diff mbox series

[v2,03/15] dt-bindings: display: mediatek: merge: Add compatible for MT8188

Message ID 20230614073125.17958-4-shawn.sung@mediatek.com
State New
Headers
Series Add display driver for MT8188 VDOSYS1 |

Commit Message

Shawn Sung (宋孝謙) June 14, 2023, 7:31 a.m. UTC 
  Add compatible name for MediaTek MT8188 MERGE.

Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 .../devicetree/bindings/display/mediatek/mediatek,merge.yaml   | 3 +++
 1 file changed, 3 insertions(+)

--
2.18.0

Comments

AngeloGioacchino Del Regno June 14, 2023, 11:41 a.m. UTC | #1 
Il 14/06/23 09:31, Hsiao Chien Sung ha scritto:
> Add compatible name for MediaTek MT8188 MERGE.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Krzysztof Kozlowski June 15, 2023, 8:28 a.m. UTC | #2 
On 14/06/2023 09:31, Hsiao Chien Sung wrote:
> Add compatible name for MediaTek MT8188 MERGE.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  .../devicetree/bindings/display/mediatek/mediatek,merge.yaml   | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
> index eead5cb8636e..5c678695162e 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
> @@ -27,6 +27,9 @@ properties:
>        - items:
>            - const: mediatek,mt6795-disp-merge
>            - const: mediatek,mt8173-disp-merge
> +      - items:
> +          - const: mediatek,mt8188-disp-merge
> +          - const: mediatek,mt8195-disp-merge

Linux next has something entirely different. I don't know the base here,
but it's really, really different and it suggests you should add mt8188
to an enum with mt8173.

Best regards,
Krzysztof
Shawn Sung (宋孝謙) June 16, 2023, 5:29 a.m. UTC | #3 
Hi Krzysztof,

Thanks for the reminder, because MT8188 is not related to MT8173, I’ll
keep it as it is for now, however, I do find that MT8195 doesn’t exist
in this dt-bindings which it should be, so there may be conflicts when
this series is going to be merged.

Best regards,
Hsiao Chien Sung

On Thu, 2023-06-15 at 10:28 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 14/06/2023 09:31, Hsiao Chien Sung wrote:
> > Add compatible name for MediaTek MT8188 MERGE.
> > 
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
> >  .../devicetree/bindings/display/mediatek/mediatek,merge.yaml   | 3
> +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.y
> aml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.y
> aml
> > index eead5cb8636e..5c678695162e 100644
> > ---
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.y
> aml
> > +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.y
> aml
> > @@ -27,6 +27,9 @@ properties:
> >        - items:
> >            - const: mediatek,mt6795-disp-merge
> >            - const: mediatek,mt8173-disp-merge
> > +      - items:
> > +          - const: mediatek,mt8188-disp-merge
> > +          - const: mediatek,mt8195-disp-merge
> 
> Linux next has something entirely different. I don't know the base
> here,
> but it's really, really different and it suggests you should add
> mt8188
> to an enum with mt8173.
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski June 16, 2023, 8:07 a.m. UTC | #4 
On 16/06/2023 07:29, Shawn Sung (宋孝謙) wrote:
> Hi Krzysztof,
> 
> Thanks for the reminder, because MT8188 is not related to MT8173,

How does it matter?

>  I’ll
> keep it as it is for now, however, I do find that MT8195 doesn’t exist
> in this dt-bindings which it should be, so there may be conflicts when
> this series is going to be merged.

Don't top post.

No, rebase on current next and implement my comment.

Best regards,
Krzysztof
Shawn Sung (宋孝謙) June 16, 2023, 8:40 a.m. UTC | #5 
On Fri, 2023-06-16 at 10:07 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 16/06/2023 07:29, Shawn Sung (宋孝謙) wrote:
> > Hi Krzysztof,
> > 
> > Thanks for the reminder, because MT8188 is not related to MT8173,
> 
> How does it matter?

Because MT8188 Merge is fully compatible with MT8195, we didn't add its
compatible name to the driver, but just list it in dt-bindings, and use
MT8195's compatible name to match the ID in device table. For example,
in mt8188.dtsi:

merge1: merge@1c10c000 {
        compatible = "mediatek,mt8188-disp-merge", "mediatek,mt8195-
disp-merge";
        ...
};

If we add MT8188 Merge as an enum with MT8173, then our device tree
must be as below, and nothing will match in Merge driver.

merge1: merge@1c10c000 {
        compatible = "mediatek,mt8188-disp-
merge";
        ...
};

> 
> >  I’ll
> > keep it as it is for now, however, I do find that MT8195 doesn’t
> exist
> > in this dt-bindings which it should be, so there may be conflicts
> when
> > this series is going to be merged.
> 
> Don't top post.
> 
> No, rebase on current next and implement my comment.

Will rebase linux-next in the next version.

> 
> Best regards,
> Krzysztof
> 

Best regards,
Hsiao Chien Sung
Krzysztof Kozlowski June 16, 2023, 9:31 a.m. UTC | #6 
On 16/06/2023 10:40, Shawn Sung (宋孝謙) wrote:
> On Fri, 2023-06-16 at 10:07 +0200, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On 16/06/2023 07:29, Shawn Sung (宋孝謙) wrote:
>>> Hi Krzysztof,
>>>
>>> Thanks for the reminder, because MT8188 is not related to MT8173,
>>
>> How does it matter?
> 
> Because MT8188 Merge is fully compatible with MT8195, we didn't add its
> compatible name to the driver, but just list it in dt-bindings, and use
> MT8195's compatible name to match the ID in device table. For example,
> in mt8188.dtsi:
> 
> merge1: merge@1c10c000 {
>         compatible = "mediatek,mt8188-disp-merge", "mediatek,mt8195-
> disp-merge";
>         ...
> };
> 
> If we add MT8188 Merge as an enum with MT8173, then our device tree
> must be as below, and nothing will match in Merge driver.
> 
> merge1: merge@1c10c000 {
>         compatible = "mediatek,mt8188-disp-
> merge";
>         ...
> };

No, why? It would be incorrect with existing bindings. Again, on what
tree are you working?

> 
>>
>>>  I’ll
>>> keep it as it is for now, however, I do find that MT8195 doesn’t
>> exist
>>> in this dt-bindings which it should be, so there may be conflicts
>> when
>>> this series is going to be merged.
>>
>> Don't top post.
>>
>> No, rebase on current next and implement my comment.
> 
> Will rebase linux-next in the next version.

Rebase now - for this discussion.

Best regards,
Krzysztof
Shawn Sung (宋孝謙) June 16, 2023, 11:51 a.m. UTC | #7 
On Fri, 2023-06-16 at 11:31 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 16/06/2023 10:40, Shawn Sung (宋孝謙) wrote:
> > On Fri, 2023-06-16 at 10:07 +0200, Krzysztof Kozlowski wrote:
> >>   
> >> External email : Please do not click links or open attachments
> until
> >> you have verified the sender or the content.
> >>  On 16/06/2023 07:29, Shawn Sung (宋孝謙) wrote:
> >>> Hi Krzysztof,
> >>>
> >>> Thanks for the reminder, because MT8188 is not related to MT8173,
> >>
> >> How does it matter?
> > 
> > Because MT8188 Merge is fully compatible with MT8195, we didn't add
> its
> > compatible name to the driver, but just list it in dt-bindings, and
> use
> > MT8195's compatible name to match the ID in device table. For
> example,
> > in mt8188.dtsi:
> > 
> > merge1: merge@1c10c000 {
> >         compatible = "mediatek,mt8188-disp-merge",
> "mediatek,mt8195-
> > disp-merge";
> >         ...
> > };
> > 
> > If we add MT8188 Merge as an enum with MT8173, then our device tree
> > must be as below, and nothing will match in Merge driver.
> > 
> > merge1: merge@1c10c000 {
> >         compatible = "mediatek,mt8188-disp-
> > merge";
> >         ...
> > };
> 
> No, why? It would be incorrect with existing bindings. Again, on what
> tree are you working?

Just sent a v3 that is based on next-20230616 tag of linux-next/master
branch. After checking, still not sure what is the difference between
this modification and the others in mdp-rdma and ethdr. Could you share
more information please? 

> 
> > 
> >>
> >>>  I’ll
> >>> keep it as it is for now, however, I do find that MT8195 doesn’t
> >> exist
> >>> in this dt-bindings which it should be, so there may be conflicts
> >> when
> >>> this series is going to be merged.
> >>
> >> Don't top post.
> >>
> >> No, rebase on current next and implement my comment.
> > 
> > Will rebase linux-next in the next version.
> 
> Rebase now - for this discussion.

Done. Please refer to v3.

> 
> Best regards,
> Krzysztof
> 

Thanks,
Hsiao Chien Sung
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
index eead5cb8636e..5c678695162e 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
@@ -27,6 +27,9 @@  properties:
       - items:
           - const: mediatek,mt6795-disp-merge
           - const: mediatek,mt8173-disp-merge
+      - items:
+          - const: mediatek,mt8188-disp-merge
+          - const: mediatek,mt8195-disp-merge

   reg:
     maxItems: 1