Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <14cf568e-d7ee-886e-5122-69b2e58b8717@linaro.org>
Date:   Fri, 12 Aug 2022 14:25:42 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property
Content-Language: en-US
To:     Wei Fang <wei.fang@nxp.com>, "andrew@lunn.ch" <andrew@lunn.ch>,
        "hkallweit1@gmail.com" <hkallweit1@gmail.com>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20220812145009.1229094-1-wei.fang@nxp.com>
 <20220812145009.1229094-2-wei.fang@nxp.com>
 <0cd22a17-3171-b572-65fb-e9d3def60133@linaro.org>
 <DB9PR04MB81060AF4890DEA9E2378940288679@DB9PR04MB8106.eurprd04.prod.outlook.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <DB9PR04MB81060AF4890DEA9E2378940288679@DB9PR04MB8106.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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