* [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
@ 2016-02-09 20:01 Bernhard Walle
2016-02-09 22:12 ` Fabio Estevam
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Bernhard Walle @ 2016-02-09 20:01 UTC (permalink / raw)
To: fabio.estevam; +Cc: devicetree, linux-kernel, netdev, Bernhard Walle
We need that for a custom hardware that needs the reverse reset
sequence.
Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
---
Changes compared to v1:
- Add documentation to 'phy-reset-gpios' that flags are ignored
as suggested by Andrew Lunn.
Documentation/devicetree/bindings/net/fsl-fec.txt | 7 ++++++-
drivers/net/ethernet/freescale/fec_main.c | 8 ++++++--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index a9eb611..0caa429 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -7,11 +7,16 @@ Required properties:
- phy-mode : See ethernet.txt file in the same directory
Optional properties:
-- phy-reset-gpios : Should specify the gpio for phy reset
+- phy-reset-gpios : Should specify the gpio for phy reset. Additional
+ flags are ignored, see the non-standard 'phy-reset-active-low' property
+ instead.
- phy-reset-duration : Reset duration in milliseconds. Should present
only if property "phy-reset-gpios" is available. Missing the property
will have the duration be 1 millisecond. Numbers greater than 1000 are
invalid and 1 millisecond will be used instead.
+- phy-reset-active-low : If present then the reset sequence using the GPIO
+ specified in the "phy-reset-gpios" property is reversed (H=reset state,
+ L=operation state).
- phy-supply : regulator that powers the Ethernet PHY.
- phy-handle : phandle to the PHY device connected to this device.
- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 41c81f6..98caf87 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3229,6 +3229,7 @@ static int fec_enet_init(struct net_device *ndev)
static void fec_reset_phy(struct platform_device *pdev)
{
int err, phy_reset;
+ bool active_low = false;
int msec = 1;
struct device_node *np = pdev->dev.of_node;
@@ -3244,14 +3245,17 @@ static void fec_reset_phy(struct platform_device *pdev)
if (!gpio_is_valid(phy_reset))
return;
+ active_low = of_property_read_bool(np, "phy-reset-active-low");
+
err = devm_gpio_request_one(&pdev->dev, phy_reset,
- GPIOF_OUT_INIT_LOW, "phy-reset");
+ active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
+ "phy-reset");
if (err) {
dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
return;
}
msleep(msec);
- gpio_set_value_cansleep(phy_reset, 1);
+ gpio_set_value_cansleep(phy_reset, !active_low);
}
#else /* CONFIG_OF */
static void fec_reset_phy(struct platform_device *pdev)
--
2.7.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
2016-02-09 20:01 [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT Bernhard Walle
@ 2016-02-09 22:12 ` Fabio Estevam
2016-02-10 18:09 ` Andrew Lunn
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Fabio Estevam @ 2016-02-09 22:12 UTC (permalink / raw)
To: Bernhard Walle; +Cc: Fabio Estevam, devicetree, linux-kernel, netdev
Hi Bernhard,
On Tue, Feb 9, 2016 at 6:01 PM, Bernhard Walle <bernhard@bwalle.de> wrote:
> We need that for a custom hardware that needs the reverse reset
> sequence.
>
> Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
Looks good:
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
2016-02-09 20:01 [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT Bernhard Walle
2016-02-09 22:12 ` Fabio Estevam
@ 2016-02-10 18:09 ` Andrew Lunn
2016-02-12 15:20 ` Rob Herring
2016-02-21 19:33 ` Sergei Shtylyov
3 siblings, 0 replies; 7+ messages in thread
From: Andrew Lunn @ 2016-02-10 18:09 UTC (permalink / raw)
To: Bernhard Walle; +Cc: fabio.estevam, devicetree, linux-kernel, netdev
On Tue, Feb 09, 2016 at 09:01:12PM +0100, Bernhard Walle wrote:
> We need that for a custom hardware that needs the reverse reset
> sequence.
>
> Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
Thanks for updating the Documentation.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
> ---
> Changes compared to v1:
> - Add documentation to 'phy-reset-gpios' that flags are ignored
> as suggested by Andrew Lunn.
>
> Documentation/devicetree/bindings/net/fsl-fec.txt | 7 ++++++-
> drivers/net/ethernet/freescale/fec_main.c | 8 ++++++--
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index a9eb611..0caa429 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -7,11 +7,16 @@ Required properties:
> - phy-mode : See ethernet.txt file in the same directory
>
> Optional properties:
> -- phy-reset-gpios : Should specify the gpio for phy reset
> +- phy-reset-gpios : Should specify the gpio for phy reset. Additional
> + flags are ignored, see the non-standard 'phy-reset-active-low' property
> + instead.
> - phy-reset-duration : Reset duration in milliseconds. Should present
> only if property "phy-reset-gpios" is available. Missing the property
> will have the duration be 1 millisecond. Numbers greater than 1000 are
> invalid and 1 millisecond will be used instead.
> +- phy-reset-active-low : If present then the reset sequence using the GPIO
> + specified in the "phy-reset-gpios" property is reversed (H=reset state,
> + L=operation state).
> - phy-supply : regulator that powers the Ethernet PHY.
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 41c81f6..98caf87 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3229,6 +3229,7 @@ static int fec_enet_init(struct net_device *ndev)
> static void fec_reset_phy(struct platform_device *pdev)
> {
> int err, phy_reset;
> + bool active_low = false;
> int msec = 1;
> struct device_node *np = pdev->dev.of_node;
>
> @@ -3244,14 +3245,17 @@ static void fec_reset_phy(struct platform_device *pdev)
> if (!gpio_is_valid(phy_reset))
> return;
>
> + active_low = of_property_read_bool(np, "phy-reset-active-low");
> +
> err = devm_gpio_request_one(&pdev->dev, phy_reset,
> - GPIOF_OUT_INIT_LOW, "phy-reset");
> + active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> + "phy-reset");
> if (err) {
> dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
> return;
> }
> msleep(msec);
> - gpio_set_value_cansleep(phy_reset, 1);
> + gpio_set_value_cansleep(phy_reset, !active_low);
> }
> #else /* CONFIG_OF */
> static void fec_reset_phy(struct platform_device *pdev)
> --
> 2.7.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
2016-02-09 20:01 [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT Bernhard Walle
2016-02-09 22:12 ` Fabio Estevam
2016-02-10 18:09 ` Andrew Lunn
@ 2016-02-12 15:20 ` Rob Herring
2016-02-12 15:24 ` Fabio Estevam
2016-02-21 19:33 ` Sergei Shtylyov
3 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2016-02-12 15:20 UTC (permalink / raw)
To: Bernhard Walle; +Cc: fabio.estevam, devicetree, linux-kernel, netdev
On Tue, Feb 09, 2016 at 09:01:12PM +0100, Bernhard Walle wrote:
> We need that for a custom hardware that needs the reverse reset
> sequence.
>
> Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
> ---
> Changes compared to v1:
> - Add documentation to 'phy-reset-gpios' that flags are ignored
> as suggested by Andrew Lunn.
>
> Documentation/devicetree/bindings/net/fsl-fec.txt | 7 ++++++-
> drivers/net/ethernet/freescale/fec_main.c | 8 ++++++--
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index a9eb611..0caa429 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -7,11 +7,16 @@ Required properties:
> - phy-mode : See ethernet.txt file in the same directory
>
> Optional properties:
> -- phy-reset-gpios : Should specify the gpio for phy reset
> +- phy-reset-gpios : Should specify the gpio for phy reset. Additional
> + flags are ignored, see the non-standard 'phy-reset-active-low' property
> + instead.
> - phy-reset-duration : Reset duration in milliseconds. Should present
> only if property "phy-reset-gpios" is available. Missing the property
> will have the duration be 1 millisecond. Numbers greater than 1000 are
> invalid and 1 millisecond will be used instead.
> +- phy-reset-active-low : If present then the reset sequence using the GPIO
> + specified in the "phy-reset-gpios" property is reversed (H=reset state,
> + L=operation state).
This is what the gpio flags are for. Why can't you use that?
> - phy-supply : regulator that powers the Ethernet PHY.
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
2016-02-12 15:20 ` Rob Herring
@ 2016-02-12 15:24 ` Fabio Estevam
2016-02-12 19:44 ` Rob Herring
0 siblings, 1 reply; 7+ messages in thread
From: Fabio Estevam @ 2016-02-12 15:24 UTC (permalink / raw)
To: Rob Herring
Cc: Bernhard Walle, Fabio Estevam, devicetree, linux-kernel, netdev
On Fri, Feb 12, 2016 at 1:20 PM, Rob Herring <robh@kernel.org> wrote:
> This is what the gpio flags are for. Why can't you use that?
This has already been discussed in v1:
https://lkml.org/lkml/2016/2/8/867
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
2016-02-12 15:24 ` Fabio Estevam
@ 2016-02-12 19:44 ` Rob Herring
0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2016-02-12 19:44 UTC (permalink / raw)
To: Fabio Estevam
Cc: Bernhard Walle, Fabio Estevam, devicetree, linux-kernel, netdev
On Fri, Feb 12, 2016 at 9:24 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Feb 12, 2016 at 1:20 PM, Rob Herring <robh@kernel.org> wrote:
>
>> This is what the gpio flags are for. Why can't you use that?
>
> This has already been discussed in v1:
> https://lkml.org/lkml/2016/2/8/867
Thanks. That's fine, but that detail should be explained in the
binding. Is there a specific compatible string this can be limited to?
It should also be stated that the GPIO flags should be correct rather
than ignored implying I can put in whatever I want there. Then someday
if no more wrong dtbs are in the wild, it can switch back to using
gpio flags. Also, phy-reset-active-low should be prefixed with "fsl,".
Rob
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT
2016-02-09 20:01 [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT Bernhard Walle
` (2 preceding siblings ...)
2016-02-12 15:20 ` Rob Herring
@ 2016-02-21 19:33 ` Sergei Shtylyov
3 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2016-02-21 19:33 UTC (permalink / raw)
To: Bernhard Walle, fabio.estevam; +Cc: devicetree, linux-kernel, netdev
Hello.
On 02/09/2016 11:01 PM, Bernhard Walle wrote:
> We need that for a custom hardware that needs the reverse reset
> sequence.
>
> Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
> ---
> Changes compared to v1:
> - Add documentation to 'phy-reset-gpios' that flags are ignored
> as suggested by Andrew Lunn.
>
> Documentation/devicetree/bindings/net/fsl-fec.txt | 7 ++++++-
> drivers/net/ethernet/freescale/fec_main.c | 8 ++++++--
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index a9eb611..0caa429 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -7,11 +7,16 @@ Required properties:
> - phy-mode : See ethernet.txt file in the same directory
>
> Optional properties:
> -- phy-reset-gpios : Should specify the gpio for phy reset
> +- phy-reset-gpios : Should specify the gpio for phy reset. Additional
> + flags are ignored, see the non-standard 'phy-reset-active-low' property
> + instead.
> - phy-reset-duration : Reset duration in milliseconds. Should present
> only if property "phy-reset-gpios" is available. Missing the property
> will have the duration be 1 millisecond. Numbers greater than 1000 are
> invalid and 1 millisecond will be used instead.
> +- phy-reset-active-low : If present then the reset sequence using the GPIO
> + specified in the "phy-reset-gpios" property is reversed (H=reset state,
> + L=operation state).
It would be active high reset, no?
> - phy-supply : regulator that powers the Ethernet PHY.
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 41c81f6..98caf87 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3229,6 +3229,7 @@ static int fec_enet_init(struct net_device *ndev)
> static void fec_reset_phy(struct platform_device *pdev)
> {
> int err, phy_reset;
> + bool active_low = false;
> int msec = 1;
> struct device_node *np = pdev->dev.of_node;
>
> @@ -3244,14 +3245,17 @@ static void fec_reset_phy(struct platform_device *pdev)
> if (!gpio_is_valid(phy_reset))
> return;
>
> + active_low = of_property_read_bool(np, "phy-reset-active-low");
> +
> err = devm_gpio_request_one(&pdev->dev, phy_reset,
> - GPIOF_OUT_INIT_LOW, "phy-reset");
> + active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> + "phy-reset");
> if (err) {
> dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
> return;
> }
> msleep(msec);
> - gpio_set_value_cansleep(phy_reset, 1);
> + gpio_set_value_cansleep(phy_reset, !active_low);
I believe the above expressions should be inverted.
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-21 19:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09 20:01 [PATCH v2] net: fec: Add "phy-reset-active-low" property to DT Bernhard Walle
2016-02-09 22:12 ` Fabio Estevam
2016-02-10 18:09 ` Andrew Lunn
2016-02-12 15:20 ` Rob Herring
2016-02-12 15:24 ` Fabio Estevam
2016-02-12 19:44 ` Rob Herring
2016-02-21 19:33 ` Sergei Shtylyov
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).