| Message ID | 20190724171615.20774-5-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | TCU patchset v15 | expand |
Lee, Arnd, On Wed, Jul 24, 2019 at 01:16:06PM -0400, Paul Cercueil wrote: > device_node_to_regmap() is exactly like syscon_node_to_regmap(), but it > does not check that the node is compatible with "syscon", and won't > attach the first clock it finds to the regmap. > > The rationale behind this, is that one device node with a standard > compatible string "foo,bar" can be covered by multiple drivers sharing a > regmap, or by a single driver doing all the job without a regmap, but > these are implementation details which shouldn't reflect on the > devicetree. Does this looks like a good path forwards to you? Its use in this case is described by Documentation/devicetree/bindings/timer/ingenic,tcu.txt in patch 3 of the series. If you're OK with it an ack would be appreciated so I can take the series through mips-next, otherwise I guess we'd need to go back to the v14 approach. Thanks, Paul > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > > Notes: > v15: New patch > > drivers/mfd/syscon.c | 46 +++++++++++++++++++++++++------------- > include/linux/mfd/syscon.h | 6 +++++ > 2 files changed, 36 insertions(+), 16 deletions(-) > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > index b65e585fc8c6..660723276481 100644 > --- a/drivers/mfd/syscon.c > +++ b/drivers/mfd/syscon.c > @@ -40,7 +40,7 @@ static const struct regmap_config syscon_regmap_config = { > .reg_stride = 4, > }; > > -static struct syscon *of_syscon_register(struct device_node *np) > +static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > { > struct clk *clk; > struct syscon *syscon; > @@ -51,9 +51,6 @@ static struct syscon *of_syscon_register(struct device_node *np) > struct regmap_config syscon_config = syscon_regmap_config; > struct resource res; > > - if (!of_device_is_compatible(np, "syscon")) > - return ERR_PTR(-EINVAL); > - > syscon = kzalloc(sizeof(*syscon), GFP_KERNEL); > if (!syscon) > return ERR_PTR(-ENOMEM); > @@ -117,16 +114,18 @@ static struct syscon *of_syscon_register(struct device_node *np) > goto err_regmap; > } > > - clk = of_clk_get(np, 0); > - if (IS_ERR(clk)) { > - ret = PTR_ERR(clk); > - /* clock is optional */ > - if (ret != -ENOENT) > - goto err_clk; > - } else { > - ret = regmap_mmio_attach_clk(regmap, clk); > - if (ret) > - goto err_attach; > + if (check_clk) { > + clk = of_clk_get(np, 0); > + if (IS_ERR(clk)) { > + ret = PTR_ERR(clk); > + /* clock is optional */ > + if (ret != -ENOENT) > + goto err_clk; > + } else { > + ret = regmap_mmio_attach_clk(regmap, clk); > + if (ret) > + goto err_attach; > + } > } > > syscon->regmap = regmap; > @@ -150,7 +149,8 @@ static struct syscon *of_syscon_register(struct device_node *np) > return ERR_PTR(ret); > } > > -struct regmap *syscon_node_to_regmap(struct device_node *np) > +static struct regmap *device_node_get_regmap(struct device_node *np, > + bool check_clk) > { > struct syscon *entry, *syscon = NULL; > > @@ -165,13 +165,27 @@ struct regmap *syscon_node_to_regmap(struct device_node *np) > spin_unlock(&syscon_list_slock); > > if (!syscon) > - syscon = of_syscon_register(np); > + syscon = of_syscon_register(np, check_clk); > > if (IS_ERR(syscon)) > return ERR_CAST(syscon); > > return syscon->regmap; > } > + > +struct regmap *device_node_to_regmap(struct device_node *np) > +{ > + return device_node_get_regmap(np, false); > +} > +EXPORT_SYMBOL_GPL(device_node_to_regmap); > + > +struct regmap *syscon_node_to_regmap(struct device_node *np) > +{ > + if (!of_device_is_compatible(np, "syscon")) > + return ERR_PTR(-EINVAL); > + > + return device_node_get_regmap(np, true); > +} > EXPORT_SYMBOL_GPL(syscon_node_to_regmap); > > struct regmap *syscon_regmap_lookup_by_compatible(const char *s) > diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h > index 8cfda0554381..112dc66262cc 100644 > --- a/include/linux/mfd/syscon.h > +++ b/include/linux/mfd/syscon.h > @@ -17,12 +17,18 @@ > struct device_node; > > #ifdef CONFIG_MFD_SYSCON > +extern struct regmap *device_node_to_regmap(struct device_node *np); > extern struct regmap *syscon_node_to_regmap(struct device_node *np); > extern struct regmap *syscon_regmap_lookup_by_compatible(const char *s); > extern struct regmap *syscon_regmap_lookup_by_phandle( > struct device_node *np, > const char *property); > #else > +static inline struct regmap *device_node_to_regmap(struct device_node *np) > +{ > + return ERR_PTR(-ENOTSUPP); > +} > + > static inline struct regmap *syscon_node_to_regmap(struct device_node *np) > { > return ERR_PTR(-ENOTSUPP); > -- > 2.21.0.593.g511ec345e18 >
On Mon, Jul 29, 2019 at 6:55 PM Paul Burton <paul.burton@mips.com> wrote: > > Lee, Arnd, > > On Wed, Jul 24, 2019 at 01:16:06PM -0400, Paul Cercueil wrote: > > device_node_to_regmap() is exactly like syscon_node_to_regmap(), but it > > does not check that the node is compatible with "syscon", and won't > > attach the first clock it finds to the regmap. > > > > The rationale behind this, is that one device node with a standard > > compatible string "foo,bar" can be covered by multiple drivers sharing a > > regmap, or by a single driver doing all the job without a regmap, but > > these are implementation details which shouldn't reflect on the > > devicetree. > > Does this looks like a good path forwards to you? Its use in this case > is described by Documentation/devicetree/bindings/timer/ingenic,tcu.txt > in patch 3 of the series. > > If you're OK with it an ack would be appreciated so I can take the > series through mips-next, otherwise I guess we'd need to go back to the > v14 approach. Yes, I guess this is ok, sorry for missing the submission earlier. Acked-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index b65e585fc8c6..660723276481 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -40,7 +40,7 @@ static const struct regmap_config syscon_regmap_config = { .reg_stride = 4, }; -static struct syscon *of_syscon_register(struct device_node *np) +static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) { struct clk *clk; struct syscon *syscon; @@ -51,9 +51,6 @@ static struct syscon *of_syscon_register(struct device_node *np) struct regmap_config syscon_config = syscon_regmap_config; struct resource res; - if (!of_device_is_compatible(np, "syscon")) - return ERR_PTR(-EINVAL); - syscon = kzalloc(sizeof(*syscon), GFP_KERNEL); if (!syscon) return ERR_PTR(-ENOMEM); @@ -117,16 +114,18 @@ static struct syscon *of_syscon_register(struct device_node *np) goto err_regmap; } - clk = of_clk_get(np, 0); - if (IS_ERR(clk)) { - ret = PTR_ERR(clk); - /* clock is optional */ - if (ret != -ENOENT) - goto err_clk; - } else { - ret = regmap_mmio_attach_clk(regmap, clk); - if (ret) - goto err_attach; + if (check_clk) { + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) { + ret = PTR_ERR(clk); + /* clock is optional */ + if (ret != -ENOENT) + goto err_clk; + } else { + ret = regmap_mmio_attach_clk(regmap, clk); + if (ret) + goto err_attach; + } } syscon->regmap = regmap; @@ -150,7 +149,8 @@ static struct syscon *of_syscon_register(struct device_node *np) return ERR_PTR(ret); } -struct regmap *syscon_node_to_regmap(struct device_node *np) +static struct regmap *device_node_get_regmap(struct device_node *np, + bool check_clk) { struct syscon *entry, *syscon = NULL; @@ -165,13 +165,27 @@ struct regmap *syscon_node_to_regmap(struct device_node *np) spin_unlock(&syscon_list_slock); if (!syscon) - syscon = of_syscon_register(np); + syscon = of_syscon_register(np, check_clk); if (IS_ERR(syscon)) return ERR_CAST(syscon); return syscon->regmap; } + +struct regmap *device_node_to_regmap(struct device_node *np) +{ + return device_node_get_regmap(np, false); +} +EXPORT_SYMBOL_GPL(device_node_to_regmap); + +struct regmap *syscon_node_to_regmap(struct device_node *np) +{ + if (!of_device_is_compatible(np, "syscon")) + return ERR_PTR(-EINVAL); + + return device_node_get_regmap(np, true); +} EXPORT_SYMBOL_GPL(syscon_node_to_regmap); struct regmap *syscon_regmap_lookup_by_compatible(const char *s) diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h index 8cfda0554381..112dc66262cc 100644 --- a/include/linux/mfd/syscon.h +++ b/include/linux/mfd/syscon.h @@ -17,12 +17,18 @@ struct device_node; #ifdef CONFIG_MFD_SYSCON +extern struct regmap *device_node_to_regmap(struct device_node *np); extern struct regmap *syscon_node_to_regmap(struct device_node *np); extern struct regmap *syscon_regmap_lookup_by_compatible(const char *s); extern struct regmap *syscon_regmap_lookup_by_phandle( struct device_node *np, const char *property); #else +static inline struct regmap *device_node_to_regmap(struct device_node *np) +{ + return ERR_PTR(-ENOTSUPP); +} + static inline struct regmap *syscon_node_to_regmap(struct device_node *np) { return ERR_PTR(-ENOTSUPP);
device_node_to_regmap() is exactly like syscon_node_to_regmap(), but it does not check that the node is compatible with "syscon", and won't attach the first clock it finds to the regmap. The rationale behind this, is that one device node with a standard compatible string "foo,bar" can be covered by multiple drivers sharing a regmap, or by a single driver doing all the job without a regmap, but these are implementation details which shouldn't reflect on the devicetree. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- Notes: v15: New patch drivers/mfd/syscon.c | 46 +++++++++++++++++++++++++------------- include/linux/mfd/syscon.h | 6 +++++ 2 files changed, 36 insertions(+), 16 deletions(-)