linux-kernel - Re: [PATCH v1 3/3] arm64: dts: qcom: sc7280: include sc7280-herobrine-audio-rt5682.dtsi in villager and herobrine-r1

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 14 Jul 2022 08:12:21 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Judy Hsiao <judyhsiao@...omium.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Srini Kandagatla <srinivas.kandagatla@...aro.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Jimmy Cheng-Yi Chiang <cychiang@...gle.com>,
        Judy Hsiao <judyhsiao@...gle.com>,
        Stephen Boyd <swboyd@...omium.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Jimmy Chen <jinghung.chen3@...mail.com>
Subject: Re: [PATCH v1 3/3] arm64: dts: qcom: sc7280: include
 sc7280-herobrine-audio-rt5682.dtsi in villager and herobrine-r1

Hi,

On Wed, Jul 13, 2022 at 11:12 PM Judy Hsiao <judyhsiao@...omium.org> wrote:
>
> Include sc7280-herobrine-audio-rt5682.dtsi in villager and herobrine-r1 as
> these boards use rt5682 codec.
>
> Signed-off-by: Judy Hsiao <judyhsiao@...omium.org>
> Reviewed-by: Matthias Kaehlcke <mka@...omium.org>
> Reviewed-by: Douglas Anderson <dianders@...omium.org>
>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts | 1 +
>  arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts  | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> index c1647a85a371..98280436813d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> @@ -8,6 +8,7 @@
>  /dts-v1/;
>
>  #include "sc7280-herobrine.dtsi"
> +#include "sc7280-herobrine-audio-rt5682.dtsi"
>
>  / {
>         model = "Google Herobrine (rev1+)";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts
> index cbd8a2d1ef2a..077c58c93a65 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts
> @@ -8,6 +8,7 @@
>  /dts-v1/;
>
>  #include "sc7280-herobrine.dtsi"
> +#include "sc7280-herobrine-audio-rt5682.dtsi"

Hmmm, I suspect that this is going to conflict with Jimmy's patch [1]
since after Jimmy's patch then villager-r1 just includes villager-r0
and then adds the _other_ audio include. The net result of that will
be that the "-r1" file will end up including both audio solutions and
I'm not sure that plays well together.

I suspect that what you'll want to do is:

1. Move most of the existing "sc7280-herobrine-villager-r0.dts" into a
new file "sc7280-herobrine-villager.dtsi".

2. Make a new "sc7280-herobrine-villager-r0.dts" that includes the
dtsi and then the audio bit. This would also have the top-level
"model" and "compatible".

3. For simplicity sake, maybe you should add
"sc7280-herobrine-villager-r1.dts" in your patch series. Then Jimmy's
cover letter can document that his patch series is based atop yours
and his patch series can just focus on the LTE bits. If you do this,
then your patch series will also need a patch to add to the board
bindings file (Documentation/devicetree/bindings/arm/qcom.yaml), so
you'll need a patch that's part of Jimmy's patch #1.

Does that all make sense?

[1] https://lore.kernel.org/r/SG2PR03MB50066EA3AE8F8E98B67C920BCC889@SG2PR03MB5006.apcprd03.prod.outlook.com/