Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings
Quoting Stephen Boyd (2013-07-25 09:36:56)
> 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", ) < 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, ) == 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.
I made a simple wrapper around of_count_phandle_with_args for the basic
DT bindings series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/178836.html
I still need to make a couple of changes to that before I merge it. If
you want to base on top of the v3 version then you can use it, otherwise
just use of_count_phandle_with_args.
Regards,
Mike
>
> >
> > > + names = p;
> > > +
> > > + if (of_property_read_string(s.np, "clock-output-names",
> > > + ) < 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings
Quoting Stephen Boyd (2013-07-25 09:36:56)
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 rob.herr...@calxeda.com
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
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.
I made a simple wrapper around of_count_phandle_with_args for the basic
DT bindings series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/178836.html
I still need to make a couple of changes to that before I merge it. If
you want to base on top of the v3 version then you can use it, otherwise
just use of_count_phandle_with_args.
Regards,
Mike
+ 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings
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", ) < 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, ) == 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",
> > + ) < 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings
Hi Stephen,
Pretty good idea, but I have some comments inline.
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", ) < 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, ) == 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?
> + names = p;
> +
> + if (of_property_read_string(s.np, "clock-output-names",
> + ) < 0)
> + name = s.np->name;
You should be able to use of_clk_get_parent_name() here, instead of
parsing the properties directly.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings
Hi Stephen,
Pretty good idea, but I have some comments inline.
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 rob.herr...@calxeda.com
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
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?
+ 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.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/14] clk: Add of_init_clk_data() to parse common clock bindings
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 rob.herr...@calxeda.com
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
