[net-next,v3,2/6] dt-bindings: net: lan966x: Extend with the analyzer interrupt

Extend dt-bindings for lan966x with analyzer interrupt.
This interrupt can be generated for example when the HW learn/forgets
an entry in the MAC table.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 .../devicetree/bindings/net/microchip,lan966x-switch.yaml       | 2 ++
 1 file changed, 2 insertions(+)

On Thu, Dec 09, 2021 at 10:46:11AM +0100, Horatiu Vultur wrote:
> Extend dt-bindings for lan966x with analyzer interrupt.
> This interrupt can be generated for example when the HW learn/forgets
> an entry in the MAC table.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Why don't you describe your hardware in the device tree all at once?
Doing it piece by piece means that every time when you add a new
functionality you need to be compatible with the absence of a certain
reg, interrupt etc.

>  .../devicetree/bindings/net/microchip,lan966x-switch.yaml       | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> index 5bee665d5fcf..e79e4e166ad8 100644
> --- a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> +++ b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> @@ -37,12 +37,14 @@ properties:
>      items:
>        - description: register based extraction
>        - description: frame dma based extraction
> +      - description: analyzer interrupt
>  
>    interrupt-names:
>      minItems: 1
>      items:
>        - const: xtr
>        - const: fdma
> +      - const: ana
>  
>    resets:
>      items:
> -- 
> 2.33.0
>

The 12/09/2021 10:58, Vladimir Oltean wrote:
> 
> On Thu, Dec 09, 2021 at 10:46:11AM +0100, Horatiu Vultur wrote:
> > Extend dt-bindings for lan966x with analyzer interrupt.
> > This interrupt can be generated for example when the HW learn/forgets
> > an entry in the MAC table.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> 
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Why don't you describe your hardware in the device tree all at once?
> Doing it piece by piece means that every time when you add a new
> functionality you need to be compatible with the absence of a certain
> reg, interrupt etc.

I though it is more clear what is added in the patch series.
But then, if for example add more interrupts in DT than what the
driver support, that would not be an issue?

> 
> >  .../devicetree/bindings/net/microchip,lan966x-switch.yaml       | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > index 5bee665d5fcf..e79e4e166ad8 100644
> > --- a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > +++ b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > @@ -37,12 +37,14 @@ properties:
> >      items:
> >        - description: register based extraction
> >        - description: frame dma based extraction
> > +      - description: analyzer interrupt
> >
> >    interrupt-names:
> >      minItems: 1
> >      items:
> >        - const: xtr
> >        - const: fdma
> > +      - const: ana
> >
> >    resets:
> >      items:
> > --
> > 2.33.0
> >

On Thu, Dec 09, 2021 at 04:42:47PM +0100, Horatiu Vultur wrote:
> The 12/09/2021 10:58, Vladimir Oltean wrote:
> > 
> > On Thu, Dec 09, 2021 at 10:46:11AM +0100, Horatiu Vultur wrote:
> > > Extend dt-bindings for lan966x with analyzer interrupt.
> > > This interrupt can be generated for example when the HW learn/forgets
> > > an entry in the MAC table.
> > >
> > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > > ---
> > 
> > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > Why don't you describe your hardware in the device tree all at once?
> > Doing it piece by piece means that every time when you add a new
> > functionality you need to be compatible with the absence of a certain
> > reg, interrupt etc.
> 
> I though it is more clear what is added in the patch series.
> But then, if for example add more interrupts in DT than what the
> driver support, that would not be an issue?

I haven't kept track of the lan966x driver development. It looks like it
is pretty new, so I think it's ok in this case. But I've also seen
features introduced years after the driver was initially published (see
ocelot fdma) where device tree updates were still necessary, due to
minor things like these: an interrupt isn't there, the registers for
FDMA aren't there, etc. After that kind of time you'd expect the DT
to no longer require updates unless there is some unforeseen event
(something is broken, a driver is radically rethought). Sure there's a
fine line between how much you can add to the device tree and the
how many consumers there are in the kernel, but on the other hand the
kernel doesn't have to use everything that's in the device tree.
For example, at Mark Brown's suggestion, the DSPI nodes in ls1028a.dtsi
declare their DMA channels even though the driver does not use them
(it could, though, but it would be slower). Similarly, the DSPI driver
for LS1021A has had a while in which it ignored the interrupt line from
the device tree, because poll mode was simply faster. To me, this kind
of approach where the device tree is provisioned even for configurations
that aren't supported today makes sense, precisely because the DT blob
and the kernel have different lifetimes. It's better to have the
interrupt and not use it than to need it and not have it.