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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c82a4fbf-824e-4196-99a1-84fd4e836951@collabora.com>
Date: Mon, 25 Nov 2024 10:05:20 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Wojciech Macek <wmacek@...omium.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
 Chen-Yu Tsai <wenst@...omium.org>, Rafal Milecki <rafal@...ecki.pl>,
 Hsin-Yi Wang <hsinyi@...omium.org>, Sean Wang <sean.wang@...iatek.com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] arm64: dts: mediatek: mt8186: Add Starmie device

Il 25/11/24 09:21, Wojciech Macek ha scritto:
> Add support for Starmie Chromebooks.
> 
> Signed-off-by: Wojciech Macek <wmacek@...omium.org>
> ---
> Changelog v2-v1:
>   - no change
> 
>   arch/arm64/boot/dts/mediatek/Makefile         |   2 +
>   .../mediatek/mt8186-corsola-starmie-sku0.dts  |  29 ++
>   .../mediatek/mt8186-corsola-starmie-sku1.dts  |  46 ++
>   .../dts/mediatek/mt8186-corsola-starmie.dtsi  | 480 ++++++++++++++++++
>   4 files changed, 557 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku0.dts
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku1.dts
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie.dtsi
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 8fd7b2bb7a159..2ee6266ddf43d 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -59,6 +59,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-magneton-sku393216.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-magneton-sku393217.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-magneton-sku393218.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-rusty-sku196608.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-starmie-sku0.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-starmie-sku1.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-steelix-sku131072.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-steelix-sku131073.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-tentacool-sku327681.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku0.dts b/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku0.dts
> new file mode 100644
> index 0000000000000..ca0b8492bbef5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku0.dts
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2023 Google LLC
> + */
> +
> +/dts-v1/;
> +#include "mt8186-corsola-starmie.dtsi"
> +
> +/ {
> +	model = "Google Starmie sku0 board";
> +	compatible = "google,starmie-sku0", "google,starmie-sku2",
> +		     "google,starmie-sku3", "google,starmie",
> +		     "mediatek,mt8186";
> +};
> +
> +&panel {
> +	compatible = "starry,ili9882t";
> +};
> +
> +&i2c_tunnel {
> +	/delete-node/ sbs-battery@b;
> +
> +	battery: sbs-battery@f {
> +		compatible = "sbs,sbs-battery";
> +		reg = <0xf>;
> +		sbs,i2c-retry-count = <2>;
> +		sbs,poll-retry-count = <1>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku1.dts b/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku1.dts
> new file mode 100644
> index 0000000000000..2ba4c083a58c6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie-sku1.dts
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2023 Google LLC
> + */
> +
> +/dts-v1/;
> +#include "mt8186-corsola-starmie.dtsi"
> +
> +/ {
> +	model = "Google Starmie sku1 board";
> +	compatible = "google,starmie-sku1", "google,starmie-sku4",
> +		     "google,starmie", "mediatek,mt8186";
> +};
> +
> +&panel {
> +	compatible = "starry,himax83102-j02";
> +};
> +
> +&i2c1 {
> +	/delete-node/ touchscreen@41;
> +	touchscreen_himax: touchscreen@4f {
> +		status = "okay";

Okay is the default.

> +
> +		compatible = "hid-over-i2c";
> +		reg = <0x4f>;
> +		interrupt-parent = <&pio>;
> +		interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&touchscreen_pins>;
> +		vdd-supply = <&mt6366_vio18_reg>;
> +		panel = <&panel>;
> +		post-power-on-delay-ms = <450>;
> +		hid-descr-addr = <0x0001>;
> +	};
> +};
> +
> +&i2c_tunnel {
> +	/delete-node/ sbs-battery@b;

Would status = "disabled" not work for sbs-battery@b?

> +
> +	battery: sbs-battery@f {

You're defining sbs-battery@f in every starmie dts, you can move that to the
starmie dtsi instead, so that you can avoid all the useless duplication.

> +		compatible = "sbs,sbs-battery";
> +		reg = <0xf>;
> +		sbs,i2c-retry-count = <2>;
> +		sbs,poll-retry-count = <1>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie.dtsi b/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie.dtsi
> new file mode 100644
> index 0000000000000..28ac65d28143e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-starmie.dtsi
> @@ -0,0 +1,480 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2023 Google LLC
> + */
> +
> +/dts-v1/;
> +#include "mt8186-corsola.dtsi"
> +
> +/delete-node/ &dsi_out;

Instead of hacking in a delete-node, you can just change mt8186.dtsi at this point,
or you can use the current dsi_out phandle. I would prefer that you do the latter, 
as it's going to be more convenient later when I'll have to migrate this platform
to the full OF Graph for the display controller.

> +/delete-node/ &keyboard_controller;
> +
> +/ {
> +	en_pp6000_mipi_disp_150ma: en-pp6000-mipi-disp-150ma {
> +		compatible = "regulator-fixed";
> +		regulator-name = "en_pp6000_mipi_disp_150ma";
> +		gpio = <&pio 154 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pp6000_mipi_disp_150ma_fixed_pins>;
> +	};
> +
> +	tboard_thermistor1: thermal-sensor1 {
> +		compatible = "generic-adc-thermal";
> +		#thermal-sensor-cells = <0>;
> +		io-channels = <&auxadc 0>;
> +		io-channel-names = "sensor-channel";
> +		temperature-lookup-table = <    (-5000) 1492
> +						0 1413
> +						5000 1324
> +						10000 1227
> +						15000 1121
> +						20000 1017
> +						25000 900
> +						30000 797
> +						35000 698
> +						40000 606
> +						45000 522
> +						50000 449
> +						55000 383
> +						60000 327
> +						65000 278
> +						70000 236
> +						75000 201
> +						80000 171
> +						85000 145
> +						90000 163
> +						95000 124
> +						100000 91
> +						105000 78
> +						110000 67
> +						115000 58
> +						120000 50
> +						125000 44>;
> +	};
> +
> +	tboard_thermistor2: thermal-sensor2 {
> +		compatible = "generic-adc-thermal";
> +		#thermal-sensor-cells = <0>;
> +		io-channels = <&auxadc 1>;
> +		io-channel-names = "sensor-channel";
> +		temperature-lookup-table = <    (-5000) 1492
> +						0 1413
> +						5000 1324
> +						10000 1227
> +						15000 1121
> +						20000 1017
> +						25000 900
> +						30000 797
> +						35000 698
> +						40000 606
> +						45000 522
> +						50000 449
> +						55000 383
> +						60000 327
> +						65000 278
> +						70000 236
> +						75000 201
> +						80000 171
> +						85000 145
> +						90000 163
> +						95000 124
> +						100000 91
> +						105000 78
> +						110000 67
> +						115000 58
> +						120000 50
> +						125000 44>;
> +	};
> +};
> +
> +&cros_ec {
> +	cbas: cbas {
> +		compatible = "google,cros-cbas";
> +	};
> +
> +	keyboard-controller {
> +		compatible = "google,cros-ec-keyb-switches";
> +	};
> +};
> +
> +&dsi0 {
> +	status = "okay";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	panel: panel@0 {
> +		/* compatible will be set in board dts */
> +		reg = <0>;
> +		enable-gpios = <&pio 98 0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&panel_pins_default>;
> +		avdd-supply = <&en_pp6000_mipi_disp>;
> +		avee-supply = <&en_pp6000_mipi_disp_150ma>;
> +		pp1800-supply = <&mt6366_vio18_reg>;
> +		backlight = <&backlight_lcd0>;
> +		rotation = <270>;
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&dsi_out>;
> +			};
> +		};
> +	};
> +
> +	ports {
> +		port {
> +			dsi_out: endpoint {
> +				remote-endpoint = <&panel_in>;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c0 {
> +	status = "disabled";
> +};
> +
> +&i2c1 {
> +	touchscreen: touchscreen@41 {
> +		status = "okay";

Status is okay by default.

> +
> +		compatible = "ilitek,ili9882t";

I can't find this compatible anywhere in any kernel driver. That won't work.

> +		reg = <0x41>;
> +		interrupt-parent = <&pio>;
> +		interrupts = <12 IRQ_TYPE_LEVEL_LOW>;

interrupts-extended please

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&touchscreen_pins>;
> +		panel = <&panel>;
> +		reset-gpios = <&pio 60 GPIO_ACTIVE_LOW>;
> +		vccio-supply = <&mt6366_vio18_reg>;
> +	};
> +};
> +
> +&i2c2 {
> +	status = "disabled";
> +};
> +
> +&i2c4 {
> +	status = "disabled";
> +};
> +
> +&i2c5 {
> +	clock-frequency = <400000>;
> +
> +};
> +
> +&mmc1_pins_default {
> +	pins-clk {
> +		drive-strength = <MTK_DRIVE_8mA>;

Please stop using MTK_DRIVE_xxmA definitions. This is just <8>.

> +	};
> +
> +	pins-cmd-dat {
> +		drive-strength = <MTK_DRIVE_8mA>;
> +	};
> +};
> +
> +&mmc1_pins_uhs {
> +	pins-clk {
> +		drive-strength = <MTK_DRIVE_8mA>;
> +	};
> +
> +	pins-cmd-dat {
> +		drive-strength = <MTK_DRIVE_8mA>;
> +	};
> +};
> +
> +&pen_insert {
> +	wakeup-event-action = <EV_ACT_ANY>;
> +};
> +
> +&pio {

..snip..

> +
> +	dpi_pin_default: dpi-pin-default {
> +		pins-cmd-dat {
> +			pinmux = <PINMUX_GPIO103__FUNC_GPIO103>,
> +				 <PINMUX_GPIO104__FUNC_GPIO104>,
> +				 <PINMUX_GPIO105__FUNC_GPIO105>,
> +				 <PINMUX_GPIO106__FUNC_GPIO106>,
> +				 <PINMUX_GPIO107__FUNC_GPIO107>,
> +				 <PINMUX_GPIO108__FUNC_GPIO108>,
> +				 <PINMUX_GPIO109__FUNC_GPIO109>,
> +				 <PINMUX_GPIO110__FUNC_GPIO110>,
> +				 <PINMUX_GPIO111__FUNC_GPIO111>,
> +				 <PINMUX_GPIO112__FUNC_GPIO112>,
> +				 <PINMUX_GPIO113__FUNC_GPIO113>,
> +				 <PINMUX_GPIO114__FUNC_GPIO114>,
> +				 <PINMUX_GPIO101__FUNC_GPIO101>,
> +				 <PINMUX_GPIO100__FUNC_GPIO100>,
> +				 <PINMUX_GPIO102__FUNC_GPIO102>,
> +				 <PINMUX_GPIO99__FUNC_GPIO99>;
> +			drive-strength = <MTK_DRIVE_10mA>;

Please stop using MTK_DRIVE_xxmA definitions. This is <10>.


> +			output-low;
> +		};
> +	};
> +
> +	dpi_pin_func: dpi-pin-func {
> +		pins-cmd-dat {
> +			pinmux = <PINMUX_GPIO103__FUNC_DPI_DATA0>,
> +				 <PINMUX_GPIO104__FUNC_DPI_DATA1>,
> +				 <PINMUX_GPIO105__FUNC_DPI_DATA2>,
> +				 <PINMUX_GPIO106__FUNC_DPI_DATA3>,
> +				 <PINMUX_GPIO107__FUNC_DPI_DATA4>,
> +				 <PINMUX_GPIO108__FUNC_DPI_DATA5>,
> +				 <PINMUX_GPIO109__FUNC_DPI_DATA6>,
> +				 <PINMUX_GPIO110__FUNC_DPI_DATA7>,
> +				 <PINMUX_GPIO111__FUNC_DPI_DATA8>,
> +				 <PINMUX_GPIO112__FUNC_DPI_DATA9>,
> +				 <PINMUX_GPIO113__FUNC_DPI_DATA10>,
> +				 <PINMUX_GPIO114__FUNC_DPI_DATA11>,
> +				 <PINMUX_GPIO101__FUNC_DPI_HSYNC>,
> +				 <PINMUX_GPIO100__FUNC_DPI_VSYNC>,
> +				 <PINMUX_GPIO102__FUNC_DPI_DE>,
> +				 <PINMUX_GPIO99__FUNC_DPI_PCLK>;
> +			drive-strength = <MTK_DRIVE_10mA>;
> +		};
> +	};
> +
> +	edp_panel_fixed_pins: edp-panel-fixed-pins {
> +		pins1 {

I don't see where you're using this pin. Please don't add unused pins.

> +			pinmux = <PINMUX_GPIO153__FUNC_GPIO153>;
> +			output-low;
> +		};
> +	};
> +
> +	pp6000_mipi_disp_150ma_fixed_pins: pp6000-mipi-disp-150ma-fixed-pins {
> +		pins1 {

pins-en {

> +			pinmux = <PINMUX_GPIO154__FUNC_GPIO154>;
> +			output-low;
> +		};
> +	};
> +
> +	panel_pins_default: panel-pins-default {
> +		pins1 {

pins-en {

> +			pinmux = <PINMUX_GPIO98__FUNC_GPIO98>;
> +			output-low;
> +		};
> +	};
> +	wifi_pins_pwrseq: wifipwrseq {

Like this, that's unused.

You do have a wifi_enable_pin in mt8186-corsola.dtsi though, so override it.

> +		pins-wifi-enable {
> +			pinmux = <PINMUX_GPIO51__FUNC_GPIO51>;
> +		};
> +	};
> +};
> +
> +&usb_c1 {
> +	status = "disabled";
> +};
> +
> +&thermal_zones {
> +	tboard1 {
> +		polling-delay = <1000>; /* milliseconds */
> +		polling-delay-passive = <0>; /* milliseconds */
> +		thermal-sensors = <&tboard_thermistor1>;
> +	};
> +
> +	tboard2 {
> +		polling-delay = <1000>; /* milliseconds */
> +		polling-delay-passive = <0>; /* milliseconds */
> +		thermal-sensors = <&tboard_thermistor2>;
> +	};
> +};
> +
> +&wifi_pwrseq {
> +	reset-gpios = <&pio 51 1>;
> +};
> +
> +en_pp6000_mipi_disp: &pp3300_disp_x {

....but pp6000 is not pp3300, so move the pp3300 to the relevant board dts
and define the pp6000 here, or names won't match.

> +	regulator-name = "en_pp6000_mipi_disp";
> +	gpio = <&pio 153 GPIO_ACTIVE_HIGH>;
> +	regulator-enable-ramp-delay = <3000>;
> +	/delete-property/ regulator-boot-on;
> +};

Regards,
Angelo

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