From mboxrd@z Thu Jan 1 00:00:00 1970 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 AD3CFCE79AB for ; Wed, 20 Sep 2023 07:51:17 +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:References:In-Reply-To: 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: List-Owner; bh=WimE4xVa+7f9AZkdJnuQZFQZFEgbHuS35bc2qzDzPpQ=; b=HOZkGawBH4Uzum dIsg21+jyyFoyyBzitxpDpKaCqKROSILeZ9rgst43XeF3BUH1pPdDyNl/cBeo8yCK+PPALEDQ2RfZ hM9ujR21hic2IaEos47MbHohm+pfALgPKl36VOY1kVeIuZFqapPE53OU4jH4ZwRDn9ATxLoJQbjoA diwOQFBRgUUxB7RGrulnl2yK6PAXaF01y1v9mDVd060GeqySXG07bseFaJMVB2wAFAwvEWjpSEwg5 alEVLFbWv/3PTw+ArnG3F3yQOxqdji9EflJpWyzVH1HAZ58QgwOEl6ZoGKugNjfELyW7xPeOIfMRe 7760zha4nijzMPV8tVFA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qirz0-002BhZ-0A; Wed, 20 Sep 2023 07:50:50 +0000 Received: from mail-wr1-x434.google.com ([2a00:1450:4864:20::434]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qiryx-002Bfx-1U for linux-arm-kernel@lists.infradead.org; Wed, 20 Sep 2023 07:50:48 +0000 Received: by mail-wr1-x434.google.com with SMTP id ffacd0b85a97d-31fe3426a61so5452288f8f.1 for ; Wed, 20 Sep 2023 00:50:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695196244; x=1695801044; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=HlzHTfHgS7DIEzzPJ69/Nl7roPtenVnlUobepr30yTw=; b=V8T92g8Apij5v5Kk+hh0y0sw6i7+Efa4FDAasqgrbl0bdwMiBwATGpZQzfbrAWfNC1 YdutO4+HSvOJo9zmLBS43s2SDggOYwunJiIkQnn754f6ssZpGZarv5pKqPfvg5v0B2ht L4+DjSnZ+oCUkLkD1O+K52E34ote64i5YgNbRKVEBW3BVeghe/nLAKSjKEvgc99NRNUx aoYMn2+RrpCMI1b4wIFctjWoCBeRSzEjVX3WttSMfOVPA6EjcMdH3zhNxyHPXsKzSma+ yMpYb5sVXmFWD6O+Tzj19dqSEVHpVl5QaLi7gD6BLhNPJZJi7N1yxZLFjLO8du0sdq+C JnOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695196244; x=1695801044; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HlzHTfHgS7DIEzzPJ69/Nl7roPtenVnlUobepr30yTw=; b=OHhp+uWo3yRlk5ebOA6WAiATRja2+lwEq8bJVq14sVCgX0m/VbTwO53WaGMFjgIxBG c31XRsR93F7SfK6bnUP/rcdjz4VtXtZHmw8HXGSfzKtNL1fQqvbYrn4DaVQpEAWAx6ih 2rZPTuo63OUhbSl8AMOzq7RF5qWZ459zpM5VoLd9jvsTmT03kMLd48XJi0tbI3AwrToT NXuVoJmgQ8JJLvGaSgKUpgl73bQN67qD0oL4r9O1BIqLc7jIU7+hqnvKqPN5wDZGWrdf cGLQXEIE2axpeGnFftqZn4fZB1qSKVzaywGgurwzsL+zgnFW9fL+0V9P3yCAoQhAbvUt 1Lmg== X-Gm-Message-State: AOJu0YzcmsbRPGhHfGG8UJOzngLg/ZmxCFE4XUzmQd3iU2LksZ+pUbMZ i6mGypTgXpV2DScUxA9yYhE= X-Google-Smtp-Source: AGHT+IEEmHXQCsShaamGnvoU38VzYpHCM+Fifd+ZtlaDpeigN7nlVyFwbMp3MQeBupf+fI/pxclJFg== X-Received: by 2002:adf:ee88:0:b0:314:1b36:f440 with SMTP id b8-20020adfee88000000b003141b36f440mr1457739wro.70.1695196244358; Wed, 20 Sep 2023 00:50:44 -0700 (PDT) Received: from PCBABN.skidata.net ([91.230.2.244]) by smtp.gmail.com with ESMTPSA id q11-20020adff50b000000b0031c71693449sm17768372wro.1.2023.09.20.00.50.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Sep 2023 00:50:44 -0700 (PDT) From: Benjamin Bara To: mripard@kernel.org Cc: abelvesa@kernel.org, bbara93@gmail.com, benjamin.bara@skidata.com, conor+dt@kernel.org, devicetree@vger.kernel.org, festevam@gmail.com, frank@oltmanns.dev, kernel@pengutronix.de, krzysztof.kozlowski+dt@linaro.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-imx@nxp.com, linux-kernel@vger.kernel.org, linux@armlinux.org.uk, mturquette@baylibre.com, peng.fan@nxp.com, robh+dt@kernel.org, s.hauer@pengutronix.de, sboyd@kernel.org, shawnguo@kernel.org Subject: Re: [PATCH 05/13] clk: keep track of the trigger of an ongoing clk_set_rate Date: Wed, 20 Sep 2023 09:50:37 +0200 Message-Id: <20230920075037.1737982-1-bbara93@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230920_005047_498927_DF6EA5CB X-CRM114-Status: GOOD ( 35.18 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi! On Tue, 19 Sept 2023 at 09:06, Maxime Ripard wrote: > On Mon, Sep 18, 2023 at 12:40:01AM +0200, Benjamin Bara wrote: > > From: Benjamin Bara > > > > When we keep track of the rate change trigger, we can easily check if an > > affected clock is affiliated with the trigger. Additionally, the trigger > > is added to the notify data, so that drivers can implement workarounds > > that might be necessary if a shared parent changes. > > > > Signed-off-by: Benjamin Bara > > --- > > drivers/clk/clk.c | 12 ++++++++++++ > > include/linux/clk.h | 2 ++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index 4954d31899ce..8f4f92547768 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -33,6 +33,9 @@ static struct task_struct *enable_owner; > > static int prepare_refcnt; > > static int enable_refcnt; > > > > +/* responsible for ongoing rate change, protected by prepare_lock */ > > +static struct clk *rate_trigger_clk; > > + > > static HLIST_HEAD(clk_root_list); > > static HLIST_HEAD(clk_orphan_list); > > static LIST_HEAD(clk_notifier_list); > > @@ -1742,6 +1745,7 @@ static int __clk_notify(struct clk_core *core, unsigned long msg, > > > > cnd.old_rate = old_rate; > > cnd.new_rate = new_rate; > > + cnd.trigger = rate_trigger_clk ? : core->parent->hw->clk; > > > > list_for_each_entry(cn, &clk_notifier_list, node) { > > if (cn->clk->core == core) { > > @@ -2513,6 +2517,8 @@ int clk_set_rate(struct clk *clk, unsigned long rate) > > /* prevent racing with updates to the clock topology */ > > clk_prepare_lock(); > > > > + rate_trigger_clk = clk; > > + > > So I don't think that interacts very well with the clk_hw_set_rate > function you introduced. It looks like you only consider the initial > clock here so you wouldn't update rate_trigger_clk on a clk_hw_set_rate > call, but that creates some inconsistencies: > > - If we call clk_hw_set_rate outside of the set_rate path (but in > .init for example), then we end up with a notifier without a trigger > clock set. > > - More generally, depending on the path we're currently in, a call to > clk_hw_set_rate will notify a clock in different ways which is a bit > weird to me. The trigger clock can also be any clock, parent or > child, at any level, which definitely complicates things at the > driver level. > > The rate propagation is top-down, so could be get away with just setting > the parent clock that triggered the notification? As I mentioned in the other response, this implementation seems to be just a hack to get additional context in the notifier. I think that's also a problem Frank had in his approach. Inside the notifier, it's not clear what to do with the incoming change. Because it could be either "intended", meaning a sub-clock of the current clock has triggered the change, or "unintended" (e.g. a sibling has triggered the change, but the subtree beyond the current clock still requires the old rate, and therefore the clock needs to adapt). Therefore I think if we use req_rate here, we might be able to achieve the same thing in a better way. Thanks! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel