[PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count

Yassine Oudjana posted 10 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count
Posted by Yassine Oudjana 1 year, 11 months ago
From: Yassine Oudjana <y.oudjana@protonmail.com>

The document currently states a maximum of 1 interrupt, but the DT
has 2 specified causing a dtbs_check error. Replace the maximum limit
with a minimum and add per-interrupt descriptions to pass the check.

Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 .../devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
index 73ae6e11410b..a3a3f7fb9605 100644
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
@@ -47,7 +47,10 @@ properties:
 
   interrupts:
     description: The interrupt outputs to sysirq.
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: EINT interrupt
+      - description: EINT event_b interrupt
 
 # PIN CONFIGURATION NODES
 patternProperties:
-- 
2.38.0
Re: [PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count
Posted by Krzysztof Kozlowski 1 year, 11 months ago
On 07/10/2022 08:58, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> The document currently states a maximum of 1 interrupt, but the DT
> has 2 specified causing a dtbs_check error. Replace the maximum limit
> with a minimum and add per-interrupt descriptions to pass the check.
> 
> Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>  .../devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> index 73ae6e11410b..a3a3f7fb9605 100644
> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> @@ -47,7 +47,10 @@ properties:
>  
>    interrupts:
>      description: The interrupt outputs to sysirq.
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: EINT interrupt
> +      - description: EINT event_b interrupt

Is second interrupt really optional or you just wanted to silence the
warning?

Best regards,
Krzysztof
Re: [PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count
Posted by AngeloGioacchino Del Regno 1 year, 11 months ago
Il 10/10/22 13:13, Krzysztof Kozlowski ha scritto:
> On 07/10/2022 08:58, Yassine Oudjana wrote:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> The document currently states a maximum of 1 interrupt, but the DT
>> has 2 specified causing a dtbs_check error. Replace the maximum limit
>> with a minimum and add per-interrupt descriptions to pass the check.
>>
>> Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> ---
>>   .../devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
>> index 73ae6e11410b..a3a3f7fb9605 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
>> @@ -47,7 +47,10 @@ properties:
>>   
>>     interrupts:
>>       description: The interrupt outputs to sysirq.
>> -    maxItems: 1
>> +    minItems: 1
>> +    items:
>> +      - description: EINT interrupt
>> +      - description: EINT event_b interrupt
> 
> Is second interrupt really optional or you just wanted to silence the
> warning?
> 

The event_b interrupt exists (and fires on certain events, if configured to do so),
but it's currently unused.

It's really optional.

> Best regards,
> Krzysztof
>
Re: [PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count
Posted by Rob Herring 1 year, 11 months ago
On Mon, Oct 10, 2022 at 01:47:18PM +0200, AngeloGioacchino Del Regno wrote:
> Il 10/10/22 13:13, Krzysztof Kozlowski ha scritto:
> > On 07/10/2022 08:58, Yassine Oudjana wrote:
> > > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > > 
> > > The document currently states a maximum of 1 interrupt, but the DT
> > > has 2 specified causing a dtbs_check error. Replace the maximum limit
> > > with a minimum and add per-interrupt descriptions to pass the check.
> > > 
> > > Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> > > ---
> > >   .../devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> > > index 73ae6e11410b..a3a3f7fb9605 100644
> > > --- a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> > > +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
> > > @@ -47,7 +47,10 @@ properties:
> > >     interrupts:
> > >       description: The interrupt outputs to sysirq.
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    items:
> > > +      - description: EINT interrupt
> > > +      - description: EINT event_b interrupt
> > 
> > Is second interrupt really optional or you just wanted to silence the
> > warning?
> > 
> 
> The event_b interrupt exists (and fires on certain events, if configured to do so),
> but it's currently unused.
> 
> It's really optional.

Optional for DT means may or may not be wired up in the h/w, not what 
some OS 'currently' uses.

However, you can't really add new required properties or entries to an 
existing DT without breaking compatibility. Maybe that is not yet a 
concern.

Rob
Re: [PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count
Posted by AngeloGioacchino Del Regno 1 year, 11 months ago
Il 10/10/22 14:57, Rob Herring ha scritto:
> On Mon, Oct 10, 2022 at 01:47:18PM +0200, AngeloGioacchino Del Regno wrote:
>> Il 10/10/22 13:13, Krzysztof Kozlowski ha scritto:
>>> On 07/10/2022 08:58, Yassine Oudjana wrote:
>>>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>>>
>>>> The document currently states a maximum of 1 interrupt, but the DT
>>>> has 2 specified causing a dtbs_check error. Replace the maximum limit
>>>> with a minimum and add per-interrupt descriptions to pass the check.
>>>>
>>>> Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>>>> ---
>>>>    .../devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
>>>> index 73ae6e11410b..a3a3f7fb9605 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,pinctrl-mt6795.yaml
>>>> @@ -47,7 +47,10 @@ properties:
>>>>      interrupts:
>>>>        description: The interrupt outputs to sysirq.
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    items:
>>>> +      - description: EINT interrupt
>>>> +      - description: EINT event_b interrupt
>>>
>>> Is second interrupt really optional or you just wanted to silence the
>>> warning?
>>>
>>
>> The event_b interrupt exists (and fires on certain events, if configured to do so),
>> but it's currently unused.
>>
>> It's really optional.
> 
> Optional for DT means may or may not be wired up in the h/w, not what
> some OS 'currently' uses.
> 
> However, you can't really add new required properties or entries to an
> existing DT without breaking compatibility. Maybe that is not yet a
> concern.
> 

Right. I don't think that compatibility is a concern in this case anyway, but I
just noticed that this commit has no Fixes tag, even though it is indeed fixing
the binding, as mt6795 does have this interrupt and its devicetree does already
define the second interrupt in the pinctrl node.

Yassine, can you please resend this with a Fixes tag?

Fixes: 81557a71564a ("dt-bindings: pinctrl: Add MediaTek MT6795 pinctrl bindings")
Re: [PATCH v3 05/10] dt-bindings: pinctrl: mediatek,pinctrl-mt6795: Fix interrupt count
Posted by AngeloGioacchino Del Regno 1 year, 11 months ago
Il 07/10/22 14:58, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> The document currently states a maximum of 1 interrupt, but the DT
> has 2 specified causing a dtbs_check error. Replace the maximum limit
> with a minimum and add per-interrupt descriptions to pass the check.
> 
> Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>