| 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=S1nhhfwHU5K5ZyUhZBhvz38LOZGLnGN-Rc1ZAup_VTfkpvA@mail.gmail.com>
Date: Fri, 8 Nov 2024 12:11:57 +0800
From: Fei Shao <fshao@...omium.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] arm64: dts: mediatek: Introduce MT8188 Geralt
platform based Ciri
On Thu, Nov 7, 2024 at 6:37 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
>
> Il 07/11/24 07:58, Fei Shao ha scritto:
> > On Wed, Nov 6, 2024 at 9:19 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@...labora.com> wrote:
> >>
> >> Il 05/11/24 10:30, Fei Shao ha scritto:
> >>> 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>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - remove invalid or undocumented properties
> >>> e.g. mediatek,dai-link, maxim,dsm_param_name etc.
> >>> - remove touchscreen as the driver is not yet accepted in upstream
> >>> - update sound DAI link node name to match the binding
> >>> - add missing pinctrls in audio codec nodes
> >>>
> >>> arch/arm64/boot/dts/mediatek/Makefile | 8 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku0.dts | 11 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku1.dts | 60 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku2.dts | 56 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku3.dts | 15 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku4.dts | 43 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku5.dts | 73 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku6.dts | 69 +
> >>> .../dts/mediatek/mt8188-geralt-ciri-sku7.dts | 47 +
> >>> .../boot/dts/mediatek/mt8188-geralt-ciri.dtsi | 397 +++++
> >>> .../boot/dts/mediatek/mt8188-geralt.dtsi | 1492 +++++++++++++++++
> >>> 11 files changed, 2271 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
> >>>
> > [...]
> >
>
> [...]
>
> >>> +&pmic {
> >>> + interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>;
> >>> +};
> >>> +
> >>> +&scp {
> >>
> >> Is this SCP-dual or SCP?
> >> I see SCP, but I also see a SCP-Dual memory region... what's going on here?
> >>
> >> Of course, the SCP-Dual won't work if you don't override the compatible string...
> >
> > To clarify, the second SCP core is used for MIPI camera in downstream,
> > and I deliberately only describe the first SCP core here since the MTK
> > camera ISP driver isn't in upstream at the moment.
> > I had a fixup patch for removing the scp-dual reserved memory region,
> > but likely it was missing during the rebase... let me check again if
> > it can be removed, just in case there's firmware protecting the region
> > and the kernel shouldn't access it.
> >
>
> Hmm... but the second SCP core can still be brought up, even if the MIPI Camera
> driver is not upstreamed yet, right?
Well, that's true... and it should pave the way for validating the
driver with the upstreamed DT whenever that becomes available.
>
> That shouldn't cause lockups and/or other kinds of bad behavior, and should
> bring up a core and just never use it, without any particular issues.
>
> If we can enable the secondary core, let's just go for it.. as that will help
> specifying the exact memory layout of this board (and failing to do that may
> create some other issues, that's why I'm proposing to enable that even if it
> is not really used in this case).
>
> What do you think? :-)
>
Sure, that sounds good to me, too.
I started only with the essential DT bits to ensure the device can
boot, which it does, so I guess it's time to bring that back. I'll
incorporate that in v3.
I plan to fix up the single SCP core node to SCP-dual directly, so
please let me know if you prefer seeing that as an individual patch on
top (either option works for me).
Regards,
Fei
> >>
> >>> + firmware-name = "mediatek/mt8188/scp.img";
> >>> + memory-region = <&scp_mem>;
> >>> + pinctrl-names = "default";
> >>> + pinctrl-0 = <&scp_pins>;
> >>> + status = "okay";
> >>> +
> >>> + cros-ec-rpmsg {
> >>> + compatible = "google,cros-ec-rpmsg";
> >>> + mediatek,rpmsg-name = "cros-ec-rpmsg";
> >>> + };
> >>> +};
> >>> +
> >>> +&sound {
> >>> + compatible = "mediatek,mt8188-nau8825";
> >>> + model = "mt8188_m98390_8825";
> >>> + pinctrl-names = "aud_etdm_hp_on",
> >>> + "aud_etdm_hp_off",
> >>> + "aud_etdm_spk_on",
> >>> + "aud_etdm_spk_off",
> >>> + "aud_mtkaif_on",
> >>> + "aud_mtkaif_off";
> >>
> >> pinctrl-names = "aud_etdm_hp_on", "aud_etdm_hp_off",
> >> "aud_etdm_spk_on", "aud_etdm_spk_off",
> >> "aud_mtkaif_on", "aud_mtkaif_off";
> >
> > Acked.
> >
> >>
> >>> + pinctrl-0 = <&aud_etdm_hp_on>;
> >>> + pinctrl-1 = <&aud_etdm_hp_off>;
> >>> + pinctrl-2 = <&aud_etdm_spk_on>;
> >>> + pinctrl-3 = <&aud_etdm_spk_off>;
> >>> + pinctrl-4 = <&aud_mtkaif_on>;
> >>> + pinctrl-5 = <&aud_mtkaif_off>;
> >>
> >> Add a comment:
> >>
> >> /* The audio-routing is defined in each board dts */
> >>
> >>> + 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",
> >>> + "MIC", "Headset Mic",
> >>> + "Left Spk", "Front Left BE_OUT",
> >>> + "Right Spk", "Front Right BE_OUT",
> >>> + "Rear Left Spk", "Rear Left BE_OUT",
> >>> + "Rear Right Spk", "Rear Right BE_OUT";
> >>> +
> >>
> >> ...and remove the audio-routing from this dtsi; it's anyway being
> >> overridden by the -ciri.dtsi devicetree...
> >
> > Acknowledged, and thanks for all the feedback here.
> >
>
> You're welcome :-)
>
> Cheers,
> Angelo
>
Powered by blists - more mailing lists