Re: [RFC 3/6] pinctrl: add scmi driver

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

Hi Simon,

On Sun, Sep 10, 2023 at 01:13:27PM -0600, Simon Glass wrote:
> Hi AKASHI,
> 
> On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > pinctrl devices exposed by SCMI firmware (server).
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  drivers/pinctrl/Kconfig        |  11 +
> >  drivers/pinctrl/Makefile       |   1 +
> >  drivers/pinctrl/pinctrl-scmi.c | 537 +++++++++++++++++++++++++++++++++
> >  3 files changed, 549 insertions(+)
> >  create mode 100644 drivers/pinctrl/pinctrl-scmi.c
> 
> 
> [..]
> 
> +
> > +U_BOOT_DRIVER(scmi_pinctrl) = {
> > +       .name = "scmi_pinctrl",
> > +       .id = UCLASS_PINCTRL,
> > +       .ops = &scmi_pinctrl_ops,
> > +       .probe = scmi_pinctrl_probe,
> > +       .priv_auto      = sizeof(struct scmi_pinctrl_priv),
> > +};
> > --
> > 2.34.1
> >
> 
> Where is the compatible string for this driver?

Nothing defined.

As I mentioned in the cover letter, pinctrl driver consists of
two layers:
- helper functions which directly manipulate protocol interfaces.
- DM-compliant (that you doubt about :) pinctrl driver.

All the SCMI-based drivers (existing ones, clock, reset and voltage domain,
except base) follow this scheme.

According to the DT bindings for SCMI protocols, they are sub-nodes
of scmi node and identified by "reg" properties. When the *bind* takes
place, scmi_bind_protocols() will enumerate all the sub-nodes and
try to bind associated protocol drivers (pinctrl in my patch) based
on "reg" value.

So there is no need to have a "compatible" property for each protocol.
Do you think it is ugly?
I suppose the corresponding kernel code, scmi_probe() in
drivers/firmware/arm_scmi/driver.c, has a similar logic although
it seems a bit more complicated.

-Takahiro Akashi

> 
> Regards,
> Simon