[PATCH v8 00/14] Add Allwinner H3/H5/H6/A64 HDMI audio

[Clément Péron]

Hi,

This is the same as v7 but rebased on next-20201026 and added a comment
about slots and slot_width.

A proper sound card will be introduced later.

This was tested on H6 only.

Regards,
Clement

Change since v7:
- rebase on next-20201026
- comment about slots and slot_width

Change since v6:
- move set_channel_cfg() in first position
- convert return value to decimal

Change since v5:
- Drop HDMI simple soundcard
- Collect Chen-Yu Tsai tags
- Configure channels from 9 to 15.
- Remove DMA RX for H3/H5
- Fix Documentation for H3/H5

Change since v4:
- add more comment on get_wss() and set_channel_cfg() patch
- merge soundcard and DAI HDMI patches

Change since v3:
- add Samuel Holland patch to reconfigure FIFO_TX_REG when suspend is enabled
- readd inversion to H6 LRCK sun50i_h6_i2s_set_soc_fmt()
- Fix get_wss() for sun4i
- Add a commit to fix checkpatch warning

Change since v2:
- rebase on next-20200918
- drop revert LRCK polarity patch
- readd simple-audio-card,frame-inversion in dts
- Add patch for changing set_chan_cfg params

Change since v1:
- rebase on next-20200828
- add revert LRCK polarity
- remove all simple-audio-card,frame-inversion in dts
- add Ondrej patches for Orange Pi board
- Add arm64 defconfig patch

Clément Péron (6):
ASoC: sun4i-i2s: Change set_chan_cfg() params
ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
ASoC: sun4i-i2s: Fix sun8i volatile regs
ASoC: sun4i-i2s: fix coding-style for callback definition
arm64: defconfig: Enable Allwinner i2s driver
dt-bindings: sound: sun4i-i2s: Document H3 with missing RX channel
possibility

Jernej Skrabec (3):
ASoC: sun4i-i2s: Add support for H6 I2S
dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
arm64: dts: allwinner: h6: Add I2S1 node

Marcus Cooper (4):
ASoC: sun4i-i2s: Set sign extend sample
ASoc: sun4i-i2s: Add 20 and 24 bit support
arm64: dts: allwinner: a64: Add I2S2 node
arm: dts: sunxi: h3/h5: Add I2S2 node

Samuel Holland (1):
ASoC: sun4i-i2s: Fix setting of FIFO modes

.../sound/allwinner,sun4i-a10-i2s.yaml | 6 +-
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 13 +
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 +
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 13 +
arch/arm64/configs/defconfig | 1 +
sound/soc/sunxi/sun4i-i2s.c | 385 +++++++++++++++---
6 files changed, 376 insertions(+), 56 deletions(-)

--
2.25.1

[Clément Péron]

As slots and slot_width can be set manually using set_tdm().
These values are then kept in sun4i_i2s struct.
So we need to check if these values are set or not.

This is not done actually and will trigger a bug.
For example, if we set to the simple soundcard in the device-tree
dai-tdm-slot-width = <32> and then start a stream using S16_LE,
currently we would calculate BCLK for 32-bit slots, but program
lrck_period for 16-bit slots, making the sample rate double what we
expected.

To fix this, we need to check if these values are set or not but as
this logic is already done by the caller. Avoid duplicating this
logic and just pass the required values as params to set_chan_cfg().

Suggested-by: Samuel Holland <sam...@sholland.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---
sound/soc/sunxi/sun4i-i2s.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index f23ff29e7c1d..6c10f810b114 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -162,8 +162,9 @@ struct sun4i_i2s_quirks {
unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
s8 (*get_sr)(const struct sun4i_i2s *, int);
s8 (*get_wss)(const struct sun4i_i2s *, int);
- int (*set_chan_cfg)(const struct sun4i_i2s *,
- const struct snd_pcm_hw_params *);
+ int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width);
int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
};

@@ -399,10 +400,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
}

static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
-
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210);
@@ -419,15 +419,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
}

static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
- unsigned int slots = channels;
unsigned int lrck_period;

- if (i2s->slots)
- slots = i2s->slots;
-
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
@@ -452,11 +448,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
case SND_SOC_DAIFMT_DSP_B:
case SND_SOC_DAIFMT_LEFT_J:
case SND_SOC_DAIFMT_RIGHT_J:
- lrck_period = params_physical_width(params) * slots;
+ lrck_period = slot_width * slots;
break;

case SND_SOC_DAIFMT_I2S:
- lrck_period = params_physical_width(params);
+ lrck_period = slot_width;
break;

default:
@@ -480,9 +476,16 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int word_size = params_width(params);
- unsigned int slot_width = params_physical_width(params);
unsigned int channels = params_channels(params);
+
+ /*
+ * Here and in set_chan_cfg(), "slots" means channels per frame +
+ * padding slots, regardless of format. "slot_width" means bits
+ * per sample + padding bits, regardless of format.
+ */
unsigned int slots = channels;
+ unsigned int slot_width = params_physical_width(params);
+
int ret, sr, wss;
u32 width;

@@ -492,7 +495,7 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
if (i2s->slot_width)
slot_width = i2s->slot_width;

- ret = i2s->variant->set_chan_cfg(i2s, params);
+ ret = i2s->variant->set_chan_cfg(i2s, channels, slots, slot_width);
if (ret < 0) {
dev_err(dai->dev, "Invalid channel configuration\n");
return ret;
--
2.25.1

[Clément Péron]

We are actually using a complex formula to just return a bunch of
simple values. Also this formula is wrong for sun4i when calling
get_wss() the function return 4 instead of 3.

Replace this with a simpler switch case.

Also drop the i2s params which is unused and return a simple int as
returning an error code could be out of range for an s8 and there is
no optim to return a s8 here.

Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")
Reviewed-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 69 +++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 5bec7fbd0b30..70a2ec99f444 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -175,8 +175,8 @@ struct sun4i_i2s_quirks {
unsigned int num_mclk_dividers;

unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);

- s8 (*get_sr)(const struct sun4i_i2s *, int);
- s8 (*get_wss)(const struct sun4i_i2s *, int);
+ int (*get_sr)(unsigned int width);
+ int (*get_wss)(unsigned int width);

int (*set_chan_cfg)(const struct sun4i_i2s *i2s,

unsigned int channels, unsigned int slots,

unsigned int slot_width);
@@ -381,37 +381,56 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
return 0;
}

-static s8 sun4i_i2s_get_sr(const struct sun4i_i2s *i2s, int width)
+static int sun4i_i2s_get_sr(unsigned int width)
{
- if (width < 16 || width > 24)
- return -EINVAL;
-
- if (width % 4)
- return -EINVAL;
+ switch (width) {
+ case 16:
+ return 0;
+ case 20:
+ return 1;
+ case 24:
+ return 2;
+ }

- return (width - 16) / 4;
+ return -EINVAL;
}

-static s8 sun4i_i2s_get_wss(const struct sun4i_i2s *i2s, int width)
+static int sun4i_i2s_get_wss(unsigned int width)
{
- if (width < 16 || width > 32)
- return -EINVAL;
-
- if (width % 4)
- return -EINVAL;
+ switch (width) {
+ case 16:
+ return 0;
+ case 20:
+ return 1;
+ case 24:
+ return 2;
+ case 32:
+ return 3;
+ }

- return (width - 16) / 4;
+ return -EINVAL;
}

-static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
+static int sun8i_i2s_get_sr_wss(unsigned int width)
{
- if (width % 4)
- return -EINVAL;
-
- if (width < 8 || width > 32)
- return -EINVAL;
+ switch (width) {
+ case 8:
+ return 1;
+ case 12:
+ return 2;
+ case 16:
+ return 3;
+ case 20:
+ return 4;
+ case 24:
+ return 5;
+ case 28:
+ return 6;
+ case 32:
+ return 7;
+ }

- return (width - 8) / 4 + 1;
+ return -EINVAL;

}

static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

@@ -581,11 +600,11 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
}
i2s->playback_dma_data.addr_width = width;

- sr = i2s->variant->get_sr(i2s, word_size);
+ sr = i2s->variant->get_sr(word_size);
if (sr < 0)
return -EINVAL;

- wss = i2s->variant->get_wss(i2s, slot_width);
+ wss = i2s->variant->get_wss(slot_width);
if (wss < 0)
return -EINVAL;

--
2.25.1

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

H6 I2S is very similar to that in H3, except it supports up to 16
channels.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 222 ++++++++++++++++++++++++++++++++++++
1 file changed, 222 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 6c10f810b114..5bec7fbd0b30 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -124,6 +124,21 @@
#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58

+/* Defines required for sun50i-h6 support */
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK GENMASK(21, 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset) ((offset) << 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK GENMASK(19, 16)
+#define SUN50I_H6_I2S_TX_CHAN_SEL(chan) ((chan - 1) << 16)
+#define SUN50I_H6_I2S_TX_CHAN_EN_MASK GENMASK(15, 0)
+#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1))
+
+#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG 0x44
+#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG 0x48
+
+#define SUN50I_H6_I2S_RX_CHAN_SEL_REG 0x64
+#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG 0x68
+#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG 0x6C
+
struct sun4i_i2s;

/**
@@ -470,6 +485,60 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
return 0;
}

+static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
+{
+ unsigned int lrck_period;
+
+ /* Map the channels for playback and capture */
+ regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0xFEDCBA98);
+ regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
+ regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0xFEDCBA98);
+ regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210);
+
+ /* Configure the channels */
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
+ SUN50I_H6_I2S_TX_CHAN_SEL(channels));
+ regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
+ SUN50I_H6_I2S_TX_CHAN_SEL(channels));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
+ SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
+ SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels));
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
+ SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK,
+ SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels));
+
+ switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_DSP_A:
+ case SND_SOC_DAIFMT_DSP_B:
+ case SND_SOC_DAIFMT_LEFT_J:
+ case SND_SOC_DAIFMT_RIGHT_J:

+ lrck_period = slot_width * slots;

+ break;
+
+ case SND_SOC_DAIFMT_I2S:
+ lrck_period = slot_width;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
+ SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_EN_MASK,
+ SUN50I_H6_I2S_TX_CHAN_EN(channels));
+
+ return 0;
+}
+

static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -702,6 +771,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
return 0;
}

+static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
+ unsigned int fmt)
+{
+ u32 mode, val;
+ u8 offset;
+
+ /*
+ * DAI clock polarity
+ *
+ * The setup for LRCK contradicts the datasheet, but under a
+ * scope it's clear that the LRCK polarity is reversed
+ * compared to the expected polarity on the bus.
+ */
+ switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_IB_IF:
+ /* Invert both clocks */
+ val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ /* Invert bit clock */
+ val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+ SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ /* Invert frame clock */
+ val = 0;
+ break;
+ case SND_SOC_DAIFMT_NB_NF:
+ val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK |
+ SUN8I_I2S_FMT0_BCLK_POLARITY_MASK,
+ val);
+
+ /* DAI Mode */
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_DSP_A:
+ mode = SUN8I_I2S_CTRL_MODE_PCM;
+ offset = 1;
+ break;
+
+ case SND_SOC_DAIFMT_DSP_B:
+ mode = SUN8I_I2S_CTRL_MODE_PCM;
+ offset = 0;
+ break;
+
+ case SND_SOC_DAIFMT_I2S:
+ mode = SUN8I_I2S_CTRL_MODE_LEFT;
+ offset = 1;
+ break;
+
+ case SND_SOC_DAIFMT_LEFT_J:
+ mode = SUN8I_I2S_CTRL_MODE_LEFT;
+ offset = 0;
+ break;
+
+ case SND_SOC_DAIFMT_RIGHT_J:
+ mode = SUN8I_I2S_CTRL_MODE_RIGHT;
+ offset = 0;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+ SUN8I_I2S_CTRL_MODE_MASK, mode);
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
+ SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset));
+ regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
+ SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset));
+
+ /* DAI clock master masks */
+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ case SND_SOC_DAIFMT_CBS_CFS:
+ /* BCLK and LRCLK master */
+ val = SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT;
+ break;
+
+ case SND_SOC_DAIFMT_CBM_CFM:
+ /* BCLK and LRCLK slave */
+ val = 0;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+ SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
+ val);
+
+ return 0;
+}
+
static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)

{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);

@@ -982,6 +1153,22 @@ static const struct reg_default sun8i_i2s_reg_defaults[] = {
{ SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
};

+static const struct reg_default sun50i_h6_i2s_reg_defaults[] = {
+ { SUN4I_I2S_CTRL_REG, 0x00060000 },
+ { SUN4I_I2S_FMT0_REG, 0x00000033 },
+ { SUN4I_I2S_FMT1_REG, 0x00000030 },
+ { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
+ { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
+ { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
+ { SUN8I_I2S_CHAN_CFG_REG, 0x00000000 },
+ { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
+ { SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0x00000000 },
+ { SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x00000000 },
+ { SUN50I_H6_I2S_RX_CHAN_SEL_REG, 0x00000000 },
+ { SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0x00000000 },
+ { SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x00000000 },
+};
+
static const struct regmap_config sun4i_i2s_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
@@ -1009,6 +1196,19 @@ static const struct regmap_config sun8i_i2s_regmap_config = {
.volatile_reg = sun8i_i2s_volatile_reg,
};

+static const struct regmap_config sun50i_h6_i2s_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN50I_H6_I2S_RX_CHAN_MAP1_REG,
+ .cache_type = REGCACHE_FLAT,
+ .reg_defaults = sun50i_h6_i2s_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(sun50i_h6_i2s_reg_defaults),
+ .writeable_reg = sun4i_i2s_wr_reg,
+ .readable_reg = sun8i_i2s_rd_reg,
+ .volatile_reg = sun8i_i2s_volatile_reg,
+};
+
static int sun4i_i2s_runtime_resume(struct device *dev)
{
struct sun4i_i2s *i2s = dev_get_drvdata(dev);
@@ -1167,6 +1367,24 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {
.set_fmt = sun4i_i2s_set_soc_fmt,
};

+static const struct sun4i_i2s_quirks sun50i_h6_i2s_quirks = {
+ .has_reset = true,
+ .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
+ .sun4i_i2s_regmap = &sun50i_h6_i2s_regmap_config,
+ .field_clkdiv_mclk_en = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
+ .field_fmt_wss = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
+ .field_fmt_sr = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
+ .bclk_dividers = sun8i_i2s_clk_div,
+ .num_bclk_dividers = ARRAY_SIZE(sun8i_i2s_clk_div),
+ .mclk_dividers = sun8i_i2s_clk_div,
+ .num_mclk_dividers = ARRAY_SIZE(sun8i_i2s_clk_div),
+ .get_bclk_parent_rate = sun8i_i2s_get_bclk_parent_rate,
+ .get_sr = sun8i_i2s_get_sr_wss,
+ .get_wss = sun8i_i2s_get_sr_wss,
+ .set_chan_cfg = sun50i_h6_i2s_set_chan_cfg,
+ .set_fmt = sun50i_h6_i2s_set_soc_fmt,
+};
+
static int sun4i_i2s_init_regmap_fields(struct device *dev,
struct sun4i_i2s *i2s)
{
@@ -1336,6 +1554,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
.compatible = "allwinner,sun50i-a64-codec-i2s",
.data = &sun50i_a64_codec_i2s_quirks,
},
+ {
+ .compatible = "allwinner,sun50i-h6-i2s",
+ .data = &sun50i_h6_i2s_quirks,
+ },
{}
};
MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

On the newer SoCs such as the H3 and A64 this is set by default
to transfer a 0 after each sample in each slot. However the A10
and A20 SoCs that this driver was developed on had a default
setting where it padded the audio gain with zeros.

This isn't a problem while we have only support for 16bit audio
but with larger sample resolution rates in the pipeline then SEXT
bits should be cleared so that they also pad at the LSB. Without
this the audio gets distorted.

Set sign extend sample for all the sunxi generations even if they
are not affected. This will keep consistency and avoid relying on
default.

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 70a2ec99f444..beaca56a44ae 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -48,6 +48,9 @@
#define SUN4I_I2S_FMT0_FMT_I2S (0 << 0)

#define SUN4I_I2S_FMT1_REG 0x08
+#define SUN4I_I2S_FMT1_REG_SEXT_MASK BIT(8)
+#define SUN4I_I2S_FMT1_REG_SEXT(sext) ((sext) << 8)
+
#define SUN4I_I2S_FIFO_TX_REG 0x0c
#define SUN4I_I2S_FIFO_RX_REG 0x10

@@ -105,6 +108,9 @@
#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7)
#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 7)

+#define SUN8I_I2S_FMT1_REG_SEXT_MASK GENMASK(5, 4)
+#define SUN8I_I2S_FMT1_REG_SEXT(sext) ((sext) << 4)
+
#define SUN8I_I2S_INT_STA_REG 0x0c
#define SUN8I_I2S_FIFO_TX_REG 0x20

@@ -685,6 +691,7 @@ static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
}
regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
SUN4I_I2S_CTRL_MODE_MASK, val);
+
return 0;
}

@@ -787,6 +794,11 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
val);

+ /* Set sign extension to pad out LSB with 0 */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+ SUN8I_I2S_FMT1_REG_SEXT_MASK,
+ SUN8I_I2S_FMT1_REG_SEXT(0));
+
return 0;
}

@@ -889,6 +901,11 @@ static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
val);

+ /* Set sign extension to pad out LSB with 0 */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+ SUN8I_I2S_FMT1_REG_SEXT_MASK,
+ SUN8I_I2S_FMT1_REG_SEXT(0));
+
return 0;
}

--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Extend the functionality of the driver to include support of 20 and
24 bits per sample.

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Maxime Ripard <mri...@kernel.org>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index beaca56a44ae..eee1e64cd8f4 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -599,6 +599,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;
break;
+ case 32:
+ width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ break;
default:
dev_err(dai->dev, "Unsupported physical sample width: %d\n",
params_physical_width(params));
@@ -1080,6 +1083,10 @@ static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
return 0;
}

+#define SUN4I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S20_LE | \
+ SNDRV_PCM_FMTBIT_S24_LE)
+
static struct snd_soc_dai_driver sun4i_i2s_dai = {
.probe = sun4i_i2s_dai_probe,
.capture = {
@@ -1087,14 +1094,14 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = {
.channels_min = 1,
.channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_192000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SUN4I_FORMATS,
},
.playback = {
.stream_name = "Playback",
.channels_min = 1,
.channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_192000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SUN4I_FORMATS,
},
.ops = &sun4i_i2s_dai_ops,
.symmetric_rates = 1,
--
2.25.1

[Clément Péron]

The FIFO TX reg is volatile and sun8i i2s register
mapping is different from sun4i.

Even if in this case it's doesn't create an issue,
Avoid setting some regs that are undefined in sun8i.

Acked-by: Maxime Ripard <mri...@kernel.org>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index eee1e64cd8f4..dee8688f0d37 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1161,12 +1161,19 @@ static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)

static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
{
- if (reg == SUN8I_I2S_INT_STA_REG)
+ switch (reg) {
+ case SUN4I_I2S_FIFO_CTRL_REG:
+ case SUN4I_I2S_FIFO_RX_REG:
+ case SUN4I_I2S_FIFO_STA_REG:
+ case SUN4I_I2S_RX_CNT_REG:
+ case SUN4I_I2S_TX_CNT_REG:
+ case SUN8I_I2S_FIFO_TX_REG:
+ case SUN8I_I2S_INT_STA_REG:
return true;
- if (reg == SUN8I_I2S_FIFO_TX_REG)
- return false;

- return sun4i_i2s_volatile_reg(dev, reg);
+ default:
+ return false;
+ }
}

static const struct reg_default sun4i_i2s_reg_defaults[] = {
--
2.25.1

[Clément Péron]

From: Samuel Holland <sam...@sholland.org>

Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
regmap is cache-only are ignored. To work around this, move the
configuration to a callback that runs while the ASoC core has a
runtime PM reference to the device.

Signed-off-by: Samuel Holland <sam...@sholland.org>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index dee8688f0d37..703327dc8606 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -595,6 +595,13 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
return ret;
}

+ /* Set significant bits in our FIFOs */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+
switch (params_physical_width(params)) {

case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;

@@ -923,13 +930,6 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
return ret;
}

- /* Set significant bits in our FIFOs */
- regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
- SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
- SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
- SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
- SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
-
i2s->format = fmt;

return 0;
--
2.25.1

[Clément Péron]

Checkpatch script produces warning:
WARNING: function definition argument 'const struct sun4i_i2s *'
should also have an identifier name.

Let's fix this by adding identifier name to get_bclk_parent_rate()
and set_fmt() callback definition.

Acked-by: Maxime Ripard <mri...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 703327dc8606..08e9e71f455f 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -180,13 +180,13 @@ struct sun4i_i2s_quirks {
const struct sun4i_i2s_clk_div *mclk_dividers;
unsigned int num_mclk_dividers;

- unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
+ unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *i2s);
int (*get_sr)(unsigned int width);

int (*get_wss)(unsigned int width);
int (*set_chan_cfg)(const struct sun4i_i2s *i2s,

unsigned int channels, unsigned int slots,

unsigned int slot_width);
- int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
+ int (*set_fmt)(const struct sun4i_i2s *i2s, unsigned int fmt);
};

struct sun4i_i2s {
--
2.25.1

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

H6 I2S is very similar to H3, except that it supports up to 16 channels
and thus few registers have fields on different position.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Maxime Ripard <mri...@kernel.org>

Acked-by: Rob Herring <ro...@kernel.org>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

.../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 112ae00d63c1..606ad2d884a8 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
@@ -24,6 +24,7 @@ properties:
- items:
- const: allwinner,sun50i-a64-i2s
- const: allwinner,sun8i-h3-i2s
+ - const: allwinner,sun50i-h6-i2s

reg:
maxItems: 1
@@ -59,6 +60,7 @@ allOf:
- allwinner,sun8i-a83t-i2s
- allwinner,sun8i-h3-i2s
- allwinner,sun50i-a64-codec-i2s
+ - allwinner,sun50i-h6-i2s

then:
required:
--
2.25.1

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

Add Allwinner H6 I2S1 node connected to HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>

Acked-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 28c77d6872f6..d915aeb13297 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -609,6 +609,19 @@ mdio: mdio {
};
};

+ i2s1: i2s@5091000 {
+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun50i-h6-i2s";
+ reg = <0x05091000 0x1000>;
+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S1>, <&ccu CLK_I2S1>;
+ clock-names = "apb", "mod";
+ dmas = <&dma 4>, <&dma 4>;
+ resets = <&ccu RST_BUS_I2S1>;
+ dma-names = "rx", "tx";
+ status = "disabled";
+ };
+
spdif: spdif@5093000 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-h6-spdif";
--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add the I2S2 node connected to the HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index dc238814013c..51cc30e84e26 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -846,6 +846,20 @@ i2s1: i2s@1c22400 {
status = "disabled";
};

+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun50i-a64-i2s",
+ "allwinner,sun8i-h3-i2s";
+ reg = <0x01c22800 0x400>;
+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;

+ clock-names = "apb", "mod";

+ resets = <&ccu RST_BUS_I2S2>;

+ dma-names = "rx", "tx";

+ dmas = <&dma 27>, <&dma 27>;

+ status = "disabled";
+ };
+

dai: dai@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-codec-i2s";
--
2.25.1

[Clément Péron]

Enable Allwinner I2S driver for arm64 defconfig.

Signed-off-by: Clément Péron <peron...@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 17a2df6a263e..3f89f427a355 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -706,6 +706,7 @@ CONFIG_SND_SOC_ROCKCHIP_RT5645=m
CONFIG_SND_SOC_RK3399_GRU_SOUND=m
CONFIG_SND_SOC_SAMSUNG=y
CONFIG_SND_SOC_RCAR=m
+CONFIG_SND_SUN4I_I2S=m
CONFIG_SND_SUN4I_SPDIF=m
CONFIG_SND_SOC_TEGRA=m
CONFIG_SND_SOC_TEGRA210_AHUB=m
--
2.25.1

[Clément Péron]

Like A83T the Allwinner H3 doesn't have the DMA reception available for
some audio interfaces.

As it's already documented for A83T convert this to an enum and add the H3
interface.

Acked-by: Rob Herring <ro...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

.../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 606ad2d884a8..a16e37b01e1d 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
@@ -70,7 +70,9 @@ allOf:
properties:
compatible:
contains:
- const: allwinner,sun8i-a83t-i2s
+ enum:
+ - allwinner,sun8i-a83t-i2s
+ - allwinner,sun8i-h3-i2s

then:
properties:
--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add H3/H5 I2S2 node connected to the HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm/boot/dts/sunxi-h3-h5.dtsi | 13 +++++++++++++

1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 22d533d18992..9be13378d4df 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -662,6 +662,19 @@ i2s1: i2s@1c22400 {

status = "disabled";
};

+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;

+ compatible = "allwinner,sun8i-h3-i2s";

+ reg = <0x01c22800 0x400>;
+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;
+ clock-names = "apb", "mod";

+ dmas = <&dma 27>;

+ resets = <&ccu RST_BUS_I2S2>;

+ dma-names = "tx";

+ status = "disabled";
+ };
+

codec: codec@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun8i-h3-codec";
--
2.25.1

[Maxime Ripard]

what I meant was to put that comment next to the function pointer in the
structure sun4i_i2s_quirks, it would be fairly easy to miss here.

With that fixed,
Acked-by: Maxime Ripard <mri...@kernel.org>

Maxime

[Maxime Ripard]

On Mon, Oct 26, 2020 at 07:52:27PM +0100, Clément Péron wrote:
> From: Jernej Skrabec <jernej....@siol.net>
>
> H6 I2S is very similar to that in H3, except it supports up to 16
> channels.
>
> Signed-off-by: Jernej Skrabec <jernej....@siol.net>
> Signed-off-by: Marcus Cooper <codek...@gmail.com>
> Reviewed-by: Chen-Yu Tsai <we...@csie.org>
> Signed-off-by: Clément Péron <peron...@gmail.com>

[Maxime Ripard]

On Mon, Oct 26, 2020 at 07:52:28PM +0100, Clément Péron wrote:
> We are actually using a complex formula to just return a bunch of
> simple values. Also this formula is wrong for sun4i when calling
> get_wss() the function return 4 instead of 3.
>
> Replace this with a simpler switch case.
>
> Also drop the i2s params which is unused and return a simple int as
> returning an error code could be out of range for an s8 and there is
> no optim to return a s8 here.
>
> Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")
> Reviewed-by: Chen-Yu Tsai <we...@csie.org>
> Signed-off-by: Clément Péron <peron...@gmail.com>

[Maxime Ripard]

On Mon, Oct 26, 2020 at 07:52:29PM +0100, Clément Péron wrote:
> From: Marcus Cooper <codek...@gmail.com>
>
> On the newer SoCs such as the H3 and A64 this is set by default
> to transfer a 0 after each sample in each slot. However the A10
> and A20 SoCs that this driver was developed on had a default
> setting where it padded the audio gain with zeros.
>
> This isn't a problem while we have only support for 16bit audio
> but with larger sample resolution rates in the pipeline then SEXT
> bits should be cleared so that they also pad at the LSB. Without
> this the audio gets distorted.
>
> Set sign extend sample for all the sunxi generations even if they
> are not affected. This will keep consistency and avoid relying on
> default.
>
> Signed-off-by: Marcus Cooper <codek...@gmail.com>
> Reviewed-by: Chen-Yu Tsai <we...@csie.org>
> Signed-off-by: Clément Péron <peron...@gmail.com>

[Maxime Ripard]

On Mon, Oct 26, 2020 at 07:52:32PM +0100, Clément Péron wrote:
> From: Samuel Holland <sam...@sholland.org>
>
> Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
> regmap is cache-only are ignored. To work around this, move the
> configuration to a callback that runs while the ASoC core has a
> runtime PM reference to the device.
>
> Signed-off-by: Samuel Holland <sam...@sholland.org>
> Reviewed-by: Chen-Yu Tsai <we...@csie.org>
> Signed-off-by: Clément Péron <peron...@gmail.com>

[Clément Péron]

Hi,

This series add H6 I2S support and the I2S node missing to support
HDMI audio in different Allwinner SoC.

As we first use some TDM property to make the I2S working we the
simple soundcard. We have now drop this simple sound card and a
proper dedicated soundcard will be introduce later.

This make the title of this series wrong now :/.

Regards,
Clement

Change since v8:
- move the comment near the function prototype
- collect Maxime Ripard tags

sound/soc/sunxi/sun4i-i2s.c | 384 +++++++++++++++---
6 files changed, 376 insertions(+), 55 deletions(-)

--
2.25.1

[Clément Péron]

As slots and slot_width can be set manually using set_tdm().
These values are then kept in sun4i_i2s struct.
So we need to check if these values are set or not.

This is not done actually and will trigger a bug.
For example, if we set to the simple soundcard in the device-tree
dai-tdm-slot-width = <32> and then start a stream using S16_LE,
currently we would calculate BCLK for 32-bit slots, but program
lrck_period for 16-bit slots, making the sample rate double what we
expected.

To fix this, we need to check if these values are set or not but as
this logic is already done by the caller. Avoid duplicating this
logic and just pass the required values as params to set_chan_cfg().

Suggested-by: Samuel Holland <sam...@sholland.org>

Acked-by: Maxime Ripard <mri...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index f23ff29e7c1d..7c1f57eb2462 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -162,8 +162,15 @@ struct sun4i_i2s_quirks {

unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
s8 (*get_sr)(const struct sun4i_i2s *, int);
s8 (*get_wss)(const struct sun4i_i2s *, int);
- int (*set_chan_cfg)(const struct sun4i_i2s *,
- const struct snd_pcm_hw_params *);
+

+ /*
+ * In the set_chan_cfg() function pointer:
+ * @slots: channels per frame + padding slots, regardless of format
+ * @slot_width: bits per sample + padding bits, regardless of format
+ */

+ int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width);
int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
};

@@ -399,10 +406,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)

}

static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
-
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210);

@@ -419,15 +425,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

}

static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
- unsigned int slots = channels;
unsigned int lrck_period;

- if (i2s->slots)
- slots = i2s->slots;
-
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);

@@ -452,11 +454,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

case SND_SOC_DAIFMT_DSP_B:
case SND_SOC_DAIFMT_LEFT_J:
case SND_SOC_DAIFMT_RIGHT_J:
- lrck_period = params_physical_width(params) * slots;
+ lrck_period = slot_width * slots;
break;

case SND_SOC_DAIFMT_I2S:
- lrck_period = params_physical_width(params);
+ lrck_period = slot_width;
break;

default:

@@ -482,7 +484,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

unsigned int word_size = params_width(params);

unsigned int slot_width = params_physical_width(params);
unsigned int channels = params_channels(params);
+

unsigned int slots = channels;
+

int ret, sr, wss;
u32 width;

@@ -492,7 +496,7 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

H6 I2S is very similar to that in H3, except it supports up to 16
channels.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>

Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 222 ++++++++++++++++++++++++++++++++++++
1 file changed, 222 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 7c1f57eb2462..9aa837d4fe99 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -124,6 +124,21 @@
#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58

