diff mbox series

[v2] dt-bindings: display: mediatek: split: add clocks count constraint for MT8195

Message ID 20240927055140.19688-1-moudy.ho@mediatek.com (mailing list archive)
State New
Headers show
Series [v2] dt-bindings: display: mediatek: split: add clocks count constraint for MT8195 | expand

Commit Message

Moudy Ho Sept. 27, 2024, 5:51 a.m. UTC 
From: Moudy Ho <moudy.ho@mediatek.corp-partner.google.com>

The display node in mt8195.dtsi was triggering a CHECK_DTBS error due
to an excessively long 'clocks' property:
  display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too long

To resolve this issue, apply the limit by setting 'maxItems: 3' in MT8195
additional condition.

Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp: split each block to individual yaml")
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
--
The purpose of this patch is to separate the corrections for
MediaTek SPLIT CHECK_DTBS error from the original mailing list
mentioned below:
https://lore.kernel.org/all/20240924103156.13119-2-macpaul.lin@mediatek.com/

Changes since v1:
  - Adding functional descriptions and quantity restrictions.
---
 .../bindings/display/mediatek/mediatek,split.yaml           | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

CK Hu (胡俊光) Sept. 27, 2024, 9:01 a.m. UTC | #1 
Hi, Moudy:

On Fri, 2024-09-27 at 13:51 +0800, Moudy Ho wrote:
> From: Moudy Ho <moudy.ho@mediatek.corp-partner.google.com>
> 
> The display node in mt8195.dtsi was triggering a CHECK_DTBS error due
> to an excessively long 'clocks' property:
>   display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too long
> 
> To resolve this issue, apply the limit by setting 'maxItems: 3' in MT8195
> additional condition.

I think the main purpose of this patch is not to add limitation.
The main purpose is that you add HDMI RX Clock and HDMI Metadata Clock.
Could you describe more about these two clock?
mt8173-disp-split could work without these two clocks. Why mt8195-mdp3-split need these two clocks?

> 
> Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp: split each block to individual yaml")

The fixes patch should be 739058a9c5c3 ("dt-bindings: display: mediatek: split: add compatible for MT8195")

Regards,
CK

> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> --
> The purpose of this patch is to separate the corrections for
> MediaTek SPLIT CHECK_DTBS error from the original mailing list
> mentioned below:
> https://lore.kernel.org/all/20240924103156.13119-2-macpaul.lin@mediatek.com/
> 
> Changes since v1:
>   - Adding functional descriptions and quantity restrictions.
> ---
>  .../bindings/display/mediatek/mediatek,split.yaml           | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> index e4affc854f3d..bce1b8b866ce 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> @@ -57,6 +57,9 @@ properties:
>    clocks:
>      items:
>        - description: SPLIT Clock
> +      - description: HDMI RX Clock
> +      - description: HDMI Metadata Clock
> +    minItems: 1
>  
>  required:
>    - compatible
> @@ -72,6 +75,9 @@ allOf:
>              const: mediatek,mt8195-mdp3-split
>  
>      then:
> +      properties:
> +        clocks:
> +          maxItems: 3
>        required:
>          - mediatek,gce-client-reg
>
Krzysztof Kozlowski Sept. 27, 2024, 9:37 a.m. UTC | #2 
On Fri, Sep 27, 2024 at 01:51:40PM +0800, Moudy Ho wrote:
> From: Moudy Ho <moudy.ho@mediatek.corp-partner.google.com>
> 
> The display node in mt8195.dtsi was triggering a CHECK_DTBS error due
> to an excessively long 'clocks' property:
>   display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too long
> 
> To resolve this issue, apply the limit by setting 'maxItems: 3' in MT8195
> additional condition.
> 
> Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp: split each block to individual yaml")
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>

Your SoB does not match.

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run  and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

> --
> The purpose of this patch is to separate the corrections for
> MediaTek SPLIT CHECK_DTBS error from the original mailing list
> mentioned below:
> https://lore.kernel.org/all/20240924103156.13119-2-macpaul.lin@mediatek.com/
> 
> Changes since v1:
>   - Adding functional descriptions and quantity restrictions.
> ---
>  .../bindings/display/mediatek/mediatek,split.yaml           | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> index e4affc854f3d..bce1b8b866ce 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> @@ -57,6 +57,9 @@ properties:
>    clocks:
>      items:
>        - description: SPLIT Clock
> +      - description: HDMI RX Clock
> +      - description: HDMI Metadata Clock
> +    minItems: 1
>  
>  required:
>    - compatible
> @@ -72,6 +75,9 @@ allOf:
>              const: mediatek,mt8195-mdp3-split
>  
>      then:
> +      properties:
> +        clocks:

missing minItems

Missing constraints for all the variants.

Best regards,
Krzysztof
Moudy Ho Sept. 30, 2024, 3:20 a.m. UTC | #3 
Hi CK,

On Fri, 2024-09-27 at 09:01 +0000, CK Hu (胡俊光) wrote:
> Hi, Moudy:
> 
> On Fri, 2024-09-27 at 13:51 +0800, Moudy Ho wrote:
> > From: Moudy Ho <moudy.ho@mediatek.corp-partner.google.com>
> > 
> > The display node in mt8195.dtsi was triggering a CHECK_DTBS error
> > due
> > to an excessively long 'clocks' property:
> >   display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too
> > long
> > 
> > To resolve this issue, apply the limit by setting 'maxItems: 3' in
> > MT8195
> > additional condition.
> 
> I think the main purpose of this patch is not to add limitation.
> The main purpose is that you add HDMI RX Clock and HDMI Metadata
> Clock.
> Could you describe more about these two clock?
> mt8173-disp-split could work without these two clocks. Why mt8195-
> mdp3-split need these two clocks?
> 

