Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,

2016-09-07 9:32 GMT+09:00 Stephen Boyd :
> On 09/05, Masahiro Yamada wrote:
>> 2016-08-30 3:22 GMT+09:00 Stephen Boyd :
>> > On 08/29, Masahiro Yamada wrote:
>> >> I tried this, but it did not work.
>> >>
>> >> To make dev_get_regmap() work,
>> >> the parent device needs to call dev_regmap_init_mmio() beforehand.
>> >>
>> >>
>> >> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
>> >> (mfd: syscon: Decouple syscon interface from platform devices),
>> >> syscon_probe() is not called for platform devices,
>> >> so that never happens.
>> >>
>> >
>> > Ok. Is the syscon also a simple-mfd?
>> >
>> > It sounds like there's a device for the parent, but we've failed
>> > to attach a regmap to it. Maybe the core DT code should assign
>> > the regmap to the parent device when it creates it so that child
>> > devices don't need to know this detail? It could look for
>> > simple-mfd devices with compatible = "syscon" and then create the
>> > regmap? Here's a totally untested patch for that.
>> >
>>
>>
>> I was not quire sure about this,
>> but maybe worth submitting to DT subsystem?
>>
>> Anyway, I will go with syscon_node_to_regmap() for v7.
>> Of course, I am happy to replace it with dev_get_regmap()
>> when the issue is solved in the mainline.
>>
>
> Ok that's fine. Did my patch fix dev_get_regmap() for you though?
> That would be useful to know so that this patch can be merged in
> parallel to yours.

Sorry for my late reply.

Yup, your patch worked fine!


And, I've posted v7 for my SoC clk driver.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,

2016-09-07 9:32 GMT+09:00 Stephen Boyd :
> On 09/05, Masahiro Yamada wrote:
>> 2016-08-30 3:22 GMT+09:00 Stephen Boyd :
>> > On 08/29, Masahiro Yamada wrote:
>> >> I tried this, but it did not work.
>> >>
>> >> To make dev_get_regmap() work,
>> >> the parent device needs to call dev_regmap_init_mmio() beforehand.
>> >>
>> >>
>> >> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
>> >> (mfd: syscon: Decouple syscon interface from platform devices),
>> >> syscon_probe() is not called for platform devices,
>> >> so that never happens.
>> >>
>> >
>> > Ok. Is the syscon also a simple-mfd?
>> >
>> > It sounds like there's a device for the parent, but we've failed
>> > to attach a regmap to it. Maybe the core DT code should assign
>> > the regmap to the parent device when it creates it so that child
>> > devices don't need to know this detail? It could look for
>> > simple-mfd devices with compatible = "syscon" and then create the
>> > regmap? Here's a totally untested patch for that.
>> >
>>
>>
>> I was not quire sure about this,
>> but maybe worth submitting to DT subsystem?
>>
>> Anyway, I will go with syscon_node_to_regmap() for v7.
>> Of course, I am happy to replace it with dev_get_regmap()
>> when the issue is solved in the mainline.
>>
>
> Ok that's fine. Did my patch fix dev_get_regmap() for you though?
> That would be useful to know so that this patch can be merged in
> parallel to yours.

Sorry for my late reply.

Yup, your patch worked fine!


And, I've posted v7 for my SoC clk driver.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 09/05, Masahiro Yamada wrote:
> 2016-08-30 3:22 GMT+09:00 Stephen Boyd :
> > On 08/29, Masahiro Yamada wrote:
> >> I tried this, but it did not work.
> >>
> >> To make dev_get_regmap() work,
> >> the parent device needs to call dev_regmap_init_mmio() beforehand.
> >>
> >>
> >> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
> >> (mfd: syscon: Decouple syscon interface from platform devices),
> >> syscon_probe() is not called for platform devices,
> >> so that never happens.
> >>
> >
> > Ok. Is the syscon also a simple-mfd?
> >
> > It sounds like there's a device for the parent, but we've failed
> > to attach a regmap to it. Maybe the core DT code should assign
> > the regmap to the parent device when it creates it so that child
> > devices don't need to know this detail? It could look for
> > simple-mfd devices with compatible = "syscon" and then create the
> > regmap? Here's a totally untested patch for that.
> >
> 
> 
> I was not quire sure about this,
> but maybe worth submitting to DT subsystem?
> 
> Anyway, I will go with syscon_node_to_regmap() for v7.
> Of course, I am happy to replace it with dev_get_regmap()
> when the issue is solved in the mainline.
> 