+/* Defines required for sun50i-h6 support */
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK GENMASK(21, 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset) ((offset) << 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK GENMASK(19, 16)
+#define SUN50I_H6_I2S_TX_CHAN_SEL(chan) ((chan - 1) << 16)
+#define SUN50I_H6_I2S_TX_CHAN_EN_MASK GENMASK(15, 0)
+#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1))
+
+#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG 0x44
+#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG 0x48
+
+#define SUN50I_H6_I2S_RX_CHAN_SEL_REG 0x64
+#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG 0x68
+#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG 0x6C
+
struct sun4i_i2s;

/**

@@ -476,6 +491,60 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
return 0;
}

+static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)

+ lrck_period = slot_width * slots;

+ break;
+
+ case SND_SOC_DAIFMT_I2S:
+ lrck_period = slot_width;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
+ SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_EN_MASK,
+ SUN50I_H6_I2S_TX_CHAN_EN(channels));
+
+ return 0;
+}
+

static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)

@@ -703,6 +772,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,

return 0;
}

+static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
+ unsigned int fmt)
+{
+ u32 mode, val;
+ u8 offset;

+
+ /*

static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)

{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);

@@ -983,6 +1154,22 @@ static const struct reg_default sun8i_i2s_reg_defaults[] = {

{ SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
};

+static const struct reg_default sun50i_h6_i2s_reg_defaults[] = {
+ { SUN4I_I2S_CTRL_REG, 0x00060000 },
+ { SUN4I_I2S_FMT0_REG, 0x00000033 },
+ { SUN4I_I2S_FMT1_REG, 0x00000030 },
+ { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
+ { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
+ { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
+ { SUN8I_I2S_CHAN_CFG_REG, 0x00000000 },
+ { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
+ { SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0x00000000 },
+ { SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x00000000 },
+ { SUN50I_H6_I2S_RX_CHAN_SEL_REG, 0x00000000 },
+ { SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0x00000000 },
+ { SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x00000000 },
+};
+
static const struct regmap_config sun4i_i2s_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,

@@ -1010,6 +1197,19 @@ static const struct regmap_config sun8i_i2s_regmap_config = {

.volatile_reg = sun8i_i2s_volatile_reg,
};

+static const struct regmap_config sun50i_h6_i2s_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN50I_H6_I2S_RX_CHAN_MAP1_REG,
+ .cache_type = REGCACHE_FLAT,
+ .reg_defaults = sun50i_h6_i2s_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(sun50i_h6_i2s_reg_defaults),
+ .writeable_reg = sun4i_i2s_wr_reg,
+ .readable_reg = sun8i_i2s_rd_reg,
+ .volatile_reg = sun8i_i2s_volatile_reg,
+};
+
static int sun4i_i2s_runtime_resume(struct device *dev)
{
struct sun4i_i2s *i2s = dev_get_drvdata(dev);

@@ -1168,6 +1368,24 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {

.set_fmt = sun4i_i2s_set_soc_fmt,
};

+static const struct sun4i_i2s_quirks sun50i_h6_i2s_quirks = {
+ .has_reset = true,
+ .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
+ .sun4i_i2s_regmap = &sun50i_h6_i2s_regmap_config,
+ .field_clkdiv_mclk_en = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
+ .field_fmt_wss = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
+ .field_fmt_sr = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
+ .bclk_dividers = sun8i_i2s_clk_div,
+ .num_bclk_dividers = ARRAY_SIZE(sun8i_i2s_clk_div),
+ .mclk_dividers = sun8i_i2s_clk_div,
+ .num_mclk_dividers = ARRAY_SIZE(sun8i_i2s_clk_div),
+ .get_bclk_parent_rate = sun8i_i2s_get_bclk_parent_rate,
+ .get_sr = sun8i_i2s_get_sr_wss,
+ .get_wss = sun8i_i2s_get_sr_wss,
+ .set_chan_cfg = sun50i_h6_i2s_set_chan_cfg,
+ .set_fmt = sun50i_h6_i2s_set_soc_fmt,
+};
+
static int sun4i_i2s_init_regmap_fields(struct device *dev,
struct sun4i_i2s *i2s)
{

@@ -1337,6 +1555,10 @@ static const struct of_device_id sun4i_i2s_match[] = {

[Clément Péron]

We are actually using a complex formula to just return a bunch of
simple values. Also this formula is wrong for sun4i when calling
get_wss() the function return 4 instead of 3.

Replace this with a simpler switch case.

Also drop the i2s params which is unused and return a simple int as
returning an error code could be out of range for an s8 and there is
no optim to return a s8 here.

Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 69 +++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 9aa837d4fe99..073ee60da011 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -175,8 +175,8 @@ struct sun4i_i2s_quirks {
unsigned int num_mclk_dividers;

unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);

- s8 (*get_sr)(const struct sun4i_i2s *, int);
- s8 (*get_wss)(const struct sun4i_i2s *, int);
+ int (*get_sr)(unsigned int width);
+ int (*get_wss)(unsigned int width);

/*

* In the set_chan_cfg() function pointer:

@@ -387,37 +387,56 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,

static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

@@ -582,11 +601,11 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Extend the functionality of the driver to include support of 20 and
24 bits per sample.

Signed-off-by: Marcus Cooper <codek...@gmail.com>

Acked-by: Maxime Ripard <mri...@kernel.org>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 9f9d3e7baad0..39b56d0de1fd 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -600,6 +600,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;

break;
+ case 32:
+ width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ break;
default:
dev_err(dai->dev, "Unsupported physical sample width: %d\n",
params_physical_width(params));

@@ -1081,6 +1084,10 @@ static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)

return 0;
}

+#define SUN4I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S20_LE | \
+ SNDRV_PCM_FMTBIT_S24_LE)
+
static struct snd_soc_dai_driver sun4i_i2s_dai = {
.probe = sun4i_i2s_dai_probe,
.capture = {

@@ -1088,14 +1095,14 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = {

[Clément Péron]

The FIFO TX reg is volatile and sun8i i2s register
mapping is different from sun4i.

Even if in this case it's doesn't create an issue,
Avoid setting some regs that are undefined in sun8i.

Acked-by: Maxime Ripard <mri...@kernel.org>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 39b56d0de1fd..83537538f8ee 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1162,12 +1162,19 @@ static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)

[Clément Péron]

From: Samuel Holland <sam...@sholland.org>

Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
regmap is cache-only are ignored. To work around this, move the
configuration to a callback that runs while the ASoC core has a
runtime PM reference to the device.

Signed-off-by: Samuel Holland <sam...@sholland.org>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Acked-by: Maxime Ripard <mri...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 83537538f8ee..6a3207245ae2 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -596,6 +596,13 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

return ret;
}

+ /* Set significant bits in our FIFOs */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+
switch (params_physical_width(params)) {

case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;

@@ -924,13 +931,6 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)

[Clément Péron]

Checkpatch script produces warning:
WARNING: function definition argument 'const struct sun4i_i2s *'
should also have an identifier name.

Let's fix this by adding identifier name to get_bclk_parent_rate()
and set_fmt() callback definition.

Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 6a3207245ae2..4cf8a67efa4f 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -180,7 +180,7 @@ struct sun4i_i2s_quirks {

const struct sun4i_i2s_clk_div *mclk_dividers;
unsigned int num_mclk_dividers;

- unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
+ unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *i2s);
int (*get_sr)(unsigned int width);
int (*get_wss)(unsigned int width);

@@ -192,7 +192,7 @@ struct sun4i_i2s_quirks {

int (*set_chan_cfg)(const struct sun4i_i2s *i2s,

unsigned int channels, unsigned int slots,

unsigned int slot_width);
- int (*set_fmt)(const struct sun4i_i2s *, unsigned int);

+ int (*set_fmt)(const struct sun4i_i2s *i2s, unsigned int fmt);
};

struct sun4i_i2s {
--
2.25.1

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

H6 I2S is very similar to H3, except that it supports up to 16 channels
and thus few registers have fields on different position.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Maxime Ripard <mri...@kernel.org>

Acked-by: Rob Herring <ro...@kernel.org>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

.../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 112ae00d63c1..606ad2d884a8 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

Add Allwinner H6 I2S1 node connected to HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>

Acked-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 13 +++++++++++++

1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 28c77d6872f6..d915aeb13297 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -609,6 +609,19 @@ mdio: mdio {
};
};

+ i2s1: i2s@5091000 {

+ #sound-dai-cells = <0>;

+ compatible = "allwinner,sun50i-h6-i2s";
+ reg = <0x05091000 0x1000>;

+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S1>, <&ccu CLK_I2S1>;

+ clock-names = "apb", "mod";

+ dmas = <&dma 4>, <&dma 4>;
+ resets = <&ccu RST_BUS_I2S1>;

+ dma-names = "rx", "tx";

+ status = "disabled";
+ };
+

spdif: spdif@5093000 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-h6-spdif";
--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

On the newer SoCs such as the H3 and A64 this is set by default
to transfer a 0 after each sample in each slot. However the A10
and A20 SoCs that this driver was developed on had a default
setting where it padded the audio gain with zeros.

This isn't a problem while we have only support for 16bit audio
but with larger sample resolution rates in the pipeline then SEXT
bits should be cleared so that they also pad at the LSB. Without
this the audio gets distorted.

Set sign extend sample for all the sunxi generations even if they
are not affected. This will keep consistency and avoid relying on
default.

Signed-off-by: Marcus Cooper <codek...@gmail.com>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Acked-by: Maxime Ripard <mri...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 073ee60da011..9f9d3e7baad0 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -48,6 +48,9 @@
#define SUN4I_I2S_FMT0_FMT_I2S (0 << 0)

#define SUN4I_I2S_FMT1_REG 0x08
+#define SUN4I_I2S_FMT1_REG_SEXT_MASK BIT(8)
+#define SUN4I_I2S_FMT1_REG_SEXT(sext) ((sext) << 8)
+
#define SUN4I_I2S_FIFO_TX_REG 0x0c
#define SUN4I_I2S_FIFO_RX_REG 0x10

@@ -105,6 +108,9 @@
#define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7)
#define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL (0 << 7)

+#define SUN8I_I2S_FMT1_REG_SEXT_MASK GENMASK(5, 4)
+#define SUN8I_I2S_FMT1_REG_SEXT(sext) ((sext) << 4)
+
#define SUN8I_I2S_INT_STA_REG 0x0c
#define SUN8I_I2S_FIFO_TX_REG 0x20

@@ -686,6 +692,7 @@ static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
}
regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
SUN4I_I2S_CTRL_MODE_MASK, val);
+
return 0;
}

@@ -788,6 +795,11 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
val);

+ /* Set sign extension to pad out LSB with 0 */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+ SUN8I_I2S_FMT1_REG_SEXT_MASK,
+ SUN8I_I2S_FMT1_REG_SEXT(0));
+
return 0;
}

@@ -890,6 +902,11 @@ static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
val);

+ /* Set sign extension to pad out LSB with 0 */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+ SUN8I_I2S_FMT1_REG_SEXT_MASK,
+ SUN8I_I2S_FMT1_REG_SEXT(0));
+
return 0;
}

--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add the I2S2 node connected to the HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index dc238814013c..51cc30e84e26 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi

@@ -846,6 +846,20 @@ i2s1: i2s@1c22400 {

status = "disabled";
};

+ i2s2: i2s@1c22800 {

+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun50i-a64-i2s",
+ "allwinner,sun8i-h3-i2s";

+ reg = <0x01c22800 0x400>;

+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;

+ clock-names = "apb", "mod";

+ resets = <&ccu RST_BUS_I2S2>;

+ dma-names = "rx", "tx";

+ dmas = <&dma 27>, <&dma 27>;

+ status = "disabled";
+ };
+

