From patchwork Mon Aug 7 08:22:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ahmad Fatoum X-Patchwork-Id: 13343181 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 5C5E9EB64DD for ; Mon, 7 Aug 2023 08:22:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To: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=ejAH/sBQPu8FYvhHfnLoLx2I8cnxcNUoPhujIek3d0c=; b=CFIplgfEpI3PMb xCTWa6ueFztiPzOSLAgSM0ESgHfxpHuLhwtTg3jmZwZNvIjWlpr+hHOVX63K0ObgnZAffUNMBZuRU chYir1EZvSiLOwPEHn2mdJgqzKv+ANjEtJOEWq2Dni8948aBWRDHuK8TTvWdPyzcxbBdRjrU1sA1K +Gm0Ux8wZu7wBxD0NH6WKmW4xDh2HBJOxA4Fi5GkEv+jCNJslgwKUw26R3kMlHg5gGlX176Hth+bO kxler7IW6j4R3jrvpQXCWN2uS+3nHiqrJJ/B3NqhvjYHX3YMch7oMzXcRzglu3ZYKnhv1SMG1A5YC u/liNS7pttvHkKYJm6KQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qSvVR-00GQyr-1s; Mon, 07 Aug 2023 08:22:25 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qSvVM-00GQxb-1w for linux-arm-kernel@lists.infradead.org; Mon, 07 Aug 2023 08:22:24 +0000 Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qSvV9-00009U-TV; Mon, 07 Aug 2023 10:22:07 +0200 Received: from [2a0a:edc0:0:1101:1d::54] (helo=dude05.red.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.94.2) (envelope-from ) id 1qSvV8-001hda-UD; Mon, 07 Aug 2023 10:22:06 +0200 Received: from afa by dude05.red.stw.pengutronix.de with local (Exim 4.96) (envelope-from ) id 1qSvV8-009n6M-1f; Mon, 07 Aug 2023 10:22:06 +0200 From: Ahmad Fatoum To: Abel Vesa , Peng Fan , Michael Turquette , Stephen Boyd , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team Cc: Peng Fan , Ahmad Fatoum , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] clk: imx: composite-8m: fix clock pauses when set_rate would be a no-op Date: Mon, 7 Aug 2023 10:22:00 +0200 Message-Id: <20230807082201.2332746-1-a.fatoum@pengutronix.de> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: afa@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230807_012220_635519_5F296818 X-CRM114-Status: GOOD ( 15.69 ) 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 Reconfiguring the clock divider to the exact same value is observed on an i.MX8MN to often cause a longer than usual clock pause, probably because the divider restarts counting whenever the register is rewritten. This issue doesn't show up normally, because the clock framework will take care to not call set_rate when the clock rate is the same. However, when we reconfigure an upstream clock, the common code will call set_rate with the newly calculated rate on all children, e.g.: - sai5 is running normally and divides Audio PLL out by 16. - Audio PLL rate is increased by 32Hz (glitch-free kdiv change) - rates for children are recalculated and rates are set recursively - imx8m_clk_composite_divider_set_rate(sai5) is called with 32/16 = 2Hz more - imx8m_clk_composite_divider_set_rate computes same divider as before - divider register is written, so it restarts counting from zero and MCLK is briefly paused, so instead of e.g. 40ns, MCLK is low for 120ns. Some external clock consumers can be upset by such unexpected clock pauses, so let's make sure we only rewrite the divider value when the value to be written is actually different. Fixes: d3ff9728134e ("clk: imx: Add imx composite clock") Signed-off-by: Ahmad Fatoum Reviewed-by: Peng Fan --- v1 -> v2: - replace glitch with clock pause instead (Peng) - describe how bug triggers in more detail (Peng) --- drivers/clk/imx/clk-composite-8m.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c index cbf0d7955a00..3e9a092e136c 100644 --- a/drivers/clk/imx/clk-composite-8m.c +++ b/drivers/clk/imx/clk-composite-8m.c @@ -97,7 +97,7 @@ static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw, int prediv_value; int div_value; int ret; - u32 val; + u32 orig, val; ret = imx8m_clk_composite_compute_dividers(rate, parent_rate, &prediv_value, &div_value); @@ -106,13 +106,15 @@ static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw, spin_lock_irqsave(divider->lock, flags); - val = readl(divider->reg); - val &= ~((clk_div_mask(divider->width) << divider->shift) | - (clk_div_mask(PCG_DIV_WIDTH) << PCG_DIV_SHIFT)); + orig = readl(divider->reg); + val = orig & ~((clk_div_mask(divider->width) << divider->shift) | + (clk_div_mask(PCG_DIV_WIDTH) << PCG_DIV_SHIFT)); val |= (u32)(prediv_value - 1) << divider->shift; val |= (u32)(div_value - 1) << PCG_DIV_SHIFT; - writel(val, divider->reg); + + if (val != orig) + writel(val, divider->reg); spin_unlock_irqrestore(divider->lock, flags);