gpio_request_by_name for GPIOD_IS_OUT drives GPIO before specifically instructed to

[Sean Anderson]

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