dai: dai@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-codec-i2s";
--
2.25.1

[Clément Péron]

Enable Allwinner I2S driver for arm64 defconfig.

Signed-off-by: Clément Péron <peron...@gmail.com>
---

[Clément Péron]

Like A83T the Allwinner H3 doesn't have the DMA reception available for
some audio interfaces.

As it's already documented for A83T convert this to an enum and add the H3
interface.

Acked-by: Rob Herring <ro...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

.../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 606ad2d884a8..a16e37b01e1d 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add H3/H5 I2S2 node connected to the HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm/boot/dts/sunxi-h3-h5.dtsi | 13 +++++++++++++

1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 22d533d18992..9be13378d4df 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi

@@ -662,6 +662,19 @@ i2s1: i2s@1c22400 {

status = "disabled";
};

+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;

+ compatible = "allwinner,sun8i-h3-i2s";

+ reg = <0x01c22800 0x400>;
+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;
+ clock-names = "apb", "mod";

+ dmas = <&dma 27>;

+ resets = <&ccu RST_BUS_I2S2>;

+ dma-names = "tx";

+ status = "disabled";
+ };
+

codec: codec@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun8i-h3-codec";
--
2.25.1

[Clément Péron]

Hi Pierre-Louis,

On Tue, 27 Oct 2020 at 19:59, Pierre-Louis Bossart
<pierre-lou...@linux.intel.com> wrote:
>
>
> > @@ -452,11 +454,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > case SND_SOC_DAIFMT_DSP_B:
> > case SND_SOC_DAIFMT_LEFT_J:
> > case SND_SOC_DAIFMT_RIGHT_J:
> > - lrck_period = params_physical_width(params) * slots;
> > + lrck_period = slot_width * slots;
> > break;
> >
> > case SND_SOC_DAIFMT_I2S:
> > - lrck_period = params_physical_width(params);
> > + lrck_period = slot_width;
> > break;
>

> Aren't I2S, LEFT_J and RIGHT_J pretty much the same in terms of lrclk
> rate/period? the only thing that can change is the polarity, no?
>
> Not sure why it's handled differently here?

I just had a look at the User Manual for H3 and H6 and I didn't find
any reason why LEFT_J and RIGHT_J should be computed in a different
way as I2S.

Also the commit introducing this doesn't mention it.
7ae7834ec446 ("ASoC: sun4i-i2s: Add support for DSP formats")

I can't test it with my board but if nobody complains about it, I will
introduce a fix for this in the next version and change this also for
H6.

Thanks for your review,
Clement

[Chen-Yu Tsai]

On Wed, Oct 28, 2020 at 2:32 AM Clément Péron <peron...@gmail.com> wrote:
>
> Enable Allwinner I2S driver for arm64 defconfig.
>
> Signed-off-by: Clément Péron <peron...@gmail.com>

Acked-by: Chen-Yu Tsai <we...@csie.org>

[Samuel Holland]

On 10/27/20 4:43 PM, Clément Péron wrote:
> Hi Pierre-Louis,
>
> On Tue, 27 Oct 2020 at 19:59, Pierre-Louis Bossart
> <pierre-lou...@linux.intel.com> wrote:
>>
>>
>>> @@ -452,11 +454,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>>> case SND_SOC_DAIFMT_DSP_B:
>>> case SND_SOC_DAIFMT_LEFT_J:
>>> case SND_SOC_DAIFMT_RIGHT_J:
>>> - lrck_period = params_physical_width(params) * slots;
>>> + lrck_period = slot_width * slots;
>>> break;
>>>
>>> case SND_SOC_DAIFMT_I2S:
>>> - lrck_period = params_physical_width(params);
>>> + lrck_period = slot_width;
>>> break;
>>
>> Aren't I2S, LEFT_J and RIGHT_J pretty much the same in terms of lrclk
>> rate/period? the only thing that can change is the polarity, no?
>>
>> Not sure why it's handled differently here?
>
> I just had a look at the User Manual for H3 and H6 and I didn't find
> any reason why LEFT_J and RIGHT_J should be computed in a different
> way as I2S.

Yes, and the manual explicitly groups LEFT_J and RIGHT_J with I2S when
describing this field:

PCM Mode: Number of BCLKs within (Left + Right) channel width.
I2S/Left-Justified/Right-Justified Mode: Number of BCLKs within each
individual channel width(Left or Right)

So I agree that the code is wrong here.

Regards,
Samuel

[Clément Péron]

Hi Samuel

Thanks, I will send a fix in the v10.

Regards,
Clement

[Clément Péron]

Hi,

This series add H6 I2S support and the I2S node missing to support
HDMI audio in different Allwinner SoC.

As we first use some TDM property to make the I2S working with the

simple soundcard. We have now drop this simple sound card and a
proper dedicated soundcard will be introduce later.

This make the title of this series wrong now but to be able to
follow the previous release I keep the same name.

Regards,
Clement

Change since v9:
- fix lrck_period computation for I2S justified mode

Clément Péron (7):
ASoC: sun4i-i2s: Fix lrck_period computation for I2S justified mode

ASoC: sun4i-i2s: Change set_chan_cfg() params
ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
ASoC: sun4i-i2s: Fix sun8i volatile regs
ASoC: sun4i-i2s: fix coding-style for callback definition
arm64: defconfig: Enable Allwinner i2s driver
dt-bindings: sound: sun4i-i2s: Document H3 with missing RX channel
possibility

Jernej Skrabec (3):
ASoC: sun4i-i2s: Add support for H6 I2S
dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
arm64: dts: allwinner: h6: Add I2S1 node

Marcus Cooper (4):
ASoC: sun4i-i2s: Set sign extend sample

ASoC: sun4i-i2s: Add 20 and 24 bit support

arm64: dts: allwinner: a64: Add I2S2 node
arm: dts: sunxi: h3/h5: Add I2S2 node

Samuel Holland (1):
ASoC: sun4i-i2s: Fix setting of FIFO modes

.../sound/allwinner,sun4i-a10-i2s.yaml | 6 +-
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 13 +
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 +
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 13 +
arch/arm64/configs/defconfig | 1 +

sound/soc/sunxi/sun4i-i2s.c | 388 +++++++++++++++---
6 files changed, 378 insertions(+), 57 deletions(-)

--
2.25.1

[Clément Péron]

Left and Right justified mode are computed using the same formula
as DSP_A and DSP_B mode.
Which is wrong and the user manual explicitly says:

LRCK_PERDIOD:

PCM Mode: Number of BCLKs within (Left + Right) channel width.
I2S/Left-Justified/Right-Justified Mode: Number of BCLKs within each
individual channel width(Left or Right)

Fix this by using the same formula as the I2S mode.

Fixes: 7ae7834ec446 ("ASoC: sun4i-i2s: Add support for DSP formats")

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index f23ff29e7c1d..a994b5cf87b3 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -450,11 +450,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {

case SND_SOC_DAIFMT_DSP_A:
case SND_SOC_DAIFMT_DSP_B:
- case SND_SOC_DAIFMT_LEFT_J:
- case SND_SOC_DAIFMT_RIGHT_J:

lrck_period = params_physical_width(params) * slots;

break;

+ case SND_SOC_DAIFMT_LEFT_J:
+ case SND_SOC_DAIFMT_RIGHT_J:

case SND_SOC_DAIFMT_I2S:
lrck_period = params_physical_width(params);
break;
--
2.25.1

[Clément Péron]

As slots and slot_width can be set manually using set_tdm().
These values are then kept in sun4i_i2s struct.
So we need to check if these values are set or not.

This is not done actually and will trigger a bug.
For example, if we set to the simple soundcard in the device-tree
dai-tdm-slot-width = <32> and then start a stream using S16_LE,
currently we would calculate BCLK for 32-bit slots, but program
lrck_period for 16-bit slots, making the sample rate double what we
expected.

To fix this, we need to check if these values are set or not but as
this logic is already done by the caller. Avoid duplicating this
logic and just pass the required values as params to set_chan_cfg().

Suggested-by: Samuel Holland <sam...@sholland.org>
Acked-by: Maxime Ripard <mri...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index a994b5cf87b3..4ff2068779fd 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -162,8 +162,15 @@ struct sun4i_i2s_quirks {

unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);

s8 (*get_sr)(const struct sun4i_i2s *, int);

s8 (*get_wss)(const struct sun4i_i2s *, int);

- int (*set_chan_cfg)(const struct sun4i_i2s *,
- const struct snd_pcm_hw_params *);
+
+ /*
+ * In the set_chan_cfg() function pointer:
+ * @slots: channels per frame + padding slots, regardless of format
+ * @slot_width: bits per sample + padding bits, regardless of format
+ */

+ int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width);

int (*set_fmt)(const struct sun4i_i2s *, unsigned int);

};

@@ -399,10 +406,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)

}

static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
-

/* Map the channels for playback and capture */

regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210);

@@ -419,15 +425,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

}

static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

- const struct snd_pcm_hw_params *params)

+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
- unsigned int slots = channels;
unsigned int lrck_period;

- if (i2s->slots)
- slots = i2s->slots;
-

/* Map the channels for playback and capture */

regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);

@@ -450,13 +452,13 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_DSP_A:
case SND_SOC_DAIFMT_DSP_B:

- lrck_period = params_physical_width(params) * slots;
+ lrck_period = slot_width * slots;
break;

case SND_SOC_DAIFMT_LEFT_J:
case SND_SOC_DAIFMT_RIGHT_J:

case SND_SOC_DAIFMT_I2S:
- lrck_period = params_physical_width(params);
+ lrck_period = slot_width;
break;

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

H6 I2S is very similar to that in H3, except it supports up to 16
channels.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>

Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 222 ++++++++++++++++++++++++++++++++++++
1 file changed, 222 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 4ff2068779fd..24b3137afbc2 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -124,6 +124,21 @@
#define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
#define SUN8I_I2S_RX_CHAN_MAP_REG 0x58

+/* Defines required for sun50i-h6 support */
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK GENMASK(21, 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset) ((offset) << 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK GENMASK(19, 16)
+#define SUN50I_H6_I2S_TX_CHAN_SEL(chan) ((chan - 1) << 16)
+#define SUN50I_H6_I2S_TX_CHAN_EN_MASK GENMASK(15, 0)
+#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1))
+
+#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG 0x44
+#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG 0x48
+
+#define SUN50I_H6_I2S_RX_CHAN_SEL_REG 0x64
+#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG 0x68
+#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG 0x6C
+
struct sun4i_i2s;

/**
@@ -476,6 +491,60 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
return 0;
}

+static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)

+ lrck_period = slot_width * slots;

+ break;
+

+ case SND_SOC_DAIFMT_LEFT_J:
+ case SND_SOC_DAIFMT_RIGHT_J:

+ case SND_SOC_DAIFMT_I2S:
+ lrck_period = slot_width;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+ SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
+ SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ SUN50I_H6_I2S_TX_CHAN_EN_MASK,
+ SUN50I_H6_I2S_TX_CHAN_EN(channels));
+
+ return 0;
+}
+

static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -703,6 +772,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
return 0;
}

+static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
+ unsigned int fmt)
+{
+ u32 mode, val;
+ u8 offset;

+
+ /*

[Clément Péron]

We are actually using a complex formula to just return a bunch of
simple values. Also this formula is wrong for sun4i when calling
get_wss() the function return 4 instead of 3.

Replace this with a simpler switch case.

Also drop the i2s params which is unused and return a simple int as
returning an error code could be out of range for an s8 and there is
no optim to return a s8 here.

Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 69 +++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 24b3137afbc2..6ee9c2995b4f 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -175,8 +175,8 @@ struct sun4i_i2s_quirks {
unsigned int num_mclk_dividers;

unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);

- s8 (*get_sr)(const struct sun4i_i2s *, int);
- s8 (*get_wss)(const struct sun4i_i2s *, int);
+ int (*get_sr)(unsigned int width);
+ int (*get_wss)(unsigned int width);

/*

* In the set_chan_cfg() function pointer:

static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

On the newer SoCs such as the H3 and A64 this is set by default
to transfer a 0 after each sample in each slot. However the A10
and A20 SoCs that this driver was developed on had a default
setting where it padded the audio gain with zeros.

This isn't a problem while we have only support for 16bit audio
but with larger sample resolution rates in the pipeline then SEXT
bits should be cleared so that they also pad at the LSB. Without
this the audio gets distorted.

Set sign extend sample for all the sunxi generations even if they
are not affected. This will keep consistency and avoid relying on
default.

Signed-off-by: Marcus Cooper <codek...@gmail.com>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 6ee9c2995b4f..46e4da18c27f 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Extend the functionality of the driver to include support of 20 and
24 bits per sample.

Signed-off-by: Marcus Cooper <codek...@gmail.com>

Acked-by: Maxime Ripard <mri...@kernel.org>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 46e4da18c27f..786731191d90 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -600,6 +600,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,

case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;

[Clément Péron]

From: Samuel Holland <sam...@sholland.org>

Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
regmap is cache-only are ignored. To work around this, move the
configuration to a callback that runs while the ASoC core has a
runtime PM reference to the device.

Signed-off-by: Samuel Holland <sam...@sholland.org>

Reviewed-by: Chen-Yu Tsai <we...@csie.org>
Acked-by: Maxime Ripard <mri...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 003610c0badf..4f5cd850752d 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -596,6 +596,13 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
return ret;
}

+ /* Set significant bits in our FIFOs */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+
switch (params_physical_width(params)) {

case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;

[Clément Péron]

Checkpatch script produces warning:
WARNING: function definition argument 'const struct sun4i_i2s *'
should also have an identifier name.

Let's fix this by adding identifier name to get_bclk_parent_rate()
and set_fmt() callback definition.

Acked-by: Maxime Ripard <mri...@kernel.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 4f5cd850752d..4b8ca5be0a29 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c

@@ -180,7 +180,7 @@ struct sun4i_i2s_quirks {
const struct sun4i_i2s_clk_div *mclk_dividers;
unsigned int num_mclk_dividers;

- unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
+ unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *i2s);
int (*get_sr)(unsigned int width);
int (*get_wss)(unsigned int width);

@@ -192,7 +192,7 @@ struct sun4i_i2s_quirks {

int (*set_chan_cfg)(const struct sun4i_i2s *i2s,

unsigned int channels, unsigned int slots,

unsigned int slot_width);
- int (*set_fmt)(const struct sun4i_i2s *, unsigned int);

+ int (*set_fmt)(const struct sun4i_i2s *i2s, unsigned int fmt);
};

struct sun4i_i2s {
--
2.25.1

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

H6 I2S is very similar to H3, except that it supports up to 16 channels
and thus few registers have fields on different position.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>

Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Maxime Ripard <mri...@kernel.org>

Acked-by: Rob Herring <ro...@kernel.org>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

.../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 112ae00d63c1..606ad2d884a8 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml

[Clément Péron]

From: Jernej Skrabec <jernej....@siol.net>

Add Allwinner H6 I2S1 node connected to HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>

Acked-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 13 +++++++++++++

1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 28c77d6872f6..d915aeb13297 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -609,6 +609,19 @@ mdio: mdio {
};
};

+ i2s1: i2s@5091000 {

+ #sound-dai-cells = <0>;

+ compatible = "allwinner,sun50i-h6-i2s";
+ reg = <0x05091000 0x1000>;

+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S1>, <&ccu CLK_I2S1>;

+ clock-names = "apb", "mod";

+ dmas = <&dma 4>, <&dma 4>;
+ resets = <&ccu RST_BUS_I2S1>;

+ dma-names = "rx", "tx";

+ status = "disabled";
+ };
+

spdif: spdif@5093000 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-h6-spdif";
--
2.25.1

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add the I2S2 node connected to the HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index dc238814013c..51cc30e84e26 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi

@@ -846,6 +846,20 @@ i2s1: i2s@1c22400 {

status = "disabled";
};

+ i2s2: i2s@1c22800 {

+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun50i-a64-i2s",
+ "allwinner,sun8i-h3-i2s";

+ reg = <0x01c22800 0x400>;

+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;

+ clock-names = "apb", "mod";

+ resets = <&ccu RST_BUS_I2S2>;

+ dma-names = "rx", "tx";

+ dmas = <&dma 27>, <&dma 27>;

+ status = "disabled";
+ };
+

dai: dai@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-codec-i2s";
--
2.25.1

[Clément Péron]

Enable Allwinner I2S driver for arm64 defconfig.

Acked-by: Chen-Yu Tsai <we...@csie.org>
Signed-off-by: Clément Péron <peron...@gmail.com>
---

[Clément Péron]

Like A83T the Allwinner H3 doesn't have the DMA reception available for
some audio interfaces.

As it's already documented for A83T convert this to an enum and add the H3
interface.

Acked-by: Rob Herring <ro...@kernel.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

.../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 606ad2d884a8..a16e37b01e1d 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml

[Clément Péron]

From: Marcus Cooper <codek...@gmail.com>

Add H3/H5 I2S2 node connected to the HDMI interface.

Signed-off-by: Jernej Skrabec <jernej....@siol.net>
Signed-off-by: Marcus Cooper <codek...@gmail.com>
Acked-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

arch/arm/boot/dts/sunxi-h3-h5.dtsi | 13 +++++++++++++

1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 22d533d18992..9be13378d4df 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi

@@ -662,6 +662,19 @@ i2s1: i2s@1c22400 {

status = "disabled";
};

+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;

+ compatible = "allwinner,sun8i-h3-i2s";

+ reg = <0x01c22800 0x400>;
+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;
+ clock-names = "apb", "mod";

+ dmas = <&dma 27>;

+ resets = <&ccu RST_BUS_I2S2>;

+ dma-names = "tx";

+ status = "disabled";
+ };
+

codec: codec@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun8i-h3-codec";
--
2.25.1

[Clément Péron]

The FIFO TX reg is volatile and sun8i i2s register
mapping is different from sun4i.

Even if in this case it's doesn't create an issue,
Avoid setting some regs that are undefined in sun8i.

Acked-by: Maxime Ripard <mri...@kernel.org>
Reviewed-by: Chen-Yu Tsai <we...@csie.org>

Signed-off-by: Clément Péron <peron...@gmail.com>
---

sound/soc/sunxi/sun4i-i2s.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 786731191d90..003610c0badf 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1162,12 +1162,19 @@ static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)

