Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
References: <20200419053815.15731-1-christianshewitt@gmail.com>
User-agent: mu4e 1.3.3; emacs 26.3
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Christian Hewitt <christianshewitt@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Kevin Hilman <khilman@baylibre.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/5] arm64: dts: meson: Simplify G12/SM1 Audio Configs
In-reply-to: <20200419053815.15731-1-christianshewitt@gmail.com>
Date:   Mon, 20 Apr 2020 13:03:07 +0200
Message-ID: <1jk12apggk.fsf@starbuckisacylon.baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On Sun 19 Apr 2020 at 07:38, Christian Hewitt <christianshewitt@gmail.com> wrote:

> This series creates two new dtsi with the HDMI and HDMI+S/PDIF audio configs
> used in most G12/SM1 Android box devices and some SBC's and switches most dts
> to use them. I have not touched the SEI510/610 which have extra hardware
> in their dts, and the U200 which does not currently have audio support, but
> will likely receive an 'all possible routings' master config when Jerome
> sends his next set of changes (looking at his WIP branch) so I leave U200
> for him to address later.
>
> One advantaage of common configs is that distros that need to embed alsa
> conf files as part of their userspace support now only need to include two
> confs that will automatically support more boards and boxes as they are
> added, instead of needing to track and add confs or card aliases for every
> new device.
>
> Christian Hewitt (5):
>   arm64: dts: meson: create common hdmi/hdmi-spdif audio dtsi

I'm really not a fan of this. Yes the configuration appear to be similar
but there always the same. When they are, it is usually by lack of
knowledge of the platform and its use cases.

Using the same sound card model is particularily bad.

>   arm64: dts: meson: convert ugoos-am6 to common w400 dtsi
>   arm64: dts: meson: convert odroid-n2 to hdmi dtsi
>   arm64: dts: meson: convert khadas-vim3/vim3l to hdmi dtsi

For example on the vim3, copying the sei610 was not the best choice
possible.

- SEI 610 prepares TDM B for 8 ch HDMI because TDM A is used for the
 internal speaker
- VIM3 has the TDM B on the 40 pin header and TDM C on the M2 port.
  It would be better to use TDM A for HDMI is this case (patch will
 follow)

This is just an example. Bottom, it designs are really copy/paste of the
ref design, the dtsi should include all the platform, not just the
sound.

>   arm64: dts: meson: convert x96max to hdmi dtsi
>
>  .../amlogic/meson-g12-audio-hdmi-spdif.dtsi   | 139 +++++
>  .../dts/amlogic/meson-g12-audio-hdmi.dtsi     |  96 ++++
>  .../boot/dts/amlogic/meson-g12a-x96-max.dts   | 131 +----
>  .../dts/amlogic/meson-g12b-khadas-vim3.dtsi   |  85 ---
>  .../boot/dts/amlogic/meson-g12b-odroid-n2.dts |  89 +--
>  .../boot/dts/amlogic/meson-g12b-ugoos-am6.dts | 541 +-----------------
>  .../boot/dts/amlogic/meson-g12b-w400.dtsi     | 423 ++++++++++++++
>  .../boot/dts/amlogic/meson-khadas-vim3.dtsi   |   1 +
>  8 files changed, 663 insertions(+), 842 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12-audio-hdmi-spdif.dtsi
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12-audio-hdmi.dtsi
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi