[PATCH 09/10] dt-bindings: rtc: pcf2127: add PCF2131

[PATCH 09/10] dt-bindings: rtc: pcf2127: add PCF2131

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Add support for new NXP RTC PCF2131.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
index cde7b1675ead..57eb0a58afa3 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
@@ -14,7 +14,9 @@ maintainers:
 
 properties:
   compatible:
-    const: nxp,pcf2127
+    enum:
+      - nxp,pcf2127
+      - nxp,pcf2131
 
   reg:
     maxItems: 1
@@ -48,4 +50,19 @@ examples:
         };
     };
 
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@53 {
+            compatible = "nxp,pcf2131";
+            reg = <0x53>;
+            pinctrl-0 = <&rtc_nint_pins>;
+            interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
+            reset-source;
+        };
+    };
+
 ...
-- 
2.30.2

[PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

The PCF2131 has two output interrupt pins, named INT_A and INT_B.

Add properties to identify onto which pin we want the alarm interrupt
to be routed. It can be either one, or both.

These properties are automatically set to false for variants other
than PCF2131 (ex: PCF2127).

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
index 57eb0a58afa3..83656dd2f97f 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
@@ -24,6 +24,16 @@ properties:
   interrupts:
     maxItems: 1
 
+  alarm-output-a:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Enable alarm interrupt on INT_A output pin.
+
+  alarm-output-b:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Enable alarm interrupt on INT_B output pin.
+
   start-year: true
 
   reset-source: true
@@ -32,6 +42,18 @@ required:
   - compatible
   - reg
 
+if:
+  not:
+    properties:
+      compatible:
+        contains:
+          enum:
+            - nxp,pcf2131
+then:
+  properties:
+    alarm-output-a: false
+    alarm-output-b: false
+
 additionalProperties: false
 
 examples:
@@ -62,6 +84,7 @@ examples:
             pinctrl-0 = <&rtc_nint_pins>;
             interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
             reset-source;
+            alarm-output-b;
         };
     };
 
-- 
2.30.2

Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Rob Herring]

On Tue, Jan 25, 2022 at 03:00:09PM -0500, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> The PCF2131 has two output interrupt pins, named INT_A and INT_B.
> 
> Add properties to identify onto which pin we want the alarm interrupt
> to be routed. It can be either one, or both.
> 
> These properties are automatically set to false for variants other
> than PCF2131 (ex: PCF2127).
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> index 57eb0a58afa3..83656dd2f97f 100644
> --- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> @@ -24,6 +24,16 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  alarm-output-a:

nxp,alarm-output-a

> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Enable alarm interrupt on INT_A output pin.
> +
> +  alarm-output-b:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Enable alarm interrupt on INT_B output pin.
> +
>    start-year: true
>  
>    reset-source: true
> @@ -32,6 +42,18 @@ required:
>    - compatible
>    - reg
>  
> +if:
> +  not:
> +    properties:
> +      compatible:
> +        contains:
> +          enum:
> +            - nxp,pcf2131
> +then:
> +  properties:
> +    alarm-output-a: false
> +    alarm-output-b: false
> +
>  additionalProperties: false
>  
>  examples:
> @@ -62,6 +84,7 @@ examples:
>              pinctrl-0 = <&rtc_nint_pins>;
>              interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
>              reset-source;
> +            alarm-output-b;
>          };
>      };
>  
> -- 
> 2.30.2
> 
> 

Re: [PATCH 09/10] dt-bindings: rtc: pcf2127: add PCF2131

[Rob Herring]

On Tue, Jan 25, 2022 at 03:00:08PM -0500, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Add support for new NXP RTC PCF2131.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Hugo Villeneuve]

On Tue, 8 Feb 2022 21:20:28 -0600
Rob Herring <robh@kernel.org> wrote:

> On Tue, Jan 25, 2022 at 03:00:09PM -0500, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > The PCF2131 has two output interrupt pins, named INT_A and INT_B.
> > 
> > Add properties to identify onto which pin we want the alarm interrupt
> > to be routed. It can be either one, or both.
> > 
> > These properties are automatically set to false for variants other
> > than PCF2131 (ex: PCF2127).
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23 +++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > index 57eb0a58afa3..83656dd2f97f 100644
> > --- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > @@ -24,6 +24,16 @@ properties:
> >    interrupts:
> >      maxItems: 1
> >  
> > +  alarm-output-a:
> 
> nxp,alarm-output-a

Ok, this will be fixed for V2.

Thank you, Hugo.

-- 
Hugo Villeneuve <hugo@hugovil.com>

Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Alexandre Belloni]

On 10/02/2022 17:12:34-0500, Hugo Villeneuve wrote:
> On Tue, 8 Feb 2022 21:20:28 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Tue, Jan 25, 2022 at 03:00:09PM -0500, Hugo Villeneuve wrote:
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > 
> > > The PCF2131 has two output interrupt pins, named INT_A and INT_B.
> > > 
> > > Add properties to identify onto which pin we want the alarm interrupt
> > > to be routed. It can be either one, or both.
> > > 
> > > These properties are automatically set to false for variants other
> > > than PCF2131 (ex: PCF2127).
> > > 
> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > ---
> > >  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23 +++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > index 57eb0a58afa3..83656dd2f97f 100644
> > > --- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > @@ -24,6 +24,16 @@ properties:
> > >    interrupts:
> > >      maxItems: 1
> > >  
> > > +  alarm-output-a:
> > 
> > nxp,alarm-output-a
> 
> Ok, this will be fixed for V2.
> 

Actually, this property has to be made more generic and thought out.
There are multiple RTCs that have multiple interrupt pins where one of
the pin can be used for different interrupt or clock output.

With your binding, there is no way to separate which interrupt is going
to which pin and so there is no way to get the alarm and BLF or the
watchdog on different pins and we certainly don't want to have a
property per interrupt type.

Also, the documentation is missing the fact that the driver makes having
one of the property mandatory.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Hugo Villeneuve]

On Thu, 10 Feb 2022 23:32:32 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> On 10/02/2022 17:12:34-0500, Hugo Villeneuve wrote:
> > On Tue, 8 Feb 2022 21:20:28 -0600
> > Rob Herring <robh@kernel.org> wrote:
> > 
> > > On Tue, Jan 25, 2022 at 03:00:09PM -0500, Hugo Villeneuve wrote:
> > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > 
> > > > The PCF2131 has two output interrupt pins, named INT_A and INT_B.
> > > > 
> > > > Add properties to identify onto which pin we want the alarm interrupt
> > > > to be routed. It can be either one, or both.
> > > > 
> > > > These properties are automatically set to false for variants other
> > > > than PCF2131 (ex: PCF2127).
> > > > 
> > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > ---
> > > >  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23 +++++++++++++++++++
> > > >  1 file changed, 23 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > index 57eb0a58afa3..83656dd2f97f 100644
> > > > --- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > @@ -24,6 +24,16 @@ properties:
> > > >    interrupts:
> > > >      maxItems: 1
> > > >  
> > > > +  alarm-output-a:
> > > 
> > > nxp,alarm-output-a
> > 
> > Ok, this will be fixed for V2.
> > 
> 
> Actually, this property has to be made more generic and thought out.
> There are multiple RTCs that have multiple interrupt pins where one of
> the pin can be used for different interrupt or clock output.
> 
> With your binding, there is no way to separate which interrupt is going
> to which pin and so there is no way to get the alarm and BLF or the
> watchdog on different pins and we certainly don't want to have a
> property per interrupt type.

Hi,
can you please suggest how you would prefer it to be done?

> Also, the documentation is missing the fact that the driver makes having
> one of the property mandatory.

I will add it.

Thank you, Hugo.

-- 
Hugo Villeneuve <hugo@hugovil.com>

Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Hugo Villeneuve]

