| Message ID | 20241008082200.4002798-1-wenst@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: mediatek: mt8186-corsola-voltorb: Merge speaker codec nodes | expand |
Il 08/10/24 10:21, Chen-Yu Tsai ha scritto: > The Voltorb device uses a speaker codec different from the original > Corsola device. When the Voltorb device tree was first added, the new > codec was added as a separate node when it should have just replaced the > existing one. > > Merge the two nodes. The only differences are the compatible string and > the GPIO line property name. This keeps the device node path for the > speaker codec the same across the MT8186 Chromebook line. Ok, I agree... But, at this point, can we rename `rt1019p` to `speaker_codec` instead? Imo, that makes a bit more sense as a phandle, as it reads generic and it's not screaming "I'm RT1019P" on dts(i) files where it's actually not. > > Fixes: 321ad586e607 ("arm64: dts: mediatek: Add MT8186 Voltorb Chromebooks") > Cc: <stable@vger.kernel.org> Well, that's not a fix - it's an improvement, so we can avoid this Fixes tag :-) Cheers, Angelo > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > .../dts/mediatek/mt8186-corsola-voltorb.dtsi | 19 ++++--------------- > 1 file changed, 4 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi > index 52ec58128d56..fbcd97069df9 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi > @@ -10,12 +10,6 @@ > > / { > chassis-type = "laptop"; > - > - max98360a: max98360a { > - compatible = "maxim,max98360a"; > - sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; > - #sound-dai-cells = <0>; > - }; > }; > > &cpu6 { > @@ -59,19 +53,14 @@ &cluster1_opp_15 { > opp-hz = /bits/ 64 <2200000000>; > }; > > -&rt1019p{ > - status = "disabled"; > +&rt1019p { > + compatible = "maxim,max98360a"; > + sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; > + /delete-property/ sdb-gpios; > }; > > &sound { > compatible = "mediatek,mt8186-mt6366-rt5682s-max98360-sound"; > - status = "okay"; > - > - spk-hdmi-playback-dai-link { > - codec { > - sound-dai = <&it6505dptx>, <&max98360a>; > - }; > - }; > }; > > &spmi {
On Tue, Oct 8, 2024 at 4:51 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 08/10/24 10:21, Chen-Yu Tsai ha scritto: > > The Voltorb device uses a speaker codec different from the original > > Corsola device. When the Voltorb device tree was first added, the new > > codec was added as a separate node when it should have just replaced the > > existing one. > > > > Merge the two nodes. The only differences are the compatible string and > > the GPIO line property name. This keeps the device node path for the > > speaker codec the same across the MT8186 Chromebook line. > > Ok, I agree... > > But, at this point, can we rename `rt1019p` to `speaker_codec` instead? > > Imo, that makes a bit more sense as a phandle, as it reads generic and it's not > screaming "I'm RT1019P" on dts(i) files where it's actually not. Works for me. > > > > Fixes: 321ad586e607 ("arm64: dts: mediatek: Add MT8186 Voltorb Chromebooks") > > Cc: <stable@vger.kernel.org> > > Well, that's not a fix - it's an improvement, so we can avoid this Fixes tag :-) I'd like to see it backported though, so we minimize the different DTS files. Guess I'll add Cc stable instead? Not sure if that works without a Fixes tag. ChenYu > Cheers, > Angelo > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > --- > > .../dts/mediatek/mt8186-corsola-voltorb.dtsi | 19 ++++--------------- > > 1 file changed, 4 insertions(+), 15 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi > > index 52ec58128d56..fbcd97069df9 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi > > @@ -10,12 +10,6 @@ > > > > / { > > chassis-type = "laptop"; > > - > > - max98360a: max98360a { > > - compatible = "maxim,max98360a"; > > - sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; > > - #sound-dai-cells = <0>; > > - }; > > }; > > > > &cpu6 { > > @@ -59,19 +53,14 @@ &cluster1_opp_15 { > > opp-hz = /bits/ 64 <2200000000>; > > }; > > > > -&rt1019p{ > > - status = "disabled"; > > +&rt1019p { > > + compatible = "maxim,max98360a"; > > + sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; > > + /delete-property/ sdb-gpios; > > }; > > > > &sound { > > compatible = "mediatek,mt8186-mt6366-rt5682s-max98360-sound"; > > - status = "okay"; > > - > > - spk-hdmi-playback-dai-link { > > - codec { > > - sound-dai = <&it6505dptx>, <&max98360a>; > > - }; > > - }; > > }; > > > > &spmi { >
Il 08/10/24 12:14, Chen-Yu Tsai ha scritto: > On Tue, Oct 8, 2024 at 4:51 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 08/10/24 10:21, Chen-Yu Tsai ha scritto: >>> The Voltorb device uses a speaker codec different from the original >>> Corsola device. When the Voltorb device tree was first added, the new >>> codec was added as a separate node when it should have just replaced the >>> existing one. >>> >>> Merge the two nodes. The only differences are the compatible string and >>> the GPIO line property name. This keeps the device node path for the >>> speaker codec the same across the MT8186 Chromebook line. >> >> Ok, I agree... >> >> But, at this point, can we rename `rt1019p` to `speaker_codec` instead? >> >> Imo, that makes a bit more sense as a phandle, as it reads generic and it's not >> screaming "I'm RT1019P" on dts(i) files where it's actually not. > > Works for me. > >>> >>> Fixes: 321ad586e607 ("arm64: dts: mediatek: Add MT8186 Voltorb Chromebooks") >> > Cc: <stable@vger.kernel.org> >> >> Well, that's not a fix - it's an improvement, so we can avoid this Fixes tag :-) > > I'd like to see it backported though, so we minimize the different DTS files. > Guess I'll add Cc stable instead? Not sure if that works without a Fixes tag. > Well, try to send it to the stable mailing list as well... I fully understand your concern but backporting is for fixes and *not* for improvements, so I doubt that you'll get that backported. It's Sasha's call then, anyway. > ChenYu > >> Cheers, >> Angelo >> >>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> >>> --- >>> .../dts/mediatek/mt8186-corsola-voltorb.dtsi | 19 ++++--------------- >>> 1 file changed, 4 insertions(+), 15 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi >>> index 52ec58128d56..fbcd97069df9 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi >>> @@ -10,12 +10,6 @@ >>> >>> / { >>> chassis-type = "laptop"; >>> - >>> - max98360a: max98360a { >>> - compatible = "maxim,max98360a"; >>> - sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; >>> - #sound-dai-cells = <0>; >>> - }; >>> }; >>> >>> &cpu6 { >>> @@ -59,19 +53,14 @@ &cluster1_opp_15 { >>> opp-hz = /bits/ 64 <2200000000>; >>> }; >>> >>> -&rt1019p{ >>> - status = "disabled"; >>> +&rt1019p { >>> + compatible = "maxim,max98360a"; >>> + sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; >>> + /delete-property/ sdb-gpios; >>> }; >>> >>> &sound { >>> compatible = "mediatek,mt8186-mt6366-rt5682s-max98360-sound"; >>> - status = "okay"; >>> - >>> - spk-hdmi-playback-dai-link { >>> - codec { >>> - sound-dai = <&it6505dptx>, <&max98360a>; >>> - }; >>> - }; >>> }; >>> >>> &spmi { >>
diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi index 52ec58128d56..fbcd97069df9 100644 --- a/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-voltorb.dtsi @@ -10,12 +10,6 @@ / { chassis-type = "laptop"; - - max98360a: max98360a { - compatible = "maxim,max98360a"; - sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; - #sound-dai-cells = <0>; - }; }; &cpu6 { @@ -59,19 +53,14 @@ &cluster1_opp_15 { opp-hz = /bits/ 64 <2200000000>; }; -&rt1019p{ - status = "disabled"; +&rt1019p { + compatible = "maxim,max98360a"; + sdmode-gpios = <&pio 150 GPIO_ACTIVE_HIGH>; + /delete-property/ sdb-gpios; }; &sound { compatible = "mediatek,mt8186-mt6366-rt5682s-max98360-sound"; - status = "okay"; - - spk-hdmi-playback-dai-link { - codec { - sound-dai = <&it6505dptx>, <&max98360a>; - }; - }; }; &spmi {
The Voltorb device uses a speaker codec different from the original Corsola device. When the Voltorb device tree was first added, the new codec was added as a separate node when it should have just replaced the existing one. Merge the two nodes. The only differences are the compatible string and the GPIO line property name. This keeps the device node path for the speaker codec the same across the MT8186 Chromebook line. Fixes: 321ad586e607 ("arm64: dts: mediatek: Add MT8186 Voltorb Chromebooks") Cc: <stable@vger.kernel.org> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- .../dts/mediatek/mt8186-corsola-voltorb.dtsi | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)