From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings Date: Thu, 25 Jul 2013 09:36:56 -0700 Message-ID: <20130725163656.GC29694@codeaurora.org> References: <1374713022-6049-1-git-send-email-sboyd@codeaurora.org> <1374713022-6049-3-git-send-email-sboyd@codeaurora.org> <1714620.Z8J8zTxIRB@flatron> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1714620.Z8J8zTxIRB@flatron> Sender: linux-kernel-owner@vger.kernel.org To: Tomasz Figa Cc: linux-arm-kernel@lists.infradead.org, Mike Turquette , linux-arm-msm@vger.kernel.org, Saravana Kannan , Rob Herring , linux-kernel@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org On 07/25, Tomasz Figa wrote: > On Wednesday 24 of July 2013 17:43:30 Stephen Boyd wrote: > > Consolidate DT parsing for the common bits of a clock binding in > > one place to simplify clock drivers. This also has the added > > benefit of standardizing how the clock names used by the common > > clock framework are generated from the DT bindings. We always use > > the first clock-output-names string if it exists, otherwise we > > fall back to the node name. > > > > To be slightly more efficient and make the caller's life easier, > > we introduce a shallow copy flag so that the clock core knows to > > just copy the pointers to the strings and not the string > > contents. Otherwise the callers of this function would have to > > free the strings allocated here which could be cumbersome. > > > > Cc: Rob Herring > > Signed-off-by: Stephen Boyd > > --- > > drivers/clk/clk.c | 59 > > +++++++++++++++++++++++++++++++++++++++++++- > > include/linux/clk-provider.h | 7 ++++++ > > 2 files changed, 65 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index 1ed9bdd..ea8e951b 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -1809,6 +1809,10 @@ static int _clk_register(struct device *dev, > > struct clk_hw *hw, struct clk *clk) { > > int i, ret; > > > > + hw->clk = clk; > > + if (hw->init->flags & CLK_SHALLOW_COPY) > > + return PTR_RET(__clk_register(dev, hw)); > > + > > clk->name = kstrdup(hw->init->name, GFP_KERNEL); > > if (!clk->name) { > > pr_err("%s: could not allocate clk->name\n", __func__); > > @@ -1819,7 +1823,6 @@ static int _clk_register(struct device *dev, > > struct clk_hw *hw, struct clk *clk) clk->hw = hw; > > clk->flags = hw->init->flags; > > clk->num_parents = hw->init->num_parents; > > - hw->clk = clk; > > > > /* allocate local copy in case parent_names is __initdata */ > > clk->parent_names = kzalloc((sizeof(char*) * clk->num_parents), > > @@ -2232,4 +2235,58 @@ void __init of_clk_init(const struct of_device_id > > *matches) clk_init_cb(np); > > } > > } > > + > > +/** > > + * of_init_clk_data() - Initialize a clk_init_data struct from a DT > > node + * @np: node to initialize struct from > > + * @init: struct to initialize > > + * > > + * Populates the clk_init_data struct by parsing the device node for > > + * properties matching the common clock binding. Returns 0 on success > > + * and a negative error code on failure. > > + */ > > +int of_init_clk_data(struct device_node *np, struct clk_init_data > > *init) +{ > > + struct of_phandle_args s; > > + const char **names = NULL, **p; > > + const char *name; > > + int i; > > + > > + if (of_property_read_string(np, "clock-output-names", &name) < 0) > > + name = np->name; > > + init->name = kstrdup(name, GFP_KERNEL); > > + if (!init->name) > > + return -ENOMEM; > > + > > + for (i = 0; of_parse_phandle_with_args(np, "clocks", "#clock- > cells", > > + i, &s) == 0; i++) { > > + p = krealloc(names, sizeof(*names) * (i + 1), GFP_KERNEL); > > + if (!p) > > + goto err; > > What about using of_count_phandle_with_args() first to get the parent > count? Sure that simplifies things. > > > + names = p; > > + > > + if (of_property_read_string(s.np, "clock-output-names", > > + &name) < 0) > > + name = s.np->name; > > You should be able to use of_clk_get_parent_name() here, instead of > parsing the properties directly. This must be new too. Will do in v2. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation From mboxrd@z Thu Jan 1 00:00:00 1970 From: sboyd@codeaurora.org (Stephen Boyd) Date: Thu, 25 Jul 2013 09:36:56 -0700 Subject: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings In-Reply-To: <1714620.Z8J8zTxIRB@flatron> References: <1374713022-6049-1-git-send-email-sboyd@codeaurora.org> <1374713022-6049-3-git-send-email-sboyd@codeaurora.org> <1714620.Z8J8zTxIRB@flatron> Message-ID: <20130725163656.GC29694@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 07/25, Tomasz Figa wrote: > On Wednesday 24 of July 2013 17:43:30 Stephen Boyd wrote: > > Consolidate DT parsing for the common bits of a clock binding in > > one place to simplify clock drivers. This also has the added > > benefit of standardizing how the clock names used by the common > > clock framework are generated from the DT bindings. We always use > > the first clock-output-names string if it exists, otherwise we > > fall back to the node name. > > > > To be slightly more efficient and make the caller's life easier, > > we introduce a shallow copy flag so that the clock core knows to > > just copy the pointers to the strings and not the string > > contents. Otherwise the callers of this function would have to > > free the strings allocated here which could be cumbersome. > > > > Cc: Rob Herring > > Signed-off-by: Stephen Boyd > > --- > > drivers/clk/clk.c | 59 > > +++++++++++++++++++++++++++++++++++++++++++- > > include/linux/clk-provider.h | 7 ++++++ > > 2 files changed, 65 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index 1ed9bdd..ea8e951b 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -1809,6 +1809,10 @@ static int _clk_register(struct device *dev, > > struct clk_hw *hw, struct clk *clk) { > > int i, ret; > > > > + hw->clk = clk; > > + if (hw->init->flags & CLK_SHALLOW_COPY) > > + return PTR_RET(__clk_register(dev, hw)); > > + > > clk->name = kstrdup(hw->init->name, GFP_KERNEL); > > if (!clk->name) { > > pr_err("%s: could not allocate clk->name\n", __func__); > > @@ -1819,7 +1823,6 @@ static int _clk_register(struct device *dev, > > struct clk_hw *hw, struct clk *clk) clk->hw = hw; > > clk->flags = hw->init->flags; > > clk->num_parents = hw->init->num_parents; > > - hw->clk = clk; > > > > /* allocate local copy in case parent_names is __initdata */ > > clk->parent_names = kzalloc((sizeof(char*) * clk->num_parents), > > @@ -2232,4 +2235,58 @@ void __init of_clk_init(const struct of_device_id > > *matches) clk_init_cb(np); > > } > > } > > + > > +/** > > + * of_init_clk_data() - Initialize a clk_init_data struct from a DT > > node + * @np: node to initialize struct from > > + * @init: struct to initialize > > + * > > + * Populates the clk_init_data struct by parsing the device node for > > + * properties matching the common clock binding. Returns 0 on success > > + * and a negative error code on failure. > > + */ > > +int of_init_clk_data(struct device_node *np, struct clk_init_data > > *init) +{ > > + struct of_phandle_args s; > > + const char **names = NULL, **p; > > + const char *name; > > + int i; > > + > > + if (of_property_read_string(np, "clock-output-names", &name) < 0) > > + name = np->name; > > + init->name = kstrdup(name, GFP_KERNEL); > > + if (!init->name) > > + return -ENOMEM; > > + > > + for (i = 0; of_parse_phandle_with_args(np, "clocks", "#clock- > cells", > > + i, &s) == 0; i++) { > > + p = krealloc(names, sizeof(*names) * (i + 1), GFP_KERNEL); > > + if (!p) > > + goto err; > > What about using of_count_phandle_with_args() first to get the parent > count? Sure that simplifies things. > > > + names = p; > > + > > + if (of_property_read_string(s.np, "clock-output-names", > > + &name) < 0) > > + name = s.np->name; > > You should be able to use of_clk_get_parent_name() here, instead of > parsing the properties directly. This must be new too. Will do in v2. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation