linux-kernel - Re: [PATCH v2 2/2] arm64: dts: mediatek: mt8390-genio-700-evk: add keys and USB HUB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8cdce399-1f42-4558-9cdb-c36b96205212@collabora.com>
Date: Tue, 8 Oct 2024 10:20:01 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Macpaul Lin <macpaul.lin@...iatek.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 Alexandre Mergnat <amergnat@...libre.com>
Cc: Bear Wang <bear.wang@...iatek.com>, Pablo Sun <pablo.sun@...iatek.com>,
 Macpaul Lin <macpaul@...il.com>, Sen Chu <sen.chu@...iatek.com>,
 Chris-qj chen <chris-qj.chen@...iatek.com>,
 MediaTek Chromebook Upstream
 <Project_Global_Chrome_Upstream_Group@...iatek.com>,
 Chen-Yu Tsai <wenst@...omium.org>
Subject: Re: [PATCH v2 2/2] arm64: dts: mediatek: mt8390-genio-700-evk: add
 keys and USB HUB

Il 07/10/24 11:02, Macpaul Lin ha scritto:
> 1. Add i2c, mmc to aliases.
> 4. Add PMIC_KEY setting.
> 5. Add USB HUB TUSB8020 to xhci1.
> 6. Re-order spi2 node.

Please either add the aliases in a different commit, or add that to the title.

arm64: dts: mediatek: mt8390-genio-700-evk: Add aliases, keys and USB HUB

Additionally, I'd really like to see a "conversation-like" description instead
of a kind-of-checkbox list.

Something like..

"
Add aliases for the I2C and MMC/SD controllers to keep the numbering consistent and
describe the TUSB8020 hub present on the USB XHCI1 controller instance to enable
resetting it with its specific reset GPIO.

While at it, also move the spi2 node to keep nodes alphabetically ordered.
"

> 
> Signed-off-by: Macpaul Lin <macpaul.lin@...iatek.com>
> ---
>   .../dts/mediatek/mt8390-genio-700-evk.dts     | 57 +++++++++++++++----
>   1 file changed, 47 insertions(+), 10 deletions(-)
> 
> Changes for v2:
>   - Fix order of spi2.
>   - Update pinctrl in i2c4 and rt1715.
>   - Drop IT5205 and RT1715 nodes since the DTS are not completed yet.
>   - Add #address-cells and #size-cells to xhci1 for supporting USB hubs.
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts b/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts
> index 96b272567cb1..3e77f59f2c74 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dts
> @@ -23,6 +23,15 @@ / {
>   		     "mediatek,mt8188";
>   
>   	aliases {
> +		i2c0 = &i2c0;
> +		i2c1 = &i2c1;
> +		i2c2 = &i2c2;
> +		i2c3 = &i2c3;
> +		i2c4 = &i2c4;
> +		i2c5 = &i2c5;
> +		i2c6 = &i2c6;
> +		mmc0 = &mmc0;
> +		mmc1 = &mmc1;
>   		serial0 = &uart0;
>   	};
>   
> @@ -249,7 +258,6 @@ &i2c3 {
>   &i2c4 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&i2c4_pins>;
> -	pinctrl-1 = <&rt1715_int_pins>;

What is this extra change?
Please describe it in the commit description.

>   	clock-frequency = <1000000>;
>   	status = "okay";
>   };
> @@ -867,6 +875,17 @@ pins-wifi-enable {
>   &pmic {
>   	interrupt-parent = <&pio>;
>   	interrupts = <222 IRQ_TYPE_LEVEL_HIGH>;
> +
> +	mt6359keys: keys {

Is there any reason why we can't just put this node in mt6359.dtsi?

Cheers,
Angelo

> +		compatible = "mediatek,mt6359-keys";
> +		mediatek,long-press-mode = <1>;
> +		power-off-time-sec = <0>;
> +
> +		power-key {
> +			linux,keycodes = <KEY_POWER>;
> +			wakeup-source;
> +		};
> +	};
>   };
>   
>   &scp {
> @@ -874,6 +893,15 @@ &scp {
>   	status = "okay";
>   };
>   
> +&spi2 {
> +	pinctrl-0 = <&spi2_pins>;
> +	pinctrl-names = "default";
> +	mediatek,pad-select = <0>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +};
> +
>   &uart0 {
>   	pinctrl-0 = <&uart0_pins>;
>   	pinctrl-names = "default";
> @@ -892,15 +920,6 @@ &uart2 {
>   	status = "okay";
>   };
>   
> -&spi2 {
> -	pinctrl-0 = <&spi2_pins>;
> -	pinctrl-names = "default";
> -	mediatek,pad-select = <0>;
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -	status = "okay";
> -};
> -
>   &u3phy0 {
>   	status = "okay";
>   };
> @@ -921,6 +940,24 @@ &xhci0 {
>   &xhci1 {
>   	status = "okay";
>   	vusb33-supply = <&mt6359_vusb_ldo_reg>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	hub_2_0: hub@1 {
> +		compatible = "usb451,8025";
> +		reg = <1>;
> +		peer-hub = <&hub_3_0>;
> +		reset-gpios = <&pio 7 GPIO_ACTIVE_HIGH>;
> +		vdd-supply = <&usb_hub_fixed_3v3>;
> +	};
> +
> +	hub_3_0: hub@2 {
> +		compatible = "usb451,8027";
> +		reg = <2>;
> +		peer-hub = <&hub_2_0>;
> +		reset-gpios = <&pio 7 GPIO_ACTIVE_HIGH>;
> +		vdd-supply = <&usb_hub_fixed_3v3>;
> +	};
>   };
>   
>   &xhci2 {