[PATCH 0/4] Allwinner H6 SPDIF support

[PATCH 0/4] Allwinner H6 SPDIF support

[Clément Péron]

*H6 DMA support IS REQUIRED*

Allwinner H6 SoC has a SPDIF controller called One Wire Audio (OWA) which
is different from the previous H3 generation but still compatible.

Difference are an increase of fifo sizes and there is now the possibility
to output the master clock on a pin.

Also this series require the DMA working on H6, a first version has been
submitted by Jernej Škrabec but is not yet accepted (as this moment):
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=89011

This patch has been tested on Beelink GS1 on top of sunxi/for-next with
the series above applied.

Clément Péron (4):
  dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible
  arm64: dts: allwinner: Add SPDIF node for Allwinner H6
  arm64: dts: allwinner: h6: Enable SPDIF for Beelink GS1
  arm64: defconfig: Enable Sun4i SPDIF module

 .../bindings/sound/sunxi,sun4i-spdif.txt      |  3 +-
 .../dts/allwinner/sun50i-h6-beelink-gs1.dts   |  4 ++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 39 +++++++++++++++++++
 arch/arm64/configs/defconfig                  |  1 +
 4 files changed, 46 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Clément Péron]

Allwinner H6 has a SPDIF controller with an increase of the fifo
size and the possibility to output the master clock.
However it's still compatible with the already existing driver for
Allwiner H3.

Add this compatible in the device-tree bindings documentation.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt b/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt
index 0c64a209c2e9..e33180354dce 100644
--- a/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt
+++ b/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt
@@ -7,10 +7,11 @@ For now only playback is supported.
 
 Required properties:
 
-  - compatible		: should be one of the following:
+  - compatible		: Should be one of the following:
     - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
     - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC
     - "allwinner,sun8i-h3-spdif": for the Allwinner H3 SoC
+    - "allwinner,sun50i-h6-spdif", "allwinner,sun8i-h3-spdif": for H6 SoC
 
   - reg			: Offset and length of the register set for the device.
 
-- 
2.17.1

[PATCH 2/4] arm64: dts: allwinner: Add SPDIF node for Allwinner H6

[Clément Péron]

The Allwinner H6 has a SPDIF controller called OWA (One Wire Audio).

Add this node in the SoC device-tree and as they are only one pinmuxing
available set it as default.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 39 ++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index ad2077830f73..5e850b359be5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -83,6 +83,24 @@
 		method = "smc";
 	};
 
+	sound_spdif {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "On-board SPDIF";
+
+		simple-audio-card,cpu {
+			sound-dai = <&spdif>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&spdif_out>;
+		};
+	};
+
+	spdif_out: spdif-out {
+		#sound-dai-cells = <0>;
+		compatible = "linux,spdif-dit";
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13
@@ -264,6 +282,11 @@
 				bias-pull-up;
 			};
 
+			spdif_tx_pin: spdif-tx-pin {
+				pins = "PH7";
+				function = "spdif";
+			};
+
 			uart0_ph_pins: uart0-ph-pins {
 				pins = "PH0", "PH1";
 				function = "uart0";
@@ -391,6 +414,22 @@
 			};
 		};
 
+		spdif: spdif@5093000 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun50i-h6-spdif",
+				     "allwinner,sun8i-h3-spdif";
+			reg = <0x05093000 0x400>;
+			interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_SPDIF>, <&ccu CLK_SPDIF>;
+			clock-names = "apb", "spdif";
+			resets = <&ccu RST_BUS_SPDIF>;
+			dmas = <&dma 2>;
+			dma-names = "tx";
+			pinctrl-names = "default";
+			pinctrl-0 = <&spdif_tx_pin>;
+			status = "disabled";
+		};
+
 		usb2otg: usb@5100000 {
 			compatible = "allwinner,sun50i-h6-musb",
 				     "allwinner,sun8i-a33-musb";
-- 
2.17.1

[PATCH 3/4] arm64: dts: allwinner: h6: Enable SPDIF for Beelink GS1

[Clément Péron]

Beelink GS1 board has a SPDIF out connector, so enable it in
the device-tree of the board.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
index 0dc33c90dd60..76a95ad33dc5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
@@ -243,6 +243,10 @@
 	vcc-pm-supply = <&reg_aldo1>;
 };
 
