diff mbox series

[1/2] dt-bindings: arm64: dts: mediatek: Add mt8395-evk board

Message ID 20230904092043.5157-1-macpaul.lin@mediatek.com
State Changes Requested
Headers show
Series [1/2] dt-bindings: arm64: dts: mediatek: Add mt8395-evk board | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Macpaul Lin Sept. 4, 2023, 9:20 a.m. UTC
Add bindings for the MediaTek mt8395-evk board.
The mt8359-evk board is also named as "Genio 1200-EVK".
MT8195 and MT8395 are the same family series SoC could share
many of the peripheral drivers.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski Sept. 4, 2023, 9:33 a.m. UTC | #1
On 04/09/2023 11:20, Macpaul Lin wrote:
> Add bindings for the MediaTek mt8395-evk board.
> The mt8359-evk board is also named as "Genio 1200-EVK".
> MT8195 and MT8395 are the same family series SoC could share

How can be the same and have different numbers? You sill need dedicated
compatible.




Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2023, 9:37 a.m. UTC | #2
On 04/09/2023 11:20, Macpaul Lin wrote:
> Add basic device-tree for the Genio 1200 EVK board. The
> Demo board is made by MediaTek and has a MT8395 SoC (MT8195 family),
> associated with the MT6359 and MT6360 PMICs, and
> the MT7921 connectivity chip.
> 
> The IOs available on that board are:
> * 1 USB Type-C connector with DP aux mode support
> * 2 USB Type-A connector with a USB hub
> * 1 micro-USB port for gadget or OTG support
> * 1 full size HDMI RX and 1 full size HDMI TX connector
> * 1 micro SD slot
> * 40 pins header
> * SPI interface header
> * 1 M.2 slot
> * 1 audio jack
> * 1 micro-USB port for serial debug
> * 2 connectors for DSI displays, 1 of the DSI panel is installed
> * 3 connectors for CSI cameras
> * 1 connector for a eDP panel
> * 1 MMC storage
> * 1 Touch Panel (installed DSI display)
> * 1 M.2 slot for 5G dongle
> 
> This commit adds basic support in order to be able to boot.
> 
> Signed-off-by: Ben Lok <ben.lok@mediatek.com>
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>  .../boot/dts/mediatek/genio-1200-evk.dts      | 931 ++++++++++++++++++
>  2 files changed, 932 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/genio-1200-evk.dts
> 
> Notes for v1:
>  - This dts patch has been checked with 'make dtbs_check W=1', however, some
>    warnings are caused by mt8195.dtsi, should be fixed by separate patches. 
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index c99c3372a4b5..5bf29581f08b 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_MEDIATEK) += genio-1200-evk.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/genio-1200-evk.dts
> new file mode 100644
> index 000000000000..696ad535ac8f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/genio-1200-evk.dts
> @@ -0,0 +1,931 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2023 MediaTek Inc.
> + * Author: Ben Lok <ben.lok@mediatek.com>
> + *	   Macpaul Lin <macpaul.lin@mediatek.com>
> + */
> +/dts-v1/;
> +
> +#include "mt8195.dtsi"
> +#include "mt6359.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> +#include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
> +#include <dt-bindings/spmi/spmi.h>
> +#include <dt-bindings/usb/pd.h>
> +
> +/ {
> +	model = "MediaTek Genio 1200 EVK-P1V2-EMMC";
> +	compatible = "mediatek,mt8395-evk", "mediatek,mt8195";

In the binding patch you said mt8395 is a SoC, so you miss here its
compatible.

> +
> +	aliases {
> +		serial0 = &uart0;
> +		ethernet0 = &eth;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:921600n8";
> +	};
> +
> +	firmware {
> +		optee {
> +			compatible = "linaro,optee-tz";
> +			method = "smc";
> +		};
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0 0x40000000 0x2 0x00000000>;
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		/* 12 MiB reserved for OP-TEE (BL32)

Use Linux coding style comments.


> +	can_clk: can-clk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <20000000>;
> +		clock-output-names = "can-clk";
> +	};
> +
> +	wifi_pwr_fixed_3v3: wifi-pwr-fixed-3v3 {

And this is not a regulator? All other nodes are called regulator-X

> +		compatible = "regulator-fixed";
> +		regulator-name = "wifi_pwr_fixed_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 135 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-always-on;
> +	};
> +};

....

> +
> +&i2c6 {
> +	clock-frequency = <400000>;
> +	pinctrl-0 = <&i2c6_pins>;
> +	pinctrl-names = "default";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	mt6360: mt6360@34 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +		compatible = "mediatek,mt6360";
> +		reg = <0x34>;
> +		interrupts = <128 IRQ_TYPE_EDGE_FALLING>;
> +		interrupt-names = "IRQB";
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		pinctrl-0 = <&mt6360_pins>;
> +
> +		charger {
> +			compatible = "mediatek,mt6360-chg";
> +			richtek,vinovp-microvolt = <14500000>;
> +
> +			otg_vbus_regulator: usb-otg-vbus-regulator {
> +				regulator-name = "usb-otg-vbus";
> +				regulator-min-microvolt = <4425000>;
> +				regulator-max-microvolt = <5825000>;
> +			};
> +		};
> +
> +		regulator {
> +			compatible = "mediatek,mt6360-regulator";
> +			LDO_VIN3-supply = <&mt6360_buck2>;
> +
> +			mt6360_buck1: buck1 {
> +				regulator-name = "emi_vdd2";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP
> +							   MT6360_OPMODE_ULP>;
> +				regulator-always-on;
> +			};
> +
> +			mt6360_buck2: buck2 {
> +				regulator-name = "emi_vddq";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP
> +							   MT6360_OPMODE_ULP>;
> +				regulator-always-on;
> +			};
> +
> +			mt6360_ldo1: ldo1 {
> +				regulator-name = "tp1_p3v0";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP>;
> +				regulator-always-on;
> +			};
> +
> +			mt6360_ldo2: ldo2 {
> +				regulator-name = "panel1_p1v8";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP>;
> +			};
> +
> +			mt6360_ldo3: ldo3 {
> +				regulator-name = "vmc_pmu";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <3600000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP>;
> +			};
> +
> +			mt6360_ldo5: ldo5 {
> +				regulator-name = "vmch_pmu";
> +				regulator-min-microvolt = <2700000>;
> +				regulator-max-microvolt = <3600000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP>;
> +			};
> +
> +			/* This is a measure point, which name is mt6360_ldo1 on schematic */
> +			mt6360_ldo6: ldo6 {
> +				regulator-name = "mt6360_ldo1";
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <2100000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP>;
> +			};
> +
> +			mt6360_ldo7: ldo7 {
> +				regulator-name = "emi_vmddr_en";
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <2100000>;
> +				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> +							   MT6360_OPMODE_LP>;
> +				regulator-always-on;
> +			};
> +		};
> +

Stray blank line

> +	};
> +};
> +
> +&spi1 {
> +	pinctrl-0 = <&spi1_pins>;
> +	pinctrl-names = "default";
> +	mediatek,pad-select = <0>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +	cs-gpios = <&pio 64 GPIO_ACTIVE_LOW>;
> +
> +	can0: can@0 {
> +		compatible = "microchip,mcp2518fd";
> +		reg = <0>;
> +		clocks = <&can_clk>;
> +		spi-max-frequency = <20000000>;
> +		interrupts-extended = <&pio 16 IRQ_TYPE_LEVEL_LOW>;
> +		vdd-supply = <&mt6359_vcn33_2_bt_ldo_reg>;
> +		xceiver-supply = <&mt6359_vcn33_2_bt_ldo_reg>;
> +	};
> +};
> +
> +&spi2 {
> +	pinctrl-0 = <&spi2_pins>;
> +	pinctrl-names = "default";
> +	mediatek,pad-select = <0>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +};
> +
> +&xhci0 {
> +	status = "okay";
> +};
> +
> +&xhci1 {
> +	vusb33-supply = <&mt6359_vusb_ldo_reg>;
> +	status = "okay";
> +};
> +
> +&xhci2 {
> +	vusb33-supply = <&mt6359_vusb_ldo_reg>;
> +	status = "okay";
> +};
> +
> +&xhci3 {
> +	vusb33-supply = <&mt6359_vusb_ldo_reg>;
> +	status = "okay";
> +};
> +
> +&u3port0 {
> +	status = "okay";
> +};
> +

