[v3,2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

Message ID 20230118063822.14521-3-okan.sahin@analog.com
State New
Headers
Series Add MAX77541/MAX77540 PMIC Support |

Commit Message

Sahin, Okan Jan. 18, 2023, 6:38 a.m. UTC
  Add ADI MAX77541/MAX77540 devicetree document.

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
  

Comments

Krzysztof Kozlowski Jan. 18, 2023, 8:22 a.m. UTC | #1
On 18/01/2023 07:38, Okan Sahin wrote:
> Add ADI MAX77541/MAX77540 devicetree document.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/adi,max77541.yaml b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> new file mode 100644
> index 000000000000..91d15e9ca2e3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/adi,max77541.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAX77540/MAX77541 PMIC from ADI
> +
> +maintainers:
> +  - Okan Sahin <okan.sahin@analog.com>
> +
> +description: |
> +  MAX77540 is a Power Management IC with 2 buck regulators.
> +
> +  MAX77541 is a Power Management IC with 2 buck regulators and 1 ADC.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,max77540
> +      - adi,max77541
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  regulators:
> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#

No improvements regarding bisectability - this patch fails. If you
tested this patch, you would see it.

Instead of ignoring comments, either implement them or ask for
clarification.


> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,max77540
> +    then:
> +      properties:
> +        regulator:

You do not have 'regulator' property.

> +          properties:
> +            compatible:
> +              const: adi,max77540-regulator
> +    else:
> +      properties:
> +        regulator:

Same problem.

> +          properties:
> +            compatible:
> +              const: adi,max77541-regulator
> +
> +additionalProperties: false
> +

Best regards,
Krzysztof
  
Rob Herring Jan. 18, 2023, 1:10 p.m. UTC | #2
On Wed, 18 Jan 2023 09:38:09 +0300, Okan Sahin wrote:
> Add ADI MAX77541/MAX77540 devicetree document.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/mfd/adi,max77541.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/regulator/adi,max77541-regulator.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.example.dtb: pmic@69: regulators: False schema does not allow {'buck1': {'regulator-min-microvolt': [[500000]], 'regulator-max-microvolt': [[5200000]], 'regulator-boot-on': True, 'regulator-always-on': True}, 'buck2': {'regulator-min-microvolt': [[500000]], 'regulator-max-microvolt': [[5200000]], 'regulator-boot-on': True, 'regulator-always-on': True}}
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230118063822.14521-3-okan.sahin@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  
Sahin, Okan Jan. 31, 2023, 12:02 p.m. UTC | #3
Hi Krzysztof,

Thank you for your feedback and efforts. There is something that confuses me so I would like to ask below

On Wed, 18 Jan 2023 11:23 AM
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

>On 18/01/2023 07:38, Okan Sahin wrote:
>> Add ADI MAX77541/MAX77540 devicetree document.
>>
>> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
>> ---
>>  .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 88 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/mfd/adi,max77541.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
>> b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
>> new file mode 100644
>> index 000000000000..91d15e9ca2e3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
>> @@ -0,0 +1,87 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
>> +---
>> +$id:
>> +https://urldefense.com/v3/__http://devicetree.org/schemas/mfd/adi,max
>>
>+77541.yaml*__;Iw!!A3Ni8CS0y2Y!8996jdM8k1vZNHCTUma_rPZJgJFp5YspnfHn9
>Az
>> +1iRZER2Won_Nt-egtD6XnaFy2I2YTAHhCTJA0DcP6xCbYS_SPe8iy$
>> +$schema:
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>>
>+aml*__;Iw!!A3Ni8CS0y2Y!8996jdM8k1vZNHCTUma_rPZJgJFp5YspnfHn9Az1iRZE
>R2
>> +Won_Nt-egtD6XnaFy2I2YTAHhCTJA0DcP6xCbYS-wr9IcZ$
>> +
>> +title: MAX77540/MAX77541 PMIC from ADI
>> +
>> +maintainers:
>> +  - Okan Sahin <okan.sahin@analog.com>
>> +
>> +description: |
>> +  MAX77540 is a Power Management IC with 2 buck regulators.
>> +
>> +  MAX77541 is a Power Management IC with 2 buck regulators and 1 ADC.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - adi,max77540
>> +      - adi,max77541
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  regulators:
>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>
>No improvements regarding bisectability - this patch fails. If you tested this patch,
>you would see it.
>
>Instead of ignoring comments, either implement them or ask for clarification.
>
>
Sorry for misunderstanding, I checked patchset as a whole not one by one this is why I did not get failure after "make dt_binding_check " . Right now, I understand why you are saying this patch fails, but what is your suggestion?  what is the correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5. In the light of discussion, should I remove all the parts related to regulator in patch 2/5, then add adi,max77541-regulator.yaml and update adi,max77541.yaml in patch 4/5? or should I add new patch to update adi,max77541.yaml?

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: adi,max77540
>> +    then:
>> +      properties:
>> +        regulator:
>
>You do not have 'regulator' property.
>
>> +          properties:
>> +            compatible:
>> +              const: adi,max77540-regulator
>> +    else:
>> +      properties:
>> +        regulator:
>
>Same problem.
>
>> +          properties:
>> +            compatible:
>> +              const: adi,max77541-regulator
>> +
>> +additionalProperties: false
>> +
>
>Best regards,
>Krzysztof

Regards,
Okan Sahin
  
Krzysztof Kozlowski Jan. 31, 2023, 4:44 p.m. UTC | #4
On 31/01/2023 13:02, Sahin, Okan wrote:
>>> +  regulators:
>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>
>> No improvements regarding bisectability - this patch fails. If you tested this patch,
>> you would see it.
>>
>> Instead of ignoring comments, either implement them or ask for clarification.
>>
>>
> Sorry for misunderstanding, I checked patchset as a whole not one by one this is why I did not get failure after "make dt_binding_check " . Right now, I understand why you are saying this patch fails, but what is your suggestion?  what is the correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5. In the light of discussion, should I remove all the parts related to regulator in patch 2/5, then add adi,max77541-regulator.yaml and update adi,max77541.yaml in patch 4/5? or should I add new patch to update adi,max77541.yaml?

Regulator binding patch should be first in the series (bindings are
before usage), then the MFD binding should come. Your cover letter
should clearly at the top mention the dependency. You can also mention
dependency in MFD patch after ---, because many of us do not really read
cover letters...


Best regards,
Krzysztof
  
Sahin, Okan Jan. 31, 2023, 9:28 p.m. UTC | #5
On Tue, 31 Jan 2023 7:44 PM
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

>On 31/01/2023 13:02, Sahin, Okan wrote:
>>>> +  regulators:
>>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>>
>>> No improvements regarding bisectability - this patch fails. If you
>>> tested this patch, you would see it.
>>>
>>> Instead of ignoring comments, either implement them or ask for clarification.
>>>
>>>
>> Sorry for misunderstanding, I checked patchset as a whole not one by one this is
>why I did not get failure after "make dt_binding_check " . Right now, I understand
>why you are saying this patch fails, but what is your suggestion?  what is the
>correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5.
>In the light of discussion, should I remove all the parts related to regulator in
>patch 2/5, then add adi,max77541-regulator.yaml and update
>adi,max77541.yaml in patch 4/5? or should I add new patch to update
>adi,max77541.yaml?
>
>Regulator binding patch should be first in the series (bindings are before usage),
>then the MFD binding should come. Your cover letter should clearly at the top
>mention the dependency. You can also mention dependency in MFD patch after --
>-, because many of us do not really read cover letters...
>
>
>Best regards,
>Krzysztof
Hi Krzysztof,

Thank you for your feedback. I tried to explain in cover letter .However, I understand that it was not clear enough. I do not want to take your time, but let me ask one thing to understand the case completely. Right now, my order is like below
[cover letter]->[mfd driver]->[mfd binding]->[regulator driver]->[regulator binding]->[adc]. 
Should I completely change the ordering  e.g. starting with regulator ending with mfd or is it sufficient to just get the regulator binding just before the mfd bindings?

Regards,
Okan
  
Krzysztof Kozlowski Feb. 1, 2023, 7:25 a.m. UTC | #6
On 31/01/2023 22:28, Sahin, Okan wrote:
> On Tue, 31 Jan 2023 7:44 PM
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 31/01/2023 13:02, Sahin, Okan wrote:
>>>>> +  regulators:
>>>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>>>
>>>> No improvements regarding bisectability - this patch fails. If you
>>>> tested this patch, you would see it.
>>>>
>>>> Instead of ignoring comments, either implement them or ask for clarification.
>>>>
>>>>
>>> Sorry for misunderstanding, I checked patchset as a whole not one by one this is
>> why I did not get failure after "make dt_binding_check " . Right now, I understand
>> why you are saying this patch fails, but what is your suggestion?  what is the
>> correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5.
>> In the light of discussion, should I remove all the parts related to regulator in
>> patch 2/5, then add adi,max77541-regulator.yaml and update
>> adi,max77541.yaml in patch 4/5? or should I add new patch to update
>> adi,max77541.yaml?
>>
>> Regulator binding patch should be first in the series (bindings are before usage),
>> then the MFD binding should come. Your cover letter should clearly at the top
>> mention the dependency. You can also mention dependency in MFD patch after --
>> -, because many of us do not really read cover letters...
>>
>>
>> Best regards,
>> Krzysztof
> Hi Krzysztof,
> 
> Thank you for your feedback. I tried to explain in cover letter .However, I understand that it was not clear enough. I do not want to take your time, but let me ask one thing to understand the case completely. Right now, my order is like below
> [cover letter]->[mfd driver]->[mfd binding]->[regulator driver]->[regulator binding]->[adc]. 
> Should I completely change the ordering  e.g. starting with regulator ending with mfd or is it sufficient to just get the regulator binding just before the mfd bindings?

"bindings are before usage" - what's unclear?

How can you use binding before defining it?

Best regards,
Krzysztof
  
Sahin, Okan Feb. 1, 2023, 7:46 a.m. UTC | #7
>> On Wed, 1 Feb 2023 10:26 AM
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

>On 31/01/2023 22:28, Sahin, Okan wrote:
>> On Tue, 31 Jan 2023 7:44 PM
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 31/01/2023 13:02, Sahin, Okan wrote:
>>>>>> +  regulators:
>>>>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>>>>
>>>>> No improvements regarding bisectability - this patch fails. If you
>>>>> tested this patch, you would see it.
>>>>>
>>>>> Instead of ignoring comments, either implement them or ask for
>clarification.
>>>>>
>>>>>
>>>> Sorry for misunderstanding, I checked patchset as a whole not one by
>>>> one this is
>>> why I did not get failure after "make dt_binding_check " . Right now,
>>> I understand why you are saying this patch fails, but what is your
>>> suggestion?  what is the correct order for this patchset? I sent adi,max77541-
>regulator.yaml in path 4/5.
>>> In the light of discussion, should I remove all the parts related to
>>> regulator in patch 2/5, then add adi,max77541-regulator.yaml and
>>> update adi,max77541.yaml in patch 4/5? or should I add new patch to
>>> update adi,max77541.yaml?
>>>
>>> Regulator binding patch should be first in the series (bindings are
>>> before usage), then the MFD binding should come. Your cover letter
>>> should clearly at the top mention the dependency. You can also
>>> mention dependency in MFD patch after -- -, because many of us do not really
>read cover letters...
>>>
>>>
>>> Best regards,
>>> Krzysztof
>> Hi Krzysztof,
>>
>> Thank you for your feedback. I tried to explain in cover letter
>> .However, I understand that it was not clear enough. I do not want to take your
>time, but let me ask one thing to understand the case completely. Right now, my
>order is like below [cover letter]->[mfd driver]->[mfd binding]->[regulator driver]-
>>[regulator binding]->[adc].
>> Should I completely change the ordering  e.g. starting with regulator ending
>with mfd or is it sufficient to just get the regulator binding just before the mfd
>bindings?
>
>"bindings are before usage" - what's unclear?
>
>How can you use binding before defining it?
>
>Best regards,
>Krzysztof

Hi Krysztof,

It is crystal clear that "bindings are before usage", but what I want to know is what is the correct order of patchset for mfd device? Max77541 is multi-functional device. It has both adc and regulator subdevices. I thought I need to put mfd driver first, yet it looks wrong to me right now? 

Regards,
Okan
  
Krzysztof Kozlowski Feb. 1, 2023, 8:08 a.m. UTC | #8
On 01/02/2023 08:46, Sahin, Okan wrote:
>> "bindings are before usage" - what's unclear?
>>
>> How can you use binding before defining it?
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krysztof,
> 
> It is crystal clear that "bindings are before usage", but what I want to know is what is the correct order of patchset for mfd device? Max77541 is multi-functional device. It has both adc and regulator subdevices. I thought I need to put mfd driver first, yet it looks wrong to me right now?

MFD driver is also usually the last.

Best regards,
Krzysztof
  

Patch

diff --git a/Documentation/devicetree/bindings/mfd/adi,max77541.yaml b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
new file mode 100644
index 000000000000..91d15e9ca2e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
@@ -0,0 +1,87 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/adi,max77541.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX77540/MAX77541 PMIC from ADI
+
+maintainers:
+  - Okan Sahin <okan.sahin@analog.com>
+
+description: |
+  MAX77540 is a Power Management IC with 2 buck regulators.
+
+  MAX77541 is a Power Management IC with 2 buck regulators and 1 ADC.
+
+properties:
+  compatible:
+    enum:
+      - adi,max77540
+      - adi,max77541
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  regulators:
+    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,max77540
+    then:
+      properties:
+        regulator:
+          properties:
+            compatible:
+              const: adi,max77540-regulator
+    else:
+      properties:
+        regulator:
+          properties:
+            compatible:
+              const: adi,max77541-regulator
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@69 {
+            compatible = "adi,max77541";
+            reg = <0x69>;
+            interrupt-parent = <&gpio>;
+            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+
+            regulators {
+                buck1 {
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <5200000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+                buck2 {
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <5200000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index af94d06bb9f0..22f5a9c490e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12501,6 +12501,7 @@  MAXIM MAX77541 PMIC MFD DRIVER
 M:	Okan Sahin <okan.sahin@analog.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
 F:	drivers/mfd/max77541.c
 F:	include/linux/mfd/max77541.h