All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Stephen Boyd <sboyd@kernel.org>
Cc: u.kleine-koenig@pengutronix.de,
	Michael Turquette <mturquette@baylibre.com>,
	Simon South <simon@simonsouth.net>,
	linux-clk@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH] clk: Warn when clk_get_rate is called for a disabled clk
Date: Sat, 13 Feb 2021 16:54:06 +0000	[thread overview]
Message-ID: <20210213165406.GQ1463@shell.armlinux.org.uk> (raw)
In-Reply-To: <161317887654.1254594.6078241024095194891@swboyd.mtv.corp.google.com>

On Fri, Feb 12, 2021 at 05:14:36PM -0800, Stephen Boyd wrote:
> Quoting Uwe (2021-02-11 00:05:09)
> > Hello Stephen,
> > 
> > On Wed, Feb 10, 2021 at 07:14:09PM -0800, Stephen Boyd wrote:
> > 
> > > What problem are you trying to address? Is there some issue you've
> > > encountered in the kernel that would have been fixed by having this
> > > warning?
> > 
> > The warning obviously doesn't fix anything. My eventual goal is to
> > answer the question in the initial mail in this thread. The motivating
> > situation is: Should I continue to tell patch authors who use
> > clk_get_rate() that they have to ensure that the given clk must be
> > enabled as the documentation suggests? And if yes: Can we please check
> > this automatically (e.g. with my patch or by returning 0 for a disabled
> > clk) and don't rely on human review to adhere to this rule.
> > 
> 
> I suggest to stop telling folks that they must enable the clk before
> getting the rate. The documentation says
> 
>  "This is only valid once the clock source has been enabled"
> 
> which is really ambiguous. What is "this?" supposed to be?

Please Cc me on CLK API matters, as per my entry in MAINTAINERS.

The subject is the clk_get_rate() function, and as it is in the
section describing that function, I don't see it is ambiguous.

From what I remember, the restriction came after some discussion,
and the problem that on some platforms, the clock tree would not
be known prior to the clock being enabled (at that time, which
was before the "prepare" stuff got added.) Consequently, because
the child/parent relationships were not known, and PLLs were not
initialised, the rate that a particular clock would be ticking
could be different before and after it being enabled.

For this reason, it was decided that the only sensible approach
was to declare that the return value of clk_get_rate() on a
disabled clock is undefined.

That said, finding the discussion is proving difficult, so I may
be misremembering.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2021-02-13 16:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-21  9:27 clk_get_rate for disabled clks Uwe Kleine-König
2020-12-21 12:57 ` Marc Kleine-Budde
2021-01-13  8:30 ` [PATCH] clk: Warn when clk_get_rate is called for a disabled clk Uwe Kleine-König
2021-02-11  3:14   ` Stephen Boyd
2021-02-11  8:05     ` Uwe Kleine-König
2021-02-13  1:14       ` Stephen Boyd
2021-02-13 16:54         ` Russell King - ARM Linux admin [this message]
2021-11-12  8:34           ` Uwe Kleine-König
2021-11-12  8:49             ` Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210213165406.GQ1463@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=kernel@pengutronix.de \
    --cc=linux-clk@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    --cc=simon@simonsouth.net \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.