Re: [net-next PATCH 1/2] dt-bindings: Document QCA808x PHYs

[Christian Marangi <ansuelsmth@gmail.com>]

On Mon, Dec 11, 2023 at 11:19:50AM +0100, Krzysztof Kozlowski wrote:
> On 09/12/2023 02:48, Christian Marangi wrote:
> > Add Documentation for QCA808x PHYs for the additional property for the
> > active high LED setting and also document the LED configuration for this
> > PHY.
> > 
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
>

Yes sorry.

> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/net/qca,qca808x.yaml  | 66 +++++++++++++++++++
> >  1 file changed, 66 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/qca,qca808x.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/qca,qca808x.yaml b/Documentation/devicetree/bindings/net/qca,qca808x.yaml
> > new file mode 100644
> > index 000000000000..73cfff357311
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/qca,qca808x.yaml
> > @@ -0,0 +1,66 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> 
> Dual license as checkpath and writing-bindings ask.
> 

Oh didn't notice the warning.

> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/qca,qca808x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Atheros QCA808X PHY
> > +
> > +maintainers:
> > +  - Christian Marangi <ansuelsmth@gmail.com>
> > +
> > +description:
> > +  Bindings for Qualcomm Atheros QCA808X PHYs
> 
> Drop "Bindings for" and then entire sentence seems not useful.
> 

Was following the pattern used for other qcom PHY. Ok will drop!

> > +
> > +  QCA808X PHYs can have up to 3 LEDs attached.
> > +  All 3 LEDs are disabled by default.
> > +  2 LEDs have dedicated pins with the 3rd LED having the
> > +  double function of Interrupt LEDs/GPIO or additional LED.
> > +
> > +  By default this special PIN is set to LED function.
> > +
> > +allOf:
> > +  - $ref: ethernet-phy.yaml#
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - ethernet-phy-id004d.d101
> 
> I have impression that this is continuation of some other patchset...
> Anyway, id004d.d101 is specific to QCA808x?
> 

I used enum assuming eventually more qca808x PHY will come... Yes that
ID is specific and it's the id of QCA8081. Better to use const?

> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  qca,led-active-high:
> > +    description: Set all the LEDs to active high to be turned on.
> > +    type: boolean
> 
> 
> Best regards,
> Krzysztof
> 

-- 
	Ansuel