Received-SPF: pass (google.com: domain of linux-kernel+bounces-204664-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 6 Jun 2024 18:11:08 +0200
From: Maxime Ripard <mripard@kernel.org>
To: Matteo Martelli <matteomartelli3@gmail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>, 
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, Chen-Yu Tsai <wens@csie.org>, 
	Jernej Skrabec <jernej.skrabec@gmail.com>, Samuel Holland <samuel@sholland.org>, 
	Marcus Cooper <codekipper@gmail.com>, =?utf-8?B?Q2zDqW1lbnQgUMOpcm9u?= <peron.clem@gmail.com>, 
	linux-sound@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] ASoC: sunxi: sun4i-i2s: fix LRCLK polarity in i2s
 mode
Message-ID: <20240606-savvy-wallaby-of-champagne-d4a50e@houat>
References: <20240529140658.180966-2-matteomartelli3@gmail.com>
 <20240529140658.180966-3-matteomartelli3@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha384;
	protocol="application/pgp-signature"; boundary="4nk5dehwshah5pj6"
Content-Disposition: inline
In-Reply-To: <20240529140658.180966-3-matteomartelli3@gmail.com>


--4nk5dehwshah5pj6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Wed, May 29, 2024 at 04:00:15PM GMT, Matteo Martelli wrote:
> This fixes the LRCLK polarity for sun8i-h3 and sun50i-h6 in i2s mode
> which was wrongly inverted.
>=20
> 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 =3D> normal" or "1 =3D> inverted" according to the
> expected bus operation, but it means "0 =3D> frame starts on low edge" and
> "1 =3D> frame starts on high edge" (from the User Manuals).
>=20
> 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.
>=20
> Fixes: dd657eae8164 ("ASoC: sun4i-i2s: Fix the LRCK polarity")
> Fixes: 73adf87b7a58 ("ASoC: sun4i-i2s: Add support for H6 I2S")
> Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 143 ++++++++++++++++++------------------
>  1 file changed, 73 insertions(+), 70 deletions(-)
>=20
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 5f8d979585b6..a200ba41679a 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -100,8 +100,8 @@
>  #define SUN8I_I2S_CTRL_MODE_PCM			(0 << 4)
> =20
>  #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)
> @@ -729,65 +729,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;
> =20
> -	/*
> -	 * 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.
> -	 */

I think we should keep that comment somewhere.

Maxime

--4nk5dehwshah5pj6
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iJUEABMJAB0WIQTkHFbLp4ejekA/qfgnX84Zoj2+dgUCZmHflAAKCRAnX84Zoj2+
dkp4AYDD2C89et3GX+eTXxMU3fw2ApYremNJgsBVdE7CDulWRzcHlpW2kUP83Bfd
pT1zxkcBfR2hOGFtRyy3sMoLOytPZezcLRdE7sh1OTYn1SAPA/T5otrsqk1PkCp0
t7BzSI8rzw==
=ri98
-----END PGP SIGNATURE-----

--4nk5dehwshah5pj6--