[permalink] [raw]

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

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>

Thanks again!

Cheers,
Angelo

2023-04-12 13:14:33

by Alexandre Mergnat

[permalink] [raw]

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

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
>>> <[email protected]>
>>> ---
>>> 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.

--
Regards,
Alexandre

2023-04-12 13:22:41

by Alexandre Mergnat

[permalink] [raw]

Il 13/04/23 10:52, Matthias Brugger ha scritto:
>
>
> On 12/04/2023 23:03, Rob Herring wrote:
>>
>> On Wed, 12 Apr 2023 13:27:13 +0200, AngeloGioacchino Del Regno wrote:
>>> Add a compatible string for MediaTek Helio X10 MT6795's display PWM
>>> block: this is the same as MT8173.
>>>
>>> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>
>> Running 'make dtbs_check' with the schema in this patch gives the
>> following warnings. Consider if they are expected or the schema is
>> incorrect. These may not be new warnings.
>>
>
> These are not new warnings. I think we should address them in a different patch. In
> my opinion it shouldn't block this patch. In the end it only add as compatible here.
>

I agree. These warnings should not block this patch, as they're totally unrelated
to it.

Regards,
Angelo

2023-04-14 05:54:09

2023-04-14 08:44:46

by Uwe Kleine-König

[permalink] [raw]

Subject: Re: [PATCH 01/27] dt-bindings: pwm: Add compatible for MediaTek MT6795

On Fri, Apr 14, 2023 at 10:21:05AM +0200, Krzysztof Kozlowski wrote:
> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
> > Add a compatible string for MediaTek Helio X10 MT6795's display PWM
> > block: this is the same as MT8173.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> > ---
> > Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml b/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml
> > index 0088bc8e7c54..153e146df7d4 100644
> > --- a/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml
> > +++ b/Documentation/devicetree/bindings/pwm/mediatek,pwm-disp.yaml
> > @@ -22,7 +22,9 @@ properties:
> > - mediatek,mt8173-disp-pwm
> > - mediatek,mt8183-disp-pwm
> > - items:
> > - - const: mediatek,mt8167-disp-pwm
> > + - enum:
> > + - mediatek,mt6795-disp-pwm
> > + - mediatek,mt8167-disp-pwm
>
> This does not look correct. You do not add compatible, you replace
> breaking all mt8167-disp-pwm. At least it looks like this from context.

I thought the old semantic to be:

"mediatek,mt8167-disp-pwm"

and the new

"mediatek,mt6795-disp-pwm" or "mediatek,mt8167-disp-pwm"

. What am I missing?

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachments:

(No filename) (1.58 kB)
signature.asc (499.00 B)
Download all attachments

2023-04-14 08:47:48

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH 13/27] dt-bindings: display: mediatek: ufoe: Add compatible for MediaTek MT6795

2023-04-14 08:48:20

by Krzysztof Kozlowski

[permalink] [raw]

On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
> This smartphone uses the Helio X10 standard MT6331+MT6332 combo PMICs:
> include the mt6331 devicetree and add the required interrupt.
>
> Note that despite there being two interrupts, one for MT6331 and one
> for MT6332, in configurations using the companion PMIC, the interrupt
> of the latter fires for both events on MT6331 and for ones on MT6332,
> while the interrupt for the main PMIC fires only for events of the
> main PMIC.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>

Applied, thanks

> ---
> arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts
> index a0e01a756f03..debe0f2553d9 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts
> @@ -7,6 +7,7 @@
> /dts-v1/;
> #include <dt-bindings/gpio/gpio.h>
> #include "mt6795.dtsi"
> +#include "mt6331.dtsi"
>
> / {
> model = "Sony Xperia M5";
> @@ -219,6 +220,15 @@ pins-tx {
> };
> };
>
> +&pmic {
> + /*
> + * Smartphones, including the Xperia M5, are equipped with a companion
> + * MT6332 PMIC: when this is present, the main MT6331 PMIC will fire
> + * an interrupt on the companion, so we use the MT6332 IRQ GPIO.
> + */
> + interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
> +};
> +
> &uart0 {
> status = "okay";
>

2023-06-13 00:46:03

by Chun-Kuang Hu

[permalink] [raw]

Subject: Re: [PATCH 03/27] dt-bindings: display: mediatek: dpi: Add compatible for MediaTek MT6795

Hi, Matthias:

Matthias Brugger <[email protected]> 於 2023年5月29日週一下午10:07寫道：
>
> Hi Chun-Kuang Hu,
>
> Can you help to merge the missing DT-binding patches in this series?

Apply display binding of this series to mediatek-drm-next [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

>
> Thanks a lot,
> Matthias
>
> On 12/04/2023 13:27, AngeloGioacchino Del Regno wrote:
> > Add a compatible string for the MediaTek Helio X10 MT6795 SoC, using
> > the same parameters as MT8183.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> > ---
> > .../display/mediatek/mediatek,dpi.yaml | 23 +++++++++++--------
> > 1 file changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> > index d976380801e3..803c00f26206 100644
> > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> > @@ -17,15 +17,20 @@ description: |
> >
> > properties:
> > compatible:
> > - enum:
> > - - mediatek,mt2701-dpi
> > - - mediatek,mt7623-dpi
> > - - mediatek,mt8173-dpi
> > - - mediatek,mt8183-dpi
> > - - mediatek,mt8186-dpi
> > - - mediatek,mt8188-dp-intf
> > - - mediatek,mt8192-dpi
> > - - mediatek,mt8195-dp-intf
> > + oneOf:
> > + - enum:
> > + - mediatek,mt2701-dpi
> > + - mediatek,mt7623-dpi
> > + - mediatek,mt8173-dpi
> > + - mediatek,mt8183-dpi
> > + - mediatek,mt8186-dpi
> > + - mediatek,mt8188-dp-intf
> > + - mediatek,mt8192-dpi
> > + - mediatek,mt8195-dp-intf
> > + - items:
> > + - enum:
> > + - mediatek,mt6795-dpi
> > + - const: mediatek,mt8183-dpi
> >
> > reg:
> > maxItems: 1