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

From: "Sahin, Okan" <Okan.Sahin@analog.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Lee Jones <lee@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Caleb Connolly <caleb.connolly@linaro.org>,
	Marcus Folkesson <marcus.folkesson@gmail.com>,
	"Bolboaca, Ramona" <Ramona.Bolboaca@analog.com>,
	William Breathitt Gray <william.gray@linaro.org>,
	ChiYuan Huang <cy_huang@richtek.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Subject: RE: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540
Date: Tue, 31 Jan 2023 12:02:57 +0000	[thread overview]
Message-ID: <MN2PR03MB51685B56D49CBB590BBE6B7EE7D09@MN2PR03MB5168.namprd03.prod.outlook.com> (raw)
In-Reply-To: <c4433cba-ce35-e5d3-f04b-ba8f9f501732@linaro.org>

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