diff mbox series

[02/27] dt-bindings: phy: mediatek,dsi-phy: Add compatible for MT6795 Helio X10

Message ID 20230412112739.160376-3-angelogioacchino.delregno@collabora.com
State New
Headers
Series MediaTek Helio X10 - Mailbox, Display, MMC/SD/SDIO |

Commit Message

AngeloGioacchino Del Regno April 12, 2023, 11:27 a.m. UTC 
  Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
the same DSI PHY as MT8173.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Matthias Brugger April 12, 2023, 12:34 p.m. UTC | #1 
On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
> the same DSI PHY as MT8173.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> index 26f2b887cfc1..a9f78344efdb 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> @@ -24,6 +24,10 @@ properties:
>             - enum:
>                 - mediatek,mt7623-mipi-tx
>             - const: mediatek,mt2701-mipi-tx
> +      - items:
> +          - enum:
> +              - mediatek,mt6795-mipi-tx
> +          - const: mediatek,mt8173-mipi-tx

I suppose you expect more SoCs to share the same fallback in the future, apart 
from keeping in sync with other comaptibles described here, so:

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>



>         - items:
>             - enum:
>                 - mediatek,mt8365-mipi-tx
Alexandre Mergnat April 12, 2023, 12:59 p.m. UTC | #2 
On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
> the same DSI PHY as MT8173.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> index 26f2b887cfc1..a9f78344efdb 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> @@ -24,6 +24,10 @@ properties:
>             - enum:
>                 - mediatek,mt7623-mipi-tx
>             - const: mediatek,mt2701-mipi-tx
> +      - items:
> +          - enum:
> +              - mediatek,mt6795-mipi-tx
> +          - const: mediatek,mt8173-mipi-tx

AFAIK, it should be:
       - items:
           - const: mediatek,mt6795-mipi-tx
           - const: mediatek,mt8173-mipi-tx

Since it isn't respected above for mt7623, it may be tolerated.
Please, take this comment as a suggestion, isn't a NAK from me.

>         - items:
>             - enum:
>                 - mediatek,mt8365-mipi-tx

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
AngeloGioacchino Del Regno April 12, 2023, 1:03 p.m. UTC | #3 
Il 12/04/23 14:59, Alexandre Mergnat ha scritto:
> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
>> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
>> the same DSI PHY as MT8173.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml 
>> b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>> index 26f2b887cfc1..a9f78344efdb 100644
>> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>> @@ -24,6 +24,10 @@ properties:
>>             - enum:
>>                 - mediatek,mt7623-mipi-tx
>>             - const: mediatek,mt2701-mipi-tx
>> +      - items:
>> +          - enum:
>> +              - mediatek,mt6795-mipi-tx
>> +          - const: mediatek,mt8173-mipi-tx
> 
> AFAIK, it should be:
>        - items:
>            - const: mediatek,mt6795-mipi-tx
>            - const: mediatek,mt8173-mipi-tx
> 
> Since it isn't respected above for mt7623, it may be tolerated.
> Please, take this comment as a suggestion, isn't a NAK from me.
> 

First of all, Thanks!
I want to explain, though, the reason for that.

If you check all the commits, on some I did it as you just proposed, while
on some others I did it with an enum before const: that's simply because I
*totally expect* some to grow, while others (const - const) I was either
unsure, or totally *not* expecting them to grow soon!

>>         - items:
>>             - enum:
>>                 - mediatek,mt8365-mipi-tx
> 
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> 

Thanks again!

Cheers,
Angelo
Alexandre Mergnat April 12, 2023, 1:12 p.m. UTC | #4 
On 12/04/2023 15:03, AngeloGioacchino Del Regno wrote:
> Il 12/04/23 14:59, Alexandre Mergnat ha scritto:
>> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
>>> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
>>> the same DSI PHY as MT8173.
>>>
>>> Signed-off-by: AngeloGioacchino Del Regno 
>>> <angelogioacchino.delregno@collabora.com>
>>> ---
>>>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml 
>>> b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>> index 26f2b887cfc1..a9f78344efdb 100644
>>> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>> @@ -24,6 +24,10 @@ properties:
>>>             - enum:
>>>                 - mediatek,mt7623-mipi-tx
>>>             - const: mediatek,mt2701-mipi-tx
>>> +      - items:
>>> +          - enum:
>>> +              - mediatek,mt6795-mipi-tx
>>> +          - const: mediatek,mt8173-mipi-tx
>>
>> AFAIK, it should be:
>>        - items:
>>            - const: mediatek,mt6795-mipi-tx
>>            - const: mediatek,mt8173-mipi-tx
>>
>> Since it isn't respected above for mt7623, it may be tolerated.
>> Please, take this comment as a suggestion, isn't a NAK from me.
>>
> 
> First of all, Thanks!
> I want to explain, though, the reason for that.
> 
> If you check all the commits, on some I did it as you just proposed, while
> on some others I did it with an enum before const: that's simply because I
> *totally expect* some to grow, while others (const - const) I was either
> unsure, or totally *not* expecting them to grow soon!


That's what I thought. IMHO, if someone add another compat later, he 
will be on charge to change the const by enum front of your 
"mediatek,mt6795-mipi-tx". But my opinion is probably not the most popular.