static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
{
- if (reg == SUN8I_I2S_INT_STA_REG)
+ switch (reg) {
+ case SUN4I_I2S_FIFO_CTRL_REG:
+ case SUN4I_I2S_FIFO_RX_REG:
+ case SUN4I_I2S_FIFO_STA_REG:
+ case SUN4I_I2S_RX_CNT_REG:
+ case SUN4I_I2S_TX_CNT_REG:
+ case SUN8I_I2S_FIFO_TX_REG:
+ case SUN8I_I2S_INT_STA_REG:
return true;
- if (reg == SUN8I_I2S_FIFO_TX_REG)
- return false;

- return sun4i_i2s_volatile_reg(dev, reg);
+ default:
+ return false;
+ }
}

static const struct reg_default sun4i_i2s_reg_defaults[] = {
--
2.25.1

[Maxime Ripard]

On Fri, Oct 30, 2020 at 03:46:34PM +0100, Clément Péron wrote:
> Left and Right justified mode are computed using the same formula
> as DSP_A and DSP_B mode.
> Which is wrong and the user manual explicitly says:
>
> LRCK_PERDIOD:
> PCM Mode: Number of BCLKs within (Left + Right) channel width.
> I2S/Left-Justified/Right-Justified Mode: Number of BCLKs within each
> individual channel width(Left or Right)
>
> Fix this by using the same formula as the I2S mode.
>
> Fixes: 7ae7834ec446 ("ASoC: sun4i-i2s: Add support for DSP formats")
> Signed-off-by: Clément Péron <peron...@gmail.com>

Acked-by: Maxime Ripard <mri...@kernel.org>
Thanks!
Maxime

[Mark Brown]

On Fri, Oct 30, 2020 at 03:46:47PM +0100, Clément Péron wrote:
> Like A83T the Allwinner H3 doesn't have the DMA reception available for
> some audio interfaces.

Please if you're going to mix dts updates in with subsystem changes like
this keep the bits for different subsystems grouped, this makes it much
easier to handle things.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[Clément Péron]

Hi Mark,

Sorry i have based the commit log on this file regarding the previous
commit and it was these:
5c7404bb30bc ("dt-bindings: Change maintainer address")
eb5b12843b06 ("dt-bindings: sound: sun4i-i2s: Document that the RX
channel can be missing")
0a0ca8e94ca3 ("dt-bindings: sound: Convert Allwinner I2S binding to YAML")

But basically for the next dt-bindings change you will prefer somethings like:
"ASoC: dt-bindings: sun4i-i2s: Document H3 with missing RX channel possibility"

Regards,
Clement

[Clément Péron]

Hi Maxime,

Thanks for the ACK

just noticed that the case was not properly sorted.

Do you agree that sorting like this is better ?
case SND_SOC_DAIFMT_I2S:

+ case SND_SOC_DAIFMT_LEFT_J:
+ case SND_SOC_DAIFMT_RIGHT_J:

If I have to push a new series I will fix it

Regards,
Clement

> Thanks!
> Maxime

[Mark Brown]

On Fri, 30 Oct 2020 15:46:33 +0100, Clément Péron wrote:
> This series add H6 I2S support and the I2S node missing to support
> HDMI audio in different Allwinner SoC.
>
> As we first use some TDM property to make the I2S working with the
> simple soundcard. We have now drop this simple sound card and a
> proper dedicated soundcard will be introduce later.
>

> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[01/11] ASoC: sun4i-i2s: Fix lrck_period computation for I2S justified mode
commit: 93c0210671d8f3ec2262da703fab93a1497158a8
[02/11] ASoC: sun4i-i2s: Change set_chan_cfg() params
commit: c779e2de0ac6156bea63e759481ee383587336cc
[03/11] ASoC: sun4i-i2s: Add support for H6 I2S
commit: 73adf87b7a5882408b0a17da59e69df4be12a968
[04/11] ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
commit: 9c2d255f0e63f8e54bd8345f9c59c4060cf4bbd4
[05/11] ASoC: sun4i-i2s: Set sign extend sample
commit: d8659dd9a13ce7a92c017c352aea1c390f300937
[06/11] ASoC: sun4i-i2s: Add 20 and 24 bit support
commit: 6ad7ca6297f8679162ee62ed672b603e8d004146
[07/11] ASoC: sun4i-i2s: Fix sun8i volatile regs
commit: 64359246abe4421ad409be5b0bc9a534caa18b7d
[08/11] ASoC: sun4i-i2s: Fix setting of FIFO modes
commit: 38d7adc0a003298013786cfffe5f4cc907009d30
[09/11] ASoC: sun4i-i2s: fix coding-style for callback definition
commit: 08c7b7d546fddce76d500e5e5767aa08836f7cae
[10/11] ASoC: sun4i-i2s: Add H6 compatible
commit: e84f44ba4604e55a51e7caf01464f220d0eabef4
[11/11] ASoC: sun4i-i2s: Document H3 with missing RX channel possibility
commit: 0bc1bf241de551842535c3d0b080e0f38c11aed1

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

[Mark Brown]

On Fri, Oct 30, 2020 at 08:05:31PM +0100, Clément Péron wrote:

> But basically for the next dt-bindings change you will prefer somethings like:
> "ASoC: dt-bindings: sun4i-i2s: Document H3 with missing RX channel possibility"

Yes.

[Stefan Monnier]

>> This series add H6 I2S support and the I2S node missing to support
>> HDMI audio in different Allwinner SoC.

> Applied to
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Yay!

Now, I wonder: will that make it easier to add support for HDMI-Audio for
the A10/A20?

(there was a patch for that submitted earlier this year by Stefan
Mavrodiev <ste...@olimex.com>, but it seems there hasn't been any
progress on it since then (I think the last message on it concluded that
it should be rewritten to use ALSA instead of ASoC)).

[ To clarify, don't know what's the difference between ALSA and ASoC;
I'm only interested here as an owner of an A20 box on which I'd
love to be able to use the HDMI-Audio. ]


-- Stefan

[Jernej Škrabec]

Dne petek, 30. oktober 2020 ob 21:50:43 CET je Stefan Monnier napisal(a):

> >> This series add H6 I2S support and the I2S node missing to support
> >> HDMI audio in different Allwinner SoC.
> >
> > Applied to
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
> > for-next
>
> Yay!

Note that this doesn't bring HDMI audio card just yet. Another driver will be
needed for that.

>
> Now, I wonder: will that make it easier to add support for HDMI-Audio for
> the A10/A20?

No, A10/A20 HDMI audio uses completely different interface.

>
> (there was a patch for that submitted earlier this year by Stefan
> Mavrodiev <ste...@olimex.com>, but it seems there hasn't been any
> progress on it since then (I think the last message on it concluded that
> it should be rewritten to use ALSA instead of ASoC)).

IIUC original author left Olimex, so work stalled.

Best regards,
Jernej

[Stefan Monnier]

>> Yay!
> Note that this doesn't bring HDMI audio card just yet. Another driver will be
> needed for that.

That's what I suspected, but thanks for clarifying.

>> Now, I wonder: will that make it easier to add support for HDMI-Audio for
>> the A10/A20?
> No, A10/A20 HDMI audio uses completely different interface.

Oh, well!

> IIUC original author left Olimex, so work stalled.

Aha, so that's what happened.
I wish I had the energy to take his patch and adapt it.


Stefan

[Maxime Ripard]

Applied the rest, thanks!
Maxime