+&spdif {
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_ph_pins>;
-- 
2.17.1

[PATCH 4/4] arm64: defconfig: Enable Sun4i SPDIF module

[Clément Péron]

Allwinner A64 and H6 use the Sun4i SPDIF driver.

Enable this to allow a proper support for these SoCs.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2d9c39033c1a..423413768d79 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -531,6 +531,7 @@ CONFIG_SND_SOC_RK3399_GRU_SOUND=m
 CONFIG_SND_MESON_AXG_SOUND_CARD=m
 CONFIG_SND_SOC_SAMSUNG=y
 CONFIG_SND_SOC_RCAR=m
+CONFIG_SND_SUN4I_SPDIF=m
 CONFIG_SND_SOC_AK4613=m
 CONFIG_SND_SOC_PCM3168A_I2C=m
 CONFIG_SND_SIMPLE_CARD=m
-- 
2.17.1

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

Hi,

On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> Allwinner H6 has a SPDIF controller with an increase of the fifo
> size and the possibility to output the master clock.
> However it's still compatible with the already existing driver for
> Allwiner H3.
>
> Add this compatible in the device-tree bindings documentation.

How was this tested?

After looking at the datasheet, it looks like it's still significantly
different. In particular:

  - The MCLK divider isn't at the same offset
  - The bits to flush the FIFO aren't at the same offset
  - The FIFO trigger levels aren't at the same offset
  - The FIFO status register layout is almost entirely different

So it doesn't really look compatible to me

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Clément Péron]

Hi,

On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > size and the possibility to output the master clock.
> > However it's still compatible with the already existing driver for
> > Allwiner H3.
> >
> > Add this compatible in the device-tree bindings documentation.
>
> How was this tested?

Using Alsa utils on a Beelink GS1 and I check correct output sound on
my home cinema.
>
> After looking at the datasheet, it looks like it's still significantly
> different. In particular:
>
>   - The MCLK divider isn't at the same offset
We don't use the MCLK
>   - The bits to flush the FIFO aren't at the same offset
>   - The FIFO trigger levels aren't at the same offset
>   - The FIFO status register layout is almost entirely different
We use DMA not FIFO

Clement
>
> So it doesn't really look compatible to me
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Clément Péron]

On Mon, 15 Apr 2019 at 10:09, Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi,
>
> On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > Hi,
> >
> > On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > > size and the possibility to output the master clock.
> > > However it's still compatible with the already existing driver for
> > > Allwiner H3.
> > >
> > > Add this compatible in the device-tree bindings documentation.
> >
> > How was this tested?
>
> Using Alsa utils on a Beelink GS1 and I check correct output sound on
> my home cinema.
> >
> > After looking at the datasheet, it looks like it's still significantly
> > different. In particular:
> >
> >   - The MCLK divider isn't at the same offset
> We don't use the MCLK
> >   - The bits to flush the FIFO aren't at the same offset
> >   - The FIFO trigger levels aren't at the same offset
> >   - The FIFO status register layout is almost entirely different
> We use DMA not FIFO

And also only SPDIF Tx has been tested.

>
> Clement
> >
> > So it doesn't really look compatible to me
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1428 bytes --]

On Mon, Apr 15, 2019 at 10:09:11AM +0200, Clément Péron wrote:
> Hi,
>
> On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > Hi,
> >
> > On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > > size and the possibility to output the master clock.
> > > However it's still compatible with the already existing driver for
> > > Allwiner H3.
> > >
> > > Add this compatible in the device-tree bindings documentation.
> >
> > How was this tested?
>
> Using Alsa utils on a Beelink GS1 and I check correct output sound on
> my home cinema.
>
> > After looking at the datasheet, it looks like it's still significantly
> > different. In particular:
> >
> >   - The MCLK divider isn't at the same offset
>
> We don't use the MCLK
>
> >   - The bits to flush the FIFO aren't at the same offset
> >   - The FIFO trigger levels aren't at the same offset
> >   - The FIFO status register layout is almost entirely different
>
> We use DMA not FIFO

Well, DMA still uses the FIFOs. And even then, you're kind of missing
the point. The device tree's compatible is about whether a particular
device can be used as another one. How you are using it in the driver
is completely irrelevant.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Clément Péron]

On Mon, 15 Apr 2019 at 10:18, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Apr 15, 2019 at 10:09:11AM +0200, Clément Péron wrote:
> > Hi,
> >
> > On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > > > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > > > size and the possibility to output the master clock.
> > > > However it's still compatible with the already existing driver for
> > > > Allwiner H3.
> > > >
> > > > Add this compatible in the device-tree bindings documentation.
> > >
> > > How was this tested?
> >
> > Using Alsa utils on a Beelink GS1 and I check correct output sound on
> > my home cinema.
> >
> > > After looking at the datasheet, it looks like it's still significantly
> > > different. In particular:
> > >
> > >   - The MCLK divider isn't at the same offset
> >
> > We don't use the MCLK
> >
> > >   - The bits to flush the FIFO aren't at the same offset
> > >   - The FIFO trigger levels aren't at the same offset
> > >   - The FIFO status register layout is almost entirely different
> >
> > We use DMA not FIFO
>
> Well, DMA still uses the FIFOs. And even then, you're kind of missing
> the point. The device tree's compatible is about whether a particular
> device can be used as another one. How you are using it in the driver
> is completely irrelevant.

