gpio_request_by_name for GPIOD_IS_OUT drives GPIO before specifically instructed to

[Tim Harvey]

On Tue, Mar 1, 2022 at 11:15 AM Sean Anderson <sean.anderson at seco.com> wrote:
>
>
>
> On 3/1/22 2:12 PM, Tim Harvey wrote:
> > On Tue, Mar 1, 2022 at 10:59 AM Sean Anderson <sean.anderson at seco.com> wrote:
> >>
> >> Hi Tim,
> >>
> >> On 3/1/22 1:45 PM, Tim Harvey wrote:
> >> > Greetings,
> >> >
> >> > I'm seeing an issue in U-Boot caused by gpio_request_by_name driving a
> >> > GPIO output before it has been given an output level with
> >> > {dm_}gpio_set_value.
> >> >
> >> > In my particular instance I have a network PHY that can encounter
> >> > errata if it gets reset more than once (fun time with this one!)
> >> > declared liks this:
> >> >
> >> > &ethphy0 {
> >> >        reset-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
> >> >        reset-assert-us = <1000>;
> >> >        reset-deassert-us = <300000>;
> >> > };
> >> >
> >> > Through testing it has been found that this PHY must be held in reset
> >> > on power-up and released from reset only once. This is probably the
> >> > third time I've run into such an issue in the 20 years I've worked
> >> > with hardware - it's rare and certainly a result of poor reset
> >> > handling in IC's but not a unique situation (especially at a time
> >> > where we can't be too choosy about what parts go on boards).
> >> >
> >> > When eth-phy-uclass.c calls gpio_request_by_name(dev, "reset-gpios",
> >> > 0, &uc_priv->reset_gpio, GPIOD_IS_OUT) direction_output is called
> >> > which sets the GPIO to a high effectively taking the PHY out of reset
> >> > before eth_phy_reset is called to do so. Setting the gpio as
> >> > GPIO_ACTIVE_LOW will drive it low keeping it in reset until ready to
> >> > de-assert it but then the polarity is inverted which results in the
> >> > PHY being left in reset permanently.
> >>
> >> Can you try
> >>
> >> gpio_request_by_name(dev, "reset-gpios", 0, &uc_priv->reset_gpio, \
> >>         GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE)
> >>
> >
> > Sean,
> >
> > Thanks - this does work! When looking into this flag originally I
> > misunderstood what that flag does. It causes the GPIO to be
> > initialized in the 'active' (asserted) state vs the 'inactive'
> > (de-asserted) state.
> >
> > I would certainly think that reset gpios should be initialized in the
> > de-asserted case but I wonder if I go and change phy-reset-gpio in
> > drivers/net/fec_mxc.c and phy-reset in dirvers/net/eth-phy-uclass.c if
> > this would cause issues for other boards?
>
> Well, it seems to be what Linux does at least
>
> https://lore.kernel.org/all/20210202143239.10714-1-mike.looijmans@topic.nl/
>

Sean,

That exactly describes the issue! Thanks for pointing me to that thread.

I quick scan of U-Boot code shows me 42 different
'gpio_request_by_name' calls requesting a gpio with 'reset' in the
name and the only one that requests it as GPIOD_IS_OUT_ACTIVE is
drivers/net/dwc_eth_qos.c. I guess all I can do is to patch the ones
that I see causing issues for me and request some testing.

Best regards,

Tim