[PATCH 0/3] clk: support regmap

[PATCH 0/3] clk: support regmap

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
reset functionality, so we need make sure the access to the PCC register
be protected to avoid concurrent access from clk and reset subsystem.

So let's use regmap here.

The i.MX specific code will be posted if this patchset is ok for you.

Peng Fan (3):
  clk: mux: support regmap
  clk: fractional-divider: support regmap
  clk: gate: support regmap

 drivers/clk/clk-fractional-divider.c | 26 +++++++++++++++++++++++---
 drivers/clk/clk-gate.c               | 26 +++++++++++++++++++++++---
 drivers/clk/clk-mux.c                | 26 +++++++++++++++++++++++---
 include/linux/clk-provider.h         | 14 ++++++++++++++
 4 files changed, 83 insertions(+), 9 deletions(-)

-- 
2.30.0

[PATCH 1/3] clk: mux: support regmap

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
reset functionality, so we need make sure the access to the PCC register
be protected to avoid concurrent access from clk and reset subsystem.

So let's use regmap here.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk-mux.c        | 26 +++++++++++++++++++++++---
 include/linux/clk-provider.h |  6 ++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 20582aae7a35..90facb6eafe6 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -10,6 +10,7 @@
 #include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/err.h>
@@ -26,18 +27,37 @@
 
 static inline u32 clk_mux_readl(struct clk_mux *mux)
 {
-	if (mux->flags & CLK_MUX_BIG_ENDIAN)
+	int ret;
+	uint32_t val;
+
+	if (mux->flags & CLK_MUX_BIG_ENDIAN) {
 		return ioread32be(mux->reg);
+	} else if (mux->flags & CLK_MUX_REGMAP) {
+		ret = regmap_read(mux->regmap, mux->reg_off, &val);
+		if (ret < 0) {
+			pr_warn("%s: failed read %x, %d\n", __func__, mux->reg_off, ret);
+			return ret;
+		} else {
+			return val;
+		}
+	}
 
 	return readl(mux->reg);
 }
 
 static inline void clk_mux_writel(struct clk_mux *mux, u32 val)
 {
-	if (mux->flags & CLK_MUX_BIG_ENDIAN)
+	int ret;
+
+	if (mux->flags & CLK_MUX_BIG_ENDIAN) {
 		iowrite32be(val, mux->reg);
-	else
+	} else if (mux->flags & CLK_MUX_REGMAP) {
+		ret = regmap_write(mux->regmap, mux->reg_off, val);
+		if (ret < 0)
+			pr_warn("%s: failed write %x, %d\n", __func__, mux->reg_off, ret);
+	} else {
 		writel(val, mux->reg);
+	}
 }
 
 int clk_mux_val_to_index(struct clk_hw *hw, u32 *table, unsigned int flags,
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 162a2e5546a3..6bd9288b953d 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -837,6 +837,8 @@ void clk_hw_unregister_divider(struct clk_hw *hw);
  * @mask:	mask of mutliplexer bit field
  * @flags:	hardware-specific flags
  * @lock:	register lock
+ * @regmap:	register controlling regmap
+ * @reg_off:	register offset
  *
  * Clock with multiple selectable parents.  Implements .get_parent, .set_parent
  * and .recalc_rate
@@ -855,6 +857,7 @@ void clk_hw_unregister_divider(struct clk_hw *hw);
  * CLK_MUX_BIG_ENDIAN - By default little endian register accesses are used for
  *	the mux register.  Setting this flag makes the register accesses big
  *	endian.
+ * CLK_MUX_REGMAP - Indicate the accessing method is using regmap API.
  */
 struct clk_mux {
 	struct clk_hw	hw;
@@ -864,6 +867,8 @@ struct clk_mux {
 	u8		shift;
 	u8		flags;
 	spinlock_t	*lock;
+	struct regmap	*regmap;
+	u32		reg_off;
 };
 
 #define to_clk_mux(_hw) container_of(_hw, struct clk_mux, hw)
@@ -874,6 +879,7 @@ struct clk_mux {
 #define CLK_MUX_READ_ONLY		BIT(3) /* mux can't be changed */
 #define CLK_MUX_ROUND_CLOSEST		BIT(4)
 #define CLK_MUX_BIG_ENDIAN		BIT(5)
+#define CLK_MUX_REGMAP			BIT(6)
 
 extern const struct clk_ops clk_mux_ops;
 extern const struct clk_ops clk_mux_ro_ops;
-- 
2.30.0

[PATCH 2/3] clk: fractional-divider: support regmap

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
reset functionality, so we need make sure the access to the PCC register
be protected to avoid concurrent access from clk and reset subsystem.

So let's use regmap here.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk-fractional-divider.c | 26 +++++++++++++++++++++++---
 include/linux/clk-provider.h         |  5 +++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index b1e556f20911..f2df5d278214 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -11,23 +11,43 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/rational.h>
 
 static inline u32 clk_fd_readl(struct clk_fractional_divider *fd)
 {
-	if (fd->flags & CLK_FRAC_DIVIDER_BIG_ENDIAN)
+	int ret;
+	u32 val;
+
+	if (fd->flags & CLK_FRAC_DIVIDER_BIG_ENDIAN) {
 		return ioread32be(fd->reg);
+	} else if (fd->flags & CLK_FRAC_DIVIDER_REGMAP) {
+		ret = regmap_read(fd->regmap, fd->reg_off, &val);
+		if (ret < 0) {
+			pr_warn("%s: failed %x, %d\n", __func__, fd->reg_off, ret);
+			return ret;
+		} else {
+			return val;
+		}
+	}
 
 	return readl(fd->reg);
 }
 
 static inline void clk_fd_writel(struct clk_fractional_divider *fd, u32 val)
 {
-	if (fd->flags & CLK_FRAC_DIVIDER_BIG_ENDIAN)
+	int ret;
+
+	if (fd->flags & CLK_FRAC_DIVIDER_BIG_ENDIAN) {
 		iowrite32be(val, fd->reg);
-	else
+	} else if (fd->flags & CLK_FRAC_DIVIDER_REGMAP) {
+		ret = regmap_write(fd->regmap, fd->reg_off, val);
+		if (ret < 0)
+			pr_warn("%s: failed %x, %d\n", __func__, fd->reg_off, ret);
+	} else {
 		writel(val, fd->reg);
+	}
 }
 
 static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 6bd9288b953d..3902f883cdaf 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -990,6 +990,8 @@ struct clk_hw *devm_clk_hw_register_fixed_factor(struct device *dev,
  * @nshift:	shift to the denominator bit field
  * @nwidth:	width of the denominator bit field
  * @lock:	register lock
+ * @regmap:	register regmap
+ * @reg_off:	register offset
  *
  * Clock with adjustable fractional divider affecting its output frequency.
  *
@@ -1012,6 +1014,8 @@ struct clk_fractional_divider {
 	u8		nwidth;
 	u32		nmask;
 	u8		flags;
+	struct regmap	*regmap;
+	u32		reg_off;
 	void		(*approximation)(struct clk_hw *hw,
 				unsigned long rate, unsigned long *parent_rate,
 				unsigned long *m, unsigned long *n);
@@ -1022,6 +1026,7 @@ struct clk_fractional_divider {
 
 #define CLK_FRAC_DIVIDER_ZERO_BASED		BIT(0)
 #define CLK_FRAC_DIVIDER_BIG_ENDIAN		BIT(1)
+#define CLK_FRAC_DIVIDER_REGMAP			BIT(2)
 
 extern const struct clk_ops clk_fractional_divider_ops;
 struct clk *clk_register_fractional_divider(struct device *dev,
-- 
2.30.0

[PATCH 3/3] clk: gate: support regmap

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
reset functionality, so we need make sure the access to the PCC register
be protected to avoid concurrent access from clk and reset subsystem.

So let's use regmap here.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk-gate.c       | 26 +++++++++++++++++++++++---
 include/linux/clk-provider.h |  3 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 070dc47e95a1..1acaa2f5a969 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -8,6 +8,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/err.h>
@@ -25,18 +26,37 @@
 
 static inline u32 clk_gate_readl(struct clk_gate *gate)
 {
-	if (gate->flags & CLK_GATE_BIG_ENDIAN)
+	int ret;
+	u32 val;
+
+	if (gate->flags & CLK_GATE_BIG_ENDIAN) {
 		return ioread32be(gate->reg);
+	} else if (gate->flags & CLK_GATE_REGMAP) {
+		ret = regmap_read(gate->regmap, gate->reg_off, &val);
+		if (ret < 0) {
+			pr_warn("%s: failed %x, %d\n", __func__, gate->reg_off, ret);
+			return ret;
+		} else {
+			return val;
+		}
+	}
 
 	return readl(gate->reg);
 }
 
 static inline void clk_gate_writel(struct clk_gate *gate, u32 val)
 {
-	if (gate->flags & CLK_GATE_BIG_ENDIAN)
+	int ret;
+
+	if (gate->flags & CLK_GATE_BIG_ENDIAN) {
 		iowrite32be(val, gate->reg);
-	else
+	} else if (gate->flags & CLK_GATE_REGMAP) {
+		ret = regmap_write(gate->regmap, gate->reg_off, val);
+		if (ret < 0)
+			pr_warn("%s: %x: %d\n", __func__, gate->reg_off, ret);
+	} else {
 		writel(val, gate->reg);
+	}
 }
 
 /*
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 3902f883cdaf..0a4c01a023cc 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -474,6 +474,8 @@ struct clk_gate {
 	u8		bit_idx;
 	u8		flags;
 	spinlock_t	*lock;
+	struct regmap	*regmap;
+	u32		reg_off;
 };
 
 #define to_clk_gate(_hw) container_of(_hw, struct clk_gate, hw)
@@ -481,6 +483,7 @@ struct clk_gate {
 #define CLK_GATE_SET_TO_DISABLE		BIT(0)
 #define CLK_GATE_HIWORD_MASK		BIT(1)
 #define CLK_GATE_BIG_ENDIAN		BIT(2)
+#define CLK_GATE_REGMAP			BIT(3)
 
 extern const struct clk_ops clk_gate_ops;
 struct clk_hw *__clk_hw_register_gate(struct device *dev,
-- 
2.30.0

Re: [PATCH 0/3] clk: support regmap

[Stephen Boyd]

Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
> From: Peng Fan <peng.fan@nxp.com>
> 
> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
> reset functionality, so we need make sure the access to the PCC register
> be protected to avoid concurrent access from clk and reset subsystem.
> 
> So let's use regmap here.
> 
> The i.MX specific code will be posted if this patchset is ok for you.

We have a couple regmap clk drivers in the tree. Either combine the
different regmap clk drivers or duplicate it into the imx directory. I'd
prefer we combine them but last time I couldn't agree on the approach
when Jerome wanted to do it. Maybe now is the time to combine them all
into one common piece of code.

> 
> Peng Fan (3):
>   clk: mux: support regmap
>   clk: fractional-divider: support regmap
>   clk: gate: support regmap

Re: [PATCH 0/3] clk: support regmap

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1244 bytes --]

On 6/2/21 10:18 AM, Stephen Boyd wrote:
> Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
>> reset functionality, so we need make sure the access to the PCC register
>> be protected to avoid concurrent access from clk and reset subsystem.
>>
>> So let's use regmap here.
>>
>> The i.MX specific code will be posted if this patchset is ok for you.
> 
> We have a couple regmap clk drivers in the tree. Either combine the
> different regmap clk drivers or duplicate it into the imx directory. I'd
> prefer we combine them but last time I couldn't agree on the approach
> when Jerome wanted to do it. Maybe now is the time to combine them all
> into one common piece of code.

IMHO for the basic drivers, such as gate, divider, mux, mux-div, etc... it makes
no sense to have them in an arch specific subdir, like meson.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH 0/3] clk: support regmap

[Peng Fan]

> Subject: Re: [PATCH 0/3] clk: support regmap
> 
> On 6/2/21 10:18 AM, Stephen Boyd wrote:
> > Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
> >> From: Peng Fan <peng.fan@nxp.com>
> >>
> >> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and
> >> peripheral reset functionality, so we need make sure the access to
> >> the PCC register be protected to avoid concurrent access from clk and
> reset subsystem.
> >>
> >> So let's use regmap here.
> >>
> >> The i.MX specific code will be posted if this patchset is ok for you.
> >
> > We have a couple regmap clk drivers in the tree. Either combine the
> > different regmap clk drivers or duplicate it into the imx directory.
> > I'd prefer we combine them but last time I couldn't agree on the
> > approach when Jerome wanted to do it. Maybe now is the time to combine
> > them all into one common piece of code.
> 
> IMHO for the basic drivers, such as gate, divider, mux, mux-div, etc... it makes
> no sense to have them in an arch specific subdir, like meson.

Yeah. Let's make it as common piece code could be reused by others.

Thanks,
Peng.

> 
> regards,
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/3] clk: support regmap

[Jerome Brunet]

On Wed 02 Jun 2021 at 10:21, Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> On 6/2/21 10:18 AM, Stephen Boyd wrote:
>> Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
>>> reset functionality, so we need make sure the access to the PCC register
>>> be protected to avoid concurrent access from clk and reset subsystem.
>>>
>>> So let's use regmap here.
>>>
>>> The i.MX specific code will be posted if this patchset is ok for you.
>> 
>> We have a couple regmap clk drivers in the tree. Either combine the
>> different regmap clk drivers or duplicate it into the imx directory. I'd
>> prefer we combine them but last time I couldn't agree on the approach
>> when Jerome wanted to do it. Maybe now is the time to combine them all
>> into one common piece of code.
>
> IMHO for the basic drivers, such as gate, divider, mux, mux-div, etc... it makes
> no sense to have them in an arch specific subdir, like meson.

Indeed, those basic types were not meant to remain platform
specific. Some framework (ASoC for ex) make heavy use of regmap and
could welcome regmap based basic clock types.

At the time, Stephen (qcom) and I (meson) had slightly different
approaches. Before having those types spread through the kernel, I think
testing things out was a good thing ... this is why these are platform
specific ATM.

It's been 3 years now ... and it has not been a total disaster :)

In the end things are not so different. Let's compare:
a. Both have a generic "clk_regmap" type common to all regmap based
  types. This is very useful to easily fix the regmap pointer in static
  clocks (which are heavily used by both platform)

b. Meson uses a generic pointer to store the type specific info.
  Qcom embeds the generic clk_regmap into the specific type one.
  => In the end, I don't see any advantage to the meson
  approach. Switching to the qcom method would be quite a big bang
  for meson but it is doable (nothing difficult, just a huge line count)
  
c. Qcom basic regmap type deviates a bit from the regular basic ones
  when it comes to the actual data. The qcom "clk_regmap" also provides
  the gate, mux have the qcom "parent_map". In meson, I tried to keep as
  close as possible to regular basic types ... at least what they were 3
  years ago. Only the register poking part should be different actually.
  => I'd be in favor of keeping close to meson here.

A possible plan could be:
1. Rework meson as explained in [b] above.
2. reword types in qcom where necessary to avoid name clashes (add
   "_qcom" extension for ex)
3. Move the clk_regmap implementation out of meson to drivers/clk
4. Things are yours to play with ...

I can take care of 1. and 3. I would welcome help for 2. especially since
I won't be able to test it.

>
> regards,
> Marc

Re: [PATCH 0/3] clk: support regmap

[Jerome Brunet]

On Wed 02 Jun 2021 at 11:32, Jerome Brunet <jbrunet@baylibre.com> wrote:

> On Wed 02 Jun 2021 at 10:21, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
>> On 6/2/21 10:18 AM, Stephen Boyd wrote:
>>> Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>
>>>> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
>>>> reset functionality, so we need make sure the access to the PCC register
>>>> be protected to avoid concurrent access from clk and reset subsystem.
>>>>
>>>> So let's use regmap here.
>>>>
>>>> The i.MX specific code will be posted if this patchset is ok for you.
>>> 
>>> We have a couple regmap clk drivers in the tree. Either combine the
>>> different regmap clk drivers or duplicate it into the imx directory. I'd
>>> prefer we combine them but last time I couldn't agree on the approach
>>> when Jerome wanted to do it. Maybe now is the time to combine them all
>>> into one common piece of code.
>>
>> IMHO for the basic drivers, such as gate, divider, mux, mux-div, etc... it makes
>> no sense to have them in an arch specific subdir, like meson.
>
> Indeed, those basic types were not meant to remain platform
> specific. Some framework (ASoC for ex) make heavy use of regmap and
> could welcome regmap based basic clock types.
>
> At the time, Stephen (qcom) and I (meson) had slightly different
> approaches. Before having those types spread through the kernel, I think
> testing things out was a good thing ... this is why these are platform
> specific ATM.
>
> It's been 3 years now ... and it has not been a total disaster :)
>
> In the end things are not so different. Let's compare:
> a. Both have a generic "clk_regmap" type common to all regmap based
>   types. This is very useful to easily fix the regmap pointer in static
>   clocks (which are heavily used by both platform)
>
> b. Meson uses a generic pointer to store the type specific info.
>   Qcom embeds the generic clk_regmap into the specific type one.
>   => In the end, I don't see any advantage to the meson
>   approach. Switching to the qcom method would be quite a big bang
>   for meson but it is doable (nothing difficult, just a huge line count)
>   
> c. Qcom basic regmap type deviates a bit from the regular basic ones
>   when it comes to the actual data. The qcom "clk_regmap" also provides
>   the gate, mux have the qcom "parent_map". In meson, I tried to keep as
>   close as possible to regular basic types ... at least what they were 3
>   years ago. Only the register poking part should be different actually.
>   => I'd be in favor of keeping close to meson here.
>
> A possible plan could be:
> 1. Rework meson as explained in [b] above.
> 2. reword types in qcom where necessary to avoid name clashes (add
>    "_qcom" extension for ex)
> 3. Move the clk_regmap implementation out of meson to drivers/clk
> 4. Things are yours to play with ...
>
> I can take care of 1. and 3. I would welcome help for 2. especially since
> I won't be able to test it.
>

Hi Stephen,

What do you think of the proposition above ?
There rework is going to take some time to do. I'll start only if this
OK with you.

Cheers
Jerome

>>
>> regards,
>> Marc

Re: [PATCH 0/3] clk: support regmap

[Stephen Boyd]

Quoting Jerome Brunet (2021-06-18 07:30:59)
> 
> On Wed 02 Jun 2021 at 11:32, Jerome Brunet <jbrunet@baylibre.com> wrote:
> 
> > On Wed 02 Jun 2021 at 10:21, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> >
> >> On 6/2/21 10:18 AM, Stephen Boyd wrote:
> >>> Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
> >>>> From: Peng Fan <peng.fan@nxp.com>
> >>>>
> >>>> To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and peripheral
> >>>> reset functionality, so we need make sure the access to the PCC register
> >>>> be protected to avoid concurrent access from clk and reset subsystem.
> >>>>
> >>>> So let's use regmap here.
> >>>>
> >>>> The i.MX specific code will be posted if this patchset is ok for you.
> >>> 
> >>> We have a couple regmap clk drivers in the tree. Either combine the
> >>> different regmap clk drivers or duplicate it into the imx directory. I'd
> >>> prefer we combine them but last time I couldn't agree on the approach
> >>> when Jerome wanted to do it. Maybe now is the time to combine them all
> >>> into one common piece of code.
> >>
> >> IMHO for the basic drivers, such as gate, divider, mux, mux-div, etc... it makes
> >> no sense to have them in an arch specific subdir, like meson.
> >
> > Indeed, those basic types were not meant to remain platform
> > specific. Some framework (ASoC for ex) make heavy use of regmap and
> > could welcome regmap based basic clock types.
> >
> > At the time, Stephen (qcom) and I (meson) had slightly different
> > approaches. Before having those types spread through the kernel, I think
> > testing things out was a good thing ... this is why these are platform
> > specific ATM.
> >
> > It's been 3 years now ... and it has not been a total disaster :)
> >
> > In the end things are not so different. Let's compare:
> > a. Both have a generic "clk_regmap" type common to all regmap based
> >   types. This is very useful to easily fix the regmap pointer in static
> >   clocks (which are heavily used by both platform)
> >
> > b. Meson uses a generic pointer to store the type specific info.
> >   Qcom embeds the generic clk_regmap into the specific type one.
> >   => In the end, I don't see any advantage to the meson
> >   approach. Switching to the qcom method would be quite a big bang
> >   for meson but it is doable (nothing difficult, just a huge line count)
> >   
> > c. Qcom basic regmap type deviates a bit from the regular basic ones
> >   when it comes to the actual data. The qcom "clk_regmap" also provides
> >   the gate, mux have the qcom "parent_map". In meson, I tried to keep as
> >   close as possible to regular basic types ... at least what they were 3
> >   years ago. Only the register poking part should be different actually.
> >   => I'd be in favor of keeping close to meson here.
> >
> > A possible plan could be:
> > 1. Rework meson as explained in [b] above.
> > 2. reword types in qcom where necessary to avoid name clashes (add
> >    "_qcom" extension for ex)
> > 3. Move the clk_regmap implementation out of meson to drivers/clk
> > 4. Things are yours to play with ...
> >
> > I can take care of 1. and 3. I would welcome help for 2. especially since
> > I won't be able to test it.
> >
> 
> Hi Stephen,
> 
> What do you think of the proposition above ?
> There rework is going to take some time to do. I'll start only if this
> OK with you.

Hey Jerome,

Sorry I wrote a long email and then it got lost when my machine
rebooted. :( That was weeks ago! :( :(

Anyway, we have struct device now associated with clks when they're
registered (yay!), so I wonder if perhaps we can make an API or two like
clk_hw_get_regmap(struct clk_hw *) that takes a clk_hw and finds the
device associated with the clk and then calls dev_get_regmap() with that
device pointer to get the regmap pointer. Then clk drivers can use the
regmap pointer wherever they want. The basic clk type code can be
modified to either have a flag like "I_AM_A_REGMAP_CLK" (please pick
better name), or to have an init clk_op that stores the regmap pointer
in the basic type structure if grabbing it all the time is too costly
(no idea here so it really needs a measurement).

We do have some clks that are registered with devices that have
more than one regmap. The clk_hw_get_regmap() API would need to probably
take a string argument too, similar to dev_get_regmap() that lets clk
providers figure out what regmap they're asking for if there's more than
one associated with it.

With that API (and probably one for clk_hw_get_dev(struct clk_hw *)) we
should be able to avoid having to consolidate on a 'struct clk_regmap'
and let the various drivers keep doing what they're doing with the
regmap pointer. I know this may not be what you're envisioning, but I
think it would be a good step to take so that we can simplify some of the
registration code and add regmap support to the basic types in a way
that isn't heavily invasive because we're changing structs or struct
members all over the place.

I for one don't look forward to reviewing a huge rewrite of any clk
driver :)

RE: [PATCH 0/3] clk: support regmap

[Peng Fan]

> Subject: Re: [PATCH 0/3] clk: support regmap
> 
> Quoting Peng Fan (OSS) (2021-05-28 04:34:00)
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > To i.MX8ULP, a PCC register provides clk(mux, gate, divider) and
> > peripheral reset functionality, so we need make sure the access to the
> > PCC register be protected to avoid concurrent access from clk and reset
> subsystem.
> >
> > So let's use regmap here.
> >
> > The i.MX specific code will be posted if this patchset is ok for you.
> 
> We have a couple regmap clk drivers in the tree. Either combine the different
> regmap clk drivers or duplicate it into the imx directory. I'd prefer we combine
> them but last time I couldn't agree on the approach when Jerome wanted to
> do it.

You mean Jerome has some work to combine them all into one common?
Could you please share me the patch link?

 Maybe now is the time to combine them all into one common piece of
> code.

ok, I see.

Thanks,
Peng.

> 
> >
> > Peng Fan (3):
> >   clk: mux: support regmap
> >   clk: fractional-divider: support regmap
> >   clk: gate: support regmap