diff mbox series

[v3,6/8] firmware: scmi: sandbox test for SCMI clocks

Message ID 20200907145000.30587-6-etienne.carriere@linaro.org
State Superseded
Headers show
Series [v3,1/8] firmware: add SCMI agent uclass | expand

Commit Message

Etienne Carriere Sept. 7, 2020, 2:49 p.m. UTC
Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
is used to get clock resources, allowing further clock manipulation.

Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
Add DM test scmi_clocks to test these 3 clocks.
Update DM test sandbox_scmi_agent with load/remove test sequences
factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Cc: Simon Glass <sjg@chromium.org>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---

Changes in v3:
- New commit in the series, addresses review comments on test support.
  ut_dm_scmi_clocks test SCMI are found and behave as expected for the
  implemented clk uclass methods.
---
 arch/sandbox/dts/test.dts                    |  15 ++
 arch/sandbox/include/asm/scmi_test.h         |  37 ++++
 configs/sandbox_defconfig                    |   1 +
 drivers/firmware/scmi/Makefile               |   2 +-
 drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-
 drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++
 test/dm/scmi.c                               | 139 ++++++++++++++-
 7 files changed, 438 insertions(+), 11 deletions(-)
 create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

-- 
2.17.1

Comments

Simon Glass Sept. 8, 2020, 3:20 p.m. UTC | #1
Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>

> Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c

> is used to get clock resources, allowing further clock manipulation.

>

> Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.

> Add DM test scmi_clocks to test these 3 clocks.

> Update DM test sandbox_scmi_agent with load/remove test sequences

> factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.

>

> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

> Cc: Simon Glass <sjg@chromium.org>

> Cc: Peng Fan <peng.fan@nxp.com>

> Cc: Sudeep Holla <sudeep.holla@arm.com>

> ---

>

> Changes in v3:

> - New commit in the series, addresses review comments on test support.

>   ut_dm_scmi_clocks test SCMI are found and behave as expected for the

>   implemented clk uclass methods.

> ---

>  arch/sandbox/dts/test.dts                    |  15 ++

>  arch/sandbox/include/asm/scmi_test.h         |  37 ++++

>  configs/sandbox_defconfig                    |   1 +

>  drivers/firmware/scmi/Makefile               |   2 +-

>  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-

>  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++

>  test/dm/scmi.c                               | 139 ++++++++++++++-

>  7 files changed, 438 insertions(+), 11 deletions(-)

>  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c


Reviewed-by: Simon Glass <sjg@chromium.org>


Nits below

[..]

> diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c

> new file mode 100644

> index 0000000000..2ce8e664df

> --- /dev/null

> +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c

> @@ -0,0 +1,86 @@

> +// SPDX-License-Identifier: GPL-2.0

> +/*

> + * Copyright (C) 2020, Linaro Limited

> + */

> +

> +#include <common.h>

> +#include <clk.h>

> +#include <dm.h>

> +#include <malloc.h>

> +#include <asm/io.h>

> +#include <asm/scmi_test.h>

> +#include <dm/device_compat.h>

> +

> +/*

> + * Simulate to some extend a SCMI exhange.


extend, exchange

> + * This drivers gets SCMI resources and offers API function to the

> + * SCMI test sequence manipulate the resources.

> + */

> +

> +#define SCMI_TEST_DEVICES_CLK_COUNT            3

> +

> +/*

> + * These tables store de handles used in the various uclasses device function


s/de/the/ ?

> + * that are instancied when probed through the SCMI agent. Use a static


spelling

> + * memory allocation to ease sharing with test sequence implementation.

> + */

> +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];

> +static struct sandbox_scmi_devices sandbox_scmi_devhld;


This should really be in a struct, I think, pointed to by
dev_get_priv() on this device. I do try to avoid BSS with driver
model, although it is not a hard rule with test code.

> +

> +struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)

> +{

> +       return &sandbox_scmi_devhld;

> +}

> +

> +static void dereference_device_handles(struct sandbox_scmi_devices *devices)

> +{

> +       devices->clk = NULL;

> +       devices->clk_count = 0;

> +}

> +

> +static int sandbox_scmi_devices_remove(struct udevice *dev)

> +{

> +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();

> +

> +       dereference_device_handles(devices);

> +

> +       return 0;

> +}

> +

> +static int sandbox_scmi_devices_probe(struct udevice *dev)

> +{

> +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();

> +       int rc;

> +       size_t n;

> +

> +       devices->clk = sandbox_scmi_clk_device;

> +       devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;

> +

> +       for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {

> +               rc = clk_get_by_index(dev, n, devices->clk + n);

> +               if (rc) {

> +                       dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);

> +                       goto err_clk;

> +               }

> +       }

> +

> +       return 0;

> +

> +err_clk:

> +       dereference_device_handles(devices);

> +

> +       return rc;

> +}

> +

> +static const struct udevice_id sandbox_scmi_devices_ids[] = {

> +       { .compatible = "sandbox,scmi-devices" },

> +       { }

> +};

> +

> +U_BOOT_DRIVER(sandbox_scmi_devices) = {

> +       .name = "sandbox-scmi_devices",

> +       .id = UCLASS_MISC,

> +       .of_match = sandbox_scmi_devices_ids,

> +       .remove = sandbox_scmi_devices_remove,

> +       .probe = sandbox_scmi_devices_probe,

> +};

[..]

Regards,
Simon
Etienne Carriere Sept. 9, 2020, 9:58 a.m. UTC | #2
On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg@chromium.org> wrote:
>

> Hi Etienne,

>

> On Mon, 7 Sep 2020 at 08:50, Etienne Carriere

> <etienne.carriere@linaro.org> wrote:

> >

> > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c

> > is used to get clock resources, allowing further clock manipulation.

> >

> > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.

> > Add DM test scmi_clocks to test these 3 clocks.

> > Update DM test sandbox_scmi_agent with load/remove test sequences

> > factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.

> >

> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

> > Cc: Simon Glass <sjg@chromium.org>

> > Cc: Peng Fan <peng.fan@nxp.com>

> > Cc: Sudeep Holla <sudeep.holla@arm.com>

> > ---

> >

> > Changes in v3:

> > - New commit in the series, addresses review comments on test support.

> >   ut_dm_scmi_clocks test SCMI are found and behave as expected for the

> >   implemented clk uclass methods.

> > ---

> >  arch/sandbox/dts/test.dts                    |  15 ++

> >  arch/sandbox/include/asm/scmi_test.h         |  37 ++++

> >  configs/sandbox_defconfig                    |   1 +

> >  drivers/firmware/scmi/Makefile               |   2 +-

> >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-

> >  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++

> >  test/dm/scmi.c                               | 139 ++++++++++++++-

> >  7 files changed, 438 insertions(+), 11 deletions(-)

> >  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

>

> Reviewed-by: Simon Glass <sjg@chromium.org>

>

> Nits below

>

> [..]

>

> > diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c

> > new file mode 100644

> > index 0000000000..2ce8e664df

> > --- /dev/null

> > +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c

> > @@ -0,0 +1,86 @@

> > +// SPDX-License-Identifier: GPL-2.0

> > +/*

> > + * Copyright (C) 2020, Linaro Limited

> > + */

> > +

> > +#include <common.h>

> > +#include <clk.h>

> > +#include <dm.h>

> > +#include <malloc.h>

> > +#include <asm/io.h>

> > +#include <asm/scmi_test.h>

> > +#include <dm/device_compat.h>

> > +

> > +/*

> > + * Simulate to some extend a SCMI exhange.

>

> extend, exchange


acked.

>

> > + * This drivers gets SCMI resources and offers API function to the

> > + * SCMI test sequence manipulate the resources.

> > + */

> > +

> > +#define SCMI_TEST_DEVICES_CLK_COUNT            3

> > +

> > +/*

> > + * These tables store de handles used in the various uclasses device function

>

> s/de/the/ ?


acked.

>

> > + * that are instancied when probed through the SCMI agent. Use a static

>

> spelling


acked, thanks.

>

> > + * memory allocation to ease sharing with test sequence implementation.

> > + */

> > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];

> > +static struct sandbox_scmi_devices sandbox_scmi_devhld;

>

> This should really be in a struct, I think, pointed to by

> dev_get_priv() on this device. I do try to avoid BSS with driver

> model, although it is not a hard rule with test code.


I used a static structure here to ease sharing context reference with
the test sequence implementation.
Context reference returned by sandbox_scmi_devices_ctx() is always
reliable for the sequence.
(not possibly dereference in which case the test may segfault).

I can go this way if you prefer no BSS in drivers (Note this is a
sandbox driver). I'll update the test accordingly.

etienne

>

> > +

> > +struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)

> > +{

> > +       return &sandbox_scmi_devhld;

> > +}

> > +

> > +static void dereference_device_handles(struct sandbox_scmi_devices *devices)

> > +{

> > +       devices->clk = NULL;

> > +       devices->clk_count = 0;

> > +}

> > +

> > +static int sandbox_scmi_devices_remove(struct udevice *dev)

> > +{

> > +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();

> > +

> > +       dereference_device_handles(devices);

> > +

> > +       return 0;

> > +}

> > +

> > +static int sandbox_scmi_devices_probe(struct udevice *dev)

> > +{

> > +       struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();

> > +       int rc;

> > +       size_t n;

> > +

> > +       devices->clk = sandbox_scmi_clk_device;

> > +       devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;

> > +

> > +       for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {

> > +               rc = clk_get_by_index(dev, n, devices->clk + n);

> > +               if (rc) {

> > +                       dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);

> > +                       goto err_clk;

> > +               }

> > +       }

> > +

> > +       return 0;

> > +

> > +err_clk:

> > +       dereference_device_handles(devices);

> > +

> > +       return rc;

> > +}

> > +

> > +static const struct udevice_id sandbox_scmi_devices_ids[] = {

> > +       { .compatible = "sandbox,scmi-devices" },

> > +       { }

> > +};

> > +

> > +U_BOOT_DRIVER(sandbox_scmi_devices) = {

> > +       .name = "sandbox-scmi_devices",

> > +       .id = UCLASS_MISC,

> > +       .of_match = sandbox_scmi_devices_ids,

> > +       .remove = sandbox_scmi_devices_remove,

> > +       .probe = sandbox_scmi_devices_probe,

> > +};

> [..]

>

> Regards,

> Simon
Simon Glass Sept. 9, 2020, 2:35 p.m. UTC | #3
Hi Etienne,

On Wed, 9 Sep 2020 at 03:58, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>

> On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg@chromium.org> wrote:

> >

> > Hi Etienne,

> >

> > On Mon, 7 Sep 2020 at 08:50, Etienne Carriere

> > <etienne.carriere@linaro.org> wrote:

> > >

> > > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c

> > > is used to get clock resources, allowing further clock manipulation.

> > >

> > > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.

> > > Add DM test scmi_clocks to test these 3 clocks.

> > > Update DM test sandbox_scmi_agent with load/remove test sequences

> > > factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.

> > >

> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

> > > Cc: Simon Glass <sjg@chromium.org>

> > > Cc: Peng Fan <peng.fan@nxp.com>

> > > Cc: Sudeep Holla <sudeep.holla@arm.com>

> > > ---

> > >

> > > Changes in v3:

> > > - New commit in the series, addresses review comments on test support.

> > >   ut_dm_scmi_clocks test SCMI are found and behave as expected for the

> > >   implemented clk uclass methods.

> > > ---

> > >  arch/sandbox/dts/test.dts                    |  15 ++

> > >  arch/sandbox/include/asm/scmi_test.h         |  37 ++++

> > >  configs/sandbox_defconfig                    |   1 +

> > >  drivers/firmware/scmi/Makefile               |   2 +-

> > >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-

> > >  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++

> > >  test/dm/scmi.c                               | 139 ++++++++++++++-

> > >  7 files changed, 438 insertions(+), 11 deletions(-)

> > >  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

> >

> > Reviewed-by: Simon Glass <sjg@chromium.org>

> >

> > Nits below

> >

> > [..]


> > > + * memory allocation to ease sharing with test sequence implementation.

> > > + */

> > > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];

> > > +static struct sandbox_scmi_devices sandbox_scmi_devhld;

> >

> > This should really be in a struct, I think, pointed to by

> > dev_get_priv() on this device. I do try to avoid BSS with driver

> > model, although it is not a hard rule with test code.

>

> I used a static structure here to ease sharing context reference with

> the test sequence implementation.


Is that in a different file?

> Context reference returned by sandbox_scmi_devices_ctx() is always

> reliable for the sequence.

> (not possibly dereference in which case the test may segfault).

>

> I can go this way if you prefer no BSS in drivers (Note this is a

> sandbox driver). I'll update the test accordingly.


If you have a reason to use BSS for a sandbox driver that is OK. I
don't quite understand the problem you are solving though. If private
data is allocated too late, you can allocate the info as platdata
which happens when the device is bound, and then use
dev_get_platdata().

But ultimately it is up to you, as this is a test driver. It just
helps to avoid people copying a pattern to their own driver.

[..]

Regards,
Simon
Etienne Carriere Sept. 9, 2020, 2:55 p.m. UTC | #4
On Wed, 9 Sep 2020 at 16:35, Simon Glass <sjg@chromium.org> wrote:
>

> Hi Etienne,

>

> On Wed, 9 Sep 2020 at 03:58, Etienne Carriere

> <etienne.carriere@linaro.org> wrote:

