| Message ID | 375a82230accf26dfbbb74e6f928243ea8e38b50.1415557680.git.moinejf@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Nov 08, 2014 at 07:33:22PM +0100, Jean-Francois Moine wrote: > Some device nodes are sometimes referenced by const pointers. > > This patch avoids to cast these pointers when calling the functions > of_clk_get() and of_node_put(). NAK. Firstly, of_node_put contains a kref, which is /definitely/ not a const item when kobject_put() is called on it. Inside the kobject is a kref, which kobject_put() will call kref_put() on. kref_put() in turn will internally call atomic_sub_and_test() on a value embedded within the kref struct. Ergo, of_node_put() modifies the struct device_node contents. Therefore, of_node_put() definitely not treating the data pointed to as read-only, and therefore it is completely inappropriate for it to be marked "const". What this then means is that it fundamentally undermines the idea of storing the pointer to a device_node as a const pointer, as the device node must always be modified when you're done with it (because it's a ref-counted structure.) So, having it const in your code is a bug. What this also means is that every other place that you've added const below is also very dubious.
On Sun, 9 Nov 2014 18:50:19 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > What this then means is that it fundamentally undermines the idea of > storing the pointer to a device_node as a const pointer, as the device > node must always be modified when you're done with it (because it's a > ref-counted structure.) So, having it const in your code is a bug. > > What this also means is that every other place that you've added const > below is also very dubious. Thanks for the explanation. The correct patch will be simpler: in the ASoC structures, I found only three places where the device_node pointer is const.
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index da4bda8..eaee11e 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -53,7 +53,7 @@ struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec) return clk; } -struct clk *of_clk_get(struct device_node *np, int index) +struct clk *of_clk_get(const struct device_node *np, int index) { struct of_phandle_args clkspec; struct clk *clk; diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index f297891..64eb5ba 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -34,7 +34,7 @@ EXPORT_SYMBOL(of_node_get); * @node: Node to dec refcount, NULL is supported to simplify writing of * callers */ -void of_node_put(struct device_node *node) +void of_node_put(const struct device_node *node) { if (node) kobject_put(&node->kobj); diff --git a/include/linux/clk.h b/include/linux/clk.h index c7f258a..4e077a4 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -425,11 +425,11 @@ struct device_node; 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 *of_clk_get(const 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 -static inline struct clk *of_clk_get(struct device_node *np, int index) +static inline struct clk *of_clk_get(const struct device_node *np, int index) { return ERR_PTR(-ENOENT); } diff --git a/include/linux/of.h b/include/linux/of.h index 6545e7a..b738681 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -95,14 +95,14 @@ static inline int of_node_is_attached(struct device_node *node) #ifdef CONFIG_OF_DYNAMIC extern struct device_node *of_node_get(struct device_node *node); -extern void of_node_put(struct device_node *node); +extern void of_node_put(const struct device_node *node); #else /* CONFIG_OF_DYNAMIC */ /* Dummy ref counting routines - to be implemented later */ static inline struct device_node *of_node_get(struct device_node *node) { return node; } -static inline void of_node_put(struct device_node *node) { } +static inline void of_node_put(const struct device_node *node) { } #endif /* !CONFIG_OF_DYNAMIC */ #ifdef CONFIG_OF
Some device nodes are sometimes referenced by const pointers. This patch avoids to cast these pointers when calling the functions of_clk_get() and of_node_put(). Signed-off-by: Jean-Francois Moine <moinejf@free.fr> --- drivers/clk/clkdev.c | 2 +- drivers/of/dynamic.c | 2 +- include/linux/clk.h | 4 ++-- include/linux/of.h | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-)