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

[Florian Fainelli <f.fainelli@gmail.com>]

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