* [PATCH net-next v2 0/1] stmmac: honor the GPIO flags for the PHY reset GPIO @ 2019-06-12 19:31 Martin Blumenstingl 2019-06-12 19:31 ` [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset Martin Blumenstingl 0 siblings, 1 reply; 7+ messages in thread From: Martin Blumenstingl @ 2019-06-12 19:31 UTC (permalink / raw) To: netdev, peppe.cavallaro, alexandre.torgue, joabreu, davem, andrew Cc: maxime.ripard, linus.walleij, linux-kernel, linux-gpio, Martin Blumenstingl, linux-arm-kernel Recent Amlogic SoCs (G12A which includes S905X2 and S905D2 as well as G12B which includes S922X) use GPIOZ_14 or GPIOZ_15 for the PHY reset line. These GPIOs are special because they are marked as "3.3V input tolerant open drain (OD) pins" which means they can only drive the pin output LOW (to reset the PHY) or to switch to input mode (to take the PHY out of reset). The GPIO subsystem already supports this with the GPIO_OPEN_DRAIN and GPIO_OPEN_SOURCE flags in the devicetree bindings. The goal of this series to add support for these special GPIOs in stmmac (even though the "snps,reset-gpio" binding is deprecated). My test-cases were: - X96 Max: snps,reset-gpio = <&gpio GPIOZ_15 0> with and without snps,reset-active-low before these patches. The PHY was not detected. - X96 Max: snps,reset-gpio = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>. The PHY is now detected correctly - Meson8b EC100: snps,reset-gpio = <&gpio GPIOH_4 0> with snps,reset-active-low. Before and after these patches the PHY is detected correctly. - Meson8b EC100: snps,reset-gpio = <&gpio GPIOH_4 0> without snps,reset-active-low. Before and after these patches the PHY is not detected (this is expected because we need to set the output LOW to take the PHY out of reset). - Meson8b EC100: snps,reset-gpio = <&gpio GPIOH_4 GPIO_ACTIVE_LOW> but without snps,reset-active-low. Before these patches the PHY was not detected. With these patches the PHY is now detected correctly. Changes since RFC v1 at [0]: - dropped all patches except the main patch which changes stmmac_mdio_reset to use GPIO descriptors (I will send the cleanup patches in a separate series once this patch is merged) - drop the active_low field from struct stmmac_mdio_bus_data - added Linus Walleij's Reviewed-by (thank you!) DEPENDENCIES: This has a runtime dependency on the preparation patch [0] from Linus W.'s GPIO tree. Without that dependency the snps,reset-active-low property (which quite a few .dts files use) will be ignored. Linus created an immutable branch which can be pulled into net-next: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-snps-reset-gpio gitweb for this immutable branch: [2] [0] https://patchwork.kernel.org/cover/10983801/ [1] https://patchwork.ozlabs.org/cover/1113217/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-snps-reset-gpio Martin Blumenstingl (1): net: stmmac: use GPIO descriptors in stmmac_mdio_reset .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 27 +++++++++---------- include/linux/stmmac.h | 2 +- 2 files changed, 14 insertions(+), 15 deletions(-) -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset 2019-06-12 19:31 [PATCH net-next v2 0/1] stmmac: honor the GPIO flags for the PHY reset GPIO Martin Blumenstingl @ 2019-06-12 19:31 ` Martin Blumenstingl 2019-06-12 21:09 ` Andrew Lunn ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Martin Blumenstingl @ 2019-06-12 19:31 UTC (permalink / raw) To: netdev, peppe.cavallaro, alexandre.torgue, joabreu, davem, andrew Cc: maxime.ripard, linus.walleij, linux-kernel, linux-gpio, Martin Blumenstingl, linux-arm-kernel Switch stmmac_mdio_reset to use GPIO descriptors. GPIO core handles the "snps,reset-gpio" for GPIO descriptors so we don't need to take care of it inside the driver anymore. The advantage of this is that we now preserve the GPIO flags which are passed via devicetree. This is required on some newer Amlogic boards which use an Open Drain pin for the reset GPIO. This pin can only output a LOW signal or switch to input mode but it cannot output a HIGH signal. There are already devicetree bindings for these special cases and GPIO core already takes care of them but only if we use GPIO descriptors instead of GPIO numbers. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> --- .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 27 +++++++++---------- include/linux/stmmac.h | 2 +- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c index 093a223fe408..f1c39dd048e7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c @@ -20,11 +20,11 @@ Maintainer: Giuseppe Cavallaro <peppe.cavallaro@st.com> *******************************************************************************/ +#include <linux/gpio/consumer.h> #include <linux/io.h> #include <linux/iopoll.h> #include <linux/mii.h> #include <linux/of.h> -#include <linux/of_gpio.h> #include <linux/of_mdio.h> #include <linux/phy.h> #include <linux/slab.h> @@ -251,37 +251,36 @@ int stmmac_mdio_reset(struct mii_bus *bus) #ifdef CONFIG_OF if (priv->device->of_node) { + struct gpio_desc *reset_gpio; + if (data->reset_gpio < 0) { struct device_node *np = priv->device->of_node; if (!np) return 0; - data->reset_gpio = of_get_named_gpio(np, - "snps,reset-gpio", 0); - if (data->reset_gpio < 0) - return 0; + reset_gpio = devm_gpiod_get_optional(priv->device, + "snps,reset", + GPIOD_OUT_LOW); + if (IS_ERR(reset_gpio)) + return PTR_ERR(reset_gpio); - data->active_low = of_property_read_bool(np, - "snps,reset-active-low"); of_property_read_u32_array(np, "snps,reset-delays-us", data->delays, 3); + } else { + reset_gpio = gpio_to_desc(data->reset_gpio); - if (devm_gpio_request(priv->device, data->reset_gpio, - "mdio-reset")) - return 0; + gpiod_direction_output(reset_gpio, 0); } - gpio_direction_output(data->reset_gpio, - data->active_low ? 1 : 0); if (data->delays[0]) msleep(DIV_ROUND_UP(data->delays[0], 1000)); - gpio_set_value(data->reset_gpio, data->active_low ? 0 : 1); + gpiod_set_value_cansleep(reset_gpio, 1); if (data->delays[1]) msleep(DIV_ROUND_UP(data->delays[1], 1000)); - gpio_set_value(data->reset_gpio, data->active_low ? 1 : 0); + gpiod_set_value_cansleep(reset_gpio, 0); if (data->delays[2]) msleep(DIV_ROUND_UP(data->delays[2], 1000)); } diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index 4335bd771ce5..816edb545592 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -97,7 +97,7 @@ struct stmmac_mdio_bus_data { int *irqs; int probed_phy_irq; #ifdef CONFIG_OF - int reset_gpio, active_low; + int reset_gpio; u32 delays[3]; #endif }; -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset 2019-06-12 19:31 ` [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset Martin Blumenstingl @ 2019-06-12 21:09 ` Andrew Lunn 2019-06-15 2:08 ` David Miller 2019-06-15 9:07 ` Linus Walleij 2 siblings, 0 replies; 7+ messages in thread From: Andrew Lunn @ 2019-06-12 21:09 UTC (permalink / raw) To: Martin Blumenstingl Cc: alexandre.torgue, maxime.ripard, netdev, linus.walleij, linux-kernel, linux-gpio, joabreu, peppe.cavallaro, davem, linux-arm-kernel On Wed, Jun 12, 2019 at 09:31:15PM +0200, Martin Blumenstingl wrote: > Switch stmmac_mdio_reset to use GPIO descriptors. GPIO core handles the > "snps,reset-gpio" for GPIO descriptors so we don't need to take care of > it inside the driver anymore. > > The advantage of this is that we now preserve the GPIO flags which are > passed via devicetree. This is required on some newer Amlogic boards > which use an Open Drain pin for the reset GPIO. This pin can only output > a LOW signal or switch to input mode but it cannot output a HIGH signal. > There are already devicetree bindings for these special cases and GPIO > core already takes care of them but only if we use GPIO descriptors > instead of GPIO numbers. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset 2019-06-12 19:31 ` [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset Martin Blumenstingl 2019-06-12 21:09 ` Andrew Lunn @ 2019-06-15 2:08 ` David Miller 2019-06-15 9:07 ` Linus Walleij 2 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2019-06-15 2:08 UTC (permalink / raw) To: martin.blumenstingl Cc: andrew, alexandre.torgue, maxime.ripard, netdev, linus.walleij, linux-kernel, linux-gpio, joabreu, peppe.cavallaro, linux-arm-kernel From: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Date: Wed, 12 Jun 2019 21:31:15 +0200 > Switch stmmac_mdio_reset to use GPIO descriptors. GPIO core handles the > "snps,reset-gpio" for GPIO descriptors so we don't need to take care of > it inside the driver anymore. > > The advantage of this is that we now preserve the GPIO flags which are > passed via devicetree. This is required on some newer Amlogic boards > which use an Open Drain pin for the reset GPIO. This pin can only output > a LOW signal or switch to input mode but it cannot output a HIGH signal. > There are already devicetree bindings for these special cases and GPIO > core already takes care of them but only if we use GPIO descriptors > instead of GPIO numbers. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Applied. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset 2019-06-12 19:31 ` [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset Martin Blumenstingl 2019-06-12 21:09 ` Andrew Lunn 2019-06-15 2:08 ` David Miller @ 2019-06-15 9:07 ` Linus Walleij 2019-06-15 9:11 ` Martin Blumenstingl 2 siblings, 1 reply; 7+ messages in thread From: Linus Walleij @ 2019-06-15 9:07 UTC (permalink / raw) To: Martin Blumenstingl Cc: Andrew Lunn, Alexandre TORGUE, Maxime Ripard, netdev, linux-kernel, open list:GPIO SUBSYSTEM, Jose Abreu, Giuseppe CAVALLARO, David S. Miller, Linux ARM Hi Martin! Thanks for fixing this up! A hint for a follow-up: On Wed, Jun 12, 2019 at 9:31 PM Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > index 4335bd771ce5..816edb545592 100644 > --- a/include/linux/stmmac.h > +++ b/include/linux/stmmac.h > @@ -97,7 +97,7 @@ struct stmmac_mdio_bus_data { > int *irqs; > int probed_phy_irq; > #ifdef CONFIG_OF > - int reset_gpio, active_low; > + int reset_gpio; Nothing in the kernel seems to be using this reset_gpio either. I think it can be deleted with associated code, any new users should use machine descriptors if they insist on board files. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset 2019-06-15 9:07 ` Linus Walleij @ 2019-06-15 9:11 ` Martin Blumenstingl 2019-06-15 9:34 ` Linus Walleij 0 siblings, 1 reply; 7+ messages in thread From: Martin Blumenstingl @ 2019-06-15 9:11 UTC (permalink / raw) To: Linus Walleij Cc: Andrew Lunn, Alexandre TORGUE, Maxime Ripard, netdev, linux-kernel, open list:GPIO SUBSYSTEM, Jose Abreu, Giuseppe CAVALLARO, David S. Miller, Linux ARM Hi Linus, On Sat, Jun 15, 2019 at 11:08 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > Hi Martin! > > Thanks for fixing this up! you're welcome I think I finally understand why you want to switch everything over to GPIO descriptors > A hint for a follow-up: > > On Wed, Jun 12, 2019 at 9:31 PM Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > > index 4335bd771ce5..816edb545592 100644 > > --- a/include/linux/stmmac.h > > +++ b/include/linux/stmmac.h > > @@ -97,7 +97,7 @@ struct stmmac_mdio_bus_data { > > int *irqs; > > int probed_phy_irq; > > #ifdef CONFIG_OF > > - int reset_gpio, active_low; > > + int reset_gpio; > > Nothing in the kernel seems to be using this reset_gpio either. > > I think it can be deleted with associated code, any new users > should use machine descriptors if they insist on board files. good catch, thank you - I'll put that in my cleanup series that I want to send anyways Martin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset 2019-06-15 9:11 ` Martin Blumenstingl @ 2019-06-15 9:34 ` Linus Walleij 0 siblings, 0 replies; 7+ messages in thread From: Linus Walleij @ 2019-06-15 9:34 UTC (permalink / raw) To: Martin Blumenstingl Cc: Andrew Lunn, Alexandre TORGUE, Maxime Ripard, netdev, linux-kernel, open list:GPIO SUBSYSTEM, Jose Abreu, Giuseppe CAVALLARO, David S. Miller, Linux ARM On Sat, Jun 15, 2019 at 11:11 AM Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > I think I finally understand why you want to switch everything over to > GPIO descriptors A bit after-the-fact reasoning, but after I was made aware of an interesting lecture by John Ousterhout I feel the best description of the descriptor refactoring is the idea to make the GPIO API narrow and deep, make it simple for consumers (hence <linux/gpio/consumer.h>) and handle as much special cases (polarity inversion, open drain...) as possible behind the scenes. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-15 9:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-12 19:31 [PATCH net-next v2 0/1] stmmac: honor the GPIO flags for the PHY reset GPIO Martin Blumenstingl 2019-06-12 19:31 ` [PATCH net-next v2 1/1] net: stmmac: use GPIO descriptors in stmmac_mdio_reset Martin Blumenstingl 2019-06-12 21:09 ` Andrew Lunn 2019-06-15 2:08 ` David Miller 2019-06-15 9:07 ` Linus Walleij 2019-06-15 9:11 ` Martin Blumenstingl 2019-06-15 9:34 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).