RE: [PATCH v3.1 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Conor Dooley,

Thanks for the feedback.

> -----Original Message-----
> From: Conor Dooley <conor@xxxxxxxxxx>
> Sent: Tuesday, December 5, 2023 5:12 PM
> Subject: Re: [PATCH v3.1 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062
> to json-schema
> 
> On Mon, Dec 04, 2023 at 05:25:10PM +0000, Biju Das wrote:
> 
> > @@ -60,8 +85,65 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > -  - interrupts
> > -  - interrupt-controller
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - dlg,da9063
> > +              - dlg,da9063l
> > +    then:
> > +      properties:
> > +        thermal: false
> > +        gpio: false
> > +        gpio-controller: false
> > +        "#gpio-cells": false
> > +        regulators:
> > +          patternProperties:
> > +            "^buck[1-4]$": false
> > +      required:
> > +        - interrupts
> > +        - interrupt-controller
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - dlg,da9062
> > +    then:
> > +      properties:
> > +        regulators:
> > +          patternProperties:
> > +            "^(ldo([5-9]|10|11)|bcore([1-2]|s-
> merged)|b(pro|mem|io|peri)|bmem-bio-merged)$": false
> > +      required:
> > +        - gpio
> > +        - onkey
> > +        - rtc
> > +        - thermal
> > +        - watchdog
> 
> Why are these required for the 9062 (and another set for the 9061)?
> The original binding does not seem to require them, nor does the yaml
> binding for the 9063.

The core drivers for 9063 and 9062 devices are different. Absence of child node using da9062 core driver
is giving error messages like (eg: da9062-gpio: Failed to locate of_node [id: -1]).

So, Geert suggested to make them as required one[1]

[1]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20231201110840.37408-6-biju.das.jz@xxxxxxxxxxxxxx/


> 
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - dlg,da9061
> > +    then:
> > +      properties:
> > +        gpio: false
> > +        gpio-controller: false
> > +        "#gpio-cells": false
> > +        regulators:
> > +          patternProperties:
> > +            "^(ldo([5-9]|10|11)|bcore([1-2]|s-
> merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck4)$": false
> > +        rtc: false
> > +      required:
> > +        - onkey
> > +        - thermal
> > +        - watchdog
> >
> >  additionalProperties: false
> >
> > @@ -118,4 +200,98 @@ examples:
> >          };
> >        };
> >      };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/regulator/dlg,da9063-regulator.h>
> > +    i2c {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      pmic@58 {
> > +        compatible = "dlg,da9062";
> > +        reg = <0x58>;
> > +        #interrupt-cells = <2>;
> > +        interrupt-parent = <&gpio1>;
> > +        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> > +        interrupt-controller;
> > +
> > +        gpio {
> > +          compatible = "dlg,da9062-gpio";
> > +          status = "disabled";
> 
> Why add it disabled? This should be enabled IMO.

Same as above.

Cheers,
Biju

> 
> > +        rtc {
> > +          compatible = "dlg,da9062-rtc";
> > +          status = "disabled";
> > +        };
> > +
> > +        thermal {
> > +          compatible = "dlg,da9062-thermal";
> > +          status = "disabled";
> > +        };
> 
> Ditto for these.
> 
> Thanks,
> Conor.