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.133.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 90FE03C9ECE for ; Fri, 20 Mar 2026 14:44:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774017859; cv=none; b=erDb7WCkeDJaNn8uT/UGcJq1G92vrLTyu86Ki7AHZLMbRLrFLrq7sUdmVyTzNoqYUJn0aC1Mg3poHGQLumXNOiKY8joZpSKyWdVP/vMP0TzmifJLR6lHDSYqbxDa1Xku2HAtinF3AWj50mf6mvWFtzP0Z1pYr/aXQbPgdsINdog= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774017859; c=relaxed/simple; bh=jeHzfq9gYIUk5jAxaykyy4WmrX69AwNpx2AR/murGTg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tUuWDq7UQTVRzZTPZ7Jx//2uZaZgpBjxU09uyCMY9wAcMRq1gXD14d/cdSZFY6LkYkYxn3aKNlkYPU2o6wh65DxaFbLXgOrnCTKKUMsvSu7Klp5+EQgtbkdA07Xz58qKoPaPe/JPMrpCBFRBDYJui6u4OfwQWYF8PYvDPOFDfLg= 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=OGQz6xss; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=FxHZFPXi; arc=none smtp.client-ip=170.10.133.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="OGQz6xss"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="FxHZFPXi" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1774017856; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6ghmqrm9W8jJunYiq3s7F5CdRqev6baYKcblb4T9Ru8=; b=OGQz6xssFcim9HwKyXxq6IhP8jTXU+Vr33XDkbebMLrr16FoUxuZIBzHRp1oBsuW9eTk8u UY/XlojEv0j0Tx486R9yqIwzSHxxG0hMmy2KM5HTDmNmWGb+mKhXKNxffYI3ArujPcr+2b OZt3ktnkw38IlX0SWvte60xkvoltkUg= Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-538-S7MG0J7qN5W57CEyWEJ4zg-1; Fri, 20 Mar 2026 10:44:14 -0400 X-MC-Unique: S7MG0J7qN5W57CEyWEJ4zg-1 X-Mimecast-MFC-AGG-ID: S7MG0J7qN5W57CEyWEJ4zg_1774017851 Received: by mail-qv1-f72.google.com with SMTP id 6a1803df08f44-899eacb58a5so66124506d6.1 for ; Fri, 20 Mar 2026 07:44:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1774017851; x=1774622651; darn=vger.kernel.org; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=6ghmqrm9W8jJunYiq3s7F5CdRqev6baYKcblb4T9Ru8=; b=FxHZFPXiJSnJojzFlSbNbRFsYA7qL3Co/daYaRsrMfTXB4YHlEMUWLo9afNMaWxVFz XIaZ0zQrd/Xu81ra0vlw6GWEEFBXosXh6z2N2L68HlDTfCIn0teJTWdsPmLnhp1qBzhi 7okGLV6KpMiW1KNUW4aVS5tNGPhsLGUSaLvkQcC3nlsBLAp09quEpy0YnM49tYGrPR4K p/2B8NsoibL6ariMZacoqBl/jr4Qw/GopbIgorBzkauiI26qVP9t2obF3pMkTaBZpYdU dQCrwfBS5svdSHkyhiHjUYDTYQDyxk4LnShkC1hVcWxOHUAP4cgnsHlRoJcr2vhVttGq kk1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774017851; x=1774622651; h=user-agent:in-reply-to:content-transfer-encoding :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=6ghmqrm9W8jJunYiq3s7F5CdRqev6baYKcblb4T9Ru8=; b=cAsz8N3WHW8jC9o5deyI3LBYFWoAW3qaK54L68CByWptMakfa+7b1V7pcksxdeHyCF 0IYXTcMIYG5IHWzD+9eCTInMJCJCV3OUODji3M5k/yDIS57wxQyimimVCb5taQufRzwt 9a2zRpUFsdBLJ9zMBeooKqMtNyU2c+p7ykGJHLzF3Kjd4QGY4966MPY/di++4kiQiDzP 5Q4KtOfgTQxJcfbo/J3ln4tbUpo841NaXzdIbcCGVfQBChlda5QxNnXzN6QCviVou+v7 o1enB2iA6mangtPumwjhyaitbEX1spv79b07JQAViWAfN7vmLE3wjSDiRdFqqJcGXZA0 qu6g== X-Forwarded-Encrypted: i=1; AJvYcCU2793n2eIdQSIvn1AGtcOkDS9PqFu1wJ+JS6IyU2vVNxPe+Qosc+2dF3OmZpCUq+a0fFM95S6aUaquEp0=@vger.kernel.org X-Gm-Message-State: AOJu0YywyqUSYSv3v0vwHcU9zcj8igytvc22G/AqqCFgNel2QxrGRJfa MH/kiUzcAhtrZjx/AxQ3MC94Z1HpmJoLLSex2j3mDZtqmvOIleP0KbdURZvl3CHp+AHgYzlXpea mrhlR8MEBCMCc+oew7Z5744hYIhPmp+d/9ZPghXWA5+kTi4ZGbgTI2CP9plv58u935Q== X-Gm-Gg: ATEYQzw0S4cIca46tCBWPT6bK7dD0ezzODbpHKBpilgGDynNMRC+cKiUqTmbGsDPEhD k4fmgZT7oP+sdOUHZVWMd/9cz07flIxRvv7zvcnF6d9DKNZLGVOmUwBnmx2WolUkpsc1tu1ZDub PwDwiS6eslX2QxGEWRUpWoK6RRon4A+GSmHF9A8+tVYa86ZSOKCN8cGXyxcjLtimTy8JDC7yG6Q s90fnv6wsHUMtkLWXgkuagmRyMG/3n7ZUJYol/tOWQMB6GrhprAO70tgw61j1MSeK7FrK99yava k9WVZUNmKO5uGHREMS6bkqGO+sJRsZv60ilgw3YKISglK0aN7nbxY27KKguKen63gDcCD6MqX7J WnyfNtED47iQB7/PlIblA8DGihriIC5O+bp4Ztn5/qsl9+bQt0LlGqaPg X-Received: by 2002:a05:6214:3018:b0:899:fcce:b1f7 with SMTP id 6a1803df08f44-89c85a7e246mr50079926d6.45.1774017850607; Fri, 20 Mar 2026 07:44:10 -0700 (PDT) X-Received: by 2002:a05:6214:3018:b0:899:fcce:b1f7 with SMTP id 6a1803df08f44-89c85a7e246mr50079166d6.45.1774017849900; Fri, 20 Mar 2026 07:44:09 -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-89c85251b04sm20895876d6.17.2026.03.20.07.44.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Mar 2026 07:44:08 -0700 (PDT) Date: Fri, 20 Mar 2026 10:44:07 -0400 From: Brian Masney To: Maxime Ripard Cc: Michael Turquette , Stephen Boyd , Alberto Ruiz , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 5/7] clk: introduce new flag CLK_V2_RATE_NEGOTIATION for sensitive clocks Message-ID: References: <20260313-clk-scaling-v6-0-ce89968c5247@redhat.com> <20260313-clk-scaling-v6-5-ce89968c5247@redhat.com> <20260319-quaint-coati-of-genius-9ffc0c@houat> <20260320-amiable-amazing-armadillo-f48820@houat> <20260320-accomplished-wrasse-of-temperance-5806e6@houat> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260320-accomplished-wrasse-of-temperance-5806e6@houat> User-Agent: Mutt/2.3.0 (2026-01-25) On Fri, Mar 20, 2026 at 03:33:36PM +0100, Maxime Ripard wrote: > On Fri, Mar 20, 2026 at 03:31:41PM +0100, Maxime Ripard wrote: > > On Thu, Mar 19, 2026 at 06:35:56AM -0400, Brian Masney wrote: > > > On Thu, Mar 19, 2026 at 5:35 AM Maxime Ripard wrote: > > > > On Fri, Mar 13, 2026 at 12:43:12PM -0400, Brian Masney wrote: > > > > > As demonstrated by the kunit tests, clk_calc_subtree() in the clk core > > > > > can overwrite a sibling clk with the parent rate. Clocks that are used > > > > > for some subsystems like DRM and sound are particularly sensitive to > > > > > this issue. > > > > > > > > > > I consider this to be a logic bug in the clk subsystem, however this > > > > > functionality has existed since the clk core was introduced with > > > > > commit b2476490ef11 ("clk: introduce the common clock framework"), > > > > > and some boards are unknowingly dependent on this behavior. > > > > > > > > > > Let's add support for a v2 rate negotiation logic that addresses the > > > > > logic error. Clks can opt into this new behavior by adding the flag > > > > > CLK_V2_RATE_NEGOTIATION, or globally on all clks with the kernel > > > > > parameter clk_v2_rate_negotiation. > > > > > > > > > > Link: https://lore.kernel.org/linux-clk/aUSWU7UymULCXOeF@redhat.com/ > > > > > Link: https://lpc.events/event/19/contributions/2152/ > > > > > Signed-off-by: Brian Masney > > > > > --- > > > > > drivers/clk/clk.c | 70 ++++++++++++++++++++++++++++++++++++-------- > > > > > include/linux/clk-provider.h | 3 ++ > > > > > 2 files changed, 60 insertions(+), 13 deletions(-) > > > > > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > > > index 051fe755e3bf1b0a06db254b92f8a02889456db9..64c6de5ff5df2117b8d1aca663d40b41d974bf92 100644 > > > > > --- a/drivers/clk/clk.c > > > > > +++ b/drivers/clk/clk.c > > > > > @@ -886,6 +886,43 @@ unsigned long clk_hw_get_children_lcm(struct clk_hw *hw, struct clk_hw *requesti > > > > > } > > > > > EXPORT_SYMBOL_GPL(clk_hw_get_children_lcm); > > > > > > > > > > +static bool clk_v2_rate_negotiation_enabled; > > > > > +static int __init clk_v2_rate_negotiation_setup(char *__unused) > > > > > +{ > > > > > + clk_v2_rate_negotiation_enabled = true; > > > > > + return 1; > > > > > +} > > > > > +__setup("clk_v2_rate_negotiation", clk_v2_rate_negotiation_setup); > > > > > + > > > > > +/** > > > > > + * clk_has_v2_rate_negotiation - Check if a clk should use v2 rate negotiation > > > > > + * @core: The clock core to check > > > > > + * > > > > > + * This function recursively checks if the clk or any of its descendants have > > > > > + * the CLK_V2_RATE_NEGOTIATION flag set. The v2 behavior can also be enabled > > > > > + * globally by adding clk_v2_rate_negotiation to the kernel command line. > > > > > + * > > > > > + * Returns: true if the v2 logic should be used; false otherwise > > > > > + */ > > > > > +bool clk_has_v2_rate_negotiation(const struct clk_core *core) > > > > > +{ > > > > > + struct clk_core *child; > > > > > + > > > > > + if (clk_v2_rate_negotiation_enabled) > > > > > + return true; > > > > > + > > > > > + if (core->flags & CLK_V2_RATE_NEGOTIATION) > > > > > + return true; > > > > > + > > > > > + hlist_for_each_entry(child, &core->children, child_node) { > > > > > + if (clk_has_v2_rate_negotiation(child)) > > > > > + return true; > > > > > + } > > > > > + > > > > > + return false; > > > > > +} > > > > > +EXPORT_SYMBOL_GPL(clk_has_v2_rate_negotiation); > > > > > + > > > > > > > > Do we really need to export it? I'd expect it to be abstracted away for > > > > consumers and providers? > > > > > > This is abstracted away for the consumers, however the provider needs > > > to be aware if it wants to support the v2 logic. Patch 6 of this > > > series that adds support to clk-divider.c uses this export. Well > > > technically clk-divider.c is built with CONFIG_COMMON_CLK, however > > > other clk providers that want to use v2 logic will need this export. > > > > > > The way I see it is that there are provider-specific things that need > > > to change if the v2 logic is used. For instance, the current behavior > > > of clk-divider.c is that when CLK_V2_RATE_NEGOTIATION is set, the > > > parent rate is just set to the new desired child rate, without taking > > > into account any of the sibling rates. We need to keep this behavior > > > for the v1 logic for existing boards. Moving this to the clk core > > > won't work since it doesn't know what kind of clock this is. Maybe > > > some need to compute the LCM, others may need the GCD, some may need > > > something else possibly. Some of the existing providers will need to > > > change to support this. > > > > > > Now if we decided to not support the v1 logic, and just go all on on > > > v2 logic everywhere, then we won't need this export, and can just > > > update the providers. > > > > Yeah... If we take a step back, in your previous version, we were > > reworking the entire rate propagation logic, which was potentially > > pretty hard to test and not easy to do a partial test and opt-in for. > > > > With your current work, you have a clock flag that does the opt-in on a > > clock by clock basis which makes it much easier to enable, and also > > wouldn't create any unforeseen side-effects. > > > > So I'm not sure we need the module parameter now. > > I'd even think it makes it *harder* to test since now you have to test > two combinations for each clock you convert instead of one (with one > being unlikely to happen). It's something that would be easily > overlooked, and would receive less real-world testing as well. That's a good point. The only reason I included the module parameter was for testing purposes. If a board happens to use clk-divider, and has the clk scaling issue, then we could ask them to boot the kernel with this parameter to see if it addresses the problem. I'll omit the kernel parameter from the next version. If we want someone to test this, we can just have them add the clk flag to the relevant part(s) of the clk tree. Brian