| Message ID | 20220125200009.900660-11-hugo@hugovil.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/10] rtc: pcf2127: add variant-specific configuration structure | expand |
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 > >
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.
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.
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.
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. >
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. > > >
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; }; };