From patchwork Thu Aug 1 12:07:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matteo Martelli X-Patchwork-Id: 13750281 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id F0651C3DA64 for ; Thu, 1 Aug 2024 12:32:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Cc:To:Message-Id: Content-Transfer-Encoding:Content-Type:MIME-Version:Subject:Date:From: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=MWpJKDgAAIKrTcDAo93c65X2vfalBn4m5VoyMmFWUps=; b=Qvjzdv9vkvcxjQM4t8oB7GO4u2 YugM5hREEG+R/gzarkk0JpGsy5flD9eeW95rrJMdVSd0MeEUv+WwVgqLioAKIXco+XssNXs3Azne3 xTZfzuBUqOE7abXSk6vwbdL65vHG2/8qYe2kB+5ljEMKu43rxaWmyCTJBEobBIKwzvFW4bH5nehTR 1JHNxf0N5qHeynQd63B5zdmq2jUSTeOAUuYaRwQv681iXqN2BSXzpd7M+JCAewLsIFGxDU5qtzkC6 JbXID+JajlmzHkOcM+DDSsOMMyBzJDCsOoQ0ly13L8nvM6hesZo0SW/sG9359IfZU04pjXSwuhnJu jge1Vhqg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sZUyC-00000005CLY-297H; Thu, 01 Aug 2024 12:31:48 +0000 Received: from mail-lj1-x22e.google.com ([2a00:1450:4864:20::22e]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sZUao-000000051yG-1TtK for linux-arm-kernel@lists.infradead.org; Thu, 01 Aug 2024 12:07:40 +0000 Received: by mail-lj1-x22e.google.com with SMTP id 38308e7fff4ca-2ef2cb7d562so88153441fa.3 for ; Thu, 01 Aug 2024 05:07:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722514056; x=1723118856; darn=lists.infradead.org; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:from:to:cc:subject:date:message-id:reply-to; bh=MWpJKDgAAIKrTcDAo93c65X2vfalBn4m5VoyMmFWUps=; b=K0pojWgWufPBgMYOuEJVn6XCLSFPSXYjnjp8lfpDI4C9mV00Pjg3HCxSuzv8hDNy4B oOf1TE1M0tFe4Mfgqb6livbMGIOdZNgGvR9gO4x3oFfxHv5OxmfH1Mg/uzffbdmwK97a w8Uz5RXQDiXyu/EZY+Vv4hKIrpMDfNgNAwT3RpgzpLcvvFzZnp03W5oyHb0fPXxhbihT oZoyHYgT9PP2NSJo2Vae4s1GLx7B4y7p0NU3px9CbXCegqjrWo4tpK2kEooQv1ew8iNr wV4NGxK4kLsUn+031Jy9LpsfojKVFDl6k67ut66ob26YX8Giz1G+3W2XtjKqgqWnMEVF nPkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722514056; x=1723118856; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=MWpJKDgAAIKrTcDAo93c65X2vfalBn4m5VoyMmFWUps=; b=taw2cCxmWhUK/qyY/LyH9r0CFDZggq4U9QvxjVjU+rMKtizrSVz43oqh6J9oSBInX6 xiAWvGBFdGcbxmrbq/XWfXfWTbZMVW/XtiWb4viFPaVG2LpV2VP0ctsEZbzJ7mmkz0ED JKp/GpGAkykHzupZo0/MPd4pmCnKnsNzBbI+SHd5vhgBFth4KvPAJQnlxlAIYfPnUC73 JkeObvhS231izwOC8VgxgD/MpOMjOyDJBQV0Go78PaSVI34HFAvYJTxa8GJApm7PPYQA 6XFAK+9S55Qh9I//pjqT2WfCIOCHo27ny5AA3Kez763/0XpOEWUMTHqPzfpXJxNcg2cR zD6w== X-Forwarded-Encrypted: i=1; AJvYcCXctNIKz6GZjz5BxiWTfWyl6A/PRKn+I+DhshvMfnhw+amkN1V6XkD3SGY1WYjuxw73RzZh1hPKmdpAjXtmKnIEMtbp/6DgVDBYx7VhxEhauw9sZpM= X-Gm-Message-State: AOJu0YwWc4ffKU6K3b1fO4WGeu4u42nqG5Z/c6bvcnP8Ex7zKFjgQ9RB usRRbgiEQMHPcEeAWZExF5AQYAnVLpx0nx/7absXB2/JqHd99ije X-Google-Smtp-Source: AGHT+IFZNbzC+YgMBit6OkJtk5dwL+OfkqPRYepqPLnWQLve6sNGUlEaHfkxbiBTBghGKCIoMPkEbw== X-Received: by 2002:a2e:7011:0:b0:2ef:18ae:5cc2 with SMTP id 38308e7fff4ca-2f15aac0e67mr168391fa.21.1722514055505; Thu, 01 Aug 2024 05:07:35 -0700 (PDT) Received: from localhost (host-82-58-19-206.retail.telecomitalia.it. [82.58.19.206]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36b36857d4fsm19471856f8f.86.2024.08.01.05.07.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Aug 2024 05:07:35 -0700 (PDT) From: Matteo Martelli Date: Thu, 01 Aug 2024 14:07:19 +0200 Subject: [PATCH v2] ASoC: sunxi: sun4i-i2s: fix LRCLK polarity in i2s mode MIME-Version: 1.0 Message-Id: <20240801-asoc-fix-sun4i-i2s-v2-1-a8e4e9daa363@gmail.com> X-B4-Tracking: v=1; b=H4sIAHZ6q2YC/0XMQQ6DIBCF4auYWXcMIFrpqvdoukAcdRIRA7RpY rx7STddfsl7/wGJIlOCW3VApDcnDluBulTgFrvNhDwWgxJKi2sj0abgcOIPptemGVklNFaYcRi kbt0E5bhHKoNf9PEsnmLwmJdI9p9qlZFadG1fy16YrkOF3uZMwduYaV25uc/e8lq74OE8v0Law /CoAAAA To: Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Maxime Ripard , =?utf-8?b?Q2zDqW1lbnQgUMOpcm9u?= , Marcus Cooper Cc: linux-sound@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Matteo Martelli X-Mailer: b4 0.14.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240801_050738_437873_0161133E X-CRM114-Status: GOOD ( 25.67 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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: dd657eae8164 ("ASoC: sun4i-i2s: Fix the LRCK polarity") Fixes: 73adf87b7a58 ("ASoC: sun4i-i2s: Add support for H6 I2S") Signed-off-by: Matteo Martelli --- Note that this fix applies for all the i2s controllers compatible with sun8i-h3-i2s and sun50i-h6-i2s. The issue had likely been introduced in commit dd657eae8164 ("ASoC: sun4i-i2s: Fix the LRCK polarity") due to a misinterpreted bit from the H3 Datasheet (see link below). Then also introduced for the H6 SoC in commit 73adf87b7a58 ("ASoC: sun4i-i2s: Add support for H6 I2S"). Datasheets: - H3: https://linux-sunxi.org/images/4/4b/Allwinner_H3_Datasheet_V1.2.pdf section 8.6.7.2 - H6: https://linux-sunxi.org/images/4/46/Allwinner_H6_V200_User_Manual_V1.1.pdf section 7.2.5.2 - A64: https://linux-sunxi.org/images/b/b4/Allwinner_A64_User_Manual_V1.1.pdf section 7.6.7.2 Tests: - Tested capture and playback in all supported formats (I2S, LEFT_J, RIGHT_J, DSP_A and DSP_B) in both normal and frame-inverted mode. - Test setup: SoC DAI as master, ES8311 codec as slave. Bus monitored using a logic analyzer. - Tested on A64 SoC (Pine64 board). - Tested on H3 SoC (NanoPi Neo Air board). - Not tested on H6 SoC. --- Changes in v2: - Fix LRCLK inversion mask: used BCLK mask by mistake in previous version - Re-add clock word in DAI polarity comment - Update patch description with a note about recent tests and datasheets - Remove separate cover letter - Rebase on top of asoc/for-next - Link to v1: https://lore.kernel.org/all/20240529140658.180966-2-matteomartelli3@gmail.com --- sound/soc/sunxi/sun4i-i2s.c | 143 ++++++++++++++++++++++---------------------- 1 file changed, 73 insertions(+), 70 deletions(-) --- base-commit: ead2709553e9182c196c232100058ac008981cf1 change-id: 20240731-asoc-fix-sun4i-i2s-9a09dbb145cf Best regards, diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 5f8d979585b6..3af0b2aab291 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) #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; - /* - * 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; @@ -805,6 +777,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: @@ -836,65 +837,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; @@ -912,6 +885,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: