| Message ID | 20230503084608.14008-5-biju.das.jz@bp.renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Hi Biju, CC Trent's latest address On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208 > IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the > external oscillator is determined by the PMIC revision. > > Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic > property to handle these differences. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > index 04d51887855f..888a832ed1db 100644 > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > @@ -18,6 +18,7 @@ properties: > - isil,isl1209 > - isil,isl1218 > - isil,isl1219 > + - renesas,raa215300-isl1208 That sounds a bit over-complicated. What about just "renesas,raa215300-rtc"? If you consider them sufficiently compatible, you could add "isil,isl1208" as a fallback. > > reg: > maxItems: 1 > @@ -40,6 +41,10 @@ properties: > <0> : Enable internal pull-up > <1> : Disable internal pull-up > > + renesas,raa215300-pmic: "renesas,pmic"? > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle to the pmic device with isl1208 built-in RTC. > + > required: > - compatible > - reg > @@ -58,6 +63,14 @@ allOf: > - interrupts-extended > - interrupt-names > - isil,ev-evienb > + - if: > + properties: > + compatible: > + contains: > + const: renesas,raa215300-isl1208 > + then: > + required: > + - renesas,raa215300-pmic > > unevaluatedProperties: false Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in > RTC device on PMIC RAA215300 > > Hi Biju, > > CC Trent's latest address Thanks, I will fix this in bindings. > > On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > The Built-in RTC device found on PMIC RAA215300 is similar to the > > isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity > > of the external oscillator is determined by the PMIC revision. > > > > Document renesas,raa215300-isl1208 compatible and > > renesas,raa215300-pmic property to handle these differences. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > index 04d51887855f..888a832ed1db 100644 > > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > @@ -18,6 +18,7 @@ properties: > > - isil,isl1209 > > - isil,isl1218 > > - isil,isl1219 > > + - renesas,raa215300-isl1208 > > That sounds a bit over-complicated. > What about just "renesas,raa215300-rtc"? OK, good to me. > If you consider them sufficiently compatible, you could add "isil,isl1208" > as a fallback. The pmic has to enable RTC block to make it functional. The registers and functionality are compatible. But the configuration of Oscillator polarity is different on PMIC version. So we need to handle it here. You mean like below? + - items: + - enum: + - renesas, raa215300-rtc + - const: isil,isl1208 > > > > > reg: > > maxItems: 1 > > @@ -40,6 +41,10 @@ properties: > > <0> : Enable internal pull-up > > <1> : Disable internal pull-up > > > > + renesas,raa215300-pmic: > > "renesas,pmic"? OK. Agreed. Cheers, Biju > > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle to the pmic device with isl1208 built-in RTC. > > + > > required: > > - compatible > > - reg > > @@ -58,6 +63,14 @@ allOf: > > - interrupts-extended > > - interrupt-names > > - isil,ev-evienb > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: renesas,raa215300-isl1208 > > + then: > > + required: > > + - renesas,raa215300-pmic > > > > unevaluatedProperties: false > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Hi Biju, On Wed, May 3, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in > > RTC device on PMIC RAA215300 > > On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > The Built-in RTC device found on PMIC RAA215300 is similar to the > > > isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity > > > of the external oscillator is determined by the PMIC revision. > > > > > > Document renesas,raa215300-isl1208 compatible and > > > renesas,raa215300-pmic property to handle these differences. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > > b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > > index 04d51887855f..888a832ed1db 100644 > > > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > > @@ -18,6 +18,7 @@ properties: > > > - isil,isl1209 > > > - isil,isl1218 > > > - isil,isl1219 > > > + - renesas,raa215300-isl1208 > > > > That sounds a bit over-complicated. > > What about just "renesas,raa215300-rtc"? > > OK, good to me. > > > If you consider them sufficiently compatible, you could add "isil,isl1208" > > as a fallback. > > The pmic has to enable RTC block to make it functional. > The registers and functionality are compatible. > But the configuration of Oscillator polarity is different on PMIC version. > So we need to handle it here. > > You mean like below? > > + - items: > + - enum: > + - renesas, raa215300-rtc > + - const: isil,isl1208 That's indeed what I meant. But given the inverted osc bit, I think the fallback is not appropriate. Gr{oetje,eeting}s, Geert
On 03/05/2023 10:46, Biju Das wrote: > The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208 > IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the > external oscillator is determined by the PMIC revision. > > Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic > property to handle these differences. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > index 04d51887855f..888a832ed1db 100644 > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > @@ -18,6 +18,7 @@ properties: > - isil,isl1209 > - isil,isl1218 > - isil,isl1219 > + - renesas,raa215300-isl1208 > > reg: > maxItems: 1 > @@ -40,6 +41,10 @@ properties: > <0> : Enable internal pull-up > <1> : Disable internal pull-up > > + renesas,raa215300-pmic: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle to the pmic device with isl1208 built-in RTC. No. You don't need cross-linking. We do not represent one device as two and then fix this by cross-linking them. The existing binding does not have it, so it should be a hint for you. Best regards, Krzysztof
Hi Krzysztof, On Thu, May 4, 2023 at 9:11 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 03/05/2023 10:46, Biju Das wrote: > > The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208 > > IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the > > external oscillator is determined by the PMIC revision. > > > > Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic > > property to handle these differences. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > index 04d51887855f..888a832ed1db 100644 > > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > @@ -18,6 +18,7 @@ properties: > > - isil,isl1209 > > - isil,isl1218 > > - isil,isl1219 > > + - renesas,raa215300-isl1208 > > > > reg: > > maxItems: 1 > > @@ -40,6 +41,10 @@ properties: > > <0> : Enable internal pull-up > > <1> : Disable internal pull-up > > > > + renesas,raa215300-pmic: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle to the pmic device with isl1208 built-in RTC. > > No. You don't need cross-linking. We do not represent one device as two > and then fix this by cross-linking them. The existing binding does not > have it, so it should be a hint for you. Makes sense. So there should be a single device node with 2 reg cells, and a "renesas,raa215300" compatible value. On the Linux side, the "renesas,raa215300" MFD driver can instantiate a PMIC and an RTC cell, the latter served by the (enhanced) existing rtc-isl1208 driver. Gr{oetje,eeting}s, Geert
On 04/05/2023 09:47, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Thu, May 4, 2023 at 9:11 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 03/05/2023 10:46, Biju Das wrote: >>> The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208 >>> IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the >>> external oscillator is determined by the PMIC revision. >>> >>> Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic >>> property to handle these differences. >>> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>> --- >>> .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml >>> index 04d51887855f..888a832ed1db 100644 >>> --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml >>> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml >>> @@ -18,6 +18,7 @@ properties: >>> - isil,isl1209 >>> - isil,isl1218 >>> - isil,isl1219 >>> + - renesas,raa215300-isl1208 >>> >>> reg: >>> maxItems: 1 >>> @@ -40,6 +41,10 @@ properties: >>> <0> : Enable internal pull-up >>> <1> : Disable internal pull-up >>> >>> + renesas,raa215300-pmic: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: phandle to the pmic device with isl1208 built-in RTC. >> >> No. You don't need cross-linking. We do not represent one device as two >> and then fix this by cross-linking them. The existing binding does not >> have it, so it should be a hint for you. > > Makes sense. > So there should be a single device node with 2 reg cells, and > a "renesas,raa215300" compatible value. Yes. > > On the Linux side, the "renesas,raa215300" MFD driver can instantiate > a PMIC and an RTC cell, the latter served by the (enhanced) existing > rtc-isl1208 driver. Right. Best regards, Krzysztof
Hi Krystoff, > Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in > RTC device on PMIC RAA215300 > > On 04/05/2023 09:47, Geert Uytterhoeven wrote: > > Hi Krzysztof, > > > > On Thu, May 4, 2023 at 9:11 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> On 03/05/2023 10:46, Biju Das wrote: > >>> The Built-in RTC device found on PMIC RAA215300 is similar to the > >>> isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the > >>> polarity of the external oscillator is determined by the PMIC revision. > >>> > >>> Document renesas,raa215300-isl1208 compatible and > >>> renesas,raa215300-pmic property to handle these differences. > >>> > >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >>> --- > >>> .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > >>> 1 file changed, 13 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > >>> b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > >>> index 04d51887855f..888a832ed1db 100644 > >>> --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > >>> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > >>> @@ -18,6 +18,7 @@ properties: > >>> - isil,isl1209 > >>> - isil,isl1218 > >>> - isil,isl1219 > >>> + - renesas,raa215300-isl1208 > >>> > >>> reg: > >>> maxItems: 1 > >>> @@ -40,6 +41,10 @@ properties: > >>> <0> : Enable internal pull-up > >>> <1> : Disable internal pull-up > >>> > >>> + renesas,raa215300-pmic: > >>> + $ref: /schemas/types.yaml#/definitions/phandle > >>> + description: phandle to the pmic device with isl1208 built-in RTC. > >> > >> No. You don't need cross-linking. We do not represent one device as > >> two and then fix this by cross-linking them. The existing binding > >> does not have it, so it should be a hint for you. > > > > Makes sense. > > So there should be a single device node with 2 reg cells, and a > > "renesas,raa215300" compatible value. > > Yes. > > > > > On the Linux side, the "renesas,raa215300" MFD driver can instantiate > > a PMIC and an RTC cell, the latter served by the (enhanced) existing > > rtc-isl1208 driver. > > Right. We cannot use MFD driver to instantiate RTC as it is not platform device. Thanks to Geert for pointing out "i2c_new_ancillary_device". With this, we can register rtc device from pmic driver Using the api "raa215300_rtc_probe(struct i2c_client *client, unsigned int pmic_version)". Cheers, Biju
Hi Krzysztof Kozlowski, Thanks for the feedback. > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Thursday, May 4, 2023 8:11 AM > To: Biju Das <biju.das.jz@bp.renesas.com>; Alessandro Zummo > <a.zummo@towertech.it>; Alexandre Belloni <alexandre.belloni@bootlin.com>; > Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Magnus Damm > <magnus.damm@gmail.com>; Trent Piepho <tpiepho@impinj.com>; linux- > rtc@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas- > soc@vger.kernel.org; Fabrizio Castro <fabrizio.castro.jz@renesas.com> > Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in > RTC device on PMIC RAA215300 > > On 03/05/2023 10:46, Biju Das wrote: > > The Built-in RTC device found on PMIC RAA215300 is similar to the > > isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity > > of the external oscillator is determined by the PMIC revision. > > > > Document renesas,raa215300-isl1208 compatible and > > renesas,raa215300-pmic property to handle these differences. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > index 04d51887855f..888a832ed1db 100644 > > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml > > @@ -18,6 +18,7 @@ properties: > > - isil,isl1209 > > - isil,isl1218 > > - isil,isl1219 > > + - renesas,raa215300-isl1208 As with the below model, above compatible is not required. raa215300: pmic@12 { compatible = "renesas,raa215300"; reg = <0x12 0x6f>; reg-names = "main", "rtc"; }; > > > > reg: > > maxItems: 1 > > @@ -40,6 +41,10 @@ properties: > > <0> : Enable internal pull-up > > <1> : Disable internal pull-up > > > > + renesas,raa215300-pmic: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle to the pmic device with isl1208 built-in RTC. > > No. You don't need cross-linking. We do not represent one device as two and > then fix this by cross-linking them. The existing binding does not have it, > so it should be a hint for you. OK will remove as discussed above. Cheers, Biju
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml index 04d51887855f..888a832ed1db 100644 --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml @@ -18,6 +18,7 @@ properties: - isil,isl1209 - isil,isl1218 - isil,isl1219 + - renesas,raa215300-isl1208 reg: maxItems: 1 @@ -40,6 +41,10 @@ properties: <0> : Enable internal pull-up <1> : Disable internal pull-up + renesas,raa215300-pmic: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the pmic device with isl1208 built-in RTC. + required: - compatible - reg @@ -58,6 +63,14 @@ allOf: - interrupts-extended - interrupt-names - isil,ev-evienb + - if: + properties: + compatible: + contains: + const: renesas,raa215300-isl1208 + then: + required: + - renesas,raa215300-pmic unevaluatedProperties: false
The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the external oscillator is determined by the PMIC revision. Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic property to handle these differences. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- .../devicetree/bindings/rtc/isil,isl1208.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)