lkml.org 
[lkml]   [2022]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property
    From
    On 12/08/2022 12:02, Wei Fang wrote:
    >
    >
    >> -----Original Message-----
    >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
    >> Sent: 2022年8月12日 15:28
    >> To: Wei Fang <wei.fang@nxp.com>; andrew@lunn.ch; hkallweit1@gmail.com;
    >> linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com;
    >> kuba@kernel.org; pabeni@redhat.com; robh+dt@kernel.org;
    >> krzysztof.kozlowski+dt@linaro.org; f.fainelli@gmail.com;
    >> netdev@vger.kernel.org; devicetree@vger.kernel.org;
    >> linux-kernel@vger.kernel.org
    >> Subject: Re: [PATCH net 1/2] dt: ar803x: Document disable-hibernation
    >> property
    >>
    >> On 12/08/2022 17:50, wei.fang@nxp.com wrote:
    >>> From: Wei Fang <wei.fang@nxp.com>
    >>>
    >>
    >> Please use subject prefix matching subsystem.
    >>
    > Ok, I'll add the subject prefix.
    >
    >>> The hibernation mode of Atheros AR803x PHYs is default enabled.
    >>> When the cable is unplugged, the PHY will enter hibernation mode and
    >>> the PHY clock does down. For some MACs, it needs the clock to support
    >>> it's logic. For instance, stmmac needs the PHY inputs clock is present
    >>> for software reset completion. Therefore, It is reasonable to add a DT
    >>> property to disable hibernation mode.
    >>>
    >>> Signed-off-by: Wei Fang <wei.fang@nxp.com>
    >>> ---
    >>> Documentation/devicetree/bindings/net/qca,ar803x.yaml | 6 ++++++
    >>> 1 file changed, 6 insertions(+)
    >>>
    >>> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
    >>> b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
    >>> index b3d4013b7ca6..d08431d79b83 100644
    >>> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
    >>> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
    >>> @@ -40,6 +40,12 @@ properties:
    >>> Only supported on the AR8031.
    >>> type: boolean
    >>>
    >>> + qca,disable-hibernation:
    >>> + description: |
    >>> + If set, the PHY will not enter hibernation mode when the cable is
    >>> + unplugged.
    >>
    >> Wrong indentation. Did you test the bindings?
    >>
    > Sorry, I just checked the patch and forgot to check the dt-bindings.
    >
    >> Unfortunately the property describes driver behavior not hardware, so it is not
    >> suitable for DT. Instead describe the hardware
    >> characteristics/features/bugs/constraints. Not driver behavior. Both in property
    >> name and property description.
    >>
    > Thanks for your review and feedback. Actually, the hibernation mode is a feature of hardware, I will modify the property name and description to be more in line with the requirements of the DT property.

    hibernation is a feature, but 'disable-hibernation' is not. DTS
    describes the hardware, not policy or driver bejhvior. Why disabling
    hibernation is a property of hardware? How you described, it's not,
    therefore either property is not for DT or it has to be phrased
    correctly to describe the hardware.

    Best regards,
    Krzysztof

    \
     
     \ /
      Last update: 2022-08-12 13:26    [W:4.250 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site