> >

> > On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg@chromium.org> wrote:

> > >

> > > Hi Etienne,

> > >

> > > On Mon, 7 Sep 2020 at 08:50, Etienne Carriere

> > > <etienne.carriere@linaro.org> wrote:

> > > >

> > > > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c

> > > > is used to get clock resources, allowing further clock manipulation.

> > > >

> > > > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.

> > > > Add DM test scmi_clocks to test these 3 clocks.

> > > > Update DM test sandbox_scmi_agent with load/remove test sequences

> > > > factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.

> > > >

> > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

> > > > Cc: Simon Glass <sjg@chromium.org>

> > > > Cc: Peng Fan <peng.fan@nxp.com>

> > > > Cc: Sudeep Holla <sudeep.holla@arm.com>

> > > > ---

> > > >

> > > > Changes in v3:

> > > > - New commit in the series, addresses review comments on test support.

> > > >   ut_dm_scmi_clocks test SCMI are found and behave as expected for the

> > > >   implemented clk uclass methods.

> > > > ---

> > > >  arch/sandbox/dts/test.dts                    |  15 ++

> > > >  arch/sandbox/include/asm/scmi_test.h         |  37 ++++

> > > >  configs/sandbox_defconfig                    |   1 +

> > > >  drivers/firmware/scmi/Makefile               |   2 +-

> > > >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++++++++++++++++++-

> > > >  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++++++++++

> > > >  test/dm/scmi.c                               | 139 ++++++++++++++-

> > > >  7 files changed, 438 insertions(+), 11 deletions(-)

> > > >  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

> > >

> > > Reviewed-by: Simon Glass <sjg@chromium.org>

> > >

> > > Nits below

> > >

> > > [..]

>

> > > > + * memory allocation to ease sharing with test sequence implementation.

> > > > + */

> > > > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];

> > > > +static struct sandbox_scmi_devices sandbox_scmi_devhld;

> > >

> > > This should really be in a struct, I think, pointed to by

> > > dev_get_priv() on this device. I do try to avoid BSS with driver

> > > model, although it is not a hard rule with test code.

> >

> > I used a static structure here to ease sharing context reference with

> > the test sequence implementation.

>

> Is that in a different file?


I made the helper function sandbox_scmi_devices_ctx() exported by
sandbox-scmi_devices.c that references probed devices.
The function is called from test/dm/scmi.c to run the test (actions &
verifications).


>

> > Context reference returned by sandbox_scmi_devices_ctx() is always

> > reliable for the sequence.

> > (not possibly dereference in which case the test may segfault).

> >

> > I can go this way if you prefer no BSS in drivers (Note this is a

> > sandbox driver). I'll update the test accordingly.

>

> If you have a reason to use BSS for a sandbox driver that is OK. I

> don't quite understand the problem you are solving though. If private

> data is allocated too late, you can allocate the info as platdata

> which happens when the device is bound, and then use

> dev_get_platdata().

>

> But ultimately it is up to you, as this is a test driver. It just

> helps to avoid people copying a pattern to their own driver.


Ok.
I'll try to keep it simple.

Regards,
etienne

>

> [..]

>

> Regards,

> Simon
diff mbox series

Patch

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index dd3b43885e..61acd8d79f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -361,6 +361,11 @@ 
 			compatible = "sandbox,scmi-agent";
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			clk_scmi0: protocol@14 {
+				reg = <0x14>;
+				#clock-cells = <1>;
+			};
 		};
 
 		sandbox-scmi-agent@1 {
@@ -368,6 +373,11 @@ 
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			clk_scmi1: protocol@14 {
+				reg = <0x14>;
+				#clock-cells = <1>;
+			};
+
 			protocol@10 {
 				reg = <0x10>;
 			};
@@ -1052,6 +1062,11 @@ 
 		compatible = "sandbox,virtio2";
 	};
 
+	sandbox_scmi {
+		compatible = "sandbox,scmi-devices";
+		clocks = <&clk_scmi0 7>, <&clk_scmi0 3>, <&clk_scmi1 1>;
+	};
+
 	pinctrl {
 		compatible = "sandbox,pinctrl";
 
diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
index a811fe19c3..4e09957bc7 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -10,12 +10,28 @@  struct udevice;
 struct sandbox_scmi_agent;
 struct sandbox_scmi_service;
 
+/**
+ * struct sandbox_scmi_clk - Simulated clock exposed by SCMI
+ * @id:		Identifier of the clock used in the SCMI protocol
+ * @enabled:	Clock state: true if enabled, false if disabled
+ * @rate:	Clock rate in Hertz
+ */
+struct sandbox_scmi_clk {
+	uint id;
+	bool enabled;
+	ulong rate;
+};
+
 /**
  * struct sandbox_scmi_agent - Simulated SCMI service seen by SCMI agent
  * @idx:	Identifier for the SCMI agent, its index
+ * @clk:	Simulated clocks
+ * @clk_count:	Simulated clocks array size
  */
 struct sandbox_scmi_agent {
 	uint idx;
+	struct sandbox_scmi_clk *clk;
+	size_t clk_count;
 };
 
 /**
@@ -28,16 +44,37 @@  struct sandbox_scmi_service {
 	size_t agent_count;
 };
 
+/**
+ * struct sandbox_scmi_devices - Reference to devices probed through SCMI
+ * @clk:		Array the clock devices
+ * @clk_count:		Number of clock devices probed
+ */
+struct sandbox_scmi_devices {
+	struct clk *clk;
+	size_t clk_count;
+};
+
 #ifdef CONFIG_SCMI_FIRMWARE
 /**
  * sandbox_scmi_service_context - Get the simulated SCMI services context
  * @return:	Reference to backend simulated resources state
  */
 struct sandbox_scmi_service *sandbox_scmi_service_ctx(void);
+
+/**
+ * sandbox_scmi_devices_get_ref - Get references to devices accessed through SCMI
+ * @return:	Reference to the devices probed by the SCMI test
+ */
+struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void);
 #else
 static inline struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
 {
 	return NULL;
 }
+
+static inline struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
+{
+	return NULL;
+}
 #endif /* CONFIG_SCMI_FIRMWARE */
 #endif /* __SANDBOX_SCMI_TEST_H */
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 2c130c01f0..7d71c805dc 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -122,6 +122,7 @@  CONFIG_BUTTON=y
 CONFIG_BUTTON_GPIO=y
 CONFIG_CLK=y
 CONFIG_CLK_COMPOSITE_CCF=y
+CONFIG_CLK_SCMI=y
 CONFIG_SANDBOX_CLK_CCF=y
 CONFIG_CPU=y
 CONFIG_DM_DEMO=y
diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
index 2f782bbd55..e1e0224066 100644
--- a/drivers/firmware/scmi/Makefile
+++ b/drivers/firmware/scmi/Makefile
@@ -2,4 +2,4 @@  obj-y	+= scmi_agent-uclass.o
 obj-y	+= smt.o
 obj-$(CONFIG_ARM_SMCCC) 	+= smccc_agent.o
 obj-$(CONFIG_DM_MAILBOX)	+= mailbox_agent.o
-obj-$(CONFIG_SANDBOX)		+= sandbox-scmi_agent.o
+obj-$(CONFIG_SANDBOX)		+= sandbox-scmi_agent.o sandbox-scmi_devices.o
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index ce0d49c951..262efbd12c 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -16,18 +16,34 @@ 
 /*
  * The sandbox SCMI agent driver simulates to some extend a SCMI message
  * processing. It simulates few of the SCMI services for some of the
- * SCMI protocols embedded in U-Boot. Currently none.
+ * SCMI protocols embedded in U-Boot. Currently:
+ * - SCMI clock protocol: emulate 2 agents each exposing few clocks
  *
- * This driver simulates 2 SCMI agents for test purpose.
+ * Agent #0 simulates 2 clocks.
+ * See IDs in scmi0_clk[] and "sandbox-scmi-agent@0" in test.dts.
+ *
+ * Agent #1 simulates 1 clock.
+ * See IDs in scmi1_clk[] and "sandbox-scmi-agent@1" in test.dts.
+ *
+ * All clocks are default disabled.
  *
  * This Driver exports sandbox_scmi_service_ct() for the test sequence to
  * get the state of the simulated services (clock state, rate, ...) and
  * check back-end device state reflects the request send through the
- * various uclass devices, currently nothing.
+ * various uclass devices, currently only clock controllers.
  */
 
 #define SANDBOX_SCMI_AGENT_COUNT	2
 
+static struct sandbox_scmi_clk scmi0_clk[] = {
+	{ .id = 7, .rate = 1000 },
+	{ .id = 3, .rate = 333 },
+};
+
+static struct sandbox_scmi_clk scmi1_clk[] = {
+	{ .id = 1, .rate = 44 },
+};
+
 /* The list saves to simulted end devices references for test purpose */
 struct sandbox_scmi_agent *sandbox_scmi_agent_list[SANDBOX_SCMI_AGENT_COUNT];
 
@@ -51,17 +67,158 @@  static void debug_print_agent_state(struct udevice *dev, char *str)
 	struct sandbox_scmi_agent *agent = dev2agent(dev);
 
 	dev_dbg(dev, "Dump sandbox_scmi_agent %u: %s\n", agent->idx, str);
+	dev_dbg(dev, " scmi%u_clk   (%zu): %d/%ld, %d/%ld, %d/%ld, ...\n",
+		agent->idx,
+		agent->clk_count,
+		agent->clk_count ? agent->clk[0].enabled : -1,
+		agent->clk_count ? agent->clk[0].rate : -1,
+		agent->clk_count > 1 ? agent->clk[1].enabled : -1,
+		agent->clk_count > 1 ? agent->clk[1].rate : -1,
+		agent->clk_count > 2 ? agent->clk[2].enabled : -1,
+		agent->clk_count > 2 ? agent->clk[2].rate : -1);
 };
 
+static struct sandbox_scmi_clk *get_scmi_clk_state(uint agent_id, uint clock_id)
+{
+	struct sandbox_scmi_clk *target = NULL;
+	size_t target_count = 0;
+	size_t n;
+
+	switch (agent_id) {
+	case 0:
+		target = scmi0_clk;
+		target_count = ARRAY_SIZE(scmi0_clk);
+		break;
+	case 1:
+		target = scmi1_clk;
+		target_count = ARRAY_SIZE(scmi1_clk);
+		break;
+	default:
+		return NULL;
+	}
+
+	for (n = 0; n < target_count; n++)
+		if (target[n].id == clock_id)
+			return target + n;
+
+	return NULL;
+}
+
+/*
+ * Sandbox SCMI agent ops
+ */
+
+static int sandbox_scmi_clock_rate_set(struct udevice *dev,
+				       struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_clk_rate_set_in *in = NULL;
+	struct scmi_clk_rate_set_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_clk_rate_set_in *)msg->in_msg;
+	out = (struct scmi_clk_rate_set_out *)msg->out_msg;
+
+	clk_state = get_scmi_clk_state(agent->idx, in->clock_id);
+	if (!clk_state) {
+		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		u64 rate = ((u64)in->rate_msb << 32) + in->rate_lsb;
+
+		clk_state->rate = (ulong)rate;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+static int sandbox_scmi_clock_rate_get(struct udevice *dev,
+				       struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_clk_rate_get_in *in = NULL;
+	struct scmi_clk_rate_get_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_clk_rate_get_in *)msg->in_msg;
+	out = (struct scmi_clk_rate_get_out *)msg->out_msg;
+
+	clk_state = get_scmi_clk_state(agent->idx, in->clock_id);
+	if (!clk_state) {
+		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else {
+		out->rate_msb = (u32)((u64)clk_state->rate >> 32);
+		out->rate_lsb = (u32)clk_state->rate;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
+static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
+{
+	struct sandbox_scmi_agent *agent = dev2agent(dev);
+	struct scmi_clk_state_in *in = NULL;
+	struct scmi_clk_state_out *out = NULL;
+	struct sandbox_scmi_clk *clk_state = NULL;
+
+	if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
+	    !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+		return -EINVAL;
+
+	in = (struct scmi_clk_state_in *)msg->in_msg;
+	out = (struct scmi_clk_state_out *)msg->out_msg;
+
+	clk_state = get_scmi_clk_state(agent->idx, in->clock_id);
+	if (!clk_state) {
+		dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
+
+		out->status = SCMI_NOT_FOUND;
+	} else if (in->attributes > 1) {
+		out->status = SCMI_PROTOCOL_ERROR;
+	} else {
+		clk_state->enabled = in->attributes;
+
+		out->status = SCMI_SUCCESS;
+	}
+
+	return 0;
+}
+
 static int sandbox_scmi_test_process_msg(struct udevice *dev,
 					 struct scmi_msg *msg)
 {
 	switch (msg->protocol_id) {
+	case SCMI_PROTOCOL_ID_CLOCK:
+		switch (msg->message_id) {
+		case SCMI_CLOCK_RATE_SET:
+			return sandbox_scmi_clock_rate_set(dev, msg);
+		case SCMI_CLOCK_RATE_GET:
+			return sandbox_scmi_clock_rate_get(dev, msg);
+		case SCMI_CLOCK_CONFIG_SET:
+			return sandbox_scmi_clock_gate(dev, msg);
+		default:
+			break;
+		}
+		break;
 	case SCMI_PROTOCOL_ID_BASE:
 	case SCMI_PROTOCOL_ID_POWER_DOMAIN:
 	case SCMI_PROTOCOL_ID_SYSTEM:
 	case SCMI_PROTOCOL_ID_PERF:
-	case SCMI_PROTOCOL_ID_CLOCK:
 	case SCMI_PROTOCOL_ID_SENSOR:
 	case SCMI_PROTOCOL_ID_RESET_DOMAIN:
 		*(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED;
@@ -106,11 +263,15 @@  static int sandbox_scmi_test_probe(struct udevice *dev)
 	case '0':
 		*agent = (struct sandbox_scmi_agent){
 			.idx = 0,
+			.clk = scmi0_clk,
+			.clk_count = ARRAY_SIZE(scmi0_clk),
 		};
 		break;
 	case '1':
 		*agent = (struct sandbox_scmi_agent){
 			.idx = 1,
+			.clk = scmi1_clk,
+			.clk_count = ARRAY_SIZE(scmi1_clk),
 		};
 		break;
 	default:
diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
new file mode 100644
index 0000000000..2ce8e664df
--- /dev/null
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -0,0 +1,86 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Linaro Limited
+ */
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <malloc.h>
+#include <asm/io.h>
+#include <asm/scmi_test.h>
+#include <dm/device_compat.h>
+
+/*
+ * Simulate to some extend a SCMI exhange.
+ * This drivers gets SCMI resources and offers API function to the
+ * SCMI test sequence manipulate the resources.
+ */
+
+#define SCMI_TEST_DEVICES_CLK_COUNT		3
+
+/*
+ * These tables store de handles used in the various uclasses device function
+ * that are instancied when probed through the SCMI agent. Use a static
+ * memory allocation to ease sharing with test sequence implementation.
+ */
+static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
+static struct sandbox_scmi_devices sandbox_scmi_devhld;
+
+struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
+{
+	return &sandbox_scmi_devhld;
+}
+
+static void dereference_device_handles(struct sandbox_scmi_devices *devices)
+{
+	devices->clk = NULL;
+	devices->clk_count = 0;
+}
+
+static int sandbox_scmi_devices_remove(struct udevice *dev)
+{
+	struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
+
+	dereference_device_handles(devices);
+
+	return 0;
+}
+
+static int sandbox_scmi_devices_probe(struct udevice *dev)
+{
+	struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
+	int rc;
+	size_t n;
+
+	devices->clk = sandbox_scmi_clk_device;
+	devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;
+
+	for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
+		rc = clk_get_by_index(dev, n, devices->clk + n);
+		if (rc) {
+			dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);
+			goto err_clk;
+		}
+	}
+
+	return 0;
+
+err_clk:
+	dereference_device_handles(devices);
+
+	return rc;
+}
+
+static const struct udevice_id sandbox_scmi_devices_ids[] = {
+	{ .compatible = "sandbox,scmi-devices" },
+	{ }
+};
+
+U_BOOT_DRIVER(sandbox_scmi_devices) = {
+	.name = "sandbox-scmi_devices",
+	.id = UCLASS_MISC,
+	.of_match = sandbox_scmi_devices_ids,
+	.remove = sandbox_scmi_devices_remove,
+	.probe = sandbox_scmi_devices_probe,
+};
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index d8c1e71f12..e652dbf52b 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -13,26 +13,153 @@ 
  */
 
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
 #include <asm/scmi_test.h>
 #include <dm/device-internal.h>
 #include <dm/test.h>
+#include <linux/kconfig.h>
 #include <test/ut.h>
 
+static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+
+	ut_assertnonnull(scmi_ctx);
+
+	if (scmi_ctx->agent_count)
+		ut_asserteq(2, scmi_ctx->agent_count);
+
+	ut_assertnonnull(scmi_devices);
+
+	if (scmi_devices->clk_count)
+		ut_asserteq(3, scmi_devices->clk_count);
+
+	return 0;
+}
+
+static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+
+	/* Device references to check context against test sequence */
+	ut_assertnonnull(scmi_devices);
+	if (IS_ENABLED(CONFIG_CLK_SCMI))
+		ut_asserteq(3, scmi_devices->clk_count);
+
+	/* State of the simulated SCMI server exposed */
+	ut_asserteq(2, scmi_ctx->agent_count);
+
+	ut_assertnonnull(scmi_ctx->agent[0]);
+	ut_asserteq(2, scmi_ctx->agent[0]->clk_count);
+	ut_assertnonnull(scmi_ctx->agent[0]->clk);
+
+	ut_assertnonnull(scmi_ctx->agent[1]);
+	ut_assertnonnull(scmi_ctx->agent[1]->clk);
+	ut_asserteq(1, scmi_ctx->agent[1]->clk_count);
+
+	return 0;
+}
+
+static int load_sandbox_scmi_test_devices(struct unit_test_state *uts,
+					  struct udevice **dev)
+{
+	int rc;
+
+	rc = ut_assert_scmi_state_preprobe(uts);
+	if (rc)
+		return rc;
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "sandbox_scmi",
+					      dev));
+
+	return ut_assert_scmi_state_postprobe(uts);
+}
+
+static int release_sandbox_scmi_test_devices(struct unit_test_state *uts,
+					     struct udevice *dev)
+{
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
+
+	/* Not sure test devices are fully removed, agent may not be visible */
+	return 0;
+}
+
 /*
  * Test SCMI states when loading and releasing resources
  * related to SCMI drivers.
  */
 static int dm_test_scmi_sandbox_agent(struct unit_test_state *uts)
 {
-	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+	struct udevice *dev = NULL;
+	int rc;
 
-	ut_assertnonnull(scmi_ctx);
-	ut_asserteq(2, scmi_ctx->agent_count);
-	ut_assertnull(scmi_ctx->agent[0]);
-	ut_assertnull(scmi_ctx->agent[1]);
+	rc = load_sandbox_scmi_test_devices(uts, &dev);
+	if (!rc)
+		rc = release_sandbox_scmi_test_devices(uts, dev);
 
-	return 0;
+	return rc;
 }
 
 DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
