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

[Doug Anderson]

Hi,

On Wed, Jul 13, 2022 at 11:12 PM Judy Hsiao <judyhsiao@xxxxxxxxxxxx> wrote:
>
> Include sc7280-herobrine-audio-rt5682.dtsi in villager and herobrine-r1 as
> these boards use rt5682 codec.
>
> Signed-off-by: Judy Hsiao <judyhsiao@xxxxxxxxxxxx>
> Reviewed-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/