mbox series

[v3,0/7] Allwinner H6 SPDIF support

Message ID 20190525162323.20216-1-peron.clem@gmail.com
Headers show
Series Allwinner H6 SPDIF support | expand

Message

Clément Péron May 25, 2019, 4:23 p.m. UTC 
*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 and not compatible.

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

Actually all these features are unused and only a bit for flushing the TX
fifo is required.

Also this series requires the DMA working on H6, a first version has been
submitted by Jernej Škrabec[1] but has not been accepted yet.

[1] https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=89011

Changes since v2:
 - Split quirks and H6 support patch
 - Add specific section for quirks comment

Changes since v1:
 - Remove H3 compatible
 - Add TX fifo bit flush quirks
 - Add H6 bindings in SPDIF driver

Clément Péron (7):
  dt-bindings: sound: sun4i-spdif: Add Allwinner H6 compatible
  ASoC: sun4i-spdif: Move quirks to the top
  ASoC: sun4i-spdif: Add TX fifo bit flush quirks
  ASoC: sun4i-spdif: Add support for H6 SoC
  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  | 38 ++++++++++++++
 arch/arm64/configs/defconfig                  |  1 +
 sound/soc/sunxi/sun4i-spdif.c                 | 49 ++++++++++++++++---
 5 files changed, 88 insertions(+), 7 deletions(-)

Comments

Maxime Ripard May 26, 2019, 6:22 p.m. UTC | #1 
On Sat, May 25, 2019 at 06:23:18PM +0200, Clément Péron wrote:
> The quirks are actually defines in the middle of the file with
> short explanation.
>
> Move this at the top and add a section to have coherency with
> sun4i-i2s.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard May 26, 2019, 6:24 p.m. UTC | #2 
On Sat, May 25, 2019 at 06:23:19PM +0200, Clément Péron wrote:
> Allwinner H6 has a different bit to flush the TX FIFO.
>
> Add a quirks to prepare introduction of H6 SoC.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-spdif.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
> index b6c66a62e915..8317bbee0712 100644
> --- a/sound/soc/sunxi/sun4i-spdif.c
> +++ b/sound/soc/sunxi/sun4i-spdif.c
> @@ -166,10 +166,12 @@
>   *
>   * @reg_dac_tx_data: TX FIFO offset for DMA config.
>   * @has_reset: SoC needs reset deasserted.
> + * @reg_fctl_ftx: TX FIFO flush bitmask.

It's a bit weird to use the same prefix for a register offset
(reg_dac_tx_data) and a value (reg_fctl_ftx).

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard May 26, 2019, 6:27 p.m. UTC | #3 
On Sat, May 25, 2019 at 06:23:21PM +0200, Clément Péron wrote:
> The Allwinner H6 has a SPDIF controller called OWA (One Wire Audio).
>
> Only one pinmuxing is available so set it as default.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 38 ++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index f4ea596c82ce..307d3c896ff2 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 {

This generates a warning in DTC, underscores are not valid characters
for a node name.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Clément Péron May 26, 2019, 7 p.m. UTC | #4 
Hi Maxime,

On Sun, 26 May 2019 at 20:24, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sat, May 25, 2019 at 06:23:19PM +0200, Clément Péron wrote:
> > Allwinner H6 has a different bit to flush the TX FIFO.
> >
> > Add a quirks to prepare introduction of H6 SoC.
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  sound/soc/sunxi/sun4i-spdif.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
> > index b6c66a62e915..8317bbee0712 100644
> > --- a/sound/soc/sunxi/sun4i-spdif.c
> > +++ b/sound/soc/sunxi/sun4i-spdif.c
> > @@ -166,10 +166,12 @@
> >   *
> >   * @reg_dac_tx_data: TX FIFO offset for DMA config.
> >   * @has_reset: SoC needs reset deasserted.
> > + * @reg_fctl_ftx: TX FIFO flush bitmask.
>
> It's a bit weird to use the same prefix for a register offset
> (reg_dac_tx_data) and a value (reg_fctl_ftx).

I just look at sun4i-codec and they use a regmap, But I think it's a
bit overkill no?

What do you think about val_fctl_ftx ?

Thanks for your review,
Clément

>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard May 27, 2019, 12:28 p.m. UTC | #5 
On Sun, May 26, 2019 at 09:00:30PM +0200, Clément Péron wrote:
> Hi Maxime,
>
> On Sun, 26 May 2019 at 20:24, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Sat, May 25, 2019 at 06:23:19PM +0200, Clément Péron wrote:
> > > Allwinner H6 has a different bit to flush the TX FIFO.
> > >
> > > Add a quirks to prepare introduction of H6 SoC.
> > >
> > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > ---
> > >  sound/soc/sunxi/sun4i-spdif.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
> > > index b6c66a62e915..8317bbee0712 100644
> > > --- a/sound/soc/sunxi/sun4i-spdif.c
> > > +++ b/sound/soc/sunxi/sun4i-spdif.c
> > > @@ -166,10 +166,12 @@
> > >   *
> > >   * @reg_dac_tx_data: TX FIFO offset for DMA config.
> > >   * @has_reset: SoC needs reset deasserted.
> > > + * @reg_fctl_ftx: TX FIFO flush bitmask.
> >
> > It's a bit weird to use the same prefix for a register offset
> > (reg_dac_tx_data) and a value (reg_fctl_ftx).
>
> I just look at sun4i-codec and they use a regmap, But I think it's a
> bit overkill no?

For a single value, yeah

> What do you think about val_fctl_ftx ?

Looks good, thanks!
Maxime

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