[PATCH] arm64: a37xx: pinctrl: probe after binding

[PATCH] arm64: a37xx: pinctrl: probe after binding

[Robert Marko]

Currently, pinctrl drivers are getting probed during post-bind, however
that is being reverted, and on A37XX pinctrl driver is the one that
registers the GPIO driver during the probe.

So, if the pinctrl driver doesn't get probed GPIO-s won't get registered
and thus they cannot be used.

This is a problem on the Methode eDPU as it just uses SB pins as GPIO-s
and without them being registered networking won't work as it only has
one SFP slot and the TX disable GPIO is on the SB controller.

So, lets just add a flag only to A37XX driver to probe after binding
in order for the GPIO driver to always get registered.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 25fbe39abd1..1be6252227d 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -745,6 +745,19 @@ static int armada_37xx_pinctrl_probe(struct udevice *dev)
 	return 0;
 }
 
+static int armada_37xx_pinctrl_bind(struct udevice *dev)
+{
+	/*
+	 * Make sure that the pinctrl driver gets probed after binding
+	 * as on A37XX the pinctrl driver is the one that is also
+	 * registering the GPIO one during probe, so if its not probed
+	 * GPIO-s are not registered as well.
+	 */
+	dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
+
+	return 0;
+}
+
 static const struct udevice_id armada_37xx_pinctrl_of_match[] = {
 	{
 		.compatible = "marvell,armada3710-sb-pinctrl",
@@ -762,6 +775,7 @@ U_BOOT_DRIVER(armada_37xx_pinctrl) = {
 	.id = UCLASS_PINCTRL,
 	.of_match = of_match_ptr(armada_37xx_pinctrl_of_match),
 	.probe = armada_37xx_pinctrl_probe,
+	.bind = armada_37xx_pinctrl_bind,
 	.priv_auto	= sizeof(struct armada_37xx_pinctrl),
 	.ops = &armada_37xx_pinctrl_ops,
 };
-- 
2.39.0

Re: [PATCH] arm64: a37xx: pinctrl: probe after binding

[Stefan Roese]

On 1/17/23 15:08, Robert Marko wrote:
> Currently, pinctrl drivers are getting probed during post-bind, however
> that is being reverted, and on A37XX pinctrl driver is the one that
> registers the GPIO driver during the probe.
> 
> So, if the pinctrl driver doesn't get probed GPIO-s won't get registered
> and thus they cannot be used.
> 
> This is a problem on the Methode eDPU as it just uses SB pins as GPIO-s
> and without them being registered networking won't work as it only has
> one SFP slot and the TX disable GPIO is on the SB controller.
> 
> So, lets just add a flag only to A37XX driver to probe after binding
> in order for the GPIO driver to always get registered.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

Reviewed--by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 25fbe39abd1..1be6252227d 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -745,6 +745,19 @@ static int armada_37xx_pinctrl_probe(struct udevice *dev)
>   	return 0;
>   }
>   
> +static int armada_37xx_pinctrl_bind(struct udevice *dev)
> +{
> +	/*
> +	 * Make sure that the pinctrl driver gets probed after binding
> +	 * as on A37XX the pinctrl driver is the one that is also
> +	 * registering the GPIO one during probe, so if its not probed
> +	 * GPIO-s are not registered as well.
> +	 */
> +	dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
> +
> +	return 0;
> +}
> +
>   static const struct udevice_id armada_37xx_pinctrl_of_match[] = {
>   	{
>   		.compatible = "marvell,armada3710-sb-pinctrl",
> @@ -762,6 +775,7 @@ U_BOOT_DRIVER(armada_37xx_pinctrl) = {
>   	.id = UCLASS_PINCTRL,
>   	.of_match = of_match_ptr(armada_37xx_pinctrl_of_match),
>   	.probe = armada_37xx_pinctrl_probe,
> +	.bind = armada_37xx_pinctrl_bind,
>   	.priv_auto	= sizeof(struct armada_37xx_pinctrl),
>   	.ops = &armada_37xx_pinctrl_ops,
>   };

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH] arm64: a37xx: pinctrl: probe after binding

[Simon Glass]

Hi,

On Thu, 19 Jan 2023 at 00:00, Stefan Roese <sr@denx.de> wrote:
>
> On 1/17/23 15:08, Robert Marko wrote:
> > Currently, pinctrl drivers are getting probed during post-bind, however
> > that is being reverted, and on A37XX pinctrl driver is the one that
> > registers the GPIO driver during the probe.
> >
> > So, if the pinctrl driver doesn't get probed GPIO-s won't get registered
> > and thus they cannot be used.
> >
> > This is a problem on the Methode eDPU as it just uses SB pins as GPIO-s
> > and without them being registered networking won't work as it only has
> > one SFP slot and the TX disable GPIO is on the SB controller.
> >
> > So, lets just add a flag only to A37XX driver to probe after binding
> > in order for the GPIO driver to always get registered.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>
> Reviewed--by: Stefan Roese <sr@denx.de>
>
> Thanks,
> Stefan
>
> > ---
> >   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> > index 25fbe39abd1..1be6252227d 100644
> > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> > @@ -745,6 +745,19 @@ static int armada_37xx_pinctrl_probe(struct udevice *dev)
> >       return 0;
> >   }
> >
> > +static int armada_37xx_pinctrl_bind(struct udevice *dev)
> > +{
> > +     /*
> > +      * Make sure that the pinctrl driver gets probed after binding
> > +      * as on A37XX the pinctrl driver is the one that is also
> > +      * registering the GPIO one during probe, so if its not probed
> > +      * GPIO-s are not registered as well.
> > +      */
> > +     dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
> > +
> > +     return 0;
> > +}
> > +
> >   static const struct udevice_id armada_37xx_pinctrl_of_match[] = {
> >       {
> >               .compatible = "marvell,armada3710-sb-pinctrl",
> > @@ -762,6 +775,7 @@ U_BOOT_DRIVER(armada_37xx_pinctrl) = {
> >       .id = UCLASS_PINCTRL,
> >       .of_match = of_match_ptr(armada_37xx_pinctrl_of_match),
> >       .probe = armada_37xx_pinctrl_probe,
> > +     .bind = armada_37xx_pinctrl_bind,
> >       .priv_auto      = sizeof(struct armada_37xx_pinctrl),
> >       .ops = &armada_37xx_pinctrl_ops,
> >   };
>

This is OK if you really want to do this. Is it not possible to do the
bind of the GPIO devices in the pinctrl bind() handler, as is done by
other SoCs? Why do we need to probe the pinctrl driver first?

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH] arm64: a37xx: pinctrl: probe after binding

[Robert Marko]

On Mon, Feb 13, 2023 at 12:36 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Thu, 19 Jan 2023 at 00:00, Stefan Roese <sr@denx.de> wrote:
> >
> > On 1/17/23 15:08, Robert Marko wrote:
> > > Currently, pinctrl drivers are getting probed during post-bind, however
> > > that is being reverted, and on A37XX pinctrl driver is the one that
> > > registers the GPIO driver during the probe.
> > >
> > > So, if the pinctrl driver doesn't get probed GPIO-s won't get registered
> > > and thus they cannot be used.
> > >
> > > This is a problem on the Methode eDPU as it just uses SB pins as GPIO-s
> > > and without them being registered networking won't work as it only has
> > > one SFP slot and the TX disable GPIO is on the SB controller.
> > >
> > > So, lets just add a flag only to A37XX driver to probe after binding
> > > in order for the GPIO driver to always get registered.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> >
> > Reviewed--by: Stefan Roese <sr@denx.de>
> >
> > Thanks,
> > Stefan
> >
> > > ---
> > >   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 ++++++++++++++
> > >   1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> > > index 25fbe39abd1..1be6252227d 100644
> > > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> > > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> > > @@ -745,6 +745,19 @@ static int armada_37xx_pinctrl_probe(struct udevice *dev)
> > >       return 0;
> > >   }
> > >
> > > +static int armada_37xx_pinctrl_bind(struct udevice *dev)
> > > +{
> > > +     /*
> > > +      * Make sure that the pinctrl driver gets probed after binding
> > > +      * as on A37XX the pinctrl driver is the one that is also
> > > +      * registering the GPIO one during probe, so if its not probed
> > > +      * GPIO-s are not registered as well.
> > > +      */
> > > +     dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >   static const struct udevice_id armada_37xx_pinctrl_of_match[] = {
> > >       {
> > >               .compatible = "marvell,armada3710-sb-pinctrl",
> > > @@ -762,6 +775,7 @@ U_BOOT_DRIVER(armada_37xx_pinctrl) = {
> > >       .id = UCLASS_PINCTRL,
> > >       .of_match = of_match_ptr(armada_37xx_pinctrl_of_match),
> > >       .probe = armada_37xx_pinctrl_probe,
> > > +     .bind = armada_37xx_pinctrl_bind,
> > >       .priv_auto      = sizeof(struct armada_37xx_pinctrl),
> > >       .ops = &armada_37xx_pinctrl_ops,
> > >   };
> >
>
> This is OK if you really want to do this. Is it not possible to do the
> bind of the GPIO devices in the pinctrl bind() handler, as is done by
> other SoCs? Why do we need to probe the pinctrl driver first?

Because on A37xx the pinctrl driver needs to be probed before the GPIO
one can be probed as GPIO driver is using internal data that is filled in
by the pinctrl driver.

Regards,
Robert
>
> Reviewed-by: Simon Glass <sjg@chromium.org>



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH] arm64: a37xx: pinctrl: probe after binding

[Simon Glass]

On Mon, Feb 13, 2023 at 12:36 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Thu, 19 Jan 2023 at 00:00, Stefan Roese <sr@denx.de> wrote:
> >
> > On 1/17/23 15:08, Robert Marko wrote:
> > > Currently, pinctrl drivers are getting probed during post-bind, however
> > > that is being reverted, and on A37XX pinctrl driver is the one that
> > > registers the GPIO driver during the probe.
> > >
> > > So, if the pinctrl driver doesn't get probed GPIO-s won't get registered
> > > and thus they cannot be used.
> > >
> > > This is a problem on the Methode eDPU as it just uses SB pins as GPIO-s
> > > and without them being registered networking won't work as it only has
> > > one SFP slot and the TX disable GPIO is on the SB controller.
> > >
> > > So, lets just add a flag only to A37XX driver to probe after binding
> > > in order for the GPIO driver to always get registered.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> >
> > Reviewed--by: Stefan Roese <sr@denx.de>
> >
> > Thanks,
> > Stefan
> >
> > > ---
> > >   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 ++++++++++++++
> > >   1 file changed, 14 insertions(+)
> > >
Applied to u-boot-dm, thanks!