| 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
| ||
|
Message-ID: <a9353ff0-ff29-474d-bfbe-4275d7eb36e8@collabora.com>
Date: Mon, 25 Nov 2024 13:04:19 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Wojciech Macek <wmacek@...omium.org>
Cc: 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 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@...labora.com> wrote:
>
>> 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