From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7017AD58083 for ; Mon, 25 Nov 2024 12:06:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=19cHZxR6TPJYFNcVXHasVfnpFCJVOtcnfayKu/4H3Kg=; b=dWr0IYDxqD+fW1q/4GmluRcne9 mZnnrQgtOdbo8WqRVNVxhbXNH5vHLrfE6b9nSf2KHmL1GIrTrRTxNBfH6fk/ClUlnhw78SnuBqRkX a5gK5B+gwA0Ad3l5EfoyK520BubyjAv+EJukZ3C7k/n0jdiXdaoP68cF2dRe/9K+AXlttjh/6N0mT 7m6LPKPt0QVoSW3cWQkuj6Z29prJrbg8DKtGtyIvR3amlo5P4w/s/XA3et8UXKc8tq28oFSK2R3oZ 7POYFkMIfqoqUJzOZ7pdn4tY0hKormArqPxB9H9R6IkffuFRP5Z8TyXnLf9xTaXm7qAdGNgkxohfl skaqt1Og==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tFXr5-00000007yCC-45Ah; Mon, 25 Nov 2024 12:06:15 +0000 Received: from bali.collaboradmins.com ([148.251.105.195]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tFXpG-00000007xt6-0elD; Mon, 25 Nov 2024 12:04:23 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1732536260; bh=efeNB/rKqi3tpa04/LtQ2JCpX8cpJMScuUTG9pnf4Z8=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=oZRmCsGYOdveIcb8wwttSsU6Wlj1roQjWzv2TWzf2vXlfQzSuk05qbe+TOnseUF5n g4V9rctEtM5879qAXAl1JtSuEtvxFNUvVUolmEKKOqoI3bWTwDoDGFt6y1a2XZyEI5 EMr8aLDFViRzbg8Wf9sJeu9f8HCzTQVCqQoQtNA5doFIIbhdm4ud8z2N/EEHPRR4tO UYl+6EsrK27JpO8dFzADaQE4BZwLXl1l2AnDsBFmtUd1Lbf3i0ZhY8WTDhgAwaWxFi 3K6zcdOX7Ol43nTyc6Q449i3BpUrwCjvXwPWPAfix5u7EEBs7I7LONdUxU9HYYD1gG zM8qO5O2zKVTQ== Received: from [192.168.1.100] (2-237-20-237.ip236.fastwebnet.it [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by bali.collaboradmins.com (Postfix) with ESMTPSA id 1223B17E3626; Mon, 25 Nov 2024 13:04:20 +0100 (CET) Message-ID: Date: Mon, 25 Nov 2024 13:04:19 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/2] arm64: dts: mediatek: mt8186: Add Starmie device To: Wojciech Macek Cc: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Matthias Brugger , Chen-Yu Tsai , Rafal Milecki , Hsin-Yi Wang , Sean Wang , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org References: <20241125082130.2390310-1-wmacek@chromium.org> <20241125082130.2390310-3-wmacek@chromium.org> From: AngeloGioacchino Del Regno Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241125_040422_483687_5DD04B90 X-CRM114-Status: GOOD ( 23.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Il 25/11/24 11:15, Wojciech Macek ha scritto: > Sure, I will work on it. Thanks. > >>> + >>> + compatible = "ilitek,ili9882t"; > >> I can't find this compatible anywhere in any kernel driver. That won't > work. > > Actually, the compat is defined in ./drivers/hid/i2c-hid/i2c-hid-of-elan.c > Oh, I typo'ed the compatible while grepping. Sorry, you're right about that. Cheers, Angelo > > Regards, > Wojtek > > On Mon, Nov 25, 2024 at 10:05 AM AngeloGioacchino Del Regno < > angelogioacchino.delregno@collabora.com> wrote: > >> Il 25/11/24 09:21, Wojciech Macek ha scritto: >>> Add support for Starmie Chromebooks. >>> >>> Signed-off-by: Wojciech Macek >>> --- >>> 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 = ; >> >> Please stop using MTK_DRIVE_xxmA definitions. This is just <8>. >> >>> + }; >>> + >>> + pins-cmd-dat { >>> + drive-strength = ; >>> + }; >>> +}; >>> + >>> +&mmc1_pins_uhs { >>> + pins-clk { >>> + drive-strength = ; >>> + }; >>> + >>> + pins-cmd-dat { >>> + drive-strength = ; >>> + }; >>> +}; >>> + >>> +&pen_insert { >>> + wakeup-event-action = ; >>> +}; >>> + >>> +&pio { >> >> ..snip.. >> >>> + >>> + dpi_pin_default: dpi-pin-default { >>> + pins-cmd-dat { >>> + pinmux = , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + ; >>> + drive-strength = ; >> >> Please stop using MTK_DRIVE_xxmA definitions. This is <10>. >> >> >>> + output-low; >>> + }; >>> + }; >>> + >>> + dpi_pin_func: dpi-pin-func { >>> + pins-cmd-dat { >>> + pinmux = , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + , >>> + ; >>> + drive-strength = ; >>> + }; >>> + }; >>> + >>> + 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 = ; >>> + output-low; >>> + }; >>> + }; >>> + >>> + pp6000_mipi_disp_150ma_fixed_pins: >> pp6000-mipi-disp-150ma-fixed-pins { >>> + pins1 { >> >> pins-en { >> >>> + pinmux = ; >>> + output-low; >>> + }; >>> + }; >>> + >>> + panel_pins_default: panel-pins-default { >>> + pins1 { >> >> pins-en { >> >>> + pinmux = ; >>> + 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 = ; >>> + }; >>> + }; >>> +}; >>> + >>> +&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 >> >