On Thu, 2022-02-10 at 17:55 -0500, Hugo Villeneuve wrote:
> On Thu, 10 Feb 2022 23:32:32 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > On 10/02/2022 17:12:34-0500, Hugo Villeneuve wrote:
> > > On Tue, 8 Feb 2022 21:20:28 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > > 
> > > > On Tue, Jan 25, 2022 at 03:00:09PM -0500, Hugo Villeneuve
> > > > wrote:
> > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > 
> > > > > The PCF2131 has two output interrupt pins, named INT_A and
> > > > > INT_B.
> > > > > 
> > > > > Add properties to identify onto which pin we want the alarm
> > > > > interrupt
> > > > > to be routed. It can be either one, or both.
> > > > > 
> > > > > These properties are automatically set to false for variants
> > > > > other
> > > > > than PCF2131 (ex: PCF2127).
> > > > > 
> > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > ---
> > > > >  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23
> > > > > +++++++++++++++++++
> > > > >  1 file changed, 23 insertions(+)
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > > b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > > index 57eb0a58afa3..83656dd2f97f 100644
> > > > > --- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > > > @@ -24,6 +24,16 @@ properties:
> > > > >    interrupts:
> > > > >      maxItems: 1
> > > > >  
> > > > > +  alarm-output-a:
> > > > 
> > > > nxp,alarm-output-a
> > > 
> > > Ok, this will be fixed for V2.
> > > 
> > 
> > Actually, this property has to be made more generic and thought
> > out.
> > There are multiple RTCs that have multiple interrupt pins where one
> > of
> > the pin can be used for different interrupt or clock output.

Hi,
the only example I could find of such a device is in rtc-pcf85363.c.
This device also has two interrupt pins, INT A/B, like the PCF2131.
However, in the pcf85363 driver, pin INT B is simply ignored, and all
interrupts are configured to go on INT A.

For the moment, I will simply modify my PCF2131 patches serie to mimic
the same behavior in V2. This simplifies things a lot, and support for
INT B pin could be added at a later stage (and also to pcf85363) if
anyone needs it (I don't).

Hugo.

> > With your binding, there is no way to separate which interrupt is
> > going
> > to which pin and so there is no way to get the alarm and BLF or the
> > watchdog on different pins and we certainly don't want to have a
> > property per interrupt type.
> 
> Hi,
> can you please suggest how you would prefer it to be done?
> 
> > Also, the documentation is missing the fact that the driver makes
> > having
> > one of the property mandatory.
> 
> I will add it.
> 
> Thank you, Hugo.
> 

Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and INT_B support

[Alexandre Belloni]

On 11/02/2022 20:16:27+0000, Hugo Villeneuve wrote:
> > > Actually, this property has to be made more generic and thought
> > > out.
> > > There are multiple RTCs that have multiple interrupt pins where one
> > > of
> > > the pin can be used for different interrupt or clock output.
> 
> Hi,
> the only example I could find of such a device is in rtc-pcf85363.c.
> This device also has two interrupt pins, INT A/B, like the PCF2131.
> However, in the pcf85363 driver, pin INT B is simply ignored, and all
> interrupts are configured to go on INT A.
> 

Yes, this was the RTC for which we had that discussion last time but
there is also pcf8523 and other non NXP RTCs.

> For the moment, I will simply modify my PCF2131 patches serie to mimic
> the same behavior in V2. This simplifies things a lot, and support for
> INT B pin could be added at a later stage (and also to pcf85363) if
> anyone needs it (I don't).
> 
> Hugo.
> 
> > > With your binding, there is no way to separate which interrupt is
> > > going
> > > to which pin and so there is no way to get the alarm and BLF or the
> > > watchdog on different pins and we certainly don't want to have a
> > > property per interrupt type.
> > 
> > Hi,
> > can you please suggest how you would prefer it to be done?
> > 
> > > Also, the documentation is missing the fact that the driver makes
> > > having
> > > one of the property mandatory.
> > 
> > I will add it.
> > 
> > Thank you, Hugo.
> > 
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com