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

[Etienne Carriere]

On Tue, 8 Sep 2020 at 17:21, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Etienne,
>
> On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
> <etienne.carriere at 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 at linaro.org>
> > Cc: Simon Glass <sjg at chromium.org>
> > Cc: Peng Fan <peng.fan at nxp.com>
> > Cc: Sudeep Holla <sudeep.holla at 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 at 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