Sorry I meant " We use DMA not PIO".

I didn't change the driver and the SPDIF is working what's irrelevant
in declaring the driver as compatible ?

If some feature are missing they can be introduce later as there is a
special binding for H6 no ?

Clement

>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Mon, Apr 15, 2019 at 10:30:38AM +0200, Clément Péron wrote:
> On Mon, 15 Apr 2019 at 10:18, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Apr 15, 2019 at 10:09:11AM +0200, Clément Péron wrote:
> > > Hi,
> > >
> > > On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > > > > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > > > > size and the possibility to output the master clock.
> > > > > However it's still compatible with the already existing driver for
> > > > > Allwiner H3.
> > > > >
> > > > > Add this compatible in the device-tree bindings documentation.
> > > >
> > > > How was this tested?
> > >
> > > Using Alsa utils on a Beelink GS1 and I check correct output sound on
> > > my home cinema.
> > >
> > > > After looking at the datasheet, it looks like it's still significantly
> > > > different. In particular:
> > > >
> > > >   - The MCLK divider isn't at the same offset
> > >
> > > We don't use the MCLK
> > >
> > > >   - The bits to flush the FIFO aren't at the same offset
> > > >   - The FIFO trigger levels aren't at the same offset
> > > >   - The FIFO status register layout is almost entirely different
> > >
> > > We use DMA not FIFO
> >
> > Well, DMA still uses the FIFOs. And even then, you're kind of missing
> > the point. The device tree's compatible is about whether a particular
> > device can be used as another one. How you are using it in the driver
> > is completely irrelevant.
>
> Sorry I meant " We use DMA not PIO".

I'm still not quite sure what you mean. Both DMA and PIO would be
using the FIFOs.

> I didn't change the driver and the SPDIF is working what's irrelevant
> in declaring the driver as compatible ?

It's working by accident.

This won't work for example:
https://elixir.bootlin.com/linux/latest/source/sound/soc/sunxi/sun4i-spdif.c#L181

> If some feature are missing they can be introduce later as there is a
> special binding for H6 no ?

Again, that's irrelevant. The only question you need to ask yourself
is whether, if you take an imaginary source code that would support
all the H3 SPDIF features and run it on an H6, it works just as well.

The examples above that it's not the case, so you can't claim that the
device is compatible.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Ondřej Jirman]

Hi Clement,

On Mon, Apr 15, 2019 at 10:30:38AM +0200, Clément Péron wrote:
> On Mon, 15 Apr 2019 at 10:18, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Apr 15, 2019 at 10:09:11AM +0200, Clément Péron wrote:
> > > Hi,
> > >
> > > On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > > > > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > > > > size and the possibility to output the master clock.
> > > > > However it's still compatible with the already existing driver for
> > > > > Allwiner H3.
> > > > >
> > > > > Add this compatible in the device-tree bindings documentation.
> > > >
> > > > How was this tested?
> > >
> > > Using Alsa utils on a Beelink GS1 and I check correct output sound on
> > > my home cinema.
> > >
> > > > After looking at the datasheet, it looks like it's still significantly
> > > > different. In particular:
> > > >
> > > >   - The MCLK divider isn't at the same offset
> > >
> > > We don't use the MCLK
> > >
> > > >   - The bits to flush the FIFO aren't at the same offset
> > > >   - The FIFO trigger levels aren't at the same offset
> > > >   - The FIFO status register layout is almost entirely different
> > >
> > > We use DMA not FIFO
> >
> > Well, DMA still uses the FIFOs. And even then, you're kind of missing
> > the point. The device tree's compatible is about whether a particular
> > device can be used as another one. How you are using it in the driver
> > is completely irrelevant.
> 
> Sorry I meant " We use DMA not PIO".
> 
> I didn't change the driver and the SPDIF is working what's irrelevant
> in declaring the driver as compatible ?
>
> If some feature are missing they can be introduce later as there is a
> special binding for H6 no ?

You have to drop fallback to allwinner,sun8i-h3-spdif at the very least:

+                       #sound-dai-cells = <0>;
+                       compatible = "allwinner,sun50i-h6-spdif",
+                                    "allwinner,sun8i-h3-spdif";
+                       reg = <0x05093000 0x400>;

If the H6 hardware doesn't just implement some optional superset of what H3
does.

regards,
	o.

> Clement
> 
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Clément Péron]

Hi,

On Mon, 15 Apr 2019 at 16:51, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Apr 15, 2019 at 10:30:38AM +0200, Clément Péron wrote:
> > On Mon, 15 Apr 2019 at 10:18, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Mon, Apr 15, 2019 at 10:09:11AM +0200, Clément Péron wrote:
> > > > Hi,
> > > >
> > > > On Mon, 15 Apr 2019 at 10:04, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Thu, Apr 11, 2019 at 11:44:19PM +0200, Clément Péron wrote:
> > > > > > Allwinner H6 has a SPDIF controller with an increase of the fifo
> > > > > > size and the possibility to output the master clock.
> > > > > > However it's still compatible with the already existing driver for
> > > > > > Allwiner H3.
> > > > > >
> > > > > > Add this compatible in the device-tree bindings documentation.
> > > > >
> > > > > How was this tested?
> > > >
> > > > Using Alsa utils on a Beelink GS1 and I check correct output sound on
> > > > my home cinema.
> > > >
> > > > > After looking at the datasheet, it looks like it's still significantly
> > > > > different. In particular:
> > > > >
> > > > >   - The MCLK divider isn't at the same offset
> > > >
> > > > We don't use the MCLK
> > > >
> > > > >   - The bits to flush the FIFO aren't at the same offset
> > > > >   - The FIFO trigger levels aren't at the same offset
> > > > >   - The FIFO status register layout is almost entirely different
> > > >
> > > > We use DMA not FIFO
> > >
> > > Well, DMA still uses the FIFOs. And even then, you're kind of missing
> > > the point. The device tree's compatible is about whether a particular
> > > device can be used as another one. How you are using it in the driver
> > > is completely irrelevant.
> >
> > Sorry I meant " We use DMA not PIO".
>
> I'm still not quite sure what you mean. Both DMA and PIO would be
> using the FIFOs.

Just correcting myself but yes we use FIFO with the DMA.

>
> > I didn't change the driver and the SPDIF is working what's irrelevant
> > in declaring the driver as compatible ?
>
> It's working by accident.
>
> This won't work for example:
> https://elixir.bootlin.com/linux/latest/source/sound/soc/sunxi/sun4i-spdif.c#L181
>
> > If some feature are missing they can be introduce later as there is a
> > special binding for H6 no ?
>
> Again, that's irrelevant. The only question you need to ask yourself
> is whether, if you take an imaginary source code that would support
> all the H3 SPDIF features and run it on an H6, it works just as well.
>
> The examples above that it's not the case, so you can't claim that the
> device is compatible.

Is it ok if I fix the TX Fifo flush bit and remove the compatible with H3 ?
(Looking at the driver this is the only bit used that is incompatible with H6)

Or should I send a new driver with the correct mapping ?

Thanks
Clément


>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

On Mon, Apr 15, 2019 at 06:30:10PM +0200, Clément Péron wrote:
> > > I didn't change the driver and the SPDIF is working what's irrelevant
> > > in declaring the driver as compatible ?
> >
> > It's working by accident.
> >
> > This won't work for example:
> > https://elixir.bootlin.com/linux/latest/source/sound/soc/sunxi/sun4i-spdif.c#L181
> >
> > > If some feature are missing they can be introduce later as there is a
> > > special binding for H6 no ?
> >
> > Again, that's irrelevant. The only question you need to ask yourself
> > is whether, if you take an imaginary source code that would support
> > all the H3 SPDIF features and run it on an H6, it works just as well.
> >
> > The examples above that it's not the case, so you can't claim that the
> > device is compatible.
>
> Is it ok if I fix the TX Fifo flush bit and remove the compatible with H3 ?
> (Looking at the driver this is the only bit used that is incompatible with H6)

Sure

> Or should I send a new driver with the correct mapping ?

No, we definitely don't want to duplicate a driver just for a single
bit offset that changes.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible

[Clément Péron]

Hi,

On Tue, 16 Apr 2019 at 09:34, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Apr 15, 2019 at 06:30:10PM +0200, Clément Péron wrote:
> > > > I didn't change the driver and the SPDIF is working what's irrelevant
> > > > in declaring the driver as compatible ?
> > >
> > > It's working by accident.
> > >
> > > This won't work for example:
> > > https://elixir.bootlin.com/linux/latest/source/sound/soc/sunxi/sun4i-spdif.c#L181
> > >
> > > > If some feature are missing they can be introduce later as there is a
> > > > special binding for H6 no ?
> > >
> > > Again, that's irrelevant. The only question you need to ask yourself
> > > is whether, if you take an imaginary source code that would support
> > > all the H3 SPDIF features and run it on an H6, it works just as well.
> > >
> > > The examples above that it's not the case, so you can't claim that the
> > > device is compatible.
> >
> > Is it ok if I fix the TX Fifo flush bit and remove the compatible with H3 ?
> > (Looking at the driver this is the only bit used that is incompatible with H6)
>
> Sure
>
> > Or should I send a new driver with the correct mapping ?
>
> No, we definitely don't want to duplicate a driver just for a single
> bit offset that changes.

Thanks for the advice, I will try to submit a new version with this fix.
Clement

>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com