diff mbox series

[net-next,v5,6/9] dt-bindings: net: snps,dwmac: add safety irq support

Message ID 20230817165749.672-7-jszhang@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: stmmac: add new features to xgmac | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 9 this patch: 9
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 28 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jisheng Zhang Aug. 17, 2023, 4:57 p.m. UTC
The snps dwmac IP support safety features, and those Safety Feature
Correctible Error and Uncorrectible Error irqs may be separate irqs.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 .../devicetree/bindings/net/snps,dwmac.yaml         | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Serge Semin Aug. 18, 2023, 5:39 p.m. UTC | #1
On Fri, Aug 18, 2023 at 12:57:46AM +0800, Jisheng Zhang wrote:
> The snps dwmac IP support safety features, and those Safety Feature
> Correctible Error and Uncorrectible Error irqs may be separate irqs.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml         | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index ddf9522a5dc2..ee9174f77d97 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -103,17 +103,26 @@ properties:
>  
>    interrupts:
>      minItems: 1
> +    maxItems: 5
> +    additionalItems: true
>      items:
>        - description: Combined signal for various interrupt events
>        - description: The interrupt to manage the remote wake-up packet detection
>        - description: The interrupt that occurs when Rx exits the LPI state
> +      - description: The interrupt that occurs when Safety Feature Correctible Errors happen
> +      - description: The interrupt that occurs when Safety Feature Uncorrectible Errors happen
>  
>    interrupt-names:
>      minItems: 1
> +    maxItems: 5
> +    additionalItems: true
>      items:
>        - const: macirq
> -      - enum: [eth_wake_irq, eth_lpi]
> -      - const: eth_lpi
> +      - enum:
> +          - eth_wake_irq
> +          - eth_lpi
> +          - sfty_ce
> +          - sfty_ue

IIUC this would mean the next constraints:
Item    0: must be macirq,
Item    1: any of eth_wake_irq, eth_lpi, sfty_ce, sfty_ue
Items 2:4: any bla-bla-bla.

After adding the per-DMA-channel IRQs in the next patches the array
will be extended to up to 37 any names. It doesn't look correct. What
about converting it to the position independent arrays constraint:

  interrupts:
    minItems: 1
    maxItems: 34

    
  interrupt-names:
    minItems: 1
    maxItems: 34
    items:
      oneOf:
        - description: Combined signal for various interrupt events
          const: macirq
        - description: The interrupt to manage the remote wake-up packet detection
          const: eth_wake_irq
        - description: The interrupt that occurs when Rx exits the LPI state
          const: eth_lpi
        - description: Safety Feature Correctible Errors interrupt
          const: sfty_ce
        - description: Safety Feature Uncorrectible Errors interrupt
          const: sfty_ue
        - description: DMA Tx per-channel interrupt
          pattern: '^dma_tx([0-9]|1[0-5])?$'
        - description: DMA Rx per-channel interrupt
          pattern: '^dma_rx([0-9]|1[0-1])?$'

    allOf:
      - contains:
          const: macirq

Hope neither Krzysztof nor Rob will be against such modification
especially seeing it's the only way to resolve the very much possible
case of a device having macirq and per-DMA-channel IRQs but lacking
the LPI, PMT or Safety IRQs.

Note 1. I've changed the name of the Tx/Rx IRQs to having the "dma_"
suffix to signify that these are actually DMA-channel IRQs. The MTL
queue interrupts drive the sbd_intr_o signal which is tracked by the
"macirq" line.

Note 2. I've reduced the number of DMA Rx IRQs to being up to 12 in
accordance with available to me DW XGMAC HW manual.

-Serge(y)

>  
>    clocks:
>      minItems: 1
> -- 
> 2.40.1
> 
>
Rob Herring Aug. 21, 2023, 8:33 p.m. UTC | #2
On Fri, Aug 18, 2023 at 08:39:56PM +0300, Serge Semin wrote:
> On Fri, Aug 18, 2023 at 12:57:46AM +0800, Jisheng Zhang wrote:
> > The snps dwmac IP support safety features, and those Safety Feature
> > Correctible Error and Uncorrectible Error irqs may be separate irqs.
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml         | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index ddf9522a5dc2..ee9174f77d97 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -103,17 +103,26 @@ properties:
> >  
> >    interrupts:
> >      minItems: 1
> > +    maxItems: 5
> > +    additionalItems: true
> >      items:
> >        - description: Combined signal for various interrupt events
> >        - description: The interrupt to manage the remote wake-up packet detection
> >        - description: The interrupt that occurs when Rx exits the LPI state
> > +      - description: The interrupt that occurs when Safety Feature Correctible Errors happen
> > +      - description: The interrupt that occurs when Safety Feature Uncorrectible Errors happen
> >  
> >    interrupt-names:
> >      minItems: 1
> > +    maxItems: 5
> > +    additionalItems: true
> >      items:
> >        - const: macirq
> > -      - enum: [eth_wake_irq, eth_lpi]
> > -      - const: eth_lpi
> > +      - enum:
> > +          - eth_wake_irq
> > +          - eth_lpi
> > +          - sfty_ce
> > +          - sfty_ue
> 
> IIUC this would mean the next constraints:
> Item    0: must be macirq,
> Item    1: any of eth_wake_irq, eth_lpi, sfty_ce, sfty_ue
> Items 2:4: any bla-bla-bla.

Indeed.

> 
> After adding the per-DMA-channel IRQs in the next patches the array
> will be extended to up to 37 any names. It doesn't look correct. What
> about converting it to the position independent arrays constraint:
> 
>   interrupts:
>     minItems: 1
>     maxItems: 34
> 
>     
>   interrupt-names:
>     minItems: 1
>     maxItems: 34
>     items:
>       oneOf:
>         - description: Combined signal for various interrupt events
>           const: macirq
>         - description: The interrupt to manage the remote wake-up packet detection
>           const: eth_wake_irq
>         - description: The interrupt that occurs when Rx exits the LPI state
>           const: eth_lpi
>         - description: Safety Feature Correctible Errors interrupt
>           const: sfty_ce
>         - description: Safety Feature Uncorrectible Errors interrupt
>           const: sfty_ue
>         - description: DMA Tx per-channel interrupt
>           pattern: '^dma_tx([0-9]|1[0-5])?$'
>         - description: DMA Rx per-channel interrupt
>           pattern: '^dma_rx([0-9]|1[0-1])?$'
> 
>     allOf:
>       - contains:
>           const: macirq

This would keep macirq being first:

allOf:
  - maxItems: 34
    items:
      - const: macirq

In newer json-schema the schema and list versions were split into 
"prefixItems" and "items", so we could avoid the "allOf" with that. 
Unfortunately, the former is the list version which we use everywhere. I 
don't really want to do a treewide change of that and also I find the 
'prefixItems' name kind of awkward.


> Hope neither Krzysztof nor Rob will be against such modification
> especially seeing it's the only way to resolve the very much possible
> case of a device having macirq and per-DMA-channel IRQs but lacking
> the LPI, PMT or Safety IRQs.

Don't love it, but I give up on these licensed IPs. :(

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index ddf9522a5dc2..ee9174f77d97 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -103,17 +103,26 @@  properties:
 
   interrupts:
     minItems: 1
+    maxItems: 5
+    additionalItems: true
     items:
       - description: Combined signal for various interrupt events
       - description: The interrupt to manage the remote wake-up packet detection
       - description: The interrupt that occurs when Rx exits the LPI state
+      - description: The interrupt that occurs when Safety Feature Correctible Errors happen
+      - description: The interrupt that occurs when Safety Feature Uncorrectible Errors happen
 
   interrupt-names:
     minItems: 1
+    maxItems: 5
+    additionalItems: true
     items:
       - const: macirq
-      - enum: [eth_wake_irq, eth_lpi]
-      - const: eth_lpi
+      - enum:
+          - eth_wake_irq
+          - eth_lpi
+          - sfty_ce
+          - sfty_ue
 
   clocks:
     minItems: 1