| Message ID | 1455048072-869-1-git-send-email-bernhard@bwalle.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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>
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 >
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.
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
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
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
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)
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(-)