linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/2] clk: change clk_ops' ->round_rate() prototype
Date: Wed, 6 May 2015 23:39:53 -0700	[thread overview]
Message-ID: <20150507063953.GC32399@codeaurora.org> (raw)
In-Reply-To: <1430407809-31147-2-git-send-email-boris.brezillon@free-electrons.com>

On 04/30, Boris Brezillon wrote:
> Clock rates are stored in an unsigned long field, but ->round_rate()
> (which returns a rounded rate from a requested one) returns a long
> value (errors are reported using negative error codes), which can lead
> to long overflow if the clock rate exceed 2Ghz.
> 
> Change ->round_rate() prototype to return 0 or an error code, and pass the
> requested rate as a pointer so that it can be adjusted depending on
> hardware capabilities.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Mikko Perttunen <mikko.perttunen@kapsi.fi>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

This patch is fairly invasive, and it probably doesn't even
matter for most of these clock providers to be able to round a
rate above 2GHz. I've been trying to remove the .round_rate op
from the framework by encouraging new features via the
.determine_rate op. Sadly, we still have to do a flag day and
change all the .determine_rate ops when we want to add things.

What if we changed determine_rate ops to take a struct
clk_determine_info (or some better named structure) instead of
the current list of arguments that it currently takes? Then when
we want to make these sorts of framework wide changes we can just
throw a new member into that structure and be done.

It doesn't solve the unsigned long to int return value problem
though. We can solve that by gradually introducing a new op and
handling another case in the rounding path. If we can come up
with some good name for that new op like .decide_rate or
something then it makes things nicer in the long run. I like the
name .determine_rate though :/

The benefit of all this is that we don't have to worry about
finding the random clk providers that get added into other
subsystems and fixing them up. If drivers actually care about
this problem then they'll be fixed to use the proper op. FYI,
last time we updated the function signature of .determine_rate we
broke a couple drivers along the way.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2015-05-07  6:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30 15:30 [PATCH v2 0/2] clk: adapt ->round_rate()/->determine_rate() prototypes Boris Brezillon
2015-04-30 15:30 ` [PATCH v2 1/2] clk: change clk_ops' ->round_rate() prototype Boris Brezillon
2015-05-07  6:39   ` Stephen Boyd [this message]
2015-05-07  7:37     ` Boris Brezillon
2015-05-15 15:40       ` Boris Brezillon
2015-05-16 11:14         ` Mikko Perttunen
2015-05-20  1:01           ` Stephen Boyd
2015-06-04 23:02   ` Paul Walmsley
2015-06-05  8:46     ` Jon Hunter
2015-06-05 11:39       ` Boris Brezillon
2015-06-08  8:46         ` Jon Hunter
     [not found]       ` <20150610030254.6017.98627@quantum>
2015-06-10  7:00         ` Boris Brezillon
2015-06-05 11:38     ` Boris Brezillon
2015-04-30 15:30 ` [PATCH v2 2/2] clk: change clk_ops' ->determine_rate() prototype Boris Brezillon

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=20150507063953.GC32399@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).