| Message ID | 20230906092527.18281-1-macpaul.lin@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] dt-bindings: arm64: dts: mediatek: add mt8395-evk board | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 06/09/2023 11:25, Macpaul Lin wrote: > 1. Add compatible for MT8395. > 2. Add bindings for the MediaTek mt8395-evk board, also known > as the "Genio 1200-EVK". > > The MT8195 and MT8395 belong to the same SoC family, > with only minor differences in their physical characteristics. > They utilize unique efuse values for differentiation. > > The booting process and configurations are managed by boot > loaders, firmware, and TF-A. Consequently, the part numbers > and procurement channels vary. > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > --- > Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ > 1 file changed, 5 insertions(+) Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets. > > Changes for v2: > - add more detail description for mt8395. > - add bindings for mt8395, and mt8395-evk. > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml > index ae12b1cab9fb..d7a20df640d7 100644 > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml > @@ -248,6 +248,11 @@ properties: > - enum: > - mediatek,mt8365-evk > - const: mediatek,mt8365 > + - description: MediaTek Genio 1200 Boards (Genio 1200 EVK) > + items: > + - enum: > + - mediatek,mt8395-evk > + - const: mediatek,mt8395 I don't understand. You said last time this is fully compatible with mt8195, so why it's not here? Best regards, Krzysztof
On 06/09/2023 11:25, Macpaul Lin wrote: > + edp_panel_fixed_12v: regulator-1 { > + compatible = "regulator-fixed"; > + regulator-name = "edp_backlight_12v"; > + regulator-min-microvolt = <12000000>; > + regulator-max-microvolt = <12000000>; > + enable-active-high; > + gpio = <&pio 96 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_panel_12v_en_pins>; > + }; > + > + backlight_lcd0: backlight-lcd0 { > + compatible = "pwm-backlight"; > + pwms = <&disp_pwm0 0 500000>; > + enable-gpios = <&pio 47 GPIO_ACTIVE_HIGH>; > + brightness-levels = <0 1023>; > + num-interpolated-steps = <1023>; > + default-brightness-level = <576>; > + }; > + > + backlight_lcd1: backlight-lcd1 { > + compatible = "pwm-backlight"; > + pwms = <&disp_pwm1 0 500000>; > + enable-gpios = <&pio 46 GPIO_ACTIVE_HIGH>; > + brightness-levels = <0 1023>; > + num-interpolated-steps = <1023>; > + default-brightness-level = <576>; > + }; > + > + keys: gpio-keys { > + compatible = "gpio-keys"; > + > + button-volume-up { > + wakeup-source; > + debounce-interval = <100>; > + gpios = <&pio 106 GPIO_ACTIVE_LOW>; > + label = "volume_up"; > + linux,code = <KEY_VOLUMEUP>; > + }; > + }; > + > + can_clk: can-clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <20000000>; > + clock-output-names = "can-clk"; > + }; > + > + wifi_3v3: wifi-3v3-regulator { This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. Best regards, Krzysztof
On 9/6/23 17:32, Krzysztof Kozlowski wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > On 06/09/2023 11:25, Macpaul Lin wrote: >> 1. Add compatible for MT8395. >> 2. Add bindings for the MediaTek mt8395-evk board, also known >> as the "Genio 1200-EVK". >> >> The MT8195 and MT8395 belong to the same SoC family, >> with only minor differences in their physical characteristics. >> They utilize unique efuse values for differentiation. >> >> The booting process and configurations are managed by boot >> loaders, firmware, and TF-A. Consequently, the part numbers >> and procurement channels vary. >> >> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> >> --- >> Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ >> 1 file changed, 5 insertions(+) > > Do not attach (thread) your patchsets to some other threads (unrelated > or older versions). This buries them deep in the mailbox and might > interfere with applying entire sets. > >> >> Changes for v2: >> - add more detail description for mt8395. >> - add bindings for mt8395, and mt8395-evk. >> >> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml >> index ae12b1cab9fb..d7a20df640d7 100644 >> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml >> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml >> @@ -248,6 +248,11 @@ properties: >> - enum: >> - mediatek,mt8365-evk >> - const: mediatek,mt8365 >> + - description: MediaTek Genio 1200 Boards (Genio 1200 EVK) >> + items: >> + - enum: >> + - mediatek,mt8395-evk >> + - const: mediatek,mt8395 > > > I don't understand. You said last time this is fully compatible with > mt8195, so why it's not here? > Yes, mt8395 is fully compatible with mt8195. But after reading Angelo's comment, we thought it is necessary to create a new compatible to avoid confusion for users. https://lore.kernel.org/lkml/bf8394c6-5460-8696-f46b-0c39927aaf84@collabora.com/ Although they are fully compatible, developers cannot arbitrarily replace the chip on the board with another one. So separated bindings might be better. > Best regards, > Krzysztof Thanks Macpaul Lin
On 06/09/2023 11:53, Macpaul Lin wrote: > > > On 9/6/23 17:32, Krzysztof Kozlowski wrote: >> >> >> External email : Please do not click links or open attachments until you >> have verified the sender or the content. >> >> On 06/09/2023 11:25, Macpaul Lin wrote: >>> 1. Add compatible for MT8395. >>> 2. Add bindings for the MediaTek mt8395-evk board, also known >>> as the "Genio 1200-EVK". >>> >>> The MT8195 and MT8395 belong to the same SoC family, >>> with only minor differences in their physical characteristics. >>> They utilize unique efuse values for differentiation. >>> >>> The booting process and configurations are managed by boot >>> loaders, firmware, and TF-A. Consequently, the part numbers >>> and procurement channels vary. >>> >>> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> >>> --- >>> Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ >>> 1 file changed, 5 insertions(+) >> >> Do not attach (thread) your patchsets to some other threads (unrelated >> or older versions). This buries them deep in the mailbox and might >> interfere with applying entire sets. >> >>> >>> Changes for v2: >>> - add more detail description for mt8395. >>> - add bindings for mt8395, and mt8395-evk. >>> >>> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml >>> index ae12b1cab9fb..d7a20df640d7 100644 >>> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml >>> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml >>> @@ -248,6 +248,11 @@ properties: >>> - enum: >>> - mediatek,mt8365-evk >>> - const: mediatek,mt8365 >>> + - description: MediaTek Genio 1200 Boards (Genio 1200 EVK) >>> + items: >>> + - enum: >>> + - mediatek,mt8395-evk >>> + - const: mediatek,mt8395 >> >> >> I don't understand. You said last time this is fully compatible with >> mt8195, so why it's not here? >> > > Yes, mt8395 is fully compatible with mt8195. > > But after reading Angelo's comment, we thought it is necessary to create > a new compatible to avoid confusion for users. > https://lore.kernel.org/lkml/bf8394c6-5460-8696-f46b-0c39927aaf84@collabora.com/ > New compatible is a requirement, you got clear comments from me on this as well. However we did not ask to remove a compatible of compatible devices. You also need it. Otherwise how your compatibility is going to work? Best regards, Krzysztof
On 9/6/23 17:34, Krzysztof Kozlowski wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > On 06/09/2023 11:25, Macpaul Lin wrote: >> +edp_panel_fixed_12v: regulator-1 { >> +compatible = "regulator-fixed"; >> +regulator-name = "edp_backlight_12v"; >> +regulator-min-microvolt = <12000000>; >> +regulator-max-microvolt = <12000000>; >> +enable-active-high; >> +gpio = <&pio 96 GPIO_ACTIVE_HIGH>; >> +pinctrl-names = "default"; >> +pinctrl-0 = <&edp_panel_12v_en_pins>; >> +}; >> + >> +backlight_lcd0: backlight-lcd0 { >> +compatible = "pwm-backlight"; >> +pwms = <&disp_pwm0 0 500000>; >> +enable-gpios = <&pio 47 GPIO_ACTIVE_HIGH>; >> +brightness-levels = <0 1023>; >> +num-interpolated-steps = <1023>; >> +default-brightness-level = <576>; >> +}; >> + >> +backlight_lcd1: backlight-lcd1 { >> +compatible = "pwm-backlight"; >> +pwms = <&disp_pwm1 0 500000>; >> +enable-gpios = <&pio 46 GPIO_ACTIVE_HIGH>; >> +brightness-levels = <0 1023>; >> +num-interpolated-steps = <1023>; >> +default-brightness-level = <576>; >> +}; >> + >> +keys: gpio-keys { >> +compatible = "gpio-keys"; >> + >> +button-volume-up { >> +wakeup-source; >> +debounce-interval = <100>; >> +gpios = <&pio 106 GPIO_ACTIVE_LOW>; >> +label = "volume_up"; >> +linux,code = <KEY_VOLUMEUP>; >> +}; >> +}; >> + >> +can_clk: can-clk { >> +compatible = "fixed-clock"; >> +#clock-cells = <0>; >> +clock-frequency = <20000000>; >> +clock-output-names = "can-clk"; >> +}; >> + >> +wifi_3v3: wifi-3v3-regulator { > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. This keeps a format with -regulator is because I've found some other use cases. It seems "-regulator" or "regulator-" could be arbitrary. I'm not sure if it is a new guideline for regulator's node. If there is in the devicetree document, maybe I just missed it? However if this is for the purpose of keeping the DTS format tidy, I will update it in the next version, also fix another "usb-otg-vbus-regulator" node. arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts a1d3281450ab2 (Christopher Obbard 2023-01-09 16:58:01 +0100 46) vcc5v0_sys: vcc5v0-sys-regulator { arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi 523adb553573d (Chris Morgan 2022-09-06 16:03:24 -0500 256) vcc_wifi: regulator-vcc-wifi { > Thank you. > > Best regards, > Krzysztof Thanks. Macpaul Lin > >
On 06/09/2023 12:15, Macpaul Lin wrote: >>> + >>> +wifi_3v3: wifi-3v3-regulator { >> >> This is a friendly reminder during the review process. >> >> It seems my previous comments were not fully addressed. Maybe my >> feedback got lost between the quotes, maybe you just forgot to apply it. >> Please go back to the previous discussion and either implement all >> requested changes or keep discussing them. > > > This keeps a format with -regulator is because I've found some other use > cases. It seems "-regulator" or "regulator-" could be arbitrary. I'm not > sure if it is a new guideline for regulator's node. If there is in the > devicetree document, maybe I just missed it? What is the point to name in the same DTS some of the regulators "regulator-foo" and some "foo-regulator"? Best regards, Krzysztof
Il 06/09/23 11:53, Macpaul Lin ha scritto: > > > On 9/6/23 17:32, Krzysztof Kozlowski wrote: >> >> >> External email : Please do not click links or open attachments until you have >> verified the sender or the content. >> >> On 06/09/2023 11:25, Macpaul Lin wrote: >>> 1. Add compatible for MT8395. >>> 2. Add bindings for the MediaTek mt8395-evk board, also known >>> as the "Genio 1200-EVK". >>> >>> The MT8195 and MT8395 belong to the same SoC family, >>> with only minor differences in their physical characteristics. >>> They utilize unique efuse values for differentiation. >>> >>> The booting process and configurations are managed by boot >>> loaders, firmware, and TF-A. Consequently, the part numbers >>> and procurement channels vary. >>> >>> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> >>> --- >>> Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ >>> 1 file changed, 5 insertions(+) >> >> Do not attach (thread) your patchsets to some other threads (unrelated >> or older versions). This buries them deep in the mailbox and might >> interfere with applying entire sets. >> >>> >>> Changes for v2: >>> - add more detail description for mt8395. >>> - add bindings for mt8395, and mt8395-evk. >>> >>> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml >>> b/Documentation/devicetree/bindings/arm/mediatek.yaml >>> index ae12b1cab9fb..d7a20df640d7 100644 >>> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml >>> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml >>> @@ -248,6 +248,11 @@ properties: >>> - enum: >>> - mediatek,mt8365-evk >>> - const: mediatek,mt8365 >>> + - description: MediaTek Genio 1200 Boards (Genio 1200 EVK) >>> + items: >>> + - enum: >>> + - mediatek,mt8395-evk >>> + - const: mediatek,mt8395 >> >> >> I don't understand. You said last time this is fully compatible with >> mt8195, so why it's not here? >> > > Yes, mt8395 is fully compatible with mt8195. > > But after reading Angelo's comment, we thought it is necessary to create a new > compatible to avoid confusion for users. > https://lore.kernel.org/lkml/bf8394c6-5460-8696-f46b-0c39927aaf84@collabora.com/ > > Although they are fully compatible, developers cannot arbitrarily replace the chip > on the board with another one. So separated bindings might be better. Sorry you've misunderstood; I was trying to explain to Krzysztof that the two SoCs are fully compatible... my suggestion was in like with what you're trying to do as in I am agreeing with your first version, Macpaul, to inherit MT8195. So, - enum - mediatek,mt8395-evk - const: mediatek,mt8195 - const: mediatek,mt8395 > >> Best regards, >> Krzysztof > > Thanks > Macpaul Lin
On 06/09/2023 12:48, AngeloGioacchino Del Regno wrote: >> Yes, mt8395 is fully compatible with mt8195. >> >> But after reading Angelo's comment, we thought it is necessary to create a new >> compatible to avoid confusion for users. >> https://lore.kernel.org/lkml/bf8394c6-5460-8696-f46b-0c39927aaf84@collabora.com/ >> >> Although they are fully compatible, developers cannot arbitrarily replace the chip >> on the board with another one. So separated bindings might be better. > > Sorry you've misunderstood; I was trying to explain to Krzysztof that the two > SoCs are fully compatible... my suggestion was in like with what you're trying to > do as in I am agreeing with your first version, Macpaul, to inherit MT8195. > > So, > > - enum > - mediatek,mt8395-evk > - const: mediatek,mt8195 > - const: mediatek,mt8395 The other way - mt8395 followed by mt8195 :) Best regards, Krzysztof
On 9/6/23 19:24, Krzysztof Kozlowski wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > On 06/09/2023 12:48, AngeloGioacchino Del Regno wrote: >>> Yes, mt8395 is fully compatible with mt8195. >>> >>> But after reading Angelo's comment, we thought it is necessary to create a new >>> compatible to avoid confusion for users. >>> https://lore.kernel.org/lkml/bf8394c6-5460-8696-f46b-0c39927aaf84@collabora.com/ >>> >>> Although they are fully compatible, developers cannot arbitrarily replace the chip >>> on the board with another one. So separated bindings might be better. >> >> Sorry you've misunderstood; I was trying to explain to Krzysztof that the two >> SoCs are fully compatible... my suggestion was in like with what you're trying to >> do as in I am agreeing with your first version, Macpaul, to inherit MT8195. >> >> So, >> >> - enum >> - mediatek,mt8395-evk >> - const: mediatek,mt8195 >> - const: mediatek,mt8395 > > The other way - mt8395 followed by mt8195 :) Roger that. :) > Best regards, > Krzysztof > Thanks Macpaul Lin
On 9/6/23 18:20, Krzysztof Kozlowski wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > On 06/09/2023 12:15, Macpaul Lin wrote: >>>> + >>>> +wifi_3v3: wifi-3v3-regulator { >>> >>> This is a friendly reminder during the review process. >>> >>> It seems my previous comments were not fully addressed. Maybe my >>> feedback got lost between the quotes, maybe you just forgot to apply it. >>> Please go back to the previous discussion and either implement all >>> requested changes or keep discussing them. >> >> >> This keeps a format with -regulator is because I've found some other use >> cases. It seems "-regulator" or "regulator-" could be arbitrary. I'm not >> sure if it is a new guideline for regulator's node. If there is in the >> devicetree document, maybe I just missed it? > > What is the point to name in the same DTS some of the regulators > "regulator-foo" and some "foo-regulator"? > > Best regards, > Krzysztof > It looks like I've miss the point of previous review comment. Updated new patch in v3 series. Thanks Macpaul Lin
Can you please only target Reviewers/Maintainers and mailinglists reported by get_maintainers script? I have only sent patches for mediatek and did some tests. So i'm not interested in patches for hardware i do not have here :). I guess i'm not alone... regards Frank
On 9/6/23 20:19, Frank Wunderlich wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > Can you please only target Reviewers/Maintainers and mailinglists reported by get_maintainers script? > > I have only sent patches for mediatek and did some tests. So i'm not interested in patches for hardware i do not have here :). I guess i'm not alone... > regards Frank I'm sorry for bothering you and other contributors. I've indeed run get_maintainers.sh script for these 2 patches. Here's the result. Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,commit_signer:3/4=75%) Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Matthias Brugger <matthias.bgg@gmail.com> (maintainer:ARM/Mediatek SoC support,commit_signer:3/4=75%,in file) ... AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> (reviewer:ARM/Mediatek SoC support,commit_signer:2/4=50%,authored:1/4=25%,added_lines:1/12=8%) Frank Wunderlich <frank-w@public-files.de> (commit_signer:1/4=25%,authored:1/4=25%,added_lines:1/12=8%) ... [striped...] You've contributed the similar percentage as Angelo for these files. So I think both of you could be important reviewers. I'll try to skip contributors who add lines less then 10% next time and see if this will help. Thanks! Macpaul Lin
On 07/09/2023 06:07, Macpaul Lin wrote: > > > On 9/6/23 20:19, Frank Wunderlich wrote: >> >> >> External email : Please do not click links or open attachments until you >> have verified the sender or the content. >> >> Can you please only target Reviewers/Maintainers and mailinglists reported by get_maintainers script? >> >> I have only sent patches for mediatek and did some tests. So i'm not interested in patches for hardware i do not have here :). I guess i'm not alone... >> regards Frank > > I'm sorry for bothering you and other contributors. > I've indeed run get_maintainers.sh script for these 2 patches. > Here's the result. > > Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED > DEVICE TREE BINDINGS) > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> (maintainer:OPEN > FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,commit_signer:3/4=75%) > Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND > FLATTENED DEVICE TREE BINDINGS) > Matthias Brugger <matthias.bgg@gmail.com> (maintainer:ARM/Mediatek SoC > support,commit_signer:3/4=75%,in file) > ... > AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > (reviewer:ARM/Mediatek SoC > support,commit_signer:2/4=50%,authored:1/4=25%,added_lines:1/12=8%) > Frank Wunderlich <frank-w@public-files.de> > (commit_signer:1/4=25%,authored:1/4=25%,added_lines:1/12=8%) > ... > [striped...] > > You've contributed the similar percentage as Angelo for these files. > So I think both of you could be important reviewers. > I'll try to skip contributors who add lines less then 10% next time and see > if this will help. No, commit_signer is 1. Don't cc people unrelated people. Unrelated is everyone which is not maintainer, reviewer, discussion list. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml index ae12b1cab9fb..d7a20df640d7 100644 --- a/Documentation/devicetree/bindings/arm/mediatek.yaml +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml @@ -248,6 +248,11 @@ properties: - enum: - mediatek,mt8365-evk - const: mediatek,mt8365 + - description: MediaTek Genio 1200 Boards (Genio 1200 EVK) + items: + - enum: + - mediatek,mt8395-evk + - const: mediatek,mt8395 - items: - enum: - mediatek,mt8516-pumpkin
1. Add compatible for MT8395. 2. Add bindings for the MediaTek mt8395-evk board, also known as the "Genio 1200-EVK". The MT8195 and MT8395 belong to the same SoC family, with only minor differences in their physical characteristics. They utilize unique efuse values for differentiation. The booting process and configurations are managed by boot loaders, firmware, and TF-A. Consequently, the part numbers and procurement channels vary. Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> --- Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+) Changes for v2: - add more detail description for mt8395. - add bindings for mt8395, and mt8395-evk.