Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9110244a-9e75-40e6-4c82-d2ef149e0e6a@linaro.org>
Date:   Tue, 3 May 2022 11:48:09 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH] dt-bindings: mfd: Add bindings child nodes for the
 Mediatek MT6360
Content-Language: en-US
To:     Gene Chen <gene.chen.richtek@gmail.com>, lee.jones@linaro.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        matthias.bgg@gmail.com
Cc:     devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
        gene_chen@richtek.com, cy_huang@richtek.com
References: <20220503062843.12516-1-gene.chen.richtek@gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20220503062843.12516-1-gene.chen.richtek@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/05/2022 08:28, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add bindings child nodes for the Mediatek MT6360
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> Link: https://lore.kernel.org/all/YmqTzlrVL5KaVPuz@robh.at.kernel.org/
> ---
>  .../bindings/mfd/mediatek,mt6360.yaml         | 212 +++++++++++++++++-
>  1 file changed, 201 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6360.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6360.yaml
> index 4d8769f3748c..b3eecf661bc1 100644
> --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6360.yaml
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6360.yaml
> @@ -37,6 +37,18 @@ properties:
>      description:
>        The first cell is the IRQ number.
>  
> +  regulators:
> +    $ref: ../regulator/mt6360-regulator.yaml
> +
> +  charger:
> +    $ref: ../power/supply/mt6360_charger.yaml
> +
> +  tcpc:
> +    $ref: ../usb/mediatek,mt6360-tcpc.yaml
> +
> +  led-controller:
> +    $ref: ../leds/leds-mt6360.yaml

Full paths everywhere here, so:
/schemas/leds/leds-mt6360.yaml#

> +
>  required:
>    - compatible
>    - reg
> @@ -51,17 +63,195 @@ examples:
>    - |
>      #include <dt-bindings/interrupt-controller/irq.h>
>      #include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
> +    #include <dt-bindings/leds/common.h>
> +    #include <dt-bindings/usb/pd.h>
>      i2c {
> -        #address-cells = <1>;
> -        #size-cells = <0>;
> -
> -        pmic@34 {
> -            compatible = "mediatek,mt6360";
> -            reg = <0x34>;
> -            wakeup-source;
> -            interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
> -            interrupt-names = "IRQB";
> -            interrupt-controller;
> -            #interrupt-cells = <1>;

Previous indentation was correct, don't change it from 4 spaces.

> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      mt6360@34 {

What is the reason to change correct name to wrong name when adding ONLY
child nodes?

Please split any unrelated changes to separate patches, if they are
reasonable. Using wrong node name is not reasonable.

(...)

> +              port@2 {
> +                reg = <2>;
> +                endpoint {
> +                  remote-endpoint = <&dp_aux>;
> +                };
> +              };
> +            };
> +          };
> +        };
> +

no need for blank line

> +      };
>      };


Best regards,
Krzysztof