| Message ID | 20231027162044.1011951-2-menin@carlosaurelio.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] rtc: add pcf85053a | expand |
On 27/10/2023 18:20, Carlos Menin wrote: > Add YAML bindings for NXP's PCF85053A RTC chip. > > Signed-off-by: Carlos Menin <menin@carlosaurelio.net> > Reviewed-by: Sergio Prado <sergio.prado@e-labworks.com> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time, thus I will skip this patch entirely till you follow the process allowing the patch to be tested. Please kindly resend and include all necessary To/Cc entries. > --- > .../bindings/rtc/nxp,pcf85053a.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml > new file mode 100644 > index 000000000000..80980377e27f > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml > @@ -0,0 +1,57 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/rtc/nxp,pcf85053a.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP PCF85053A Real Time Clock > + > +maintainers: > + - Carlos Menin <menin@carlosaurelio.net> > + > +properties: > + compatible: > + enum: > + - nxp,pcf85053a > + > + reg: > + maxItems: 1 > + > + quartz-load-femtofarads: > + description: > + The quartz oscillator load capacitance. > + enum: [6000, 7000, 12500] > + default: 6000 > + > + quartz-drive-control: > + description: > + The quartz oscillator drive control. > + enum: [low, normal, high] > + default: normal > + > + low-jitter: > + description: > + Low jitter mode. > + type: boolean > + default: false Cannot be anything else. > + > +allOf: > + - $ref: rtc.yaml# This goes after required: > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + rtc@6f { > + compatible = "nxp,pcf85053a"; Use 4 spaces for example indentation. > + reg = <0x6f>; > + }; > + }; Best regards, Krzysztof
On 28/10/2023 05:44, Krzysztof Kozlowski wrote: > On 27/10/2023 18:20, Carlos Menin wrote: >> Add YAML bindings for NXP's PCF85053A RTC chip. >> >> Signed-off-by: Carlos Menin <menin@carlosaurelio.net> >> Reviewed-by: Sergio Prado <sergio.prado@e-labworks.com> > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > You missed at least devicetree list (maybe more), so this won't be > tested by automated tooling. Performing review on untested code might be > a waste of time, thus I will skip this patch entirely till you follow > the process allowing the patch to be tested. > > Please kindly resend and include all necessary To/Cc entries. > >> --- >> .../bindings/rtc/nxp,pcf85053a.yaml | 57 +++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml >> >> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml >> new file mode 100644 >> index 000000000000..80980377e27f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml >> @@ -0,0 +1,57 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/rtc/nxp,pcf85053a.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: NXP PCF85053A Real Time Clock >> + >> +maintainers: >> + - Carlos Menin <menin@carlosaurelio.net> >> + >> +properties: >> + compatible: >> + enum: >> + - nxp,pcf85053a >> + >> + reg: >> + maxItems: 1 >> + >> + quartz-load-femtofarads: >> + description: >> + The quartz oscillator load capacitance. >> + enum: [6000, 7000, 12500] >> + default: 6000 >> + >> + quartz-drive-control: >> + description: >> + The quartz oscillator drive control. >> + enum: [low, normal, high] >> + default: normal >> + >> + low-jitter: >> + description: >> + Low jitter mode. >> + type: boolean >> + default: false > Cannot be anything else. Sorry, I 'm not sure I understood, just to confirm before I resend the patch, does this mean this first part of the YAML is correct? >> + >> +allOf: >> + - $ref: rtc.yaml# > This goes after required: > >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + rtc@6f { >> + compatible = "nxp,pcf85053a"; > Use 4 spaces for example indentation. > >> + reg = <0x6f>; >> + }; >> + }; > Best regards, > Krzysztof > Thanks for reviewing! Best regards, Carlos
On 01/11/2023 05:43, Carlos Aurelio wrote: > On 28/10/2023 05:44, Krzysztof Kozlowski wrote: >> On 27/10/2023 18:20, Carlos Menin wrote: >>> Add YAML bindings for NXP's PCF85053A RTC chip. >>> >>> Signed-off-by: Carlos Menin <menin@carlosaurelio.net> >>> Reviewed-by: Sergio Prado <sergio.prado@e-labworks.com> >> Please use scripts/get_maintainers.pl to get a list of necessary people >> and lists to CC. It might happen, that command when run on an older >> kernel, gives you outdated entries. Therefore please be sure you base >> your patches on recent Linux kernel. >> >> You missed at least devicetree list (maybe more), so this won't be >> tested by automated tooling. Performing review on untested code might be >> a waste of time, thus I will skip this patch entirely till you follow >> the process allowing the patch to be tested. >> >> Please kindly resend and include all necessary To/Cc entries. >> >>> --- >>> .../bindings/rtc/nxp,pcf85053a.yaml | 57 +++++++++++++++++++ >>> 1 file changed, 57 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml >>> new file mode 100644 >>> index 000000000000..80980377e27f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml >>> @@ -0,0 +1,57 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/rtc/nxp,pcf85053a.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: NXP PCF85053A Real Time Clock >>> + >>> +maintainers: >>> + - Carlos Menin <menin@carlosaurelio.net> >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - nxp,pcf85053a >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + quartz-load-femtofarads: >>> + description: >>> + The quartz oscillator load capacitance. >>> + enum: [6000, 7000, 12500] >>> + default: 6000 >>> + >>> + quartz-drive-control: >>> + description: >>> + The quartz oscillator drive control. >>> + enum: [low, normal, high] >>> + default: normal >>> + >>> + low-jitter: >>> + description: >>> + Low jitter mode. >>> + type: boolean >>> + default: false >> Cannot be anything else. > > Sorry, I 'm not sure I understood, just to confirm before I resend the > patch, does this mean this first part of the YAML is correct? No, it is not correct. I just do not perform review of code which was not even tested. It's waste of my time. You got already more than usually for such patches... Both properties quartz-drive-control and low-jitter do not look generic. If they are generic, they should be defined in common schema. Since they do not, then they miss prefixes and explanation. Explanation for example telling me why would you like to have high jitter in other cases? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml new file mode 100644 index 000000000000..80980377e27f --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053a.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rtc/nxp,pcf85053a.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP PCF85053A Real Time Clock + +maintainers: + - Carlos Menin <menin@carlosaurelio.net> + +properties: + compatible: + enum: + - nxp,pcf85053a + + reg: + maxItems: 1 + + quartz-load-femtofarads: + description: + The quartz oscillator load capacitance. + enum: [6000, 7000, 12500] + default: 6000 + + quartz-drive-control: + description: + The quartz oscillator drive control. + enum: [low, normal, high] + default: normal + + low-jitter: + description: + Low jitter mode. + type: boolean + default: false + +allOf: + - $ref: rtc.yaml# + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + rtc@6f { + compatible = "nxp,pcf85053a"; + reg = <0x6f>; + }; + };