Skip to content

Commit

Permalink
ASoC: sunxi: sun4i-i2s: fix LRCLK polarity in i2s mode
Browse files Browse the repository at this point in the history 
[ Upstream commit 3e83957 ]

This fixes the LRCLK polarity for sun8i-h3 and sun50i-h6 in i2s mode
which was wrongly inverted.

The LRCLK was being set in reversed logic compared to the DAI format:
inverted LRCLK for SND_SOC_DAIFMT_IB_NF and SND_SOC_DAIFMT_NB_NF; normal
LRCLK for SND_SOC_DAIFMT_IB_IF and SND_SOC_DAIFMT_NB_IF. Such reversed
logic applies properly for DSP_A, DSP_B, LEFT_J and RIGHT_J modes but
not for I2S mode, for which the LRCLK signal results reversed to what
expected on the bus. The issue is due to a misinterpretation of the
LRCLK polarity bit of the H3 and H6 i2s controllers. Such bit in this
case does not mean "0 => normal" or "1 => inverted" according to the
expected bus operation, but it means "0 => frame starts on low edge" and
"1 => frame starts on high edge" (from the User Manuals).

This commit fixes the LRCLK polarity by setting the LRCLK polarity bit
according to the selected bus mode and renames the LRCLK polarity bit
definition to avoid further confusion.

Fixes: dd657ea ("ASoC: sun4i-i2s: Fix the LRCK polarity")
Fixes: 73adf87 ("ASoC: sun4i-i2s: Add support for H6 I2S")
Signed-off-by: Matteo Martelli <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
  • Loading branch information
@mattmart3 @gregkh
mattmart3 authored and gregkh committed Sep 12, 2024
1 parent f39bde3 commit b9bb963
Showing 1 changed file with 73 additions and 70 deletions.
143 changes: 73 additions & 70 deletions sound/soc/sunxi/sun4i-i2s.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@
#define SUN8I_I2S_CTRL_MODE_PCM (0 << 4)

#define SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK BIT(19)
#define SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED (1 << 19)
#define SUN8I_I2S_FMT0_LRCLK_POLARITY_NORMAL (0 << 19)
#define SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH (1 << 19)
#define SUN8I_I2S_FMT0_LRCLK_POLARITY_START_LOW (0 << 19)
#define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
#define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period - 1) << 8)
#define SUN8I_I2S_FMT0_BCLK_POLARITY_MASK BIT(7)
Expand Down Expand Up @@ -727,65 +727,37 @@ static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
unsigned int fmt)
{
u32 mode, val;
u32 mode, lrclk_pol, bclk_pol, 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:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_PCM;
offset = 1;
break;

case SND_SOC_DAIFMT_DSP_B:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_PCM;
offset = 0;
break;

case SND_SOC_DAIFMT_I2S:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_LOW;
mode = SUN8I_I2S_CTRL_MODE_LEFT;
offset = 1;
break;

case SND_SOC_DAIFMT_LEFT_J:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_LEFT;
offset = 0;
break;

case SND_SOC_DAIFMT_RIGHT_J:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_RIGHT;
offset = 0;
break;
Expand All @@ -803,6 +775,35 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
SUN8I_I2S_TX_CHAN_OFFSET_MASK,
SUN8I_I2S_TX_CHAN_OFFSET(offset));

/* DAI clock polarity */
bclk_pol = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;

switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
case SND_SOC_DAIFMT_IB_IF:
/* Invert both clocks */
lrclk_pol ^= SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK;
bclk_pol = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED;
break;
case SND_SOC_DAIFMT_IB_NF:
/* Invert bit clock */
bclk_pol = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED;
break;
case SND_SOC_DAIFMT_NB_IF:
/* Invert frame clock */
lrclk_pol ^= SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK;
break;
case SND_SOC_DAIFMT_NB_NF:
/* No inversion */
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,
lrclk_pol | bclk_pol);

/* DAI clock master masks */
switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) {
case SND_SOC_DAIFMT_BP_FP:
Expand Down Expand Up @@ -834,65 +835,37 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
unsigned int fmt)
{
u32 mode, val;
u32 mode, lrclk_pol, bclk_pol, 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:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_PCM;
offset = 1;
break;

case SND_SOC_DAIFMT_DSP_B:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_PCM;
offset = 0;
break;

case SND_SOC_DAIFMT_I2S:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_LOW;
mode = SUN8I_I2S_CTRL_MODE_LEFT;
offset = 1;
break;

case SND_SOC_DAIFMT_LEFT_J:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_LEFT;
offset = 0;
break;

case SND_SOC_DAIFMT_RIGHT_J:
lrclk_pol = SUN8I_I2S_FMT0_LRCLK_POLARITY_START_HIGH;
mode = SUN8I_I2S_CTRL_MODE_RIGHT;
offset = 0;
break;
Expand All @@ -910,6 +883,36 @@ static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset));

/* DAI clock polarity */
bclk_pol = SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL;

switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
case SND_SOC_DAIFMT_IB_IF:
/* Invert both clocks */
lrclk_pol ^= SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK;
bclk_pol = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED;
break;
case SND_SOC_DAIFMT_IB_NF:
/* Invert bit clock */
bclk_pol = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED;
break;
case SND_SOC_DAIFMT_NB_IF:
/* Invert frame clock */
lrclk_pol ^= SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK;
break;
case SND_SOC_DAIFMT_NB_NF:
/* No inversion */
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,
lrclk_pol | bclk_pol);


/* DAI clock master masks */
switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) {
case SND_SOC_DAIFMT_BP_FP:
Expand Down

0 comments on commit b9bb963

Please sign in to comment.