+
+static int dm_test_scmi_clocks(struct unit_test_state *uts)
+{
+	struct sandbox_scmi_devices *scmi_devices = sandbox_scmi_devices_ctx();
+	struct sandbox_scmi_service *scmi_ctx = sandbox_scmi_service_ctx();
+	struct udevice *dev = NULL;
+	int rc_dev;
+	int rc;
+
+	if (!IS_ENABLED(CONFIG_CLK_SCMI))
+		return 0;
+
+	rc = load_sandbox_scmi_test_devices(uts, &dev);
+	if (rc)
+		return rc;
+
+	/* Test SCMI clocks rate manipulation */
+	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
+	ut_asserteq(333, clk_get_rate(&scmi_devices->clk[1]));
+	ut_asserteq(44, clk_get_rate(&scmi_devices->clk[2]));
+
+	rc_dev = clk_set_rate(&scmi_devices->clk[1], 1088);
+	ut_assert(!rc_dev || rc_dev == 1088);
+
+	ut_asserteq(1000, scmi_ctx->agent[0]->clk[0].rate);
+	ut_asserteq(1088, scmi_ctx->agent[0]->clk[1].rate);
+	ut_asserteq(44, scmi_ctx->agent[1]->clk[0].rate);
+
+	ut_asserteq(1000, clk_get_rate(&scmi_devices->clk[0]));
+	ut_asserteq(1088, clk_get_rate(&scmi_devices->clk[1]));
+	ut_asserteq(44, clk_get_rate(&scmi_devices->clk[2]));
+
+	/* restore original rate for further tests */
+	rc_dev = clk_set_rate(&scmi_devices->clk[1], 333);
+	ut_assert(!rc_dev || rc_dev == 333);
+
+	/* Test SCMI clocks gating manipulation */
+	ut_assert(!scmi_ctx->agent[0]->clk[0].enabled);
+	ut_assert(!scmi_ctx->agent[0]->clk[1].enabled);
+	ut_assert(!scmi_ctx->agent[1]->clk[0].enabled);
+
+	ut_asserteq(0, clk_enable(&scmi_devices->clk[1]));
+	ut_asserteq(0, clk_enable(&scmi_devices->clk[2]));
+
+	ut_assert(!scmi_ctx->agent[0]->clk[0].enabled);
+	ut_assert(scmi_ctx->agent[0]->clk[1].enabled);
+	ut_assert(scmi_ctx->agent[1]->clk[0].enabled);
+
+	ut_assertok(clk_disable(&scmi_devices->clk[1]));
+	ut_assertok(clk_disable(&scmi_devices->clk[2]));
+
+	ut_assert(!scmi_ctx->agent[0]->clk[0].enabled);
+	ut_assert(!scmi_ctx->agent[0]->clk[1].enabled);
+	ut_assert(!scmi_ctx->agent[1]->clk[0].enabled);
+
+	return release_sandbox_scmi_test_devices(uts, dev);
+}
+
+DM_TEST(dm_test_scmi_clocks, UT_TESTF_SCAN_FDT);