Thanks for the reminder.
In hardware design, SPLIT must enable additional clocks based on its
front-end signal source, and this also implies its position in the
display pipeline.
I will elaborate further on this part of the description.

> > 
> > Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp: split
> > each block to individual yaml")
> 
> The fixes patch should be 739058a9c5c3 ("dt-bindings: display:
> mediatek: split: add compatible for MT8195")
> 
> Regards,
> CK
> 

It will be corrected in the next version.

Sincerely,
Moudy Ho
> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> > --
> > The purpose of this patch is to separate the corrections for
> > MediaTek SPLIT CHECK_DTBS error from the original mailing list
> > mentioned below:
> > 
https://lore.kernel.org/all/20240924103156.13119-2-macpaul.lin@mediatek.com/
> > 
> > Changes since v1:
> >   - Adding functional descriptions and quantity restrictions.
> > ---
> >  .../bindings/display/mediatek/mediatek,split.yaml           | 6
> > ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,split
> > .yaml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,split
> > .yaml
> > index e4affc854f3d..bce1b8b866ce 100644
> > ---
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,split
> > .yaml
> > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,split
> > .yaml
> > @@ -57,6 +57,9 @@ properties:
> >    clocks:
> >      items:
> >        - description: SPLIT Clock
> > +      - description: HDMI RX Clock
> > +      - description: HDMI Metadata Clock
> > +    minItems: 1
> >  
> >  required:
> >    - compatible
> > @@ -72,6 +75,9 @@ allOf:
> >              const: mediatek,mt8195-mdp3-split
> >  
> >      then:
> > +      properties:
> > +        clocks:
> > +          maxItems: 3
> >        required:
> >          - mediatek,gce-client-reg
> >
Moudy Ho Sept. 30, 2024, 3:28 a.m. UTC | #4 
Hi Krzysztof,

On Fri, 2024-09-27 at 11:37 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Fri, Sep 27, 2024 at 01:51:40PM +0800, Moudy Ho wrote:
> > From: Moudy Ho <moudy.ho@mediatek.corp-partner.google.com>
> > 
> > The display node in mt8195.dtsi was triggering a CHECK_DTBS error
> due
> > to an excessively long 'clocks' property:
> >   display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too
> long
> > 
> > To resolve this issue, apply the limit by setting 'maxItems: 3' in
> MT8195
> > additional condition.
> > 
> > Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp: split
> each block to individual yaml")
> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> 
> Your SoB does not match.
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Then
> please
> run  and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the
> code
> here looks like it needs a fix. Feel free to get in touch if the
> warning
> is not clear.
> 

I apololgize for the mismatch and warning. I will make the corrections
in the next version.

> > --
> > The purpose of this patch is to separate the corrections for
> > MediaTek SPLIT CHECK_DTBS error from the original mailing list
> > mentioned below:
> > 
> https://lore.kernel.org/all/20240924103156.13119-2-macpaul.lin@mediatek.com/
> > 
> > Changes since v1:
> >   - Adding functional descriptions and quantity restrictions.
> > ---
> >  .../bindings/display/mediatek/mediatek,split.yaml           | 6
> ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.y
> aml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.y
> aml
> > index e4affc854f3d..bce1b8b866ce 100644
> > ---
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.y
> aml
> > +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.y
> aml
> > @@ -57,6 +57,9 @@ properties:
> >    clocks:
> >      items:
> >        - description: SPLIT Clock
> > +      - description: HDMI RX Clock
> > +      - description: HDMI Metadata Clock
> > +    minItems: 1
> >  
> >  required:
> >    - compatible
> > @@ -72,6 +75,9 @@ allOf:
> >              const: mediatek,mt8195-mdp3-split
> >  
> >      then:
> > +      properties:
> > +        clocks:
> 
> missing minItems
> 
> Missing constraints for all the variants.
> 

Does this mean that a 'maxItems:1' condition needs to be added for
mt8173 clock property under the 'allOf' seciton?

> Best regards,
> Krzysztof
> 

Sincerely,
Moudy Ho
Krzysztof Kozlowski Oct. 3, 2024, 10:47 a.m. UTC | #5 
On 30/09/2024 05:28, Moudy Ho (何宗原) wrote:
>>>  required:
>>>    - compatible
>>> @@ -72,6 +75,9 @@ allOf:
>>>              const: mediatek,mt8195-mdp3-split
>>>  
>>>      then:
>>> +      properties:
>>> +        clocks:
>>
>> missing minItems
>>
>> Missing constraints for all the variants.
>>
> 
> Does this mean that a 'maxItems:1' condition needs to be added for
> mt8173 clock property under the 'allOf' seciton?

This means that each variant must have clearly defined, fixed list of
clocks (other properties as well)

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
index e4affc854f3d..bce1b8b866ce 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
@@ -57,6 +57,9 @@  properties:
   clocks:
     items:
       - description: SPLIT Clock
+      - description: HDMI RX Clock
+      - description: HDMI Metadata Clock
+    minItems: 1
 
 required:
   - compatible
@@ -72,6 +75,9 @@  allOf:
             const: mediatek,mt8195-mdp3-split
 
     then:
+      properties:
+        clocks:
+          maxItems: 3
       required:
         - mediatek,gce-client-reg