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 BF80539E167 for ; Thu, 15 Jan 2026 13:02:23 +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=1768482145; cv=none; b=k8aDEz9x8C/bQMo1lszEnJ2HwCBbxBSzD0HQVztbO2LgOt+pFqy4WnwrDa4U+Cv0NtbAIWy8iDcJdQglW+Lt9Slt4uvBKO/U1e8pGF+NK3POi3/mw+gfAJ5nl0SfmChEwQXg6kiMgNOOJS2KGimespI0P5CkLcibQbGUHaU+Txw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768482145; c=relaxed/simple; bh=A69ZO/7VQGiujVunOWYUggbHdGjMD0FduSfe6mMsudA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZVyHYfDns4GHvHZjN79dpcgfybmrYC93DN5aUTRVjapx1XKTcRhwCU0rhqNhes/7vJB7C36WZXcZWY4iB6+PkIwrmANDmqi7C4DtP4SSiocab013pBntDi4n8RafXsfVAf36on83E4yYpEWBTNX4hcfx7hYRhCxV5GHvsELpQ/o= 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=UxZUlkII; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=TpG7Xuey; 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="UxZUlkII"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="TpG7Xuey" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1768482142; 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=yvng5JA6e+TbWXXcdfj/MzIBnSNMDZIedFcqPEoc/GU=; b=UxZUlkIIWGGCq6FN+qTVR0pyc3WCLWJjBnC2zrUFBeEXNdzIS1RmmAm0ePSbrFuJ7+vKp6 3cnApJb6U0Jflg/1vPBo1seP7x9j4ETTYF5ZPH/D/79gdwAGoAylICUxTfnXyVGcupBSwB dnxIwHx9uPmOGeN6K1EpkBPLTmqodcI= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-602-PXjtZVizPH-Q3oHu4s78ug-1; Thu, 15 Jan 2026 08:02:21 -0500 X-MC-Unique: PXjtZVizPH-Q3oHu4s78ug-1 X-Mimecast-MFC-AGG-ID: PXjtZVizPH-Q3oHu4s78ug_1768482141 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-5014e9d9114so23815441cf.1 for ; Thu, 15 Jan 2026 05:02:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1768482141; x=1769086941; 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=yvng5JA6e+TbWXXcdfj/MzIBnSNMDZIedFcqPEoc/GU=; b=TpG7Xueyuf5j2fd58qXahv6pBuusy2PY2hlGhJkzRaU8rSgwAPpzUiUJ2XKxxpGFLi EjtssYV2xSII12uHfkBh8Ruq5voNpOmpg2uO92VtbdiLDQaSmHvGKRvzaNjhfaLsIYBj dYhZLKaFYwzy5teoOlSSFH8rgwEpG7pRHIFNw/uYnlgxVoqsr9gHi+6JxhR7yZqDGZIG lc4CR+VRpbPIxcNhsW+Iw+SDw45acXfVtb7Zo/BqLGxs9g5RzSAOjfAejpbdX4TpyC52 GNC4rbUewOMb2FqSPBhck9CmGu2QU6jAL5NafM8O8rTsAvVMPgE4VoXNmcGKtHt/h+JI LoaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768482141; x=1769086941; 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=yvng5JA6e+TbWXXcdfj/MzIBnSNMDZIedFcqPEoc/GU=; b=u0VLxKMTSuqNdr/Ym8ho56OBQ7iSbfO9cIL+f7dwSzJMlck/V2p9/5ABAAJhUJtyHi IrftSnbj/QMX+FT7lZSXlQu72cNDRxhgWCdrKbU6QSEheuNNTDq85/zaEXjquDbUyMgc sCmUPXA/+VgWke+bmuf7X5WDnnPv6SXyWAPNxwG8SbJSFCgkjFwrWeL7lKGNLOv7TgX6 EwBPn8HDYeyUW7fiegklxsn7Ibcgz1nIWAJZofVsn+StjF380cc/VSQYehlgSlNqsbbR BuneJvXqoh4cxv8h68mlkeNv8Lz1O8O7zl6kBgUHhpW9h2Obh707BnEVH2ZjkzCTkCYQ sZ+Q== X-Forwarded-Encrypted: i=1; AJvYcCUA9Ksd3gJc5wZvBo2dvtPhIqqTXFi5HkEK/5urr85vHl0HYv9M7VDCS1kjGJpo4TJZtmwy0uS26o6y1HM=@vger.kernel.org X-Gm-Message-State: AOJu0YyWnHaME+/VtFBRdjGOmVmGApapaTrBP/31n42fobETTr+dr/Xr kJOoMyl7H2nBOHP96JC6QXjjwlVkNIrGd3l/hUvJnYbezih+Pz3kV93RtXRtEothZwyrcwbQqJM CNre0BxUQ7JF+fyfulclBU2QwmHfy4Kijg7rcHvbiCUcigEtB6Bjgiio1ycMiX+RQLA== X-Gm-Gg: AY/fxX65J+ZuuTRW20V5TdwvL81MYaUgyqH5+4MxRFHFMw3Fb6F/tUi1pOPVFQX1Twb FLOZ6YzamII1xIuZR4zRZT5lK9PPQBeJ4veFZkYomL3A/5Vvkm7g9t2fcOz373UH7cm91vMZOb1 U6DG7hqoJqBxdJDondLomTbFE54DdxwDZwe7QYeoM6qJTRKpdGbxrPdRMF9xvFDla46MvYdTPg3 C6STp6pxvlirII6gOAFs2kRdeM8J5o5MYWYBGTjy+nlcqdOc6bw9QqL4GMApf4CXwo7eqLrYm0/ sHsrFl9I/7s9HpnlpqVVCOzITeJ8hVhIJroTVv6T9fMAcmzqHlqfJf01OLW0p/Dhj8ReSy1JzhG p5GDYqGpcW0XKir4cseiJ1q1aWK2Mc/4tcGwrpfN/ybIA X-Received: by 2002:a05:622a:4203:b0:4ed:df82:ca30 with SMTP id d75a77b69052e-501481e9360mr76195471cf.13.1768482132120; Thu, 15 Jan 2026 05:02:12 -0800 (PST) X-Received: by 2002:a05:622a:4203:b0:4ed:df82:ca30 with SMTP id d75a77b69052e-501481e9360mr76172571cf.13.1768482100257; Thu, 15 Jan 2026 05:01:40 -0800 (PST) Received: from redhat.com (c-73-183-52-120.hsd1.pa.comcast.net. [73.183.52.120]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8c530be5446sm385631385a.54.2026.01.15.05.01.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Jan 2026 05:01:39 -0800 (PST) Date: Thu, 15 Jan 2026 08:01:38 -0500 From: Brian Masney To: Chuan Liu Cc: Michael Turquette , Stephen Boyd , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] clk: Ensure correct consumer's rate boundaries Message-ID: References: <20260109-fix_error_setting_clk_rate_range-v1-1-bae0b40e870f@amlogic.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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/2.2.14 (2025-02-20) Hi Chuan, On Thu, Jan 15, 2026 at 10:37:55AM +0800, Chuan Liu wrote: > On 1/15/2026 9:30 AM, Brian Masney wrote: > > On Fri, Jan 09, 2026 at 11:24:22AM +0800, Chuan Liu via B4 Relay wrote: > > > From: Chuan Liu > > > > > > If we were to have two users of the same clock, doing something like: > > > > > > clk_set_rate_range(user1, 1000, 2000); > > > clk_set_rate_range(user2, 3000, 4000); > > > > > > Even when user2's call returns -EINVAL, the min_rate and max_rate of > > > user2 are still incorrectly updated. This causes subsequent calls by > > > user1 to fail when setting the clock rate, as clk_core_get_boundaries() > > > returns corrupted boundaries (min_rate = 3000, max_rate = 2000). > > > > > > To prevent this, clk_core_check_boundaries() now rollback to the old > > > boundaries when the check fails. > > > > > > Signed-off-by: Chuan Liu > > > --- > > > drivers/clk/clk.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > index 85d2f2481acf..0dfb16bf3f31 100644 > > > --- a/drivers/clk/clk.c > > > +++ b/drivers/clk/clk.c > > > @@ -2710,13 +2710,17 @@ static int clk_set_rate_range_nolock(struct clk *clk, > > > */ > > > rate = clamp(rate, min, max); > > > ret = clk_core_set_rate_nolock(clk->core, rate); > > > + > > > +out: > > > if (ret) { > > > - /* rollback the changes */ > > > + /* > > > + * Rollback the consumer’s old boundaries if check_boundaries or > > > + * set_rate fails. > > > + */ > > > clk->min_rate = old_min; > > > clk->max_rate = old_max; > > > } > > > > > > -out: > > > if (clk->exclusive_count) > > > clk_core_rate_protect(clk->core); > > > > This looks correct to me. Just a quick question though to possibly > > simplify this further. Currently clk_set_rate_range_nolock() has the > > following code: > > > > /* Save the current values in case we need to rollback the change */ > > old_min = clk->min_rate; > > old_max = clk->max_rate; > > clk->min_rate = min; > > clk->max_rate = max; > > > > if (!clk_core_check_boundaries(clk->core, min, max)) { > > ret = -EINVAL; > > goto out; > > } > > > > Since clk_core_check_boundaries() is a readonly operation, what do you > > think about moving clk_core_check_boundaries above the code that saves the > > previous values? That way we only need to rollback in the case where > > set_rate() fails. > > > > Perhaps it would be more appropriate to move the clk_core_check_boundaries() > check before saving the previous values, like this: > > if (!clk_core_check_boundaries(clk->core, min, max)) { > ret = -EINVAL; > goto out; > } > > /* Save the current values in case we need to rollback the change */ > old_min = clk->min_rate; > old_max = clk->max_rate; > clk->min_rate = min; > clk->max_rate = max; Yes, that's what I had in mind. > The changes in this patch are intended to avoid altering the original driver > execution flow, while making the minimal modification to fix the issue where > the range is incorrectly assigned. It's ultimately up to Stephen what he wants to take. I personally have a slight preference to the approach above, however I don't have a strong opinion about it. I'm just calling this out to help with reviews. The one thing that Stephen will want though is kunit tests for this since it changes the clk core. There's already a bunch of kunit tests in drivers/clk/clk_test.c. Feel free to reach out to me if you need help writing a new test. Brian