[PATCH v2] net: fec: add post PHY reset delay DT property

[PATCH v2] net: fec: add post PHY reset delay DT property

[Quentin Schulz]

Some PHY require to wait for a bit after the reset GPIO has been
toggled. This adds support for the DT property `phy-reset-post-delay`
which gives the delay in milliseconds to wait after reset.

If the DT property is not given, no delay is observed. Post reset delay
greater than 1000ms are invalid.

Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

v2:
  - return -EINVAL when phy-reset-post-delay is greater than 1000ms
  instead of defaulting to 1ms,
  - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
  binding doc and commit log,
  - move phy-reset-post-delay property reading before
  devm_gpio_request_one(),

 Documentation/devicetree/bindings/net/fsl-fec.txt |  4 ++++
 drivers/net/ethernet/freescale/fec_main.c         | 16 +++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index a1e3693cca16..6f55bdd52f8a 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -15,6 +15,10 @@ Optional properties:
 - phy-reset-active-high : 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-reset-post-delay : Post reset delay in milliseconds. If present then
+  a delay of phy-reset-post-delay milliseconds will be observed after the
+  phy-reset-gpios has been toggled. Can be omitted thus no delay is
+  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
 - 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 56a563f90b0b..f7c8649fd28f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3192,7 +3192,7 @@ static int fec_reset_phy(struct platform_device *pdev)
 {
 	int err, phy_reset;
 	bool active_high = false;
-	int msec = 1;
+	int msec = 1, phy_post_delay = 0;
 	struct device_node *np = pdev->dev.of_node;
 
 	if (!np)
@@ -3209,6 +3209,11 @@ static int fec_reset_phy(struct platform_device *pdev)
 	else if (!gpio_is_valid(phy_reset))
 		return 0;
 
+	err = of_property_read_u32(np, "phy-reset-post-delay", &phy_post_delay);
+	/* valid reset duration should be less than 1s */
+	if (!err && phy_post_delay > 1000)
+		return -EINVAL;
+
 	active_high = of_property_read_bool(np, "phy-reset-active-high");
 
 	err = devm_gpio_request_one(&pdev->dev, phy_reset,
@@ -3226,6 +3231,15 @@ static int fec_reset_phy(struct platform_device *pdev)
 
 	gpio_set_value_cansleep(phy_reset, !active_high);
 
+	if (!phy_post_delay)
+		return 0;
+
+	if (phy_post_delay > 20)
+		msleep(phy_post_delay);
+	else
+		usleep_range(phy_post_delay * 1000,
+			     phy_post_delay * 1000 + 1000);
+
 	return 0;
 }
 #else /* CONFIG_OF */
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: fec: add post PHY reset delay DT property

[Andrew Lunn]

On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
> Some PHY require to wait for a bit after the reset GPIO has been
> toggled. This adds support for the DT property `phy-reset-post-delay`
> which gives the delay in milliseconds to wait after reset.
> 
> If the DT property is not given, no delay is observed. Post reset delay
> greater than 1000ms are invalid.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

RE: [PATCH v2] net: fec: add post PHY reset delay DT property

[Andy Duan]

From: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Sent: Tuesday, May 23, 2017 5:48 PM
>Some PHY require to wait for a bit after the reset GPIO has been toggled. This
>adds support for the DT property `phy-reset-post-delay` which gives the delay
>in milliseconds to wait after reset.
>
>If the DT property is not given, no delay is observed. Post reset delay greater
>than 1000ms are invalid.
>
>Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>---
>
>v2:
>  - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>  instead of defaulting to 1ms,
>  - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>  binding doc and commit log,
>  - move phy-reset-post-delay property reading before
>  devm_gpio_request_one(),
>
> Documentation/devicetree/bindings/net/fsl-fec.txt |  4 ++++
> drivers/net/ethernet/freescale/fec_main.c         | 16 +++++++++++++++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
>diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>b/Documentation/devicetree/bindings/net/fsl-fec.txt
>index a1e3693cca16..6f55bdd52f8a 100644
>--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>@@ -15,6 +15,10 @@ Optional properties:
> - phy-reset-active-high : 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-reset-post-delay : Post reset delay in milliseconds. If present
>+then
>+  a delay of phy-reset-post-delay milliseconds will be observed after
>+the
>+  phy-reset-gpios has been toggled. Can be omitted thus no delay is
>+  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
> - 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 56a563f90b0b..f7c8649fd28f 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -3192,7 +3192,7 @@ static int fec_reset_phy(struct platform_device
>*pdev)  {
> 	int err, phy_reset;
> 	bool active_high = false;
>-	int msec = 1;
>+	int msec = 1, phy_post_delay = 0;
> 	struct device_node *np = pdev->dev.of_node;
>
> 	if (!np)
>@@ -3209,6 +3209,11 @@ static int fec_reset_phy(struct platform_device
>*pdev)
> 	else if (!gpio_is_valid(phy_reset))
> 		return 0;
>
>+	err = of_property_read_u32(np, "phy-reset-post-delay",
>&phy_post_delay);
>+	/* valid reset duration should be less than 1s */
>+	if (!err && phy_post_delay > 1000)
>+		return -EINVAL;
>+
> 	active_high = of_property_read_bool(np, "phy-reset-active-high");
>
> 	err = devm_gpio_request_one(&pdev->dev, phy_reset, @@ -3226,6
>+3231,15 @@ static int fec_reset_phy(struct platform_device *pdev)
>
> 	gpio_set_value_cansleep(phy_reset, !active_high);
>
>+	if (!phy_post_delay)
>+		return 0;
>+
>+	if (phy_post_delay > 20)
>+		msleep(phy_post_delay);
>+	else
>+		usleep_range(phy_post_delay * 1000,
>+			     phy_post_delay * 1000 + 1000);
>+
> 	return 0;
> }
> #else /* CONFIG_OF */
>--
>2.11.0

It looks fine.
Acked-by: Fugang Duan <fugang.duan-3arQi8VN3Tc@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: fec: add post PHY reset delay DT property

[David Miller]

From: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 May 2017 11:48:08 +0200

> Some PHY require to wait for a bit after the reset GPIO has been
> toggled. This adds support for the DT property `phy-reset-post-delay`
> which gives the delay in milliseconds to wait after reset.
> 
> If the DT property is not given, no delay is observed. Post reset delay
> greater than 1000ms are invalid.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: fec: add post PHY reset delay DT property

[Rob Herring]

On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
> Some PHY require to wait for a bit after the reset GPIO has been
> toggled. This adds support for the DT property `phy-reset-post-delay`
> which gives the delay in milliseconds to wait after reset.
> 
> If the DT property is not given, no delay is observed. Post reset delay
> greater than 1000ms are invalid.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> 
> v2:
>   - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>   instead of defaulting to 1ms,
>   - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>   binding doc and commit log,
>   - move phy-reset-post-delay property reading before
>   devm_gpio_request_one(),
> 
>  Documentation/devicetree/bindings/net/fsl-fec.txt |  4 ++++
>  drivers/net/ethernet/freescale/fec_main.c         | 16 +++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index a1e3693cca16..6f55bdd52f8a 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -15,6 +15,10 @@ Optional properties:
>  - phy-reset-active-high : 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-reset-post-delay : Post reset delay in milliseconds. If present then

This needs unit suffix minimally. It should also have a vendor prefix or 
be made generic.

But really, this is a property of the phy and should be in the phy node 
as should phy-reset-gpios, phy-reset-active-high, phy-supply, etc.

> +  a delay of phy-reset-post-delay milliseconds will be observed after the
> +  phy-reset-gpios has been toggled. Can be omitted thus no delay is
> +  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
>  - 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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] net: fec: add post PHY reset delay DT property

[Andy Duan]

From: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Sent: Thursday, June 01, 2017 12:44 AM
>On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
>> Some PHY require to wait for a bit after the reset GPIO has been
>> toggled. This adds support for the DT property `phy-reset-post-delay`
>> which gives the delay in milliseconds to wait after reset.
>>
>> If the DT property is not given, no delay is observed. Post reset
>> delay greater than 1000ms are invalid.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>
>> v2:
>>   - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>>   instead of defaulting to 1ms,
>>   - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>>   binding doc and commit log,
>>   - move phy-reset-post-delay property reading before
>>   devm_gpio_request_one(),
>>
>>  Documentation/devicetree/bindings/net/fsl-fec.txt |  4 ++++
>>  drivers/net/ethernet/freescale/fec_main.c         | 16 +++++++++++++++-
>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>> b/Documentation/devicetree/bindings/net/fsl-fec.txt
>> index a1e3693cca16..6f55bdd52f8a 100644
>> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>> @@ -15,6 +15,10 @@ Optional properties:
>>  - phy-reset-active-high : 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-reset-post-delay : Post reset delay in milliseconds. If present
>> +then
>
>This needs unit suffix minimally. It should also have a vendor prefix or be
>made generic.
>
>But really, this is a property of the phy and should be in the phy node as
>should phy-reset-gpios, phy-reset-active-high, phy-supply, etc.
>
Yes, it is better to make it general.
Last year, Uwe Kleine-König's patch "Commit da47b4572056 ("phy: add support for a reset-gpio specification")" did this, but it was reverted by commit 948350140ef0 (Revert "phy: add support for a reset-gpio specification").  And in all phy device driver, only at803x.c add the gpio reset in currently. 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] net: fec: add post PHY reset delay DT property

[Florian Fainelli]

Le 05/31/17 à 18:39, Andy Duan a écrit :
> From: Rob Herring <robh@kernel.org> Sent: Thursday, June 01, 2017 12:44 AM
>> On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
>>> Some PHY require to wait for a bit after the reset GPIO has been
>>> toggled. This adds support for the DT property `phy-reset-post-delay`
>>> which gives the delay in milliseconds to wait after reset.
>>>
>>> If the DT property is not given, no delay is observed. Post reset
>>> delay greater than 1000ms are invalid.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>>> ---
>>>
>>> v2:
>>>   - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>>>   instead of defaulting to 1ms,
>>>   - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>>>   binding doc and commit log,
>>>   - move phy-reset-post-delay property reading before
>>>   devm_gpio_request_one(),
>>>
>>>  Documentation/devicetree/bindings/net/fsl-fec.txt |  4 ++++
>>>  drivers/net/ethernet/freescale/fec_main.c         | 16 +++++++++++++++-
>>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> b/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> index a1e3693cca16..6f55bdd52f8a 100644
>>> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>>> @@ -15,6 +15,10 @@ Optional properties:
>>>  - phy-reset-active-high : 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-reset-post-delay : Post reset delay in milliseconds. If present
>>> +then
>>
>> This needs unit suffix minimally. It should also have a vendor prefix or be
>> made generic.
>>
>> But really, this is a property of the phy and should be in the phy node as
>> should phy-reset-gpios, phy-reset-active-high, phy-supply, etc.
>>
> Yes, it is better to make it general.
> Last year, Uwe Kleine-König's patch "Commit da47b4572056 ("phy: add support for a reset-gpio specification")" did this, but it was reverted by commit 948350140ef0 (Revert "phy: add support for a reset-gpio specification").  And in all phy device driver, only at803x.c add the gpio reset in currently. 

Getting the binding correct does not prevent us from later moving this
reset code into PHYLIB where it's appropriate. In fact; a correct and
generic binding proposed for FEC here could be used as a basis for all
other MAC and PHY drivers.
-- 
Florian

RE: [PATCH v2] net: fec: add post PHY reset delay DT property

[Andy Duan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3374 bytes --]

From: Florian Fainelli <f.fainelli@gmail.com> Sent: Thursday, June 01, 2017 9:53 AM
>To: Andy Duan <fugang.duan@nxp.com>; Rob Herring <robh@kernel.org>;
>Quentin Schulz <quentin.schulz@free-electrons.com>
>Cc: mark.rutland@arm.com; netdev@vger.kernel.org;
>devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>thomas.petazzoni@free-electrons.com
>Subject: Re: [PATCH v2] net: fec: add post PHY reset delay DT property
>
>Le 05/31/17 à 18:39, Andy Duan a écrit :
>> From: Rob Herring <robh@kernel.org> Sent: Thursday, June 01, 2017
>> 12:44 AM
>>> On Tue, May 23, 2017 at 11:48:08AM +0200, Quentin Schulz wrote:
>>>> Some PHY require to wait for a bit after the reset GPIO has been
>>>> toggled. This adds support for the DT property
>>>> `phy-reset-post-delay` which gives the delay in milliseconds to wait after
>reset.
>>>>
>>>> If the DT property is not given, no delay is observed. Post reset
>>>> delay greater than 1000ms are invalid.
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>>>> ---
>>>>
>>>> v2:
>>>>   - return -EINVAL when phy-reset-post-delay is greater than 1000ms
>>>>   instead of defaulting to 1ms,
>>>>   - remove `default to 1ms` when phy-reset-post-delay > 1000Ms from DT
>>>>   binding doc and commit log,
>>>>   - move phy-reset-post-delay property reading before
>>>>   devm_gpio_request_one(),
>>>>
>>>>  Documentation/devicetree/bindings/net/fsl-fec.txt |  4 ++++
>>>>  drivers/net/ethernet/freescale/fec_main.c         | 16 +++++++++++++++-
>>>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>>>> b/Documentation/devicetree/bindings/net/fsl-fec.txt
>>>> index a1e3693cca16..6f55bdd52f8a 100644
>>>> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>>>> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>>>> @@ -15,6 +15,10 @@ Optional properties:
>>>>  - phy-reset-active-high : 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-reset-post-delay : Post reset delay in milliseconds. If
>>>> +present then
>>>
>>> This needs unit suffix minimally. It should also have a vendor prefix
>>> or be made generic.
>>>
>>> But really, this is a property of the phy and should be in the phy
>>> node as should phy-reset-gpios, phy-reset-active-high, phy-supply, etc.
>>>
>> Yes, it is better to make it general.
>> Last year, Uwe Kleine-König's patch "Commit da47b4572056 ("phy: add
>support for a reset-gpio specification")" did this, but it was reverted by
>commit 948350140ef0 (Revert "phy: add support for a reset-gpio
>specification").  And in all phy device driver, only at803x.c add the gpio reset in
>currently.
>
>Getting the binding correct does not prevent us from later moving this reset
>code into PHYLIB where it's appropriate. In fact; a correct and generic binding
>proposed for FEC here could be used as a basis for all other MAC and PHY
>drivers.
>--
>Florian

I agree with your opinion. Just hope to add the general phy reset interface in phylib and special device driver.

Andy
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i