Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1438037 > unrolled thread

Re: [PATCH v2] clkdev: add devm_of_clk_get()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2] clkdev: add devm_of_clk_get() Michael Turquette <mturquette@baylibre.com> - 2016-07-07 02:50 +0200
    Re: [PATCH v2] clkdev: add devm_of_clk_get() Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-07-07 14:30 +0200
      Re: [PATCH v2] clkdev: add devm_of_clk_get() Michael Turquette <mturquette@baylibre.com> - 2016-07-08 03:40 +0200

#1438037 — Re: [PATCH v2] clkdev: add devm_of_clk_get()

Quoting Kuninori Morimoto (2016-07-03 18:36:50)
> +struct clk *devm_of_clk_get(struct device *dev,
> +                           struct device_node *np, int index)

Any reason not to use devm_clk_get? Why do we need this helper?

Thanks,
Mike

> +{
> +       struct clk **ptr, *clk;
> +
> +       ptr = devres_alloc(devm_of_clk_release, sizeof(*ptr), GFP_KERNEL);
> +       if (!ptr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       clk = of_clk_get(np, index);
> +       if (!IS_ERR(clk)) {
> +               *ptr = clk;
> +               devres_add(dev, ptr);
> +       } else {
> +               devres_free(ptr);
> +       }
> +
> +       return clk;
> +}
> +EXPORT_SYMBOL(devm_of_clk_get);
> +
>  static struct clk *__of_clk_get_by_name(struct device_node *np,
>                                         const char *dev_id,
>                                         const char *name)
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index a89ba4e..33cd540 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -502,6 +502,8 @@ struct of_phandle_args;
>  
>  #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
>  struct clk *of_clk_get(struct device_node *np, int index);
> +struct clk *devm_of_clk_get(struct device *dev,
> +                           struct device_node *np, int index);
>  struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
>  struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
>  #else
> @@ -509,6 +511,11 @@ static inline struct clk *of_clk_get(struct device_node *np, int index)
>  {
>         return ERR_PTR(-ENOENT);
>  }
> +static inline struct clk *devm_of_clk_get(struct device *dev,
> +                           struct device_node *np, int index)
> +{
> +       return ERR_PTR(-ENOENT);
> +}
>  static inline struct clk *of_clk_get_by_name(struct device_node *np,
>                                              const char *name)
>  {
> -- 
> 1.9.1
> 

[toc] | [next] | [standalone]

#1438558

On Thu, Jul 07, 2016 at 09:54:03AM +0000, Kuninori Morimoto wrote:
> 
> Hi Michael
> 
> > > +struct clk *devm_of_clk_get(struct device *dev,
> > > +                           struct device_node *np, int index)
> > 
> > Any reason not to use devm_clk_get? Why do we need this helper?
> 
> Because of_clk_get() can parse "clocks", "#clock-cells" on DT.

clk_get() should also work just fine.  clk_get() uses
__of_clk_get_by_name() internally, which uses "clock-names" to
locate the index if a connection id is given.  of_clk_get() allows
lookup of a clock by index only, omitting the name, which means
you need to coordinate the order of clocks in DT with the order
that the driver wants... which sounds error prone to me.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1439055

Quoting Kuninori Morimoto (2016-07-07 17:03:00)
> 
> Hi Russell
> 
> > > > > +struct clk *devm_of_clk_get(struct device *dev,
> > > > > +                           struct device_node *np, int index)
> > > > 
> > > > Any reason not to use devm_clk_get? Why do we need this helper?
> > > 
> > > Because of_clk_get() can parse "clocks", "#clock-cells" on DT.
> > 
> > clk_get() should also work just fine.  clk_get() uses
> > __of_clk_get_by_name() internally, which uses "clock-names" to
> > locate the index if a connection id is given.  of_clk_get() allows
> > lookup of a clock by index only, omitting the name, which means
> > you need to coordinate the order of clocks in DT with the order
> > that the driver wants... which sounds error prone to me.
> 
> Thanks.
> 
> But, I have 1 issue on [devm_]clk_get().
> It can't select device_node on [devm_]clk_get(), it uses dev->of_node directly.
> 
> struct clk *clk_get(struct device *dev, const char *con_id)
> {
>         ...
>         if (dev) {
>                 clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id);
>                                            ~~~~~~~~~~~~
>                 ...
>         }
> }
> 
> I would like to select specific device_node.

Do you have access to the struct device that you want to target? Can you
pass that device into either clk_get or devm_clk_get?

Regards,
Mike

> 
>         sound_soc {
>                 ...
>                 cpu {
>                         ...
> =>                      clocks = <&xxx>;
>                 };
> 
>                 codec {
>                         ...
> =>                      clocks = <&xxx>;
>                 };
>         };
> 
> But, of_clk_get_by_name() / of_clk_get() doesn't have devm_xxx version

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web