gpio_request_by_name for GPIOD_IS_OUT drives GPIO before specifically instructed to
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:
ðphy0 { 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.
This affects a wide variety of gpio based things including gpio-hogs.
I'm looking for some input on how to best address this issue. I haven't finished walking through all the kernel code but I believe when a gpio is requested it does so without configuring that gpio. Any ideas?
Best regards,
Tim
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:
ðphy0 { 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
This affects a wide variety of gpio based things including gpio-hogs.
I'm looking for some input on how to best address this issue. I haven't finished walking through all the kernel code but I believe when a gpio is requested it does so without configuring that gpio. Any ideas?
Best regards,
Tim
On Tue, Mar 1, 2022 at 10:59 AM Sean Anderson sean.anderson@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:
ðphy0 { 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?
diff --git a/drivers/net/eth-phy-uclass.c b/drivers/net/eth-phy-uclass.c index 1f285f7afd20..27b77444a0c5 100644 --- a/drivers/net/eth-phy-uclass.c +++ b/drivers/net/eth-phy-uclass.c @@ -137,7 +137,7 @@ static int eth_phy_of_to_plat(struct udevice *dev) /* search "reset-gpios" in phy node */ ret = gpio_request_by_name(dev, "reset-gpios", 0, &uc_priv->reset_gpio, - GPIOD_IS_OUT); + GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); if (ret && ret != -ENOENT) return ret;
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 985b03844739..c1f4930172cf 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1579,7 +1579,7 @@ static int fecmxc_of_to_plat(struct udevice *dev)
#if CONFIG_IS_ENABLED(DM_GPIO) ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, - &priv->phy_reset_gpio, GPIOD_IS_OUT); + &priv->phy_reset_gpio, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); if (ret < 0) return 0; /* property is optional, don't return error! */
Tim
?
--Sean
This affects a wide variety of gpio based things including gpio-hogs.
I'm looking for some input on how to best address this issue. I haven't finished walking through all the kernel code but I believe when a gpio is requested it does so without configuring that gpio. Any ideas?
Best regards,
Tim
On 3/1/22 2:12 PM, Tim Harvey wrote:
On Tue, Mar 1, 2022 at 10:59 AM Sean Anderson sean.anderson@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:
ðphy0 { 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
On Tue, Mar 1, 2022 at 11:15 AM Sean Anderson sean.anderson@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@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:
ðphy0 { 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
participants (2)
-
Sean Anderson -
Tim Harvey