Hi Trent Piepho,
> -----Original Message-----
> From: Trent Piepho <tpiepho@xxxxxxxxx>
> Sent: Tuesday, May 9, 2023 8:04 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>; Alexandre Belloni
> <alexandre.belloni@xxxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; linux-rtc@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Geert Uytterhoeven <geert+renesas@xxxxxxxxx>;
> Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>; linux-renesas-
> soc@xxxxxxxxxxxxxxx; Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Subject: Re: [PATCH v4] dt-bindings: rtc: isl1208: Convert to json-schema
>
> On Tue, May 9, 2023 at 6:12 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > +
> > + interrupt-names:
>
> Shouldn't this have minItems: 1 and maxItems: 2 as well?
>
> > + items:
> > + - const: irq
> > + - const: evident
>
>
> > +
> > + isil,ev-evienb:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [ 0, 1 ]
> > + description: |
> > + Enable or disable internal pull on EVIN pin
> > + Default will leave the non-volatile configuration of the pullup
> > + as is.
> > + <0> : Enables internal pull-up on evin pin
> > + <1> : Disables internal pull-up on evin pin
>
> It appears this was not clear at first. The default is not to use the reset
> value, which is 0, but to leave the existing value unchanged.
> The RTC settings are battery-backed and the RTC is not reset on boot by the
> kernel. The value might have been set on a previous boot, or might have
> already been configured by the bootloader or BIOS. This was a common design
> in Linux RTC drivers. The bootloader would configure the RTC and then Linux
> driver was only design to be able to get/set the time and alarms. Stuff
> like programming the interrupt pull-ups or setting calibration registers
> wasn't done by the kernel.
Thanks for the detailed explanation. So currently are you ok with the description
right? or Do you want to change this with detailed explanation? We can always add
incremental patch to update this section later. This patch is just conversion
from txt to yaml and the content should be same as the original one as much as possible.
Please let me know.
>
> > + then:
> > + properties:
> > + interrupts:
> > + maxItems: 2
> > + else:
> > + properties:
> > + interrupts:
> > + maxItems: 1
>
> Add interrupt-names here too.
Based on the other mail thread, will update this.
>
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + rtc_twi: rtc@6f {
> > + compatible = "isil,isl1208";
> > + reg = <0x6f>;
> > + };
> > + };
>
> I agree with Geert's original comment about switching from the most complex
> to the simplest example. It's better to show as much as possible.
>
> If it's not possible to make a valid example that shows interrupts and evdet
> pull enable, then doesn't that mean the bindings don't work?
That is the normal practice right? When there is a board dts with complex case,
It is normal to add that case in example section.
I started with complex case and tested the bindings., later found that there is
no board dts with complex case. So switched to simple case.
Cheers,
Biju
