Re: [PATCH 3/4] sandbox: add SCMI power domain protocol support for testing

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Hi Simon,

Thank you for the review.

On Sun, Oct 01, 2023 at 07:17:17PM -0600, Simon Glass wrote:
> Hi AKASHI,
> 
> On Tue, 26 Sept 2023 at 01:01, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > SCMI power domain management protocol is supported on sandbox
> > for test purpose. Add fake agent interfaces and associated
> > power domain devices.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  arch/sandbox/dts/test.dts                    |   6 +
> >  arch/sandbox/include/asm/scmi_test.h         |  20 ++
> >  configs/sandbox_defconfig                    |   1 +
> >  drivers/firmware/scmi/sandbox-scmi_agent.c   | 265 ++++++++++++++++++-
> >  drivers/firmware/scmi/sandbox-scmi_devices.c |  10 +
> >  5 files changed, 301 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> nit below
> 
> >
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index 36de6a37cf6d..38fcf42cd23d 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -693,6 +693,11 @@
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >
> > +                       pwrdom_scmi: protocol@11 {
> > +                               reg = <0x11>;
> > +                               #power-domain-cells = <1>;
> > +                       };
> > +
> >                         clk_scmi: protocol@14 {
> >                                 reg = <0x14>;
> >                                 #clock-cells = <1>;
> > @@ -1589,6 +1594,7 @@
> >
> >         sandbox_scmi {
> >                 compatible = "sandbox,scmi-devices";
> > +               power-domains = <&pwrdom_scmi 2>;
> >                 clocks = <&clk_scmi 2>, <&clk_scmi 0>;
> >                 resets = <&reset_scmi 3>;
> >                 regul0-supply = <&regul0_scmi>;
> > diff --git a/arch/sandbox/include/asm/scmi_test.h b/arch/sandbox/include/asm/scmi_test.h
> > index ccb0df6c148f..1b0f4464b98f 100644
> > --- a/arch/sandbox/include/asm/scmi_test.h
> > +++ b/arch/sandbox/include/asm/scmi_test.h
> > @@ -6,10 +6,21 @@
> >  #ifndef __SANDBOX_SCMI_TEST_H
> >  #define __SANDBOX_SCMI_TEST_H
> >
> > +#include <power-domain.h>
> > +
> >  struct udevice;
> >  struct sandbox_scmi_agent;
> >  struct sandbox_scmi_service;
> >
> > +/**
> > + * struct sandbox_scmi_pwd
> > + * @id:                Identifier of the power domain used in the SCMI protocol
> 
> @pstate ?

Yes, I will add a comment.

-Takahiro Akashi


> > + */
> > +struct sandbox_scmi_pwd {
> > +       uint id;
> > +       u32 pstate;
> > +};
> > +
> >  /**
> >   * struct sandbox_scmi_clk - Simulated clock exposed by SCMI
> >   * @id:                Identifier of the clock used in the SCMI protocol
> > @@ -45,6 +56,8 @@ struct sandbox_scmi_voltd {
> >
> 
> [..]
> 
> Regards,
> Simon