Hi Uffe, > -----Original Message----- > From: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > Sent: Wednesday, June 19, 2019 7:09 PM > To: Manish Narani <MNARANI@xxxxxxxxxx> > Cc: Michal Simek <michals@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; > Mark Rutland <mark.rutland@xxxxxxx>; Adrian Hunter > <adrian.hunter@xxxxxxxxx>; Rajan Vaja <RAJANV@xxxxxxxxxx>; Jolly Shah > <JOLLYS@xxxxxxxxxx>; Nava kishore Manne <navam@xxxxxxxxxx>; Olof > Johansson <olof@xxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; DTML > <devicetree@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux- > kernel@xxxxxxxxxxxxxxx>; Linux ARM <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx> > Subject: Re: [PATCH 3/3] mmc: sdhci-of-arasan: Add support for ZynqMP > Platform Tap Delays Setup > > On Wed, 19 Jun 2019 at 10:40, Manish Narani <MNARANI@xxxxxxxxxx> wrote: > > > > Hi Uffe, > > > > > > > -----Original Message----- > > > From: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > > > Sent: Monday, June 17, 2019 5:51 PM > > [...] > > > > > > The "const struct zynqmp_eemi_ops *eemi_ops; should then be moved into > > > a clock provider specific struct, which is assigned when calling > > > sdhci_arasan_register_sdclk. I understand that all the clock data is > > > folded into struct sdhci_arasan_data today, but I think that should be > > > moved into a "sub-struct" for the clock specifics. > > > > > > Moreover, when registering the clock, we should convert from using > > > devm_clk_register() into devm_clk_hw_register() as the first one is > > > now deprecated. > > > > Just a query here: > > When we switch to using devm_clk_hw_register() here, it will register the > clk_hw and return int. > > Is there a way we can get the clk (related to the clk_hw registered) from the > > clock framework? > > I am asking this because we will need that clk pointer while calling > clk_set_phase() function. > > I assume devm_clk_get() should work fine? This clock does not come through ZynqMP Clock framework. We are initializing it in this 'sdhci-of-arasan' driver and getting only the clock name from "clock_output_names" property. So I think devm_clk_get() will not work here for our case. I have gone through the clock framework and I found one function which may be used to create clock from clock hw, that is ' clk_hw_create_clk()' which can be used from our driver, however this needs change in the clock framework as below : --- diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index aa51756..4dc69ff 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3420,6 +3420,7 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, return clk; } +EXPORT_SYMBOL_GPL(clk_hw_create_clk); static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist) { diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h index d8400d6..2319899 100644 --- a/drivers/clk/clk.h +++ b/drivers/clk/clk.h @@ -22,17 +22,9 @@ static inline struct clk_hw *of_clk_get_hw(struct device_node *np, struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id); #ifdef CONFIG_COMMON_CLK -struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, - const char *dev_id, const char *con_id); void __clk_put(struct clk *clk); #else /* All these casts to avoid ifdefs in clkdev... */ -static inline struct clk * -clk_hw_create_clk(struct device *dev, struct clk_hw *hw, const char *dev_id, - const char *con_id) -{ - return (struct clk *)hw; -} static struct clk_hw *__clk_get_hw(struct clk *clk) { return (struct clk_hw *)clk; diff --git a/include/linux/clk.h b/include/linux/clk.h index f689fc5..d3f60fe 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -18,6 +18,7 @@ struct device; struct clk; +struct clk_hw; struct device_node; struct of_phandle_args; @@ -934,4 +935,15 @@ static inline struct clk *of_clk_get_from_provider(struct of_phandle_args *clksp } #endif +#ifdef CONFIG_COMMON_CLK +struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, + const char *dev_id, const char *con_id); +#else +static inline struct clk * +clk_hw_create_clk(struct device *dev, struct clk_hw *hw, const char *dev_id, + const char *con_id) +{ + return (struct clk *)hw; +} +#endif #endif --- This change should help other drivers (outside 'drivers/clk/') as well for getting the clock created from clk_hw. Is this fine to do? Thanks, Manish