From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E3E2B351C24 for ; Thu, 16 Apr 2026 21:35:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776375330; cv=none; b=kA18sjsWpKmFlIh8Ir1HL0McUKcJN95o5/KOoJuUKupFhkegQgo6R8ZdQLF4u8yPWGimTJb2zWSiZgOzs5rXXyVTRyH/LPjZKV7crl0xNu6l3K7acGuHSb7bEcvPunyGTj20ubD1hWi4h2eu0JB61eFOarOwwLC74Y3Lxm+EAjA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776375330; c=relaxed/simple; bh=bQYDWw2yWRrPDsipu0p8v7TVMWjI3sci/lJJ3WtUqI8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qkj05rr1mLCy1z/eXghGvkaEXvrma8AvNuJc7v7a8BlsuV8zAkFck6SBJ34V5bFxleqYQfHTXCUfQbNtVOm1mcU+X1E7Z4MAQJMLuBwrVotBEe3H8jJI01zLMvCowZ6j6tHyAhggub52xR8W1rNy0lv+yyZvK2SwKLzKoWCoaTk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=YTXiTz/L; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=LjpGSOO7; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="YTXiTz/L"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="LjpGSOO7" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776375328; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YRG/q659xesZWxPYzzEVSdkuUOn2ZHM5fkzo4uR4sjQ=; b=YTXiTz/L0MEDLvvTswp7cdzl6Md2GKMfKTaAj0t0XtwacvlNs9qTOUY/PdUnqtjrnRFMQ4 gZhh6h6V5TEgDyNL0o+ojd5rnQW2Udrv2LwLk5rOVegRM5q2ZAyhPR5puugaHa59AVwB51 SHj8XqYosF3DFlDxb11SW6/wTw70g5E= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-395-XINSL3upMpWPimTX_fzlWQ-1; Thu, 16 Apr 2026 17:35:26 -0400 X-MC-Unique: XINSL3upMpWPimTX_fzlWQ-1 X-Mimecast-MFC-AGG-ID: XINSL3upMpWPimTX_fzlWQ_1776375326 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-8a14905811cso166335896d6.2 for ; Thu, 16 Apr 2026 14:35:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1776375326; x=1776980126; darn=vger.kernel.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=YRG/q659xesZWxPYzzEVSdkuUOn2ZHM5fkzo4uR4sjQ=; b=LjpGSOO7lKoizq5MRyY8hUfqoLUZgbLwNwTqY+oGo7PKu1FX54r+lDV5lVj0Uw0iNC orpUK5wT+Lx9uqKc55qamSGSW9xZNXH+PltsddwvJfC3GWyk7VvD9xiKtXeNMUmXCBlj JL8R4S5QghbN1p5sR5zd+noBOvzDs3131UA7SlgYG2HyWaXasAKDNBg2YhCt9/fyktoL tm8K5gAEGtWr3sM32D2uApXdTkV2zFiSBVz9DNE3uWz6l+ORomRTI77Vxl6MZsR3cyYB cfJ1aLOiZ2azEKn9bkYzk10eM5yRhbAkhCfl/cc++6kJ7snnN+JrtqdBhvPsOFIBTPXk NQag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776375326; x=1776980126; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=YRG/q659xesZWxPYzzEVSdkuUOn2ZHM5fkzo4uR4sjQ=; b=LrgBqgHoxgv8s3OEb29AWVikf/4nI7LWQMod/TpQg0KRsdiBsMbmaB75IpGV4zqV1w oHgDgLD5RtLf6drxMLyVsyWVZ4JHeejlNdxN2eRvtRXd6u5ABvuZheDumjh2onNwP122 3gQyzvFojpDxMmYDVmHld8m8D8G0faZJSCfn3PDvuJ6WUpl7KgWob66O0CA1PtyaaZYh 6bVy4+0fsAnxDLLHtpwjv6I3bsyVovmDJsVQT6uRgv2UuSooh4eSwZD5SdxMUfGfL9nO vlkm2G5ny/J7juMfcBU3BQt1IjS7kEEIvu5mCcUkH/eAdymLTD9iDPMGlwJYsC+EKTxE vFKw== X-Forwarded-Encrypted: i=1; AFNElJ8+J/Ki6ujwqrk0N4Nw9RkrYDn/lmG8tEz0NopDT5CZjwzATdeYn52M8DspDSYk4hK8L9mBJPfb4k4bp0A=@vger.kernel.org X-Gm-Message-State: AOJu0YzQdwg0n2wnOCbYfwTOLolF48Yq4MoDipxb61zHUBZyL+te5mD6 HRxNSSdJcFUpvBCoE3gZnhR4yue+QyI5/dGsWJxHRm3LtcF32ZKSLI8h918+Gqp3f5m9jItw+pp z1j6qMPg3hJpcllKbdK6oOZECVfjo5hrLbBTj/rge8/1DUQo2bdTbGq9vJvDzhEW5pg== X-Gm-Gg: AeBDieviP2eJ00EVDFs31eUjA5V9ybYRy0pF0J14C5UkAm0euYJKo3gQ294Vx6bwbBJ KbPNd3l6JbmKJeHR6CpaWTiHnSiD0STOsHqnSwqW1xadQ7FCIkTPVaho2Z+IYu9+xjcBBKmo33K JW17cV0nczU1OwAKN5/VCWew1nriRreQB0X4sw/29+FoFl1rbeFM7hYLKIbaAivt1VvMn9Lwkn2 ZHU2vNONRKuRAhd5fC/kQlylLitKCu/6ljL9o72+ei+KCEs74E9UV3uOHqFqhFTlbYKeAUfNhHH BZ3DwdrxgwrjLw8N4TQTdtm967GsZMODqOUUOyh1ELAdO8GSWv+1mylLLbPB7aM6eAJ7wqixsU9 YdQk0xmiwEajAfJefzf6MN5SpVUikbPyC5YzCzGMCwJJP6en90+gTcUrZB5kV+ZrjKS8= X-Received: by 2002:a05:6214:3211:b0:895:498e:e9dd with SMTP id 6a1803df08f44-8b028024c49mr5543686d6.2.1776375326029; Thu, 16 Apr 2026 14:35:26 -0700 (PDT) X-Received: by 2002:a05:6214:3211:b0:895:498e:e9dd with SMTP id 6a1803df08f44-8b028024c49mr5543066d6.2.1776375325583; Thu, 16 Apr 2026 14:35:25 -0700 (PDT) Received: from redhat.com (c-73-183-52-120.hsd1.pa.comcast.net. [73.183.52.120]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-8ae6cda5a2bsm45376176d6.33.2026.04.16.14.35.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Apr 2026 14:35:24 -0700 (PDT) Date: Thu, 16 Apr 2026 17:35:21 -0400 From: Brian Masney To: Prabhakar Cc: Michael Turquette , Stephen Boyd , Geert Uytterhoeven , linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Biju Das , Fabrizio Castro , Lad Prabhakar Subject: Re: [PATCH v2 2/2] clk: divider: Add some kunit test suites Message-ID: References: <20260413124912.3260571-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20260413124912.3260571-3-prabhakar.mahadev-lad.rj@bp.renesas.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260413124912.3260571-3-prabhakar.mahadev-lad.rj@bp.renesas.com> User-Agent: Mutt/2.3.1 (2026-03-20) Hi Lad, On Mon, Apr 13, 2026 at 01:49:12PM +0100, Prabhakar wrote: > From: Lad Prabhakar > > Add KUnit tests to verify clk_divider_bestdiv() returns the maximum > achievable rate when clk_round_rate() is called with ULONG_MAX, which > is the canonical way to probe the maximum rate a clock can produce. > > The first test uses a fixed-rate parent driving a table-based divider > with no div=1 entry. The second test places a two-input mux between > the divider and its root clocks to verify correct parent selection and > that the divider loop does not make redundant calls to > clk_hw_round_rate() for each remaining table entry after the first > overflow. > > Signed-off-by: Lad Prabhakar > --- > drivers/clk/Kconfig | 7 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-divider_test.c | 151 +++++++++++++++++++++++++++++++++ > 3 files changed, 159 insertions(+) > create mode 100644 drivers/clk/clk-divider_test.c > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index cc8743b11bb1..c8f9eaef6f6b 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -573,4 +573,11 @@ config CLK_FD_KUNIT_TEST > help > Kunit test for the clk-fractional-divider type. > > +config CLK_DIVIDER_KUNIT_TEST > + tristate "KUnit tests for clk divider bestdiv" if !KUNIT_ALL_TESTS > + depends on KUNIT Since the clk divider calls writel(), you also will need to unfortunately add: depends on !S390 This is already on CLK_GATE_KUNIT_TEST. For the reason why, look at commit a6c3da03ead11 ("clk: disable clk gate tests for s390") > + default KUNIT_ALL_TESTS > + help > + Kunit test for the clk-divider type. > + > endif > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index a3e2862ebd7e..0c915c6cf3fa 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -20,6 +20,7 @@ clk-test-y := clk_test.o \ > kunit_clk_assigned_rates_zero_consumer.dtbo.o \ > kunit_clk_hw_get_dev_of_node.dtbo.o \ > kunit_clk_parent_data_test.dtbo.o > +obj-$(CONFIG_CLK_DIVIDER_KUNIT_TEST) += clk-divider_test.o > obj-$(CONFIG_COMMON_CLK) += clk-divider.o Swap the order of these two lines above for consistency with the clk-fixed-rate and clk-gate tests where the actual implementation is first, and then the test. > obj-$(CONFIG_COMMON_CLK) += clk-fixed-factor.o > obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o > diff --git a/drivers/clk/clk-divider_test.c b/drivers/clk/clk-divider_test.c > new file mode 100644 > index 000000000000..3a5e3adccb2e > --- /dev/null > +++ b/drivers/clk/clk-divider_test.c > @@ -0,0 +1,151 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * KUnit tests for clk_divider_bestdiv() > + */ > +#include > +#include > +#include > +#include > +#include > + > +#define PARENT_RATE_1GHZ GIGA > +#define PARENT_RATE_2GHZ (2 * GIGA) > +#define PARENT_RATE_4GHZ (4 * GIGA) > + > +static u32 fake_reg_a, fake_reg_b; Right now this limits this to one implementation. Put these in a structure and use kunit_kzalloc() so that there can be multiple, and the runner can execute them in parallel. > + > +static const struct clk_div_table no_div1_table[] = { > + {0, 2}, > + {1, 4}, > + {2, 8}, > + {0, 0}, > +}; You can pass NULL for the table to simplify this code further. I don't see where you are testing anything special related to the table. I think you'll need to pass CLK_DIVIDER_ONE_BASED to the flags when you create the divider if you use a NULL table. > + > +static void unregister_fixed_rate(void *hw) > +{ > + clk_hw_unregister_fixed_rate(hw); > +} > + > +static void unregister_divider(void *hw) > +{ > + clk_hw_unregister_divider(hw); > +} > + > +static void unregister_mux(void *hw) > +{ > + clk_hw_unregister_mux(hw); > +} > + > +/* > + * Test that clk_round_rate(clk, ULONG_MAX) returns the maximum achievable > + * rate for a divider clock. > + */ > +static void clk_divider_bestdiv_ulong_max_returns_max_rate(struct kunit *test) > +{ > + struct clk_hw *parent_hw, *div_hw; > + unsigned long rate; > + > + parent_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-parent", > + NULL, 0, PARENT_RATE_1GHZ); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw); > + kunit_add_action(test, unregister_fixed_rate, parent_hw); You can put clk_hw_unregister_fixed_rate() in the call here, and then drop unregister_fixed_rate(). There's some cases of this below as well. Check the return value of kunit_add_action() here and below as well. > + > + fake_reg_a = 0; > + div_hw = clk_hw_register_divider_table(NULL, "bestdiv-div", > + "bestdiv-parent", > + CLK_SET_RATE_PARENT, > + (void __iomem *)&fake_reg_a, You'll need __force for the cast for sparse as well. > + 0, 2, 0, no_div1_table, NULL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw); > + kunit_add_action(test, unregister_divider, div_hw); Same here... you can just put clk_hw_unregister_divider() here and drop the function above. > + > + /* > + * ULONG_MAX is the canonical way to probe the maximum rate a clock > + * can produce. With a parent at 1 GHz and the smallest table divider > + * being 2, the expected maximum is 500 MHz. > + * > + * Before the fix this returned 125 MHz (PARENT_RATE / 8), the > + * minimum rate, because the search loop was bypassed entirely. The "Before the fix" comment should go in the commit log. The comment in the code should describe how the code is right now. > + */ > + rate = clk_hw_round_rate(div_hw, ULONG_MAX); > + KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_1GHZ / 2); > +} > + > +/* > + * Test that clk_round_rate(clk, ULONG_MAX) returns the correct maximum rate when > + * a mux clock sits between a divider and its parent candidates. > + * > + * Topology: > + * > + * [fixed 4 GHz] --\ > + * +--> [mux CLK_SET_RATE_PARENT] --> [div {2,4,8} CLK_SET_RATE_PARENT] > + * [fixed 2 GHz] --/ > + * > + */ > +static void clk_divider_bestdiv_mux_ulong_max_returns_max_rate(struct kunit *test) > +{ > + static const char *const mux_parents[] = { > + "bestdiv-mux-parent-a", > + "bestdiv-mux-parent-b", > + }; > + struct clk_hw *parent_a_hw, *parent_b_hw, *mux_hw, *div_hw; > + unsigned long rate; > + > + /* Higher-rate parent: the mux should select this for ULONG_MAX. */ > + parent_a_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-a", > + NULL, 0, PARENT_RATE_4GHZ); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_a_hw); > + kunit_add_action(test, unregister_fixed_rate, parent_a_hw); > + > + /* Lower-rate parent: should not be selected. */ > + parent_b_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-b", > + NULL, 0, PARENT_RATE_2GHZ); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_b_hw); > + kunit_add_action(test, unregister_fixed_rate, parent_b_hw); > + > + /* > + * 1-bit mux register selects between the two parents. > + * CLK_SET_RATE_PARENT allows the divider's rate request to > + * propagate into clk_mux_determine_rate(). > + */ > + fake_reg_a = 0; > + mux_hw = clk_hw_register_mux(NULL, "bestdiv-mux", > + mux_parents, ARRAY_SIZE(mux_parents), > + CLK_SET_RATE_PARENT, > + (void __iomem *)&fake_reg_a, > + 0, 1, 0, NULL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mux_hw); > + kunit_add_action(test, unregister_mux, mux_hw); You can put clk_hw_unregister_mux() here and drop this function above. > + > + fake_reg_b = 0; > + div_hw = clk_hw_register_divider_table(NULL, "bestdiv-mux-div", > + "bestdiv-mux", > + CLK_SET_RATE_PARENT, > + (void __iomem *)&fake_reg_b, > + 0, 2, 0, no_div1_table, NULL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw); > + kunit_add_action(test, unregister_divider, div_hw); > + > + /* > + * Expected maximum: mux selects the 4 GHz parent, divider applies > + * the smallest table entry (2): 4 GHz / 2 = 2 GHz. > + */ > + rate = clk_hw_round_rate(div_hw, ULONG_MAX); > + KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_4GHZ / 2); > +} > + > +static struct kunit_case clk_divider_bestdiv_test_cases[] = { > + KUNIT_CASE(clk_divider_bestdiv_ulong_max_returns_max_rate), > + KUNIT_CASE(clk_divider_bestdiv_mux_ulong_max_returns_max_rate), > + {} > +}; Usually I'd ask for a few other tests for basic functionality to be added rather than just testing the maximum. However there's actually some stuff broken with the existing dividers and the clk core where a clk can change the rate of it's siblings. I have a series to address this at: https://lore.kernel.org/linux-clk/20260327-clk-scaling-v8-0-86cd0aba3c5f@redhat.com/ I think the tests that you have are fine. Stephen: If you have time after the merge window closes I'd appreciate it if you could take time to provide feedback about that series. Puss in Boots please... :) https://media2.giphy.com/media/v1.Y2lkPTc5MGI3NjExaXg5cGFhbGdmbTZjdnRkdWs4aXM5d3FlYnFmbTNudWFsZ3Fwc3o5NiZlcD12MV9pbnRlcm5hbF9naWZfYnlfaWQmY3Q9Zw/qUIm5wu6LAAog/giphy.gif Brian