RE: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property

Wei Fang
2022-08-12T09:02:28Z

> -----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.

> Best regards,
> Krzysztof