From: Fei Shao <fshao@chromium.org>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
Conor Dooley <conor+dt@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Rob Herring <robh@kernel.org>,
devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 2/2] arm64: dts: mediatek: Introduce MT8188 Geralt platform based Ciri
Date: Mon, 28 Oct 2024 19:16:43 +0800 [thread overview]
Message-ID: <CAC=S1nic3HgEGeS6up_MsYh3UoM_piHC6=uuRep5bne4zOWbxQ@mail.gmail.com> (raw)
In-Reply-To: <iupuwxxslb3t76mzy3ufsruwwpd47fuop7y6tfcbwkosdagkxf@fnklbp6t6au2>
On Mon, Oct 28, 2024 at 5:02 AM Krzysztof Kozlowski <krzk@kernel.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@chromium.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
>
next prev parent reply other threads:[~2024-10-28 12:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-25 10:59 [PATCH 0/2] Add device tree for MT8188-based Chromebook "Ciri" Fei Shao
2024-10-25 10:59 ` [PATCH 1/2] dt-bindings: arm: mediatek: Add MT8188 Lenovo Chromebook Duet (11", 9) Fei Shao
2024-10-25 10:59 ` [PATCH 2/2] arm64: dts: mediatek: Introduce MT8188 Geralt platform based Ciri Fei Shao
2024-10-27 21:02 ` Krzysztof Kozlowski
2024-10-28 11:16 ` Fei Shao [this message]
2024-10-25 13:55 ` [PATCH 0/2] Add device tree for MT8188-based Chromebook "Ciri" Rob Herring (Arm)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAC=S1nic3HgEGeS6up_MsYh3UoM_piHC6=uuRep5bne4zOWbxQ@mail.gmail.com' \
--to=fshao@chromium.org \
--cc=angelogioacchino.delregno@collabora.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=krzk@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).