[PATCH 2/5] sandbox: scmi: test against a single scmi agent

[Etienne Carriere]

Hello Patrick,

On Fri, 11 Feb 2022 at 14:46, Patrick DELAUNAY
<patrick.delaunay at foss.st.com> wrote:
>
> Hi Etienne,
>
> On 2/11/22 13:07, Etienne Carriere wrote:
> > As per DT bindings since Linux kernel v5.14, the device tree can define
> > only 1 SCMI agent node that is named scmi [1]. As a consequence, change
> > implementation of the SCMI driver test through sandbox architecture to
> > reflect that.
> >
> > This change updates sandbox test DT and sandbox SCMI driver accordingly
> > since all these are impacted.
> >
> > Cc: Simon Glass <sjg at chromium.org>
> > Signed-off-by: Etienne Carriere <etienne.carriere at linaro.org>
> > ---
> >   arch/sandbox/dts/test.dts                    |  37 ++--
> >   arch/sandbox/include/asm/scmi_test.h         |  12 +-
> >   drivers/firmware/scmi/sandbox-scmi_agent.c   | 167 ++++++-------------
> >   drivers/firmware/scmi/sandbox-scmi_devices.c |   4 +-
> >   test/dm/scmi.c                               | 117 +++++++------
> >   5 files changed, 129 insertions(+), 208 deletions(-)
> > (snip)
> >
> >   static int sandbox_scmi_test_probe(struct udevice *dev)
> >   {
> > -     static const char basename[] = "sandbox-scmi-agent@";
> > +     static const char basename[] = "scmi";
> >       struct sandbox_scmi_agent *agent = dev_get_priv(dev);
> > -     const size_t basename_size = sizeof(basename) - 1;
> >
> > -     if (strncmp(basename, dev->name, basename_size))
> > +     if (strcmp(basename, dev->name))
> >               return -ENOENT;
>
> This check is still needed ?
>
> you test in drivier if device is correct
>
> - static const char basename[] = "scmi";
> ....
>
> - if (strcmp(basename, dev->name))
> -               return -ENOENT;
>


Indeed I missed that update. Thanks, i'll fix.

> >
> > (snip)
> >
> > diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> > index c938e6d4fc..2f19b99d7c 100644
> > --- a/test/dm/scmi.c
> > +++ b/test/dm/scmi.c
> > @@ -5,7 +5,7 @@
> >    * Tests scmi_agent uclass and the SCMI drivers implemented in other
> >    * uclass devices probe when a SCMI server exposes resources.
> >    *
> > - * Note in test.dts the protocol at 10 node in agent 1. Protocol 0x10 is not
> > + * Note in test.dts the protocol at 10 node in scmi node. Protocol 0x10 is not
> >    * implemented in U-Boot SCMI components but the implementation is exepected
> >    * to not complain on unknown protocol IDs, as long as it is not used. Note
> >    * in test.dts tests that SCMI drivers probing does not fail for such an
> > @@ -28,8 +28,7 @@ static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
> >       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_assertnull(scmi_ctx->agent);
> >
> >       return 0;
> >   }
> > @@ -39,35 +38,28 @@ static int ut_assert_scmi_state_postprobe(struct unit_test_state *uts,
> >   {
> >       struct sandbox_scmi_devices *scmi_devices;
> >       struct sandbox_scmi_service *scmi_ctx;
> > -     struct sandbox_scmi_agent *agent0;
> > -     struct sandbox_scmi_agent *agent1;
> > +     struct sandbox_scmi_agent *agent;
> >
> >       /* Device references to check context against test sequence */
> >       scmi_devices = sandbox_scmi_devices_ctx(dev);
> > -
> >       ut_assertnonnull(scmi_devices);
> > -     ut_asserteq(3, scmi_devices->clk_count);
> > +
> added line ?

will remove

> > +     ut_asserteq(2, scmi_devices->clk_count);
> >       ut_asserteq(1, scmi_devices->reset_count);
> >       ut_asserteq(2, scmi_devices->regul_count);
> >
> >       /* State of the simulated SCMI server exposed */
> >       scmi_ctx = sandbox_scmi_service_ctx();
> > -     agent0 = scmi_ctx->agent[0];
> > -     agent1 = scmi_ctx->agent[1];
> > -
> > -     ut_asserteq(2, scmi_ctx->agent_count);
> > -
> > -     ut_assertnonnull(agent0);
> > -     ut_asserteq(2, agent0->clk_count);
> > -     ut_assertnonnull(agent0->clk);
> > -     ut_asserteq(1, agent0->reset_count);
> > -     ut_assertnonnull(agent0->reset);
> > -     ut_asserteq(2, agent0->voltd_count);
> > -     ut_assertnonnull(agent0->voltd);
> > +     ut_assertnonnull(scmi_ctx);
> > +     agent = scmi_ctx->agent;
> > +     ut_assertnonnull(agent);
> >
> line to remove

ok

> > -     ut_assertnonnull(agent1);
> > -     ut_assertnonnull(agent1->clk);
> > -     ut_asserteq(1, agent1->clk_count);
> > +     ut_asserteq(2, agent->clk_count);
> > +     ut_assertnonnull(agent->clk);
> > +     ut_asserteq(1, agent->reset_count);
> > +     ut_assertnonnull(agent->reset);
> > +     ut_asserteq(2, agent->voltd_count);
> > +     ut_assertnonnull(agent->voltd);
> >
> >       return 0;
> >   }
> >  (snip)
>
>
> with few minor remarks
>
> Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>

I'll send the fixes. Many thanks.

Best regards,
Etienne

>
> Thanks
> Patrick
>
>