Ok that's fine. Did my patch fix dev_get_regmap() for you though?
That would be useful to know so that this patch can be merged in
parallel to yours.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 09/05, Masahiro Yamada wrote:
> 2016-08-30 3:22 GMT+09:00 Stephen Boyd :
> > On 08/29, Masahiro Yamada wrote:
> >> I tried this, but it did not work.
> >>
> >> To make dev_get_regmap() work,
> >> the parent device needs to call dev_regmap_init_mmio() beforehand.
> >>
> >>
> >> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
> >> (mfd: syscon: Decouple syscon interface from platform devices),
> >> syscon_probe() is not called for platform devices,
> >> so that never happens.
> >>
> >
> > Ok. Is the syscon also a simple-mfd?
> >
> > It sounds like there's a device for the parent, but we've failed
> > to attach a regmap to it. Maybe the core DT code should assign
> > the regmap to the parent device when it creates it so that child
> > devices don't need to know this detail? It could look for
> > simple-mfd devices with compatible = "syscon" and then create the
> > regmap? Here's a totally untested patch for that.
> >
> 
> 
> I was not quire sure about this,
> but maybe worth submitting to DT subsystem?
> 
> Anyway, I will go with syscon_node_to_regmap() for v7.
> Of course, I am happy to replace it with dev_get_regmap()
> when the issue is solved in the mainline.
> 

Ok that's fine. Did my patch fix dev_get_regmap() for you though?
That would be useful to know so that this patch can be merged in
parallel to yours.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,


2016-08-30 3:22 GMT+09:00 Stephen Boyd :
> On 08/29, Masahiro Yamada wrote:
>> Hi Stephen,
>>
>>
>> 2016-08-20 4:16 GMT+09:00 Stephen Boyd :
>> >>
>> >> >> +
>> >> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
>> >> >> node */
>> >> >> + regmap = syscon_node_to_regmap(parent);
>> >> >> + of_node_put(parent);
>> >> >
>> >> > devm_get_regmap(dev->parent) should work then? Why do we need to
>> >> > use OF APIs?
>> >>
>> >> "git grep devm_get_regmap" did not hit anything.
>> >>
>> >> Where is it defined?
>> >>
>> >
>> > Sorry I meant dev_get_regmap().
>> >
>>
>> I tried this, but it did not work.
>>
>> To make dev_get_regmap() work,
>> the parent device needs to call dev_regmap_init_mmio() beforehand.
>>
>>
>> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
>> (mfd: syscon: Decouple syscon interface from platform devices),
>> syscon_probe() is not called for platform devices,
>> so that never happens.
>>
>
> Ok. Is the syscon also a simple-mfd?
>
> It sounds like there's a device for the parent, but we've failed
> to attach a regmap to it. Maybe the core DT code should assign
> the regmap to the parent device when it creates it so that child
> devices don't need to know this detail? It could look for
> simple-mfd devices with compatible = "syscon" and then create the
> regmap? Here's a totally untested patch for that.
>
> 8<
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 2f2225e845ef..5f7d3f015b82 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -136,6 +136,17 @@ struct regmap *syscon_node_to_regmap(struct device_node 
> *np)
>  }
>  EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
>
> +int device_attach_syscon(struct device *dev)
> +{
> +   struct regmap *regmap;
> +
> +   regmap = syscon_node_to_regmap(dev->of_node);
> +   if (IS_ERR(regmap))
> +   return PTR_ERR(regmap);
> +
> +   return regmap_attach_dev(dev, regmap, _regmap_config);
> +}
> +
>  struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
>  {
> struct device_node *syscon_np;
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 8aa197691074..58a018e15006 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  const struct of_device_id of_default_bus_match_table[] = {
> { .compatible = "simple-bus", },
> @@ -383,7 +384,12 @@ static int of_platform_bus_create(struct device_node 
> *bus,
> return 0;
> }
>
> +
> dev = of_platform_device_create_pdata(bus, bus_id, platform_data, 
> parent);
> +   if (of_device_is_compatible(bus, "simple-mfd") &&
> +   of_device_is_compatible(bus, "syscon"))
> +   device_attach_syscon(>dev);
> +
> if (!dev || !of_match_node(matches, bus))
> return 0;
>
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index 40a76b97b7ab..e19e5d15f4e6 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -18,9 +18,11 @@
>  #include 
>  #include 
>
> +struct device;
>  struct device_node;
>
>  #ifdef CONFIG_MFD_SYSCON
> +extern int device_attach_syscon(struct device *dev);
>  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_pdevname(const char *s);
> @@ -28,6 +30,11 @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
> struct device_node *np,
> const char *property);
>  #else
> +static inline int device_attach_syscon(struct device *dev)
> +{
> +   return 0;
> +}
> +
>  static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>  {
> return ERR_PTR(-ENOTSUPP);


I was not quire sure about this,
but maybe worth submitting to DT subsystem?

Anyway, I will go with syscon_node_to_regmap() for v7.
Of course, I am happy to replace it with dev_get_regmap()
when the issue is solved in the mainline.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,


2016-08-30 3:22 GMT+09:00 Stephen Boyd :
> On 08/29, Masahiro Yamada wrote:
>> Hi Stephen,
>>
>>
>> 2016-08-20 4:16 GMT+09:00 Stephen Boyd :
>> >>
>> >> >> +
>> >> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
>> >> >> node */
>> >> >> + regmap = syscon_node_to_regmap(parent);
>> >> >> + of_node_put(parent);
>> >> >
>> >> > devm_get_regmap(dev->parent) should work then? Why do we need to
>> >> > use OF APIs?
>> >>
>> >> "git grep devm_get_regmap" did not hit anything.
>> >>
>> >> Where is it defined?
>> >>
>> >
>> > Sorry I meant dev_get_regmap().
>> >
>>
>> I tried this, but it did not work.
>>
>> To make dev_get_regmap() work,
>> the parent device needs to call dev_regmap_init_mmio() beforehand.
>>
>>
>> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
>> (mfd: syscon: Decouple syscon interface from platform devices),
>> syscon_probe() is not called for platform devices,
>> so that never happens.
>>
>
> Ok. Is the syscon also a simple-mfd?
>
> It sounds like there's a device for the parent, but we've failed
> to attach a regmap to it. Maybe the core DT code should assign
> the regmap to the parent device when it creates it so that child
> devices don't need to know this detail? It could look for
> simple-mfd devices with compatible = "syscon" and then create the
> regmap? Here's a totally untested patch for that.
>
> 8<
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 2f2225e845ef..5f7d3f015b82 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -136,6 +136,17 @@ struct regmap *syscon_node_to_regmap(struct device_node 
> *np)
>  }
>  EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
>
> +int device_attach_syscon(struct device *dev)
> +{
> +   struct regmap *regmap;
> +
> +   regmap = syscon_node_to_regmap(dev->of_node);
> +   if (IS_ERR(regmap))
> +   return PTR_ERR(regmap);
> +
> +   return regmap_attach_dev(dev, regmap, _regmap_config);
> +}
> +
>  struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
>  {
> struct device_node *syscon_np;
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 8aa197691074..58a018e15006 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  const struct of_device_id of_default_bus_match_table[] = {
> { .compatible = "simple-bus", },
> @@ -383,7 +384,12 @@ static int of_platform_bus_create(struct device_node 
> *bus,
> return 0;
> }
>
> +
> dev = of_platform_device_create_pdata(bus, bus_id, platform_data, 
> parent);
> +   if (of_device_is_compatible(bus, "simple-mfd") &&
> +   of_device_is_compatible(bus, "syscon"))
> +   device_attach_syscon(>dev);
> +
> if (!dev || !of_match_node(matches, bus))
> return 0;
>
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index 40a76b97b7ab..e19e5d15f4e6 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -18,9 +18,11 @@
>  #include 
>  #include 
>
> +struct device;
>  struct device_node;
>
>  #ifdef CONFIG_MFD_SYSCON
> +extern int device_attach_syscon(struct device *dev);
>  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_pdevname(const char *s);
> @@ -28,6 +30,11 @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
> struct device_node *np,
> const char *property);
>  #else
> +static inline int device_attach_syscon(struct device *dev)
> +{
> +   return 0;
> +}
> +
>  static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
>  {
> return ERR_PTR(-ENOTSUPP);


I was not quire sure about this,
but maybe worth submitting to DT subsystem?

Anyway, I will go with syscon_node_to_regmap() for v7.
Of course, I am happy to replace it with dev_get_regmap()
when the issue is solved in the mainline.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 08/29, Masahiro Yamada wrote:
> Hi Stephen,
> 
> 
> 2016-08-20 4:16 GMT+09:00 Stephen Boyd :
> >>
> >> >> +
> >> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
> >> >> node */
> >> >> + regmap = syscon_node_to_regmap(parent);
> >> >> + of_node_put(parent);
> >> >
> >> > devm_get_regmap(dev->parent) should work then? Why do we need to
> >> > use OF APIs?
> >>
> >> "git grep devm_get_regmap" did not hit anything.
> >>
> >> Where is it defined?
> >>
> >
> > Sorry I meant dev_get_regmap().
> >
> 
> I tried this, but it did not work.
> 
> To make dev_get_regmap() work,
> the parent device needs to call dev_regmap_init_mmio() beforehand.
> 
> 
> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
> (mfd: syscon: Decouple syscon interface from platform devices),
> syscon_probe() is not called for platform devices,
> so that never happens.
> 

Ok. Is the syscon also a simple-mfd?

It sounds like there's a device for the parent, but we've failed
to attach a regmap to it. Maybe the core DT code should assign
the regmap to the parent device when it creates it so that child
devices don't need to know this detail? It could look for
simple-mfd devices with compatible = "syscon" and then create the
regmap? Here's a totally untested patch for that.

8<
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 2f2225e845ef..5f7d3f015b82 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -136,6 +136,17 @@ struct regmap *syscon_node_to_regmap(struct device_node 
*np)
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
+int device_attach_syscon(struct device *dev)
+{
+   struct regmap *regmap;
+
+   regmap = syscon_node_to_regmap(dev->of_node);
+   if (IS_ERR(regmap))
+   return PTR_ERR(regmap);
+
+   return regmap_attach_dev(dev, regmap, _regmap_config);
+}
+
 struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
 {
struct device_node *syscon_np;
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8aa197691074..58a018e15006 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 const struct of_device_id of_default_bus_match_table[] = {
{ .compatible = "simple-bus", },
@@ -383,7 +384,12 @@ static int of_platform_bus_create(struct device_node *bus,
return 0;
}
 
+
dev = of_platform_device_create_pdata(bus, bus_id, platform_data, 
parent);
+   if (of_device_is_compatible(bus, "simple-mfd") &&
+   of_device_is_compatible(bus, "syscon"))
+   device_attach_syscon(>dev);
+
if (!dev || !of_match_node(matches, bus))
return 0;
 
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 40a76b97b7ab..e19e5d15f4e6 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -18,9 +18,11 @@
 #include 
 #include 
 
+struct device;
 struct device_node;
 
 #ifdef CONFIG_MFD_SYSCON
+extern int device_attach_syscon(struct device *dev);
 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_pdevname(const char *s);
@@ -28,6 +30,11 @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
struct device_node *np,
const char *property);
 #else
+static inline int device_attach_syscon(struct device *dev)
+{
+   return 0;
+}
+
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
return ERR_PTR(-ENOTSUPP);
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 08/29, Masahiro Yamada wrote:
> Hi Stephen,
> 
> 
> 2016-08-20 4:16 GMT+09:00 Stephen Boyd :
> >>
> >> >> +
> >> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
> >> >> node */
> >> >> + regmap = syscon_node_to_regmap(parent);
> >> >> + of_node_put(parent);
> >> >
> >> > devm_get_regmap(dev->parent) should work then? Why do we need to
> >> > use OF APIs?
> >>
> >> "git grep devm_get_regmap" did not hit anything.
> >>
> >> Where is it defined?
> >>
> >
> > Sorry I meant dev_get_regmap().
> >
> 
> I tried this, but it did not work.
> 
> To make dev_get_regmap() work,
> the parent device needs to call dev_regmap_init_mmio() beforehand.
> 
> 
> Since commit bdb0066df96e74a4002125467ebe459feff1ebef
> (mfd: syscon: Decouple syscon interface from platform devices),
> syscon_probe() is not called for platform devices,
> so that never happens.
> 

Ok. Is the syscon also a simple-mfd?

It sounds like there's a device for the parent, but we've failed
to attach a regmap to it. Maybe the core DT code should assign
the regmap to the parent device when it creates it so that child
devices don't need to know this detail? It could look for
simple-mfd devices with compatible = "syscon" and then create the
regmap? Here's a totally untested patch for that.

8<
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 2f2225e845ef..5f7d3f015b82 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -136,6 +136,17 @@ struct regmap *syscon_node_to_regmap(struct device_node 
*np)
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
+int device_attach_syscon(struct device *dev)
+{
+   struct regmap *regmap;
+
+   regmap = syscon_node_to_regmap(dev->of_node);
+   if (IS_ERR(regmap))
+   return PTR_ERR(regmap);
+
+   return regmap_attach_dev(dev, regmap, _regmap_config);
+}
+
 struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
 {
struct device_node *syscon_np;
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8aa197691074..58a018e15006 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 const struct of_device_id of_default_bus_match_table[] = {
{ .compatible = "simple-bus", },
@@ -383,7 +384,12 @@ static int of_platform_bus_create(struct device_node *bus,
return 0;
}
 
+
dev = of_platform_device_create_pdata(bus, bus_id, platform_data, 
parent);
+   if (of_device_is_compatible(bus, "simple-mfd") &&
+   of_device_is_compatible(bus, "syscon"))
+   device_attach_syscon(>dev);
+
if (!dev || !of_match_node(matches, bus))
return 0;
 
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 40a76b97b7ab..e19e5d15f4e6 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -18,9 +18,11 @@
 #include 
 #include 
 
+struct device;
 struct device_node;
 
 #ifdef CONFIG_MFD_SYSCON
+extern int device_attach_syscon(struct device *dev);
 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_pdevname(const char *s);
@@ -28,6 +30,11 @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
struct device_node *np,
const char *property);
 #else
+static inline int device_attach_syscon(struct device *dev)
+{
+   return 0;
+}
+
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
return ERR_PTR(-ENOTSUPP);
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,


2016-08-20 4:16 GMT+09:00 Stephen Boyd :
>>
>> >> +
>> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
>> >> node */
>> >> + regmap = syscon_node_to_regmap(parent);
>> >> + of_node_put(parent);
>> >
>> > devm_get_regmap(dev->parent) should work then? Why do we need to
>> > use OF APIs?
>>
>> "git grep devm_get_regmap" did not hit anything.
>>
>> Where is it defined?
>>
>
> Sorry I meant dev_get_regmap().
>

I tried this, but it did not work.

To make dev_get_regmap() work,
the parent device needs to call dev_regmap_init_mmio() beforehand.


Since commit bdb0066df96e74a4002125467ebe459feff1ebef
(mfd: syscon: Decouple syscon interface from platform devices),
syscon_probe() is not called for platform devices,
so that never happens.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,


2016-08-20 4:16 GMT+09:00 Stephen Boyd :
>>
>> >> +
>> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
>> >> node */
>> >> + regmap = syscon_node_to_regmap(parent);
>> >> + of_node_put(parent);
>> >
>> > devm_get_regmap(dev->parent) should work then? Why do we need to
>> > use OF APIs?
>>
>> "git grep devm_get_regmap" did not hit anything.
>>
>> Where is it defined?
>>
>
> Sorry I meant dev_get_regmap().
>

I tried this, but it did not work.

To make dev_get_regmap() work,
the parent device needs to call dev_regmap_init_mmio() beforehand.


Since commit bdb0066df96e74a4002125467ebe459feff1ebef
(mfd: syscon: Decouple syscon interface from platform devices),
syscon_probe() is not called for platform devices,
so that never happens.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 08/20, Masahiro Yamada wrote:
> 2016-08-19 9:25 GMT+09:00 Stephen Boyd :
> >> +{
> >> + struct device *dev = >dev;
> >> + const struct of_device_id *match;
> >> + struct clk_hw_onecell_data *hw_data;
> >> + struct device_node *parent;
> >> + struct regmap *regmap;
> >> + const struct uniphier_clk_data *p;
> >> + int clk_num = 0;
> >> +
> >> + match = of_match_node(uniphier_clk_match, dev->of_node);
> >> + if (!match)
> >> + return -ENODEV;
> >
> > We can use of_driver_match_device() to make this simpler.
> 
> 
> I want to use the returned "match".
> The of_driver_match_device() just checks if it matches or not,
> so I do not think it can be the replacement.
> 
> I can use of_match_device() instead, if you like.

Sorry I meant of_device_get_match_data().

> 
> 
> 
> >> +
> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
> >> node */
> >> + regmap = syscon_node_to_regmap(parent);
> >> + of_node_put(parent);
> >
> > devm_get_regmap(dev->parent) should work then? Why do we need to
> > use OF APIs?
> 
> "git grep devm_get_regmap" did not hit anything.
> 
> Where is it defined?
> 

Sorry I meant dev_get_regmap().

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 08/20, Masahiro Yamada wrote:
> 2016-08-19 9:25 GMT+09:00 Stephen Boyd :
> >> +{
> >> + struct device *dev = >dev;
> >> + const struct of_device_id *match;
> >> + struct clk_hw_onecell_data *hw_data;
> >> + struct device_node *parent;
> >> + struct regmap *regmap;
> >> + const struct uniphier_clk_data *p;
> >> + int clk_num = 0;
> >> +
> >> + match = of_match_node(uniphier_clk_match, dev->of_node);
> >> + if (!match)
> >> + return -ENODEV;
> >
> > We can use of_driver_match_device() to make this simpler.
> 
> 
> I want to use the returned "match".
> The of_driver_match_device() just checks if it matches or not,
> so I do not think it can be the replacement.
> 
> I can use of_match_device() instead, if you like.

Sorry I meant of_device_get_match_data().

> 
> 
> 
> >> +
> >> + parent = of_get_parent(dev->of_node); /* parent should be syscon 
> >> node */
> >> + regmap = syscon_node_to_regmap(parent);
> >> + of_node_put(parent);
> >
> > devm_get_regmap(dev->parent) should work then? Why do we need to
> > use OF APIs?
> 
> "git grep devm_get_regmap" did not hit anything.
> 
> Where is it defined?
> 

Sorry I meant dev_get_regmap().

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,


2016-08-19 9:25 GMT+09:00 Stephen Boyd :
>> +int uniphier_clk_probe(struct platform_device *pdev)
>
> static?


Thanks.
Will fix in v7.


>> +{
>> + struct device *dev = >dev;
>> + const struct of_device_id *match;
>> + struct clk_hw_onecell_data *hw_data;
>> + struct device_node *parent;
>> + struct regmap *regmap;
>> + const struct uniphier_clk_data *p;
>> + int clk_num = 0;
>> +
>> + match = of_match_node(uniphier_clk_match, dev->of_node);
>> + if (!match)
>> + return -ENODEV;
>
> We can use of_driver_match_device() to make this simpler.


I want to use the returned "match".
The of_driver_match_device() just checks if it matches or not,
so I do not think it can be the replacement.

I can use of_match_device() instead, if you like.



>> +
>> + parent = of_get_parent(dev->of_node); /* parent should be syscon node 
>> */
>> + regmap = syscon_node_to_regmap(parent);
>> + of_node_put(parent);
>
> devm_get_regmap(dev->parent) should work then? Why do we need to
> use OF APIs?

"git grep devm_get_regmap" did not hit anything.

Where is it defined?



>> + init.name = name;
>> + init.ops = _fixed_factor_ops;
>> + init.flags = data->parent_name ? CLK_SET_RATE_PARENT : 0;
>> + init.flags |= CLK_IS_BASIC;
>
> Please don't use CLK_IS_BASIC unless you need it. So far we've
> kept it to OMAP and I'm hoping to delete it.


Will do in v7.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Masahiro Yamada]

Hi Stephen,


2016-08-19 9:25 GMT+09:00 Stephen Boyd :
>> +int uniphier_clk_probe(struct platform_device *pdev)
>
> static?


Thanks.
Will fix in v7.


>> +{
>> + struct device *dev = >dev;
>> + const struct of_device_id *match;
>> + struct clk_hw_onecell_data *hw_data;
>> + struct device_node *parent;
>> + struct regmap *regmap;
>> + const struct uniphier_clk_data *p;
>> + int clk_num = 0;
>> +
>> + match = of_match_node(uniphier_clk_match, dev->of_node);
>> + if (!match)
>> + return -ENODEV;
>
> We can use of_driver_match_device() to make this simpler.


I want to use the returned "match".
The of_driver_match_device() just checks if it matches or not,
so I do not think it can be the replacement.

I can use of_match_device() instead, if you like.



>> +
>> + parent = of_get_parent(dev->of_node); /* parent should be syscon node 
>> */
>> + regmap = syscon_node_to_regmap(parent);
>> + of_node_put(parent);
>
> devm_get_regmap(dev->parent) should work then? Why do we need to
> use OF APIs?

"git grep devm_get_regmap" did not hit anything.

Where is it defined?



>> + init.name = name;
>> + init.ops = _fixed_factor_ops;
>> + init.flags = data->parent_name ? CLK_SET_RATE_PARENT : 0;
>> + init.flags |= CLK_IS_BASIC;
>
> Please don't use CLK_IS_BASIC unless you need it. So far we've
> kept it to OMAP and I'm hoping to delete it.


Will do in v7.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 08/02, Masahiro Yamada wrote:
> diff --git a/drivers/clk/uniphier/clk-uniphier-core.c 
> b/drivers/clk/uniphier/clk-uniphier-core.c
> new file mode 100644
> index 000..d6dfa4d
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-uniphier-core.c
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright (C) 2016 Socionext Inc.
> + *   Author: Masahiro Yamada 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "clk-uniphier.h"
> +
> +static struct clk_hw *uniphier_clk_register(struct device *dev,
> + struct regmap *regmap,
> + const struct uniphier_clk_data *data)
> +{
> + switch (data->type) {
> + case UNIPHIER_CLK_TYPE_FIXED_FACTOR:
> + return uniphier_clk_register_fixed_factor(dev, data->name,
> +   >data.factor);
> + case UNIPHIER_CLK_TYPE_FIXED_RATE:
> + return uniphier_clk_register_fixed_rate(dev, data->name,
> + >data.rate);
> + case UNIPHIER_CLK_TYPE_GATE:
> + return uniphier_clk_register_gate(dev, regmap, data->name,
> +   >data.gate);
> + case UNIPHIER_CLK_TYPE_MUX:
> + return uniphier_clk_register_mux(dev, regmap, data->name,
> +  >data.mux);
> + default:
> + dev_err(dev, "unsupported clock type\n");
> + return ERR_PTR(-EINVAL);
> + }
> +}
> +
> +static const struct of_device_id uniphier_clk_match[] = {
> + { /* sentinel */ }

Looks funny but I guess we'll fill this in later so it's ok.

> +};
> +MODULE_DEVICE_TABLE(of, uniphier_clk_match);
> +
> +int uniphier_clk_probe(struct platform_device *pdev)

static?

> +{
> + struct device *dev = >dev;
> + const struct of_device_id *match;
> + struct clk_hw_onecell_data *hw_data;
> + struct device_node *parent;
> + struct regmap *regmap;
> + const struct uniphier_clk_data *p;
> + int clk_num = 0;
> +
> + match = of_match_node(uniphier_clk_match, dev->of_node);
> + if (!match)
> + return -ENODEV;

We can use of_driver_match_device() to make this simpler.

> +
> + parent = of_get_parent(dev->of_node); /* parent should be syscon node */
> + regmap = syscon_node_to_regmap(parent);
> + of_node_put(parent);

devm_get_regmap(dev->parent) should work then? Why do we need to
use OF APIs?

> + if (IS_ERR(regmap)) {
> + dev_err(dev, "failed to get regmap (error %ld)\n",
> + PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + for (p = match->data; p->name; p++)
> + clk_num = max(clk_num, p->idx + 1);
> +
> + hw_data = devm_kzalloc(dev,
> + sizeof(*hw_data) + clk_num * sizeof(struct clk_hw *),
> + GFP_KERNEL);
> + if (!hw_data)
> + return -ENOMEM;
> +
> + hw_data->num = clk_num;
> +
> + for (p = match->data; p->name; p++) {
> + struct clk_hw *hw;
> +
> + dev_dbg(dev, "register %s (index=%d)\n", p->name, p->idx);
> + hw = uniphier_clk_register(dev, regmap, p);
> + if (IS_ERR(hw)) {
> + dev_err(dev, "failed to register %s (error %ld)\n",
> + p->name, PTR_ERR(hw));
> + return PTR_ERR(hw);
> + }
> +
> + if (p->idx >= 0)
> + hw_data->hws[p->idx] = hw;
> + }
> +
> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> +   hw_data);
> +}
> +
> +int uniphier_clk_remove(struct platform_device *pdev)

static?

> +{
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + return 0;
> +}
> diff --git a/drivers/clk/uniphier/clk-uniphier-fixed-factor.c 
> b/drivers/clk/uniphier/clk-uniphier-fixed-factor.c
> new file mode 100644
> index 000..d64ea61
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-uniphier-fixed-factor.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (C) 2016 Socionext Inc.
> + *   Author: Masahiro Yamada 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free 

Re: [PATCH v6 1/2] clk: uniphier: add core support code for UniPhier clock driver

[Stephen Boyd]

On 08/02, Masahiro Yamada wrote:
> diff --git a/drivers/clk/uniphier/clk-uniphier-core.c 
> b/drivers/clk/uniphier/clk-uniphier-core.c
> new file mode 100644
> index 000..d6dfa4d
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-uniphier-core.c
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright (C) 2016 Socionext Inc.
> + *   Author: Masahiro Yamada 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "clk-uniphier.h"
> +
> +static struct clk_hw *uniphier_clk_register(struct device *dev,
> + struct regmap *regmap,
> + const struct uniphier_clk_data *data)
> +{
> + switch (data->type) {
> + case UNIPHIER_CLK_TYPE_FIXED_FACTOR:
> + return uniphier_clk_register_fixed_factor(dev, data->name,
> +   >data.factor);
> + case UNIPHIER_CLK_TYPE_FIXED_RATE:
> + return uniphier_clk_register_fixed_rate(dev, data->name,
> + >data.rate);
> + case UNIPHIER_CLK_TYPE_GATE:
> + return uniphier_clk_register_gate(dev, regmap, data->name,
> +   >data.gate);
> + case UNIPHIER_CLK_TYPE_MUX:
> + return uniphier_clk_register_mux(dev, regmap, data->name,
> +  >data.mux);
> + default:
> + dev_err(dev, "unsupported clock type\n");
> + return ERR_PTR(-EINVAL);
> + }
> +}
> +
> +static const struct of_device_id uniphier_clk_match[] = {
> + { /* sentinel */ }

Looks funny but I guess we'll fill this in later so it's ok.

> +};
> +MODULE_DEVICE_TABLE(of, uniphier_clk_match);
> +
> +int uniphier_clk_probe(struct platform_device *pdev)

static?

> +{
> + struct device *dev = >dev;
> + const struct of_device_id *match;
> + struct clk_hw_onecell_data *hw_data;
> + struct device_node *parent;
> + struct regmap *regmap;
> + const struct uniphier_clk_data *p;
> + int clk_num = 0;
> +
> + match = of_match_node(uniphier_clk_match, dev->of_node);
> + if (!match)
> + return -ENODEV;

We can use of_driver_match_device() to make this simpler.

> +
> + parent = of_get_parent(dev->of_node); /* parent should be syscon node */
> + regmap = syscon_node_to_regmap(parent);
> + of_node_put(parent);

devm_get_regmap(dev->parent) should work then? Why do we need to
use OF APIs?

> + if (IS_ERR(regmap)) {
> + dev_err(dev, "failed to get regmap (error %ld)\n",
> + PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + for (p = match->data; p->name; p++)
> + clk_num = max(clk_num, p->idx + 1);
> +
> + hw_data = devm_kzalloc(dev,
> + sizeof(*hw_data) + clk_num * sizeof(struct clk_hw *),
> + GFP_KERNEL);
> + if (!hw_data)
> + return -ENOMEM;
> +
> + hw_data->num = clk_num;
> +
> + for (p = match->data; p->name; p++) {
> + struct clk_hw *hw;
> +
> + dev_dbg(dev, "register %s (index=%d)\n", p->name, p->idx);
> + hw = uniphier_clk_register(dev, regmap, p);
> + if (IS_ERR(hw)) {
> + dev_err(dev, "failed to register %s (error %ld)\n",
> + p->name, PTR_ERR(hw));
> + return PTR_ERR(hw);
> + }
> +
> + if (p->idx >= 0)
> + hw_data->hws[p->idx] = hw;
> + }
> +
> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> +   hw_data);
> +}
> +
> +int uniphier_clk_remove(struct platform_device *pdev)

static?

> +{
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + return 0;
> +}
> diff --git a/drivers/clk/uniphier/clk-uniphier-fixed-factor.c 
> b/drivers/clk/uniphier/clk-uniphier-fixed-factor.c
> new file mode 100644
> index 000..d64ea61
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-uniphier-fixed-factor.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (C) 2016 Socionext Inc.
> + *   Author: Masahiro Yamada 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + *