| 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: <CAC=S1nic3HgEGeS6up_MsYh3UoM_piHC6=uuRep5bne4zOWbxQ@mail.gmail.com>
Date: Mon, 28 Oct 2024 19:16:43 +0800
From: Fei Shao <fshao@...omium.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Matthias Brugger <matthias.bgg@...il.com>, Conor Dooley <conor+dt@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 2/2] arm64: dts: mediatek: Introduce MT8188 Geralt
platform based Ciri
On Mon, Oct 28, 2024 at 5:02 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On Fri, Oct 25, 2024 at 06:59:36PM +0800, Fei Shao wrote:
> > Introduce MT8188-based Chromebook Ciri, also known commercially as
> > Lenovo Chromebook Duet (11", 9).
> >
> > Ciri is a detachable device based on the Geralt design, where Geralt is
> > the codename for the MT8188 platform. Ciri offers 8 SKUs to accommodate
> > different combinations of second-source components, including:
> > - audio codecs (RT5682S and ES8326)
> > - speaker amps (TAS2563 and MAX98390)
> > - MIPI-DSI panels (BOE NV110WUM-L60 and IVO t109nw41)
> >
> > Signed-off-by: Fei Shao <fshao@...omium.org>
> > ---
> >
> > arch/arm64/boot/dts/mediatek/Makefile | 8 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku0.dts | 11 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku1.dts | 63 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku2.dts | 54 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku3.dts | 20 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku4.dts | 43 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku5.dts | 76 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku6.dts | 67 +
> > .../dts/mediatek/mt8188-geralt-ciri-sku7.dts | 52 +
> > .../boot/dts/mediatek/mt8188-geralt-ciri.dtsi | 413 +++++
> > .../boot/dts/mediatek/mt8188-geralt.dtsi | 1497 +++++++++++++++++
> > 11 files changed, 2304 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku0.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku1.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku2.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku3.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri.dtsi
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> > index 8fd7b2bb7a15..c6c34d99316b 100644
> > --- a/arch/arm64/boot/dts/mediatek/Makefile
> > +++ b/arch/arm64/boot/dts/mediatek/Makefile
> > @@ -69,6 +69,14 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-voltorb-sku589824.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-voltorb-sku589825.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-evb.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-evb.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku0.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku1.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku2.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku3.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku4.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku5.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku6.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-geralt-ciri-sku7.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8192-asurada-hayato-r1.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8192-asurada-hayato-r5-sku2.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8192-asurada-spherion-r0.dtb
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku0.dts b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku0.dts
> > new file mode 100644
> > index 000000000000..7a2edbaa74f1
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku0.dts
> > @@ -0,0 +1,11 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright 2023 Google LLC
> > + */
> > +/dts-v1/;
> > +#include "mt8188-geralt-ciri.dtsi"
> > +
> > +/ {
> > + model = "Google Ciri sku0 board";
> > + compatible = "google,ciri-sku0", "google,ciri", "mediatek,mt8188";
> > +};
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku1.dts b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku1.dts
> > new file mode 100644
> > index 000000000000..53fdd5acaa3f
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku1.dts
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright 2023 Google LLC
> > + */
> > +/dts-v1/;
> > +#include "mt8188-geralt-ciri.dtsi"
> > +
> > +/ {
> > + model = "Google Ciri sku1 board";
> > + compatible = "google,ciri-sku1", "google,ciri", "mediatek,mt8188";
> > +};
> > +
> > +&dsi_panel {
> > + compatible = "ivo,t109nw41", "himax,hx83102";
> > +};
> > +
> > +&touchscreen {
> > + himax,pid = <0x1003>;
>
> Why do you add more of undocumented properties?
I think I'll drop the Himax related thing until Himax finishes
upstreaming their drivers, so there's a standardized binding to
follow.
>
> > + firmware-name = "himax_i2chid_1003.bin";
> > +};
> > +
> > +&i2c0 {
> > + /delete-node/ audio-codec@1a;
> > +
> > + es8326: audio-codec@19 {
> > + compatible = "everest,es8326";
> > + reg = <0x19>;
> > + interrupts-extended = <&pio 108 IRQ_TYPE_LEVEL_LOW>;
> > + #sound-dai-cells = <0>;
> > + everest,jack-pol = [0e];
> > + everest,interrupt-clk = [00];
> > + };
> > +};
> > +
> > +&sound {
> > + compatible = "mediatek,mt8188-es8326";
> > + model = "mt8188_m98390_8326";
> > +
> > + audio-routing =
> > + "ETDM1_OUT", "ETDM_SPK_PIN",
> > + "ETDM2_OUT", "ETDM_HP_PIN",
> > + "ETDM1_IN", "ETDM_SPK_PIN",
> > + "ETDM2_IN", "ETDM_HP_PIN",
> > + "ADDA Capture", "MTKAIF_PIN",
> > + "Headphone Jack", "HPOL",
> > + "Headphone Jack", "HPOR",
> > + "MIC1", "Headset Mic",
> > + "Left Spk", "Front Left BE_OUT",
> > + "Right Spk", "Front Right BE_OUT";
> > + status = "okay";
> > +
> > + hs-capture-dai-link {
>
> Nope, that's not how DAI links are called.
Acknowledged, but I'm still figuring out what you want to suggest here
- does the `^dai-link@[0-9]+$` pattern sound okay?
>
> > + codec {
> > + sound-dai = <&es8326>;
> > + };
> > + };
>
> ...
>
> > +&i2c2 {
> > + status = "disabled";
> > +};
> > +
> > +&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>;
> > + };
> > +};
> > +
> > +&max98390_38 {
> > + sound-name-prefix = "Front Right";
> > + maxim,dsm_param_name = "dsm_param.bin";
>
> NAK. Not a DT property. You cannot send us ACPI stuff.
>
> Go through internal review first, which should tell you that ACPI is not
> accepted into DT. There are just few exceptions and codecs do not
> qualify.
I'll drop all these invalid lines. It was likely a misunderstanding
when the lines were added. Thanks for clarifying.
Regards,
Fei
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists