Re: [PATCH 3/6] dt-bindings: display: mediatek: Fix clocks count constraint for new SoCs

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3882 bytes --]

On Fri, Sep 27, 2024 at 02:27:14AM +0000, Moudy Ho (何宗原) wrote:
> On Thu, 2024-09-26 at 11:38 +0200, AngeloGioacchino Del Regno wrote:
> > Il 25/09/24 16:34, Conor Dooley ha scritto:
> > > On Wed, Sep 25, 2024 at 04:42:59PM +0800, Macpaul Lin wrote:
> > > > 
> > > > On 9/25/24 00:00, Conor Dooley wrote:
> > > > > On Tue, Sep 24, 2024 at 01:42:01PM +0200, AngeloGioacchino Del
> > > > > Regno wrote:
> > > > > > Il 24/09/24 12:31, Macpaul Lin ha scritto:
> > > > > > > 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, add "maxItems: 3" to the 'clocks'
> > > > > > > property in
> > > > > > > the DT schema.
> > > > > > > 
> > > > > > > Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp:
> > > > > > > split each block to individual yaml")
> > > > > > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> > > > > > > ---
> > > > > > >     .../devicetree/bindings/display/mediatek/mediatek,split
> > > > > > > .yaml     | 1 +
> > > > > > >     1 file changed, 1 insertion(+)
> > > > > > > 
> > > > > > > diff --git
> > > > > > > a/Documentation/devicetree/bindings/display/mediatek/mediat
> > > > > > > ek,split.yaml
> > > > > > > b/Documentation/devicetree/bindings/display/mediatek/mediat
> > > > > > > ek,split.yaml
> > > > > > > index e4affc854f3d..42d2d483cc29 100644
> > > > > > > ---
> > > > > > > a/Documentation/devicetree/bindings/display/mediatek/mediat
> > > > > > > ek,split.yaml
> > > > > > > +++
> > > > > > > b/Documentation/devicetree/bindings/display/mediatek/mediat
> > > > > > > ek,split.yaml
> > > > > > > @@ -57,6 +57,7 @@ properties:
> > > > > > >       clocks:
> > > > > > >         items:
> > > > > > >           - description: SPLIT Clock
> > > > > > 
> > > > > > That's at least confusing (granted that it works) - either
> > > > > > add a description for
> > > > > > each clock and then set `minItems: 1` (preferred), or remove
> > > > > > this "SPLIT Clock"
> > > > > > description and allow a maximum of 3 clocks.
> > > > > > 
> > > > > > Removing the description can be done - IMO - because "SPLIT
> > > > > > Clock" is, well,
> > > > > > saying that the SPLIT block gets a SPLIT clock ... stating
> > > > > > the obvious, anyway.
> > > > > 
> > > > > Right, but what are the other two new clocks? Are they as
> > > > > obvious?
> > > > > There's no clock-names here to give any more information as to
> > > > > what the
> > > > > other clocks are supposed to be.
> > > > > 
> > > > > Kinda unrelated, but I think that "SPLIT Clock" probably isn't
> > > > > what the
> > > > > name of the clock in the IP block is anyway, sounds more like
> > > > > the name
> > > > > for it on the provider end..
> > > > 
> > > > 
>  * Thanks for the suggestions. I think Moudy could help on the new
>    fixes
> > > > for both DT schem and mt8195.dtsi. This patch could be separated
> > > > from
> > > > origin patch set.
> > > 
> > > Not sure what you mean about separating it, if you mean correcting
> > > the
> > > description for the split clock sure. The other stuff I mentioned
> > > needs
> > > to be resolved before I'm willing to ack this.
> > 
> > He means separating this patch from the rest of the series that he
> > pushed - which
> > is okay, as it's a bit mixed anyway :-)
> > 
> > Besides ... Moudy, can you please help to clarify the description of
> > those clocks?
> 
> May I address the check error by adding 2 functional descriptions and
> specifying 'minIeims: 1' for this property, and apply the limit by
> setting 'maxItems: 3' in MT8195 additional condition?


That sounds reasonable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]