From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v12 04/10] clk: use struct clk only for external API
Date: Mon, 22 Sep 2014 19:08:07 +0100 [thread overview]
Message-ID: <20140922180807.GN5182@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1411395353-3189-1-git-send-email-tomeu.vizoso@collabora.com>
On Mon, Sep 22, 2014 at 04:15:47PM +0200, Tomeu Vizoso wrote:
> diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
> index d278572..3b3068b 100644
> --- a/drivers/clk/clk.h
> +++ b/drivers/clk/clk.h
> @@ -9,9 +9,15 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/clk-private.h>
> +
> #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
> struct clk_core *of_clk_get_by_clkspec(struct of_phandle_args *clkspec);
> struct clk_core *__of_clk_get_from_provider(struct of_phandle_args *clkspec);
> void of_clk_lock(void);
> void of_clk_unlock(void);
> #endif
> +
> +#if defined(CONFIG_COMMON_CLK)
> +struct clk *__clk_create_clk(struct clk_core *clk_core);
> +#endif
Why does any of the above need ifdefs?
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index c751d0c..060041b 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -18,6 +18,7 @@
> #include <linux/string.h>
> #include <linux/mutex.h>
> #include <linux/clk.h>
> +#include <linux/clk-private.h>
> #include <linux/clk-provider.h>
> #include <linux/clkdev.h>
> #include <linux/of.h>
> @@ -33,13 +34,13 @@ static DEFINE_MUTEX(clocks_mutex);
I'm not happy with any of these changes, because it introduces a huge
number of ifdefs.
Instead of doing this, please introduce a local typedef in this .c file
which is conditional on whether we're using your new struct clk_core
or not, eg, clkdev_ret_t.
Then, rename all the functions here to be _provider based, and arrange
for these to return clkdev_ret_t.
Then provide the original functions, with your __clk_create_clk() call
to allocate your struct clk. A dummy version which merely returns its
argument can be provided when we're not using clk_core.
This should result in substantially fewer ifdefs - probably just one
ifdef.
Another concern here is what happens when __clk_create_clk() fails.
Your __clk_create_clk() function does nothing in that regard, so any
references which the parent function took are never released.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2014-09-22 18:08 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-22 14:12 [PATCH v12 00/10] Per-user clock constraints Tomeu Vizoso
2014-09-22 14:12 ` [PATCH v12 01/10] clk: Export __clk_put and __clk_get so they can be used by modules Tomeu Vizoso
2014-09-22 14:12 ` [PATCH v12 02/10] clk: Add temporary mapping to the existing API Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 04/10] clk: use struct clk only for external API Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 05/10] clk: per-user clock accounting for debug Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 06/10] clk: Add floor and ceiling constraints to clock rates Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 07/10] clk: Warn of unbalanced clk_prepare() calls Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 08/10] clk: Take the prepare lock when updating the list of per-user clks Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 09/10] clk: Take the prepare lock when updating the per-user constraints Tomeu Vizoso
2014-09-22 14:15 ` [PATCH v12 10/10] clk: Add docs about calling clk_put after clk_get_parent Tomeu Vizoso
2014-09-22 18:08 ` Russell King - ARM Linux [this message]
2014-09-23 9:09 ` [PATCH v12 04/10] clk: use struct clk only for external API Tomeu Vizoso
2014-09-22 15:06 ` [PATCH v12 00/10] Per-user clock constraints Russell King - ARM Linux
2014-09-23 7:40 ` 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=20140922180807.GN5182@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--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).