I will not make the same feedback for the other patches in this series.
AngeloGioacchino Del Regno April 12, 2023, 1:17 p.m. UTC | #5 
Il 12/04/23 15:12, Alexandre Mergnat ha scritto:
> On 12/04/2023 15:03, AngeloGioacchino Del Regno wrote:
>> Il 12/04/23 14:59, Alexandre Mergnat ha scritto:
>>> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
>>>> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
>>>> the same DSI PHY as MT8173.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno 
>>>> <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml 
>>>> b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>>> index 26f2b887cfc1..a9f78344efdb 100644
>>>> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>>> @@ -24,6 +24,10 @@ properties:
>>>>             - enum:
>>>>                 - mediatek,mt7623-mipi-tx
>>>>             - const: mediatek,mt2701-mipi-tx
>>>> +      - items:
>>>> +          - enum:
>>>> +              - mediatek,mt6795-mipi-tx
>>>> +          - const: mediatek,mt8173-mipi-tx
>>>
>>> AFAIK, it should be:
>>>        - items:
>>>            - const: mediatek,mt6795-mipi-tx
>>>            - const: mediatek,mt8173-mipi-tx
>>>
>>> Since it isn't respected above for mt7623, it may be tolerated.
>>> Please, take this comment as a suggestion, isn't a NAK from me.
>>>
>>
>> First of all, Thanks!
>> I want to explain, though, the reason for that.
>>
>> If you check all the commits, on some I did it as you just proposed, while
>> on some others I did it with an enum before const: that's simply because I
>> *totally expect* some to grow, while others (const - const) I was either
>> unsure, or totally *not* expecting them to grow soon!
> 
> 
> That's what I thought. IMHO, if someone add another compat later, he will be on 
> charge to change the const by enum front of your "mediatek,mt6795-mipi-tx". But my 
> opinion is probably not the most popular.
> 
> I will not make the same feedback for the other patches in this series.
> 

I honestly don't know what's the most popular opinion about that... but whatever,
in any case... just want to make sure to communicate that I don't really have
strong opinions about doing it one way or the other.

The arguments in favor and against that are probably 1:1... :-D

Cheers!
Angelo
Matthias Brugger April 13, 2023, 8:58 a.m. UTC | #6 
On 12/04/2023 15:17, AngeloGioacchino Del Regno wrote:
> Il 12/04/23 15:12, Alexandre Mergnat ha scritto:
>> On 12/04/2023 15:03, AngeloGioacchino Del Regno wrote:
>>> Il 12/04/23 14:59, Alexandre Mergnat ha scritto:
>>>> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
>>>>> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
>>>>> the same DSI PHY as MT8173.
>>>>>
>>>>> Signed-off-by: AngeloGioacchino Del Regno 
>>>>> <angelogioacchino.delregno@collabora.com>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>>>>>   1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml 
>>>>> b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>>>> index 26f2b887cfc1..a9f78344efdb 100644
>>>>> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>>>> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
>>>>> @@ -24,6 +24,10 @@ properties:
>>>>>             - enum:
>>>>>                 - mediatek,mt7623-mipi-tx
>>>>>             - const: mediatek,mt2701-mipi-tx
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - mediatek,mt6795-mipi-tx
>>>>> +          - const: mediatek,mt8173-mipi-tx
>>>>
>>>> AFAIK, it should be:
>>>>        - items:
>>>>            - const: mediatek,mt6795-mipi-tx
>>>>            - const: mediatek,mt8173-mipi-tx
>>>>
>>>> Since it isn't respected above for mt7623, it may be tolerated.
>>>> Please, take this comment as a suggestion, isn't a NAK from me.
>>>>
>>>
>>> First of all, Thanks!
>>> I want to explain, though, the reason for that.
>>>
>>> If you check all the commits, on some I did it as you just proposed, while
>>> on some others I did it with an enum before const: that's simply because I
>>> *totally expect* some to grow, while others (const - const) I was either
>>> unsure, or totally *not* expecting them to grow soon!
>>
>>
>> That's what I thought. IMHO, if someone add another compat later, he will be 
>> on charge to change the const by enum front of your "mediatek,mt6795-mipi-tx". 
>> But my opinion is probably not the most popular.
>>
>> I will not make the same feedback for the other patches in this series.
>>
> 
> I honestly don't know what's the most popular opinion about that... but whatever,
> in any case... just want to make sure to communicate that I don't really have
> strong opinions about doing it one way or the other.
> 
> The arguments in favor and against that are probably 1:1... :-D
> 

Then let me throw in another one :)

Take into account that if we expect the compatible to be added somtimes in the 
future (not the near future) this code will lay around for some time. People 
will take this code as an example for new code, then we will need to explain 
it... In that sense it would make more sense to have all made const: const: and 
change this to enum once a new compatible is added to the mix.

Said all this, I leave it to the DT maintainers to decide :D

Regards,
Matthias
Krzysztof Kozlowski April 14, 2023, 8:22 a.m. UTC | #7 
On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
> the same DSI PHY as MT8173.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>  1 file changed, 4 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Matthias Brugger May 29, 2023, 2:05 p.m. UTC | #8 
On 14/04/2023 10:22, Krzysztof Kozlowski wrote:
> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
>> Add a compatible string for MediaTek Helio X10 MT6795: this SoC uses
>> the same DSI PHY as MT8173.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
> 

Applied, thanks!
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
index 26f2b887cfc1..a9f78344efdb 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
@@ -24,6 +24,10 @@  properties:
           - enum:
               - mediatek,mt7623-mipi-tx
           - const: mediatek,mt2701-mipi-tx
+      - items:
+          - enum:
+              - mediatek,mt6795-mipi-tx
+          - const: mediatek,mt8173-mipi-tx
       - items:
           - enum:
               - mediatek,mt8365-mipi-tx