You have quite surprising ordering of these overrides...

...

> +
> +&spmi {
> +	#address-cells = <2>;
> +	#size-cells = <0>;
> +
> +	mt6315_6: mt6315@6 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +		compatible = "mediatek,mt6315-regulator";
> +		reg = <0x6 SPMI_USID>;
> +
> +		regulators {
> +			mt6315_6_vbuck1: vbuck1 {
> +				regulator-compatible = "vbuck1";
> +				regulator-name = "Vbcpu";
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1193750>;
> +				regulator-enable-ramp-delay = <256>;
> +				regulator-allowed-modes = <0 1 2>;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +
> +	mt6315_7: mt6315@7 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +		compatible = "mediatek,mt6315-regulator";
> +		reg = <0x7 SPMI_USID>;
> +
> +		regulators {



Best regards,
Krzysztof
Macpaul Lin Sept. 4, 2023, 9:50 a.m. UTC | #3
On 9/4/23 17:33, Krzysztof Kozlowski wrote:
> 	
> 
> External email : Please do not click links or open attachments until you 
> have verified the sender or the content.
> 
> On 04/09/2023 11:20, Macpaul Lin wrote:
>> Add bindings for the MediaTek mt8395-evk board.
>> The mt8359-evk board is also named as "Genio 1200-EVK".
>> MT8195 and MT8395 are the same family series SoC could share
> 
> How can be the same and have different numbers? You sill need dedicated
> compatible.
> 

The SoCs mt8195 and mt8395 are designed for different market application 
and physical characteristics, using different efuse values for 
distinction. The booting flow and configurations are controllered by the 
boot loaders, firmware, and TF-A. Therefore, the part numbers and 
procurement channels are different. The detail information of these 
efuse values is proprietary, so I cant disclose it futher. Hence the 
most of peripheral drivers and base address are almost the same.

> 
> Best regards,
> Krzysztof
> 

Best regards,
Macpaul Lin
Krzysztof Kozlowski Sept. 4, 2023, 12:11 p.m. UTC | #4
On 04/09/2023 11:50, Macpaul Lin wrote:
> 
> 
> On 9/4/23 17:33, Krzysztof Kozlowski wrote:
>> 	
>>
>> External email : Please do not click links or open attachments until you 
>> have verified the sender or the content.
>>
>> On 04/09/2023 11:20, Macpaul Lin wrote:
>>> Add bindings for the MediaTek mt8395-evk board.
>>> The mt8359-evk board is also named as "Genio 1200-EVK".
>>> MT8195 and MT8395 are the same family series SoC could share
>>
>> How can be the same and have different numbers? You sill need dedicated
>> compatible.
>>
> 
> The SoCs mt8195 and mt8395 are designed for different market application 
> and physical characteristics, using different efuse values for 
> distinction. The booting flow and configurations are controllered by the 
> boot loaders, firmware, and TF-A. Therefore, the part numbers and 
> procurement channels are different. The detail information of these 
> efuse values is proprietary, so I cant disclose it futher. Hence the 
> most of peripheral drivers and base address are almost the same.

1. Drivers? So we talk about compatibility, not the same.
2. "almost the same" is not the same. Follow the guidelines for writing
bindings.


Best regards,
Krzysztof
Macpaul Lin Sept. 5, 2023, 9:36 a.m. UTC | #5
On 9/4/23 20:11, Krzysztof Kozlowski wrote:
> 	
> 
> External email : Please do not click links or open attachments until you 
> have verified the sender or the content.
> 
> On 04/09/2023 11:50, Macpaul Lin wrote:
>> 
>> 
>> On 9/4/23 17:33, Krzysztof Kozlowski wrote:
>>> 
>>>
>>> External email : Please do not click links or open attachments until you 
>>> have verified the sender or the content.
>>>
>>> On 04/09/2023 11:20, Macpaul Lin wrote:
>>>> Add bindings for the MediaTek mt8395-evk board.
>>>> The mt8359-evk board is also named as "Genio 1200-EVK".
>>>> MT8195 and MT8395 are the same family series SoC could share
>>>
>>> How can be the same and have different numbers? You sill need dedicated
>>> compatible.
>>>
>> 
>> The SoCs mt8195 and mt8395 are designed for different market application 
>> and physical characteristics, using different efuse values for 
>> distinction. The booting flow and configurations are controllered by the 
>> boot loaders, firmware, and TF-A. Therefore, the part numbers and 
>> procurement channels are different. The detail information of these 
>> efuse values is proprietary, so I cant disclose it futher. Hence the 
>> most of peripheral drivers and base address are almost the same.
> 
> 1. Drivers? So we talk about compatibility, not the same.
> 2. "almost the same" is not the same. Follow the guidelines for writing
> bindings.
> 

Thanks for the review.

After internal confirmation and discussion, it can be confirmed that the 
MT8195 and MT8395 are identical SoCs from to binding's perspective. 
MediaTek hope the mt8395 boards could directly use mt8195.dtsi, without 
the need to create a separate mt8395.dtsi to include mt8195.dtsi. 
Therefore, we hope to fully adopt the bindings of mt8195. However, I 
will submit a revised patch for compatible since they are different boards.

If the reviewer still disagrees, then MediaTek will create a new binding 
and use mt8395.dtsi to include mt8195.dtsi to apply the separate 
bindings for differentiation.

> Best regards,
> Krzysztof
> 
> 

Best regards,
Macpaul Lin
Krzysztof Kozlowski Sept. 5, 2023, 10:36 a.m. UTC | #6
On 05/09/2023 11:36, Macpaul Lin wrote:
> 
> 
> On 9/4/23 20:11, Krzysztof Kozlowski wrote:
>> 	
>>
>> External email : Please do not click links or open attachments until you 
>> have verified the sender or the content.
>>
>> On 04/09/2023 11:50, Macpaul Lin wrote:
>>>
>>>
>>> On 9/4/23 17:33, Krzysztof Kozlowski wrote:
>>>>
>>>>
>>>> External email : Please do not click links or open attachments until you 
>>>> have verified the sender or the content.
>>>>
>>>> On 04/09/2023 11:20, Macpaul Lin wrote:
>>>>> Add bindings for the MediaTek mt8395-evk board.
>>>>> The mt8359-evk board is also named as "Genio 1200-EVK".
>>>>> MT8195 and MT8395 are the same family series SoC could share
>>>>
>>>> How can be the same and have different numbers? You sill need dedicated
>>>> compatible.
>>>>
>>>
>>> The SoCs mt8195 and mt8395 are designed for different market application 
>>> and physical characteristics, using different efuse values for 
>>> distinction. The booting flow and configurations are controllered by the 
>>> boot loaders, firmware, and TF-A. Therefore, the part numbers and 
>>> procurement channels are different. The detail information of these 
>>> efuse values is proprietary, so I cant disclose it futher. Hence the 
>>> most of peripheral drivers and base address are almost the same.
>>
>> 1. Drivers? So we talk about compatibility, not the same.
>> 2. "almost the same" is not the same. Follow the guidelines for writing
>> bindings.
>>
> 
> Thanks for the review.
> 
> After internal confirmation and discussion, it can be confirmed that the 
> MT8195 and MT8395 are identical SoCs from to binding's perspective. 

I am sorry, but I really do not care what you internally discussed about
bindings. I do not think your internal review respect existing
guidelines. You talked about drivers, not "bindings perspective", so
your internal discussion is clearly discussing something else.

> MediaTek hope the mt8395 boards could directly use mt8195.dtsi, without 
> the need to create a separate mt8395.dtsi to include mt8195.dtsi. 
> Therefore, we hope to fully adopt the bindings of mt8195. However, I 
> will submit a revised patch for compatible since they are different boards.

You can disagree but then I expect arguments from your side.

Best regards,
Krzysztof
AngeloGioacchino Del Regno Sept. 5, 2023, 10:58 a.m. UTC | #7
Il 05/09/23 12:36, Krzysztof Kozlowski ha scritto:
> On 05/09/2023 11:36, Macpaul Lin wrote:
>>
>>
>> On 9/4/23 20:11, Krzysztof Kozlowski wrote:
>>> 	
>>>
>>> External email : Please do not click links or open attachments until you
>>> have verified the sender or the content.
>>>
>>> On 04/09/2023 11:50, Macpaul Lin wrote:
>>>>
>>>>
>>>> On 9/4/23 17:33, Krzysztof Kozlowski wrote:
>>>>>
>>>>>
>>>>> External email : Please do not click links or open attachments until you
>>>>> have verified the sender or the content.
>>>>>
>>>>> On 04/09/2023 11:20, Macpaul Lin wrote:
>>>>>> Add bindings for the MediaTek mt8395-evk board.
>>>>>> The mt8359-evk board is also named as "Genio 1200-EVK".
>>>>>> MT8195 and MT8395 are the same family series SoC could share
>>>>>
>>>>> How can be the same and have different numbers? You sill need dedicated
>>>>> compatible.
>>>>>
>>>>
>>>> The SoCs mt8195 and mt8395 are designed for different market application
>>>> and physical characteristics, using different efuse values for
>>>> distinction. The booting flow and configurations are controllered by the
>>>> boot loaders, firmware, and TF-A. Therefore, the part numbers and
>>>> procurement channels are different. The detail information of these
>>>> efuse values is proprietary, so I cant disclose it futher. Hence the
>>>> most of peripheral drivers and base address are almost the same.
>>>
>>> 1. Drivers? So we talk about compatibility, not the same.
>>> 2. "almost the same" is not the same. Follow the guidelines for writing
>>> bindings.
>>>
>>
>> Thanks for the review.
>>
>> After internal confirmation and discussion, it can be confirmed that the
>> MT8195 and MT8395 are identical SoCs from to binding's perspective.
> 
> I am sorry, but I really do not care what you internally discussed about
> bindings. I do not think your internal review respect existing
> guidelines. You talked about drivers, not "bindings perspective", so
> your internal discussion is clearly discussing something else.
> 
>> MediaTek hope the mt8395 boards could directly use mt8195.dtsi, without
>> the need to create a separate mt8395.dtsi to include mt8195.dtsi.
>> Therefore, we hope to fully adopt the bindings of mt8195. However, I
>> will submit a revised patch for compatible since they are different boards.
> 
> You can disagree but then I expect arguments from your side.
> 

In short - they're the same chip, as in, they behave the same on a *hardware*
perspective; what changes is the bootchain (plus stricter security from TF-A)
and allowable temperature ranges for operation, that's practically it...

...so yes the compatible for the "new soc" must be documented, but that's
practically just a revision, *not a new soc* at all.

(though, I agree that seeing a different name as in 1 -> 3 can be totally
confusing)

The drivers difference that Macpaul hinted to are about drivers needing some
SMC calls instead of direct MMIO manipulation, so, something like two bindings
for something like two drivers will need to add a 8395 compatible; speaking of
what we would have in a devicetree for this SoC, that'd be exactly 99% identical
to mt8195.dtsi.

Anyway, drivers are drivers, bindings describe hardware - and the hw is, again,
the same...

Hope that this makes things clearer! :-)

Cheers,
Angelo
Macpaul Lin Sept. 6, 2023, 2:41 a.m. UTC | #8
On 9/5/23 18:58, AngeloGioacchino Del Regno and Krzsztof Kozlowski wrote:
> Il 05/09/23 12:36, Krzysztof Kozlowski ha scritto:
>> On 05/09/2023 11:36, Macpaul Lin wrote:
>>>
>>>
>>> On 9/4/23 20:11, Krzysztof Kozlowski wrote:
>>>>
>>>>
>>>> External email : Please do not click links or open attachments until 
>>>> you
>>>> have verified the sender or the content.
>>>>
>>>> On 04/09/2023 11:50, Macpaul Lin wrote:
>>>>>
>>>>>
>>>>> On 9/4/23 17:33, Krzysztof Kozlowski wrote:
>>>>>>
>>>>>>
>>>>>> External email : Please do not click links or open attachments 
>>>>>> until you
>>>>>> have verified the sender or the content.
>>>>>>
>>>>>> On 04/09/2023 11:20, Macpaul Lin wrote:
>>>>>>> Add bindings for the MediaTek mt8395-evk board.
>>>>>>> The mt8359-evk board is also named as "Genio 1200-EVK".
>>>>>>> MT8195 and MT8395 are the same family series SoC could share
>>>>>>
>>>>>> How can be the same and have different numbers? You sill need 
>>>>>> dedicated
>>>>>> compatible.
>>>>>>
>>>>>
>>>>> The SoCs mt8195 and mt8395 are designed for different market 
>>>>> application
>>>>> and physical characteristics, using different efuse values for
>>>>> distinction. The booting flow and configurations are controllered 
>>>>> by the
>>>>> boot loaders, firmware, and TF-A. Therefore, the part numbers and
>>>>> procurement channels are different. The detail information of these
>>>>> efuse values is proprietary, so I cant disclose it futher. Hence the
>>>>> most of peripheral drivers and base address are almost the same.
>>>>
>>>> 1. Drivers? So we talk about compatibility, not the same.
>>>> 2. "almost the same" is not the same. Follow the guidelines for writing
>>>> bindings.
>>>>
>>>
>>> Thanks for the review.
>>>
>>> After internal confirmation and discussion, it can be confirmed that the
>>> MT8195 and MT8395 are identical SoCs from to binding's perspective.
>>
>> I am sorry, but I really do not care what you internally discussed about
>> bindings. I do not think your internal review respect existing
>> guidelines. You talked about drivers, not "bindings perspective", so
>> your internal discussion is clearly discussing something else.
>>
>>> MediaTek hope the mt8395 boards could directly use mt8195.dtsi, without
>>> the need to create a separate mt8395.dtsi to include mt8195.dtsi.
>>> Therefore, we hope to fully adopt the bindings of mt8195. However, I
>>> will submit a revised patch for compatible since they are different 
>>> boards.
>>
>> You can disagree but then I expect arguments from your side.
>>
> 
> In short - they're the same chip, as in, they behave the same on a 
> *hardware*
> perspective; what changes is the bootchain (plus stricter security from 
> TF-A)
> and allowable temperature ranges for operation, that's practically it...
> 
> ...so yes the compatible for the "new soc" must be documented, but that's
> practically just a revision, *not a new soc* at all.
> 
> (though, I agree that seeing a different name as in 1 -> 3 can be totally
> confusing)
> 
> The drivers difference that Macpaul hinted to are about drivers needing 
> some
> SMC calls instead of direct MMIO manipulation, so, something like two 
> bindings
> for something like two drivers will need to add a 8395 compatible; 
> speaking of
> what we would have in a devicetree for this SoC, that'd be exactly 99% 
> identical
> to mt8195.dtsi.
> 
> Anyway, drivers are drivers, bindings describe hardware - and the hw is, 
> again,
> the same...
> 
> Hope that this makes things clearer! :-)
> 
> Cheers,
> Angelo

Thanks for your patience and clarification.
I'll submit new SOC binding and revise the patches for mt8395 board.

Thanks
Macpaul Lin
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index ae12b1cab9fb..685b461fda28 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -171,6 +171,7 @@  properties:
           - enum:
               - mediatek,mt8195-demo
               - mediatek,mt8195-evb
+              - mediatek,mt8395-evk
           - const: mediatek,mt8195
       - description: Google Burnet (HP Chromebook x360 11MK G3 EE)
         items: