Re: [PATCH V8 1/7] dt-bindings: mfd: pm8008: Modify the compatible as per driver

["Satya Priya Kakitapalli (Temp)" <quic_c_skakit@quicinc.com>]

On 3/25/2022 11:32 PM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-03-25 04:58:05)
>> Modify the compatible string as per the pm8008 mfd driver.
>> Add reset-gpios property and make interrupts and interrupt-cells
>> as optional properties, they are not strictly required and may
>> cause yaml compilation errors when not added in the DT files.
> Does it have an interrupt controller inside? If so, the properties
> should be present even if the driver isn't using them.


Yes it has an interrupt controller but we are not configuring/using any 
interrupts currently in the DT node. So, if we add the properties under 
"required" list it throws a make dtbs error. Do you think we can somehow 
bypass this error?


>> Also, change the node name in example to match with the
>> pm8008_infra DT node.
>>
>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>> ---
>> Changes in V5:
>>   - Remove compatible for regulators node.
>>   - Move supply nodes of the regulators to chip level.
>>
>> Changes in V6:
>>   - No changes.
>>
>> Changes in V7:
>>   - Removed the intermediate regulators node and added ldos
>>     directly under mfd node.
>>
>> Changes in V8:
>>   - Change the compatible as per driver, remove interrupts from required
>>     properties, add reset-gpios and move regulators to separate binding.
>>
>>   Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> index ec3138c..12431ea 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> @@ -16,7 +16,7 @@ description: |
>>
>>   properties:
>>     compatible:
>> -    const: qcom,pm8008
>> +    const: qcom,pm8008-infra
> Why is the compatible being replaced with -infra postfix?


I've changed the compatible after splitting the probe into two parts 
pm8008-infra-probe and pm8008-regulators-probe. Only reason for change 
is to make it more readable/understandable.


>>     reg:
>>       description:
>> @@ -44,6 +44,10 @@ properties:
>>     "#size-cells":
>>       const: 0
>>
>> +  reset-gpios:
>> +    description: |
>> +      Specifies the GPIO to be toggled to bring pm8008 chip out of reset.
> Remove description. Add maxItems: 1


Okay.


>> +
>>   patternProperties:
>>     "^gpio@[0-9a-f]+$":
>>       type: object
>> @@ -88,10 +92,8 @@ patternProperties:
>>   required:
>>     - compatible
>>     - reg
>> -  - interrupts
>>     - "#address-cells"
>>     - "#size-cells"
>> -  - "#interrupt-cells"
> Should reset-gpios be required? I'd expect this patch to be adding
> reset-gpios and making it required and that's about it. Given that
> there isn't a DT using this compatible upstream so far it looks like we
> don't need to do anything like worry about backwards compatibility.


Okay.


>>   additionalProperties: false
>>
>> @@ -99,11 +101,12 @@ examples:
>>     - |
>>       #include <dt-bindings/mfd/qcom-pm8008.h>
>>       #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/gpio/gpio.h>
>>       qupv3_se13_i2c {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>> -      pm8008i@8 {
>> -        compatible = "qcom,pm8008";
>> +      pm8008_infra: pmic@8 {
> Remove unused phandle.


Okay.


>> +        compatible = "qcom,pm8008-infra";
>>           reg = <0x8>;
>>           #address-cells = <1>;
>>           #size-cells = <0>;
>> @@ -113,6 +116,8 @@ examples:
>>           interrupt-parent = <&tlmm>;
>>           interrupts = <32 IRQ_TYPE_EDGE_RISING>;
>>
>> +        reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>;
>> +
>>           pm8008_gpios: gpio@c000 {
>>             compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio";
>>             reg = <0xc000>;
>> --
>> 2.7.4
>>