From: Stephen Boyd <sboyd@codeaurora.org>
To: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Russell King <linux@arm.linux.org.uk>,
Mike Turquette <mturquette@linaro.org>,
Paul Walmsley <paul@pwsan.com>, Tony Lindgren <tony@atomide.com>,
linux-omap@vger.kernel.org,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Subject: Re: [PATCH 1/2] clk: Make clk API return per-user struct clk instances
Date: Mon, 06 Oct 2014 12:31:45 -0700 [thread overview]
Message-ID: <5432EE21.7030906@codeaurora.org> (raw)
In-Reply-To: <CAAObsKDa=ia3Gbu1WkoUBjXt2GCoysK7rv=3PzLarnfS7K5ZLA@mail.gmail.com>
On 10/06/2014 10:14 AM, Tomeu Vizoso wrote:
>> This is the end goal. I understand that the provider API is sort
>> of a mess with us allowing drivers to use the underscore and
>> non-underscore functions and the mixture of struct clk and struct
>> ckl_hw throughout.
>>
>> struct clk_hw <--> struct clk_core <----> struct clk
>> \-> struct clk
>> |-> struct clk
> Agree this is how it should look like at some point, but for now I
> need a reference to struct clk from struct clk_hw, so providers can
> keep using the existing API. This reference would be removed once they
> move to the new clk_hw-based API.
Ok sounds like we're on the same page.
>>> +struct clk *__clk_create_clk(struct clk_core *clk_core, const char *dev_id,
>>> + const char *con_id);
>>> +#endif
>>> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
>>> index da4bda8..fe3712f 100644
>>> --- a/drivers/clk/clkdev.c
>>> +++ b/drivers/clk/clkdev.c
>>> @@ -168,14 +172,27 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
>>> struct clk *clk_get_sys(const char *dev_id, const char *con_id)
>>> {
>>> struct clk_lookup *cl;
>>> + struct clk *clk = NULL;
>>>
>>> mutex_lock(&clocks_mutex);
>>> cl = clk_find(dev_id, con_id);
>>> - if (cl && !__clk_get(cl->clk))
>>> - cl = NULL;
>>> + if (cl) {
>>> +#if defined(CONFIG_COMMON_CLK)
>>> + clk = __clk_create_clk(cl->clk->core, dev_id, con_id);
>>> + if (clk && !__clk_get(clk)) {
>>> + kfree(clk);
>> This looks weird. It makes me suspect we've failed to reference
>> count something properly. Can we avoid this?
> Can you extend on this? But I see how the behaviour doesn't match the
> previous one because cl should be NULLed when __clk_get fails. I have
> fixed this.
It triggers my "that's not symmetric filter" because it requires the
caller to free something allocated by the callee. Do we still need
__clk_get() to be called in the common clock path? Why not just do the
stuff we do in __clk_get() in __clk_create_clk()? Then if that fails we
can return an error pointer indicating some sort of failure (-ENOENT?)
and we don't need to do any sort of cleanup otherwise.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
next prev parent reply other threads:[~2014-10-06 19:31 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-02 13:04 [PATCH 0/2] Per-user clock constraints Tomeu Vizoso
2014-10-02 13:04 ` [PATCH 1/2] clk: Make clk API return per-user struct clk instances Tomeu Vizoso
2014-10-03 23:15 ` Stephen Boyd
2014-10-06 17:14 ` Tomeu Vizoso
2014-10-06 19:31 ` Stephen Boyd [this message]
2014-10-07 15:28 ` Tomeu Vizoso
2014-10-07 18:43 ` Stephen Boyd
2014-10-02 13:04 ` [PATCH 2/2] clk: Add floor and ceiling constraints to clock rates Tomeu Vizoso
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=5432EE21.7030906@codeaurora.org \
--to=sboyd@codeaurora.org \
--cc=javier.martinez@collabora.co.uk \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=mturquette@linaro.org \
--cc=paul@pwsan.com \
--cc=tomeu.vizoso@collabora.com \
--cc=tony@atomide.com \
/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).