On 09/22/2014 08:08 PM, Russell King - ARM Linux wrote: > 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?
Because struct clk_core is only defined if CONFIG_COMMON_CLK, so clock implementations that don't use the CCF don't have to be converted to clk_core. >> 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. Sounds like a neat improvement, will send soon v13 with this. > 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. Yeah, will address this in v13 as well. Thanks a lot for the feedback, Tomeu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
