[RFC 4/6] gpio: add scmi driver based on pinctrl

[Simon Glass]

Hi,

On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> > <takahiro.akashi at linaro.org> wrote:
> > >
> > > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > > gpio devices exposed by SCMI firmware (server).
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > ---
> > >  drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
> > >  1 file changed, 539 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> > > index 3ebdad57b86c..73d385bdbfcc 100644
> > > --- a/drivers/pinctrl/pinctrl-scmi.c
> > > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > > @@ -11,21 +11,20 @@
> > >  #include <malloc.h>
> > >  #include <scmi_agent.h>
> > >  #include <scmi_protocols.h>
> > > +#include <asm-generic/gpio.h>
> > >  #include <dm/device_compat.h>
> > > +#include <dm/device-internal.h>
> > > +#include <dm/lists.h>
> > >  #include <dm/pinctrl.h>
> > >
> > [..]
> >
> > > +
> > > +U_BOOT_DRIVER(scmi_gpio) = {
> > > +       .name = "scmi_gpio",
> > > +       .id = UCLASS_GPIO,
> > > +       .of_match = scmi_gpio_ids,
> > > +       .of_to_plat = scmi_gpio_probe,
> > > +       .ops = &scmi_gpio_ops,
> > > +       .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> > > +};
> > > +
> > > +/**
> > > + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> > > + * @parent:    SCMI pinctrl device
> > > + *
> > > + * Create a pinctrl-controlled gpio device
> > > + *
> > > + * Return:     0 on success, error code on failure
> > > + */
> > > +static int scmi_gpiochip_register(struct udevice *parent)
> > > +{
> > > +       ofnode node;
> > > +       struct driver *drv;
> > > +       struct udevice *gpio_dev;
> > > +       int ret;
> > > +
> > > +       /* TODO: recovery if failed */
> > > +       dev_for_each_subnode(node, parent) {
> > > +               if (!ofnode_is_enabled(node))
> > > +                       continue;
> >
> > Can we not rely on the normal driver model binding to bind these
> > devices? Why does it need to be done manually here?
>
> First, please take a look at the cover letter.
> In this RFC, I basically assume two patterns of DT bindings,
> (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
>
> In (B), a DT node as a gpio device, which is essentially a child
> of pinctrl device, is located *under* a pinctrl device. It need
> to be probed manually as there is no implicit method to enumerate
> it as a DM device automatically.

You could add a post_bind() method to the pinctrl uclass to call
dm_scan_fdt_dev() as we do with TPM.

>
> On the other hand, in (A), the same node can be put anywhere in a DT
> as it contains a "compatible" property to identify itself.
> So if we want to only support (A), scmi_gpiochip_register() and
> scmi_pinctrl_bind() are not necessary.
>
> Since there is no discussion about bindings for GPIO managed by SCMI
> pinctrl device yet on the kernel side, I have left two solutions
> in this RFC.


OK.

[..]

Regards,
Simon