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: <dbf5fc36-11c6-ae54-a19c-5fc9924aa18d@linaro.org>
Date:   Mon, 16 Jan 2023 11:01:40 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [RESEND v3 01/13] dt-binding: mediatek: add bindings for MediaTek
 mt8195 MDP3 components
To:     =?UTF-8?B?TW91ZHkgSG8gKOS9leWul+WOnyk=?= <Moudy.Ho@mediatek.com>,
        "matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
        "mchehab@kernel.org" <mchehab@kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>
Cc:     "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
        "linux-mediatek@lists.infradead.org" 
        <linux-mediatek@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@mediatek.com>
References: <20230116032147.23607-1-moudy.ho@mediatek.com>
 <20230116032147.23607-2-moudy.ho@mediatek.com>
 <f24a54f1-2720-3345-9596-bb8d388ba16f@linaro.org>
 <a5ef36df5bf8c483d327247199f5494be13b1efa.camel@mediatek.com>
Content-Language: en-US
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <a5ef36df5bf8c483d327247199f5494be13b1efa.camel@mediatek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 16/01/2023 10:39, Moudy Ho (何宗原) wrote:
> Hi Krzysztof,
> 
> Thank you for taking the time to help review, I would like to ask a
> modification as follows.
> 
> On Mon, 2023-01-16 at 09:10 +0100, Krzysztof Kozlowski wrote:
>>>
> 
> (snip)
> 
>> On 16/01/2023 04:21, Moudy Ho wrote:
>>> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-
>>> aal.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-
>>> aal.yaml
>>> new file mode 100644
>>> index 000000000000..d2e1b5245778
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-
>>> aal.yaml
>>
>> Filename should match compatible, unless you already expect this
>> binding
>> will cover other devices. If so, why not adding them now?
>>
> 
> May I rename this file to "mediatek,mt8195-mdp3.yaml"
> 
>>>
> 
> (snip)
> 
>>> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-
>>> color.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-
>>> color.yaml
>>> new file mode 100644
>>> index 000000000000..1d8aa5dc76b9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-
>>> color.yaml
>>> @@ -0,0 +1,63 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: 
>>> https://urldefense.com/v3/__http://devicetree.org/schemas/media/mediatek,mdp3-color.yaml*__;Iw!!CTRNKA9wMg0ARbw!lcferrFFP-mshDHNL-rwJLgNKDrXF9fXoljpqL30k5YKTNvCwuC3webzR32VnQQoPeFvSvAewNkeupcT4mjdEwNEKP4V$ 
>>>  
>>> +$schema: 
>>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!lcferrFFP-mshDHNL-rwJLgNKDrXF9fXoljpqL30k5YKTNvCwuC3webzR32VnQQoPeFvSvAewNkeupcT4mjdEz618JHq$ 
>>>  
>>> +
>>> +title: MediaTek Media Data Path 3 COLOR
>>> +
>>> +maintainers:
>>> +  - Matthias Brugger <matthias.bgg@gmail.com>
>>> +  - Moudy Ho <moudy.ho@mediatek.com>
>>> +
>>> +description:
>>> +  One of Media Data Path 3 (MDP3) components used to adjust hue,
>>> luma and
>>> +  saturation to get better picture quality.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - mediatek,mt8195-mdp3-color
>>
>> This is exactly the same as previous file. Why do you split the
>> binding?
>> It really looks unnecessary.
>>
>> Probably all other files should be also squashed.
>>
> 
> and convert all other bindings into individual compatible enums to
> squash all files?
> 
>   compatible:
>     enum:
>       - mediatek,mt8195-mdp3-color
>       - mediatek,mt8195-mdp3-aal

Yes, all devices which have exactly the same properties in one binding
file. Their compatibles listed in enum.

You can keep the separate bindings which differ from each other.

Best regards,
Krzysztof