[PATCH v2 05/12] firmware: scmi: install base protocol to SCMI agent

[Simon Glass]

Hi,

On Wed, 26 Jul 2023 at 19:07, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> Hi Simon,
>
> Thank you for your extensive review.

Cheers.

>
> On Wed, Jul 26, 2023 at 06:50:23PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Wed, 26 Jul 2023 at 02:39, AKASHI Takahiro
> > <takahiro.akashi at linaro.org> wrote:
> > >
> > > SCMI base protocol is mandatory, and once SCMI node is found in a device
> > > tree, the protocol handle (udevice) is unconditionally installed to
> > > the agent. Then basic information will be retrieved from SCMI server via
> > > the protocol and saved into the agent instance's local storage.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > ---
> > > v2
> > > * use helper functions, removing direct uses of ops
> > > ---
> > >  drivers/firmware/scmi/scmi_agent-uclass.c | 116 ++++++++++++++++++++++
> > >  include/scmi_agent-uclass.h               |  70 ++++++++++++-
> > >  2 files changed, 184 insertions(+), 2 deletions(-)
> > >
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >

[..]

> > > +static inline u32 scmi_agent_id(struct udevice *dev)
> > > +{
> > > +       return ((struct scmi_agent_priv *)dev_get_uclass_plat(dev))->agent_id;
> > > +}
> >
> > Why these helper functions? It seems better to avoid inlining access
> > to dev_get_uclass_plat(). Are you worried about exposing the priv
> > struct to clients?
>
> Well, simply wanted to avoid repeating
> "((struct scmi_agent_priv *)dev_get_uclass_plat(dev))->)" in the code.

That's fine, so long as it would not be better for the caller to
obtain the priv pointer and just access the fields in the struct,

Regards,
Simon