Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[Rob Herring <robh@kernel.org>]

On Tue, Feb 25, 2025 at 10:44:53AM +0000, J. Neuschäfer wrote:
> On Fri, Feb 21, 2025 at 05:35:23PM -0600, Rob Herring wrote:
> > On Thu, Feb 20, 2025 at 06:29:23PM +0100, J. Neuschäfer wrote:
> > > Add a binding for the "Gianfar" ethernet controller, also known as
> > > TSEC/eTSEC.
> > > 
> > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > > ---
> > >  .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
> > >  .../devicetree/bindings/net/fsl-tsec-phy.txt       |  39 +---
> > >  2 files changed, 243 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..dc75ceb5dc6fdee8765bb17273f394d01cce0710
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> > > @@ -0,0 +1,242 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/fsl,gianfar.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Freescale Three-Speed Ethernet Controller (TSEC), "Gianfar"
> [...]
> > > +  "#address-cells": true
> > 
> > enum: [ 1, 2 ]
> > 
> > because 3 is not valid here.
> > 
> > > +
> > > +  "#size-cells": true
> > 
> > enum: [ 1, 2 ]
> > 
> > because 0 is not valid here.
> 
> Good point.
> 
> > 
> > 
> > > +
> > > +  cell-index:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +  interrupts:
> > > +    maxItems: 3
> > 
> > Based on the if/then schema, you need 'minItems' here if the min is not 3.
> > 
> > Really, move the descriptions here and make them work for the combined 
> > interrupt case (just a guess).
> 
> The difference here (as previously documented in prose) is by device
> variant:
> 
>  for FEC:
> 
>    - one combined interrupt
> 
>  for TSEC, eTSEC:
> 
>    - transmit interrupt
>    - receive interrupt
>    - error interrupt
> 
> Combining these cases might look like this, not sure if it's good:
> 
>   interrupts:
>     minItems: 1
>     description:
>       items:
>         - Transmit interrupt or combined interrupt
>         - Receive interrupt
>         - Error interrupt

Yep, that's good. I would say 'single combined' to make it abundantly 
clear.

Rob