[PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Marijn Suijten]

As stated in [1] I promised to tackle and send this series.

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

This series starts out by adding extra constructors for divider, mux and
fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
of some DT index or name.  Followed by individual patches performing the
conversion, one DSI PHY at a time.

dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
devm_kzalloc allocations (for the lifetime of the device) with
stack-local char arrays, like all the other DSI PHY drivers.

I couldn't help but notice that clock names are wildly varying:

- Some use underscores in the _clk suffix where others have nothing;
- Some have an _ after the %d, others have not;
- Some use a _pll suffix after dsi%d or even _phy_pll suffix.

Are there any thoughts or feelings towards unifying these?
Theoretically no clock names are used anywhere in the kernel, and
everything is based on a phandle + index in DT (I have yet to validate
this).  Obviously no .name/.fw_name will be updated to not break DT.

Which, by the way, is there a particular reason for:

  #define DSI_BYTE_PLL_CLK		0
  #define DSI_PIXEL_PLL_CLK		1

To not be in the dt-bindings and used in the DT?

And with enough future improvements out of the way, let's round out this
patch-series by stating that it has been successfully tested on:

- Sony Nile Discovery (Xperia XA2 Ultra): 14nm;
- Sony Seine PDX201 (Xperia 10II): 14nm;
- Sony Loire Suzu (Xperia X): 28nm.

And no diff is observed in debugfs's clk_summary.

Unfortunately all other devices in my collection with a 7/10nm DSI PHY
have a DSC panel which we have yet to get working.

[1]: https://lore.kernel.org/linux-arm-msm/20220502214235.s5plebunh4ttjhge@SoMainline.org/

Marijn Suijten (9):
  clk: divider: Introduce devm_clk_hw_register_divider_parent_hw()
  clk: mux: Introduce devm_clk_hw_register_mux_parent_hws()
  clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw()
  drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names
  drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers

 drivers/clk/clk-fixed-factor.c                | 57 ++++++++++--
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c    | 92 ++++++++-----------
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    | 36 ++++----
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c    | 52 +++++------
 .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c   | 26 ++----
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     | 92 +++++++++----------
 include/linux/clk-provider.h                  | 34 +++++++
 7 files changed, 209 insertions(+), 180 deletions(-)

--
2.36.1

[PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Marijn Suijten]

As stated in [1] I promised to tackle and send this series.

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

This series starts out by adding extra constructors for divider, mux and
fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
of some DT index or name.  Followed by individual patches performing the
conversion, one DSI PHY at a time.

dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
devm_kzalloc allocations (for the lifetime of the device) with
stack-local char arrays, like all the other DSI PHY drivers.

I couldn't help but notice that clock names are wildly varying:

- Some use underscores in the _clk suffix where others have nothing;
- Some have an _ after the %d, others have not;
- Some use a _pll suffix after dsi%d or even _phy_pll suffix.

Are there any thoughts or feelings towards unifying these?
Theoretically no clock names are used anywhere in the kernel, and
everything is based on a phandle + index in DT (I have yet to validate
this).  Obviously no .name/.fw_name will be updated to not break DT.

Which, by the way, is there a particular reason for:

  #define DSI_BYTE_PLL_CLK		0
  #define DSI_PIXEL_PLL_CLK		1

To not be in the dt-bindings and used in the DT?

And with enough future improvements out of the way, let's round out this
patch-series by stating that it has been successfully tested on:

- Sony Nile Discovery (Xperia XA2 Ultra): 14nm;
- Sony Seine PDX201 (Xperia 10II): 14nm;
- Sony Loire Suzu (Xperia X): 28nm.

And no diff is observed in debugfs's clk_summary.

Unfortunately all other devices in my collection with a 7/10nm DSI PHY
have a DSC panel which we have yet to get working.

[1]: https://lore.kernel.org/linux-arm-msm/20220502214235.s5plebunh4ttjhge@SoMainline.org/

Marijn Suijten (9):
  clk: divider: Introduce devm_clk_hw_register_divider_parent_hw()
  clk: mux: Introduce devm_clk_hw_register_mux_parent_hws()
  clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw()
  drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names
  drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers
  drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers

 drivers/clk/clk-fixed-factor.c                | 57 ++++++++++--
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c    | 92 ++++++++-----------
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    | 36 ++++----
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c    | 52 +++++------
 .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c   | 26 ++----
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     | 92 +++++++++----------
 include/linux/clk-provider.h                  | 34 +++++++
 7 files changed, 209 insertions(+), 180 deletions(-)

--
2.36.1

[PATCH 1/9] clk: divider: Introduce devm_clk_hw_register_divider_parent_hw()

[Marijn Suijten]

Add the devres variant of clk_hw_register_divider_parent_hw() for
registering a divider clock with clk_hw parent pointer instead of parent
name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 include/linux/clk-provider.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c10dc4c659e2..4e07621849e6 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -831,6 +831,25 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
 	__devm_clk_hw_register_divider((dev), NULL, (name), (parent_name), NULL,   \
 				  NULL, (flags), (reg), (shift), (width),     \
 				  (clk_divider_flags), NULL, (lock))
+/**
+ * devm_clk_hw_register_divider_parent_hw - register a divider clock with the clock framework
+ * @dev: device registering this clock
+ * @name: name of this clock
+ * @parent_hw: pointer to parent clk
+ * @flags: framework-specific flags
+ * @reg: register address to adjust divider
+ * @shift: number of bits to shift the bitfield
+ * @width: width of the bitfield
+ * @clk_divider_flags: divider-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+#define devm_clk_hw_register_divider_parent_hw(dev, name, parent_hw, flags,   \
+					       reg, shift, width,	      \
+					       clk_divider_flags, lock)       \
+	__devm_clk_hw_register_divider((dev), NULL, (name), NULL,	      \
+				       (parent_hw), NULL, (flags), (reg),     \
+				       (shift), (width), (clk_divider_flags), \
+				       NULL, (lock))
 /**
  * devm_clk_hw_register_divider_table - register a table based divider clock
  * with the clock framework (devres variant)
-- 
2.36.1

[PATCH 1/9] clk: divider: Introduce devm_clk_hw_register_divider_parent_hw()

[Marijn Suijten]

Add the devres variant of clk_hw_register_divider_parent_hw() for
registering a divider clock with clk_hw parent pointer instead of parent
name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 include/linux/clk-provider.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c10dc4c659e2..4e07621849e6 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -831,6 +831,25 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
 	__devm_clk_hw_register_divider((dev), NULL, (name), (parent_name), NULL,   \
 				  NULL, (flags), (reg), (shift), (width),     \
 				  (clk_divider_flags), NULL, (lock))
+/**
+ * devm_clk_hw_register_divider_parent_hw - register a divider clock with the clock framework
+ * @dev: device registering this clock
+ * @name: name of this clock
+ * @parent_hw: pointer to parent clk
+ * @flags: framework-specific flags
+ * @reg: register address to adjust divider
+ * @shift: number of bits to shift the bitfield
+ * @width: width of the bitfield
+ * @clk_divider_flags: divider-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+#define devm_clk_hw_register_divider_parent_hw(dev, name, parent_hw, flags,   \
+					       reg, shift, width,	      \
+					       clk_divider_flags, lock)       \
+	__devm_clk_hw_register_divider((dev), NULL, (name), NULL,	      \
+				       (parent_hw), NULL, (flags), (reg),     \
+				       (shift), (width), (clk_divider_flags), \
+				       NULL, (lock))
 /**
  * devm_clk_hw_register_divider_table - register a table based divider clock
  * with the clock framework (devres variant)
-- 
2.36.1

[PATCH 2/9] clk: mux: Introduce devm_clk_hw_register_mux_parent_hws()

[Marijn Suijten]

Add the devres variant of clk_hw_register_mux_hws() for registering a
mux clock with clk_hw parent pointers instead of parent names.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 include/linux/clk-provider.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4e07621849e6..316c7e082934 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -980,6 +980,13 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 			      (parent_names), NULL, NULL, (flags), (reg),     \
 			      (shift), BIT((width)) - 1, (clk_mux_flags),     \
 			      NULL, (lock))
+#define devm_clk_hw_register_mux_parent_hws(dev, name, parent_hws,	      \
+					    num_parents, flags, reg, shift,   \
+					    width, clk_mux_flags, lock)       \
+	__devm_clk_hw_register_mux((dev), NULL, (name), (num_parents), NULL,  \
+				   (parent_hws), NULL, (flags), (reg),        \
+				   (shift), BIT((width)) - 1,		      \
+				   (clk_mux_flags), NULL, (lock))
 
 int clk_mux_val_to_index(struct clk_hw *hw, const u32 *table, unsigned int flags,
 			 unsigned int val);
-- 
2.36.1

[PATCH 2/9] clk: mux: Introduce devm_clk_hw_register_mux_parent_hws()

[Marijn Suijten]

Add the devres variant of clk_hw_register_mux_hws() for registering a
mux clock with clk_hw parent pointers instead of parent names.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 include/linux/clk-provider.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4e07621849e6..316c7e082934 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -980,6 +980,13 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 			      (parent_names), NULL, NULL, (flags), (reg),     \
 			      (shift), BIT((width)) - 1, (clk_mux_flags),     \
 			      NULL, (lock))
+#define devm_clk_hw_register_mux_parent_hws(dev, name, parent_hws,	      \
+					    num_parents, flags, reg, shift,   \
+					    width, clk_mux_flags, lock)       \
+	__devm_clk_hw_register_mux((dev), NULL, (name), (num_parents), NULL,  \
+				   (parent_hws), NULL, (flags), (reg),        \
+				   (shift), BIT((width)) - 1,		      \
+				   (clk_mux_flags), NULL, (lock))
 
 int clk_mux_val_to_index(struct clk_hw *hw, const u32 *table, unsigned int flags,
 			 unsigned int val);
-- 
2.36.1

[PATCH 3/9] clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw()

[Marijn Suijten]

Add the devres and non-devres variant of
clk_hw_register_fixed_factor_parent_hw() for registering a fixed factor
clock with clk_hw parent pointer instead of parent name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/clk/clk-fixed-factor.c | 57 ++++++++++++++++++++++++++++------
 include/linux/clk-provider.h   |  8 +++++
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 54942d758ee6..fabb98d0cdb2 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -78,7 +78,8 @@ static void devm_clk_hw_register_fixed_factor_release(struct device *dev, void *
 
 static struct clk_hw *
 __clk_hw_register_fixed_factor(struct device *dev, struct device_node *np,
-		const char *name, const char *parent_name, int index,
+		const char *name, const char *parent_name,
+		const struct clk_hw *parent_hw, int index,
 		unsigned long flags, unsigned int mult, unsigned int div,
 		bool devm)
 {
@@ -108,7 +109,9 @@ __clk_hw_register_fixed_factor(struct device *dev, struct device_node *np,
 	init.name = name;
 	init.ops = &clk_fixed_factor_ops;
 	init.flags = flags;
-	if (parent_name)
+	if (parent_hw)
+		init.parent_hws = &parent_hw;
+	else if (parent_name)
 		init.parent_names = &parent_name;
 	else
 		init.parent_data = &pdata;
@@ -148,17 +151,50 @@ struct clk_hw *devm_clk_hw_register_fixed_factor_index(struct device *dev,
 		const char *name, unsigned int index, unsigned long flags,
 		unsigned int mult, unsigned int div)
 {
-	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL, index,
-					      flags, mult, div, true);
+	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL, NULL,
+					      index, flags, mult, div, true);
 }
 EXPORT_SYMBOL_GPL(devm_clk_hw_register_fixed_factor_index);
 
+/**
+ * devm_clk_hw_register_fixed_factor_parent_hw - Register a fixed factor clock with
+ * pointer to parent clock
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_hw: pointer to parent clk
+ * @flags: fixed factor flags
+ * @mult: multiplier
+ * @div: divider
+ *
+ * Return: Pointer to fixed factor clk_hw structure that was registered or
+ * an error pointer.
+ */
+struct clk_hw *devm_clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div)
+{
+	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL, parent_hw,
+					      -1, flags, mult, div, true);
+}
+EXPORT_SYMBOL_GPL(devm_clk_hw_register_fixed_factor_parent_hw);
+
+struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div)
+{
+	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL,
+					      parent_hw, -1, flags, mult, div,
+					      false);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor_parent_hw);
+
 struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
 		const char *name, const char *parent_name, unsigned long flags,
 		unsigned int mult, unsigned int div)
 {
-	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name, -1,
-					      flags, mult, div, false);
+	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name,
+					      NULL, -1, flags, mult, div,
+					      false);
 }
 EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor);
 
@@ -204,8 +240,9 @@ struct clk_hw *devm_clk_hw_register_fixed_factor(struct device *dev,
 		const char *name, const char *parent_name, unsigned long flags,
 		unsigned int mult, unsigned int div)
 {
-	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name, -1,
-			flags, mult, div, true);
+	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name,
+					      NULL, -1, flags, mult, div,
+					      true);
 }
 EXPORT_SYMBOL_GPL(devm_clk_hw_register_fixed_factor);
 
@@ -240,8 +277,8 @@ static struct clk_hw *_of_fixed_factor_clk_setup(struct device_node *node)
 	if (of_match_node(set_rate_parent_matches, node))
 		flags |= CLK_SET_RATE_PARENT;
 
-	hw = __clk_hw_register_fixed_factor(NULL, node, clk_name, NULL, 0,
-					    flags, mult, div, false);
+	hw = __clk_hw_register_fixed_factor(NULL, node, clk_name, NULL, NULL,
+					    0, flags, mult, div, false);
 	if (IS_ERR(hw)) {
 		/*
 		 * Clear OF_POPULATED flag so that clock registration can be
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 316c7e082934..94458cb669f0 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -1032,6 +1032,14 @@ struct clk_hw *devm_clk_hw_register_fixed_factor(struct device *dev,
 struct clk_hw *devm_clk_hw_register_fixed_factor_index(struct device *dev,
 		const char *name, unsigned int index, unsigned long flags,
 		unsigned int mult, unsigned int div);
+
+struct clk_hw *devm_clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div);
+
+struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div);
 /**
  * struct clk_fractional_divider - adjustable fractional divider clock
  *
-- 
2.36.1

[PATCH 3/9] clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw()

[Marijn Suijten]

Add the devres and non-devres variant of
clk_hw_register_fixed_factor_parent_hw() for registering a fixed factor
clock with clk_hw parent pointer instead of parent name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/clk/clk-fixed-factor.c | 57 ++++++++++++++++++++++++++++------
 include/linux/clk-provider.h   |  8 +++++
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 54942d758ee6..fabb98d0cdb2 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -78,7 +78,8 @@ static void devm_clk_hw_register_fixed_factor_release(struct device *dev, void *
 
 static struct clk_hw *
 __clk_hw_register_fixed_factor(struct device *dev, struct device_node *np,
-		const char *name, const char *parent_name, int index,
+		const char *name, const char *parent_name,
+		const struct clk_hw *parent_hw, int index,
 		unsigned long flags, unsigned int mult, unsigned int div,
 		bool devm)
 {
@@ -108,7 +109,9 @@ __clk_hw_register_fixed_factor(struct device *dev, struct device_node *np,
 	init.name = name;
 	init.ops = &clk_fixed_factor_ops;
 	init.flags = flags;
-	if (parent_name)
+	if (parent_hw)
+		init.parent_hws = &parent_hw;
+	else if (parent_name)
 		init.parent_names = &parent_name;
 	else
 		init.parent_data = &pdata;
@@ -148,17 +151,50 @@ struct clk_hw *devm_clk_hw_register_fixed_factor_index(struct device *dev,
 		const char *name, unsigned int index, unsigned long flags,
 		unsigned int mult, unsigned int div)
 {
-	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL, index,
-					      flags, mult, div, true);
+	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL, NULL,
+					      index, flags, mult, div, true);
 }
 EXPORT_SYMBOL_GPL(devm_clk_hw_register_fixed_factor_index);
 
+/**
+ * devm_clk_hw_register_fixed_factor_parent_hw - Register a fixed factor clock with
+ * pointer to parent clock
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_hw: pointer to parent clk
+ * @flags: fixed factor flags
+ * @mult: multiplier
+ * @div: divider
+ *
+ * Return: Pointer to fixed factor clk_hw structure that was registered or
+ * an error pointer.
+ */
+struct clk_hw *devm_clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div)
+{
+	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL, parent_hw,
+					      -1, flags, mult, div, true);
+}
+EXPORT_SYMBOL_GPL(devm_clk_hw_register_fixed_factor_parent_hw);
+
+struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div)
+{
+	return __clk_hw_register_fixed_factor(dev, NULL, name, NULL,
+					      parent_hw, -1, flags, mult, div,
+					      false);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor_parent_hw);
+
 struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
 		const char *name, const char *parent_name, unsigned long flags,
 		unsigned int mult, unsigned int div)
 {
-	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name, -1,
-					      flags, mult, div, false);
+	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name,
+					      NULL, -1, flags, mult, div,
+					      false);
 }
 EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor);
 
@@ -204,8 +240,9 @@ struct clk_hw *devm_clk_hw_register_fixed_factor(struct device *dev,
 		const char *name, const char *parent_name, unsigned long flags,
 		unsigned int mult, unsigned int div)
 {
-	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name, -1,
-			flags, mult, div, true);
+	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name,
+					      NULL, -1, flags, mult, div,
+					      true);
 }
 EXPORT_SYMBOL_GPL(devm_clk_hw_register_fixed_factor);
 
@@ -240,8 +277,8 @@ static struct clk_hw *_of_fixed_factor_clk_setup(struct device_node *node)
 	if (of_match_node(set_rate_parent_matches, node))
 		flags |= CLK_SET_RATE_PARENT;
 
-	hw = __clk_hw_register_fixed_factor(NULL, node, clk_name, NULL, 0,
-					    flags, mult, div, false);
+	hw = __clk_hw_register_fixed_factor(NULL, node, clk_name, NULL, NULL,
+					    0, flags, mult, div, false);
 	if (IS_ERR(hw)) {
 		/*
 		 * Clear OF_POPULATED flag so that clock registration can be
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 316c7e082934..94458cb669f0 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -1032,6 +1032,14 @@ struct clk_hw *devm_clk_hw_register_fixed_factor(struct device *dev,
 struct clk_hw *devm_clk_hw_register_fixed_factor_index(struct device *dev,
 		const char *name, unsigned int index, unsigned long flags,
 		unsigned int mult, unsigned int div);
+
+struct clk_hw *devm_clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div);
+
+struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev,
+		const char *name, const struct clk_hw *parent_hw,
+		unsigned long flags, unsigned int mult, unsigned int div);
 /**
  * struct clk_fractional_divider - adjustable fractional divider clock
  *
-- 
2.36.1

[PATCH 4/9] drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 52 +++++++++-------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index 48eab80b548e..6926c8ff6255 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -519,7 +519,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)

 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent1[32], parent2[32], vco_name[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref", .name = "xo",
@@ -529,7 +529,7 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 		.flags = CLK_IGNORE_UNUSED,
 	};
 	struct device *dev = &pll_28nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *analog_postdiv, *indirect_path_div2, *byte_mux;
 	int ret;

 	DBG("%d", pll_28nm->phy->id);
@@ -546,48 +546,40 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 		return ret;

 	snprintf(clk_name, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_divider(dev, clk_name,
-			parent1, CLK_SET_RATE_PARENT,
+	analog_postdiv = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_28nm->clk_hw, CLK_SET_RATE_PARENT,
 			pll_28nm->phy->pll_base +
-			REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG,
-			0, 4, 0, NULL);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+			REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG, 0, 4, 0, NULL);
+	if (IS_ERR(analog_postdiv))
+		return PTR_ERR(analog_postdiv);

 	snprintf(clk_name, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
-			parent1, CLK_SET_RATE_PARENT,
-			1, 2);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	indirect_path_div2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, analog_postdiv, CLK_SET_RATE_PARENT, 1, 2);
+	if (IS_ERR(indirect_path_div2))
+		return PTR_ERR(indirect_path_div2);

 	snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				parent1, 0, pll_28nm->phy->pll_base +
-				REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG,
-				0, 8, 0, NULL);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
+			REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG, 0, 8, 0, NULL);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	provided_clocks[DSI_PIXEL_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
-	snprintf(parent2, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_mux(dev, clk_name,
-			((const char *[]){
-				parent1, parent2
+	byte_mux = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
+			((const struct clk_hw *[]){
+				&pll_28nm->clk_hw,
+				indirect_path_div2,
 			}), 2, CLK_SET_RATE_PARENT, pll_28nm->phy->pll_base +
 			REG_DSI_28nm_PHY_PLL_VREG_CFG, 1, 1, 0, NULL);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	if (IS_ERR(byte_mux))
+		return PTR_ERR(byte_mux);

 	snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
-				parent1, CLK_SET_RATE_PARENT, 1, 4);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			byte_mux, CLK_SET_RATE_PARENT, 1, 4);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;
--
2.36.1

[PATCH 4/9] drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 52 +++++++++-------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index 48eab80b548e..6926c8ff6255 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -519,7 +519,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)

 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent1[32], parent2[32], vco_name[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref", .name = "xo",
@@ -529,7 +529,7 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 		.flags = CLK_IGNORE_UNUSED,
 	};
 	struct device *dev = &pll_28nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *analog_postdiv, *indirect_path_div2, *byte_mux;
 	int ret;

 	DBG("%d", pll_28nm->phy->id);
@@ -546,48 +546,40 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 		return ret;

 	snprintf(clk_name, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_divider(dev, clk_name,
-			parent1, CLK_SET_RATE_PARENT,
+	analog_postdiv = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_28nm->clk_hw, CLK_SET_RATE_PARENT,
 			pll_28nm->phy->pll_base +
-			REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG,
-			0, 4, 0, NULL);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+			REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG, 0, 4, 0, NULL);
+	if (IS_ERR(analog_postdiv))
+		return PTR_ERR(analog_postdiv);

 	snprintf(clk_name, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
-			parent1, CLK_SET_RATE_PARENT,
-			1, 2);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	indirect_path_div2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, analog_postdiv, CLK_SET_RATE_PARENT, 1, 2);
+	if (IS_ERR(indirect_path_div2))
+		return PTR_ERR(indirect_path_div2);

 	snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				parent1, 0, pll_28nm->phy->pll_base +
-				REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG,
-				0, 8, 0, NULL);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
+			REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG, 0, 8, 0, NULL);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	provided_clocks[DSI_PIXEL_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
-	snprintf(parent2, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_mux(dev, clk_name,
-			((const char *[]){
-				parent1, parent2
+	byte_mux = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
+			((const struct clk_hw *[]){
+				&pll_28nm->clk_hw,
+				indirect_path_div2,
 			}), 2, CLK_SET_RATE_PARENT, pll_28nm->phy->pll_base +
 			REG_DSI_28nm_PHY_PLL_VREG_CFG, 1, 1, 0, NULL);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	if (IS_ERR(byte_mux))
+		return PTR_ERR(byte_mux);

 	snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id);
-	snprintf(parent1, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
-				parent1, CLK_SET_RATE_PARENT, 1, 4);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			byte_mux, CLK_SET_RATE_PARENT, 1, 4);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;
--
2.36.1

Re: [PATCH 4/9] drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>



> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 52 +++++++++-------------
>  1 file changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> index 48eab80b548e..6926c8ff6255 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> @@ -519,7 +519,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
>
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
>  {
> -       char clk_name[32], parent1[32], parent2[32], vco_name[32];
> +       char clk_name[32], vco_name[32];

While we are at it, we might also get rid of vco_name and use clk_name
everywhere.

>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref", .name = "xo",
> @@ -529,7 +529,7 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>                 .flags = CLK_IGNORE_UNUSED,
>         };
>         struct device *dev = &pll_28nm->phy->pdev->dev;
> -       struct clk_hw *hw;
> +       struct clk_hw *hw, *analog_postdiv, *indirect_path_div2, *byte_mux;
>         int ret;
>
>         DBG("%d", pll_28nm->phy->id);
> @@ -546,48 +546,40 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>                 return ret;
>
>         snprintf(clk_name, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_divider(dev, clk_name,
> -                       parent1, CLK_SET_RATE_PARENT,
> +       analog_postdiv = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
> +                       &pll_28nm->clk_hw, CLK_SET_RATE_PARENT,
>                         pll_28nm->phy->pll_base +
> -                       REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG,
> -                       0, 4, 0, NULL);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +                       REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG, 0, 4, 0, NULL);

The diff is already hard enough to read. Could you please drop
syntax/whitespace/newline changes?

> +       if (IS_ERR(analog_postdiv))
> +               return PTR_ERR(analog_postdiv);
>
>         snprintf(clk_name, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
> -                       parent1, CLK_SET_RATE_PARENT,
> -                       1, 2);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       indirect_path_div2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
> +                       clk_name, analog_postdiv, CLK_SET_RATE_PARENT, 1, 2);
> +       if (IS_ERR(indirect_path_div2))
> +               return PTR_ERR(indirect_path_div2);
>
>         snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_divider(dev, clk_name,
> -                               parent1, 0, pll_28nm->phy->pll_base +
> -                               REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG,
> -                               0, 8, 0, NULL);
> +       hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
> +                       &pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
> +                       REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG, 0, 8, 0, NULL);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>         provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
>
>         snprintf(clk_name, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
> -       snprintf(parent2, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_mux(dev, clk_name,
> -                       ((const char *[]){
> -                               parent1, parent2
> +       byte_mux = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
> +                       ((const struct clk_hw *[]){
> +                               &pll_28nm->clk_hw,
> +                               indirect_path_div2,
>                         }), 2, CLK_SET_RATE_PARENT, pll_28nm->phy->pll_base +
>                         REG_DSI_28nm_PHY_PLL_VREG_CFG, 1, 1, 0, NULL);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       if (IS_ERR(byte_mux))
> +               return PTR_ERR(byte_mux);
>
>         snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
> -                               parent1, CLK_SET_RATE_PARENT, 1, 4);
> +       hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
> +                       byte_mux, CLK_SET_RATE_PARENT, 1, 4);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>         provided_clocks[DSI_BYTE_PLL_CLK] = hw;
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 4/9] drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>



> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 52 +++++++++-------------
>  1 file changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> index 48eab80b548e..6926c8ff6255 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> @@ -519,7 +519,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
>
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
>  {
> -       char clk_name[32], parent1[32], parent2[32], vco_name[32];
> +       char clk_name[32], vco_name[32];

While we are at it, we might also get rid of vco_name and use clk_name
everywhere.

>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref", .name = "xo",
> @@ -529,7 +529,7 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>                 .flags = CLK_IGNORE_UNUSED,
>         };
>         struct device *dev = &pll_28nm->phy->pdev->dev;
> -       struct clk_hw *hw;
> +       struct clk_hw *hw, *analog_postdiv, *indirect_path_div2, *byte_mux;
>         int ret;
>
>         DBG("%d", pll_28nm->phy->id);
> @@ -546,48 +546,40 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>                 return ret;
>
>         snprintf(clk_name, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_divider(dev, clk_name,
> -                       parent1, CLK_SET_RATE_PARENT,
> +       analog_postdiv = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
> +                       &pll_28nm->clk_hw, CLK_SET_RATE_PARENT,
>                         pll_28nm->phy->pll_base +
> -                       REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG,
> -                       0, 4, 0, NULL);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +                       REG_DSI_28nm_PHY_PLL_POSTDIV1_CFG, 0, 4, 0, NULL);

The diff is already hard enough to read. Could you please drop
syntax/whitespace/newline changes?

> +       if (IS_ERR(analog_postdiv))
> +               return PTR_ERR(analog_postdiv);
>
>         snprintf(clk_name, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%danalog_postdiv_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
> -                       parent1, CLK_SET_RATE_PARENT,
> -                       1, 2);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       indirect_path_div2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
> +                       clk_name, analog_postdiv, CLK_SET_RATE_PARENT, 1, 2);
> +       if (IS_ERR(indirect_path_div2))
> +               return PTR_ERR(indirect_path_div2);
>
>         snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_divider(dev, clk_name,
> -                               parent1, 0, pll_28nm->phy->pll_base +
> -                               REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG,
> -                               0, 8, 0, NULL);
> +       hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
> +                       &pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
> +                       REG_DSI_28nm_PHY_PLL_POSTDIV3_CFG, 0, 8, 0, NULL);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>         provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
>
>         snprintf(clk_name, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dvco_clk", pll_28nm->phy->id);
> -       snprintf(parent2, 32, "dsi%dindirect_path_div2_clk", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_mux(dev, clk_name,
> -                       ((const char *[]){
> -                               parent1, parent2
> +       byte_mux = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
> +                       ((const struct clk_hw *[]){
> +                               &pll_28nm->clk_hw,
> +                               indirect_path_div2,
>                         }), 2, CLK_SET_RATE_PARENT, pll_28nm->phy->pll_base +
>                         REG_DSI_28nm_PHY_PLL_VREG_CFG, 1, 1, 0, NULL);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       if (IS_ERR(byte_mux))
> +               return PTR_ERR(byte_mux);
>
>         snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id);
> -       snprintf(parent1, 32, "dsi%dbyte_mux", pll_28nm->phy->id);
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name,
> -                               parent1, CLK_SET_RATE_PARENT, 1, 4);
> +       hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
> +                       byte_mux, CLK_SET_RATE_PARENT, 1, 4);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>         provided_clocks[DSI_BYTE_PLL_CLK] = hw;
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

[PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c    | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index fc56cdcc9ad6..943a7e847c90 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)

 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
 {
-	char *clk_name, *parent_name, *vco_name;
+	char *clk_name, *vco_name;
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -408,10 +408,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 	if (!vco_name)
 		return -ENOMEM;

-	parent_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!parent_name)
-		return -ENOMEM;
-
 	clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
 	if (!clk_name)
 		return -ENOMEM;
@@ -429,13 +425,14 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 	bytediv->hw.init = &bytediv_init;
 	bytediv->reg = pll_28nm->phy->pll_base + REG_DSI_28nm_8960_PHY_PLL_CTRL_9;

-	snprintf(parent_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
 	snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id + 1);

 	bytediv_init.name = clk_name;
 	bytediv_init.ops = &clk_bytediv_ops;
 	bytediv_init.flags = CLK_SET_RATE_PARENT;
-	bytediv_init.parent_names = (const char * const *) &parent_name;
+	bytediv_init.parent_hws = (const struct clk_hw*[]){
+		&pll_28nm->clk_hw,
+	};
 	bytediv_init.num_parents = 1;

 	/* DIV2 */
@@ -446,10 +443,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov

 	snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id + 1);
 	/* DIV3 */
-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				parent_name, 0, pll_28nm->phy->pll_base +
-				REG_DSI_28nm_8960_PHY_PLL_CTRL_10,
-				0, 8, 0, NULL);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
+			REG_DSI_28nm_8960_PHY_PLL_CTRL_10, 0, 8, 0, NULL);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
--
2.36.1

[PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c    | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index fc56cdcc9ad6..943a7e847c90 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)

 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
 {
-	char *clk_name, *parent_name, *vco_name;
+	char *clk_name, *vco_name;
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -408,10 +408,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 	if (!vco_name)
 		return -ENOMEM;

-	parent_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!parent_name)
-		return -ENOMEM;
-
 	clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
 	if (!clk_name)
 		return -ENOMEM;
@@ -429,13 +425,14 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 	bytediv->hw.init = &bytediv_init;
 	bytediv->reg = pll_28nm->phy->pll_base + REG_DSI_28nm_8960_PHY_PLL_CTRL_9;

-	snprintf(parent_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
 	snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id + 1);

 	bytediv_init.name = clk_name;
 	bytediv_init.ops = &clk_bytediv_ops;
 	bytediv_init.flags = CLK_SET_RATE_PARENT;
-	bytediv_init.parent_names = (const char * const *) &parent_name;
+	bytediv_init.parent_hws = (const struct clk_hw*[]){
+		&pll_28nm->clk_hw,
+	};
 	bytediv_init.num_parents = 1;

 	/* DIV2 */
@@ -446,10 +443,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov

 	snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id + 1);
 	/* DIV3 */
-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				parent_name, 0, pll_28nm->phy->pll_base +
-				REG_DSI_28nm_8960_PHY_PLL_CTRL_10,
-				0, 8, 0, NULL);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
+			REG_DSI_28nm_8960_PHY_PLL_CTRL_10, 0, 8, 0, NULL);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
--
2.36.1

Re: [PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.

Can you please add a followup patch (or a preface one) removing the
rest of devm_kzalloc()'ed clock names.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Minor nit below.

>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c    | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> index fc56cdcc9ad6..943a7e847c90 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> @@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
>
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
>  {
> -       char *clk_name, *parent_name, *vco_name;
> +       char *clk_name, *vco_name;
>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref",
> @@ -408,10 +408,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>         if (!vco_name)
>                 return -ENOMEM;
>
> -       parent_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -       if (!parent_name)
> -               return -ENOMEM;
> -
>         clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
>         if (!clk_name)
>                 return -ENOMEM;
> @@ -429,13 +425,14 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>         bytediv->hw.init = &bytediv_init;
>         bytediv->reg = pll_28nm->phy->pll_base + REG_DSI_28nm_8960_PHY_PLL_CTRL_9;
>
> -       snprintf(parent_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
>         snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id + 1);
>
>         bytediv_init.name = clk_name;
>         bytediv_init.ops = &clk_bytediv_ops;
>         bytediv_init.flags = CLK_SET_RATE_PARENT;
> -       bytediv_init.parent_names = (const char * const *) &parent_name;
> +       bytediv_init.parent_hws = (const struct clk_hw*[]){
> +               &pll_28nm->clk_hw,
> +       };
>         bytediv_init.num_parents = 1;

I wonder if we can express the bytediv clock with the standard ops.
However it's definitely a separate topic.

>
>         /* DIV2 */
> @@ -446,10 +443,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>
>         snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id + 1);
>         /* DIV3 */
> -       hw = devm_clk_hw_register_divider(dev, clk_name,
> -                               parent_name, 0, pll_28nm->phy->pll_base +
> -                               REG_DSI_28nm_8960_PHY_PLL_CTRL_10,
> -                               0, 8, 0, NULL);
> +       hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
> +                       &pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
> +                       REG_DSI_28nm_8960_PHY_PLL_CTRL_10, 0, 8, 0, NULL);

Again, could you please keep the linebreak in place?


>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>         provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.

Can you please add a followup patch (or a preface one) removing the
rest of devm_kzalloc()'ed clock names.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Minor nit below.

>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c    | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> index fc56cdcc9ad6..943a7e847c90 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> @@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
>
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
>  {
> -       char *clk_name, *parent_name, *vco_name;
> +       char *clk_name, *vco_name;
>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref",
> @@ -408,10 +408,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>         if (!vco_name)
>                 return -ENOMEM;
>
> -       parent_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -       if (!parent_name)
> -               return -ENOMEM;
> -
>         clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
>         if (!clk_name)
>                 return -ENOMEM;
> @@ -429,13 +425,14 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>         bytediv->hw.init = &bytediv_init;
>         bytediv->reg = pll_28nm->phy->pll_base + REG_DSI_28nm_8960_PHY_PLL_CTRL_9;
>
> -       snprintf(parent_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
>         snprintf(clk_name, 32, "dsi%dpllbyte", pll_28nm->phy->id + 1);
>
>         bytediv_init.name = clk_name;
>         bytediv_init.ops = &clk_bytediv_ops;
>         bytediv_init.flags = CLK_SET_RATE_PARENT;
> -       bytediv_init.parent_names = (const char * const *) &parent_name;
> +       bytediv_init.parent_hws = (const struct clk_hw*[]){
> +               &pll_28nm->clk_hw,
> +       };
>         bytediv_init.num_parents = 1;

I wonder if we can express the bytediv clock with the standard ops.
However it's definitely a separate topic.

>
>         /* DIV2 */
> @@ -446,10 +443,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>
>         snprintf(clk_name, 32, "dsi%dpll", pll_28nm->phy->id + 1);
>         /* DIV3 */
> -       hw = devm_clk_hw_register_divider(dev, clk_name,
> -                               parent_name, 0, pll_28nm->phy->pll_base +
> -                               REG_DSI_28nm_8960_PHY_PLL_CTRL_10,
> -                               0, 8, 0, NULL);
> +       hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
> +                       &pll_28nm->clk_hw, 0, pll_28nm->phy->pll_base +
> +                       REG_DSI_28nm_8960_PHY_PLL_CTRL_10, 0, 8, 0, NULL);

Again, could you please keep the linebreak in place?


>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>         provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 01:44, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, 24 May 2022 at 00:38, Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > parent_hw pointers are easier to manage and cheaper to use than
> > repeatedly formatting the parent name and subsequently leaving the clk
> > framework to perform lookups based on that name.
>
> Can you please add a followup patch (or a preface one) removing the
> rest of devm_kzalloc()'ed clock names.

Argh, stupid me, you did that in the next patch. Please ignore this.

>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 01:44, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, 24 May 2022 at 00:38, Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > parent_hw pointers are easier to manage and cheaper to use than
> > repeatedly formatting the parent name and subsequently leaving the clk
> > framework to perform lookups based on that name.
>
> Can you please add a followup patch (or a preface one) removing the
> rest of devm_kzalloc()'ed clock names.

Argh, stupid me, you did that in the next patch. Please ignore this.

>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Marijn Suijten]

On 2022-05-24 01:44:49, Dmitry Baryshkov wrote:
> On Tue, 24 May 2022 at 01:44, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Tue, 24 May 2022 at 00:38, Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > parent_hw pointers are easier to manage and cheaper to use than
> > > repeatedly formatting the parent name and subsequently leaving the clk
> > > framework to perform lookups based on that name.
> >
> > Can you please add a followup patch (or a preface one) removing the
> > rest of devm_kzalloc()'ed clock names.
> 
> Argh, stupid me, you did that in the next patch. Please ignore this.

It's a fair observation, one that bothered me as well.  I've reordered
the next patch before this one for the next revision, to have clearer
separation (since this patch was currently deleting 1/3 of the
devm_kzalloc()'s).

- Marijn

Re: [PATCH 5/9] drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers

[Marijn Suijten]

On 2022-05-24 01:44:49, Dmitry Baryshkov wrote:
> On Tue, 24 May 2022 at 01:44, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Tue, 24 May 2022 at 00:38, Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > parent_hw pointers are easier to manage and cheaper to use than
> > > repeatedly formatting the parent name and subsequently leaving the clk
> > > framework to perform lookups based on that name.
> >
> > Can you please add a followup patch (or a preface one) removing the
> > rest of devm_kzalloc()'ed clock names.
> 
> Argh, stupid me, you did that in the next patch. Please ignore this.

It's a fair observation, one that bothered me as well.  I've reordered
the next patch before this one for the next revision, to have clearer
separation (since this patch was currently deleting 1/3 of the
devm_kzalloc()'s).

- Marijn

[PATCH 6/9] drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names

[Marijn Suijten]

The clock names formatted into the hw_clk's init structure are only used
for the duration of the registration function where they are kstrdup'ed,
making it unnecessary to keep the allocations alive for the duration of
the device (through devm).

Just like the other DSI PHY PLL clock trees, use a stack-local char
array and save on memory outside of the pll_28nm_register function.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index 943a7e847c90..554978fc434d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
 
 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
 {
-	char *clk_name, *vco_name;
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -404,14 +404,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 	if (!bytediv)
 		return -ENOMEM;
 
-	vco_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!vco_name)
-		return -ENOMEM;
-
-	clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!clk_name)
-		return -ENOMEM;
-
 	snprintf(vco_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
 	vco_init.name = vco_name;
 
-- 
2.36.1

[PATCH 6/9] drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names

[Marijn Suijten]

The clock names formatted into the hw_clk's init structure are only used
for the duration of the registration function where they are kstrdup'ed,
making it unnecessary to keep the allocations alive for the duration of
the device (through devm).

Just like the other DSI PHY PLL clock trees, use a stack-local char
array and save on memory outside of the pll_28nm_register function.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index 943a7e847c90..554978fc434d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
 
 static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
 {
-	char *clk_name, *vco_name;
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -404,14 +404,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
 	if (!bytediv)
 		return -ENOMEM;
 
-	vco_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!vco_name)
-		return -ENOMEM;
-
-	clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!clk_name)
-		return -ENOMEM;
-
 	snprintf(vco_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
 	vco_init.name = vco_name;
 
-- 
2.36.1

Re: [PATCH 6/9] drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> The clock names formatted into the hw_clk's init structure are only used
> for the duration of the registration function where they are kstrdup'ed,
> making it unnecessary to keep the allocations alive for the duration of
> the device (through devm).
>
> Just like the other DSI PHY PLL clock trees, use a stack-local char
> array and save on memory outside of the pll_28nm_register function.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Nit: we can use clk_name instead of vco_name too.

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> index 943a7e847c90..554978fc434d 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> @@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
>
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
>  {
> -       char *clk_name, *vco_name;
> +       char clk_name[32], vco_name[32];
>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref",
> @@ -404,14 +404,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>         if (!bytediv)
>                 return -ENOMEM;
>
> -       vco_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -       if (!vco_name)
> -               return -ENOMEM;
> -
> -       clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -       if (!clk_name)
> -               return -ENOMEM;
> -
>         snprintf(vco_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
>         vco_init.name = vco_name;
>
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 6/9] drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> The clock names formatted into the hw_clk's init structure are only used
> for the duration of the registration function where they are kstrdup'ed,
> making it unnecessary to keep the allocations alive for the duration of
> the device (through devm).
>
> Just like the other DSI PHY PLL clock trees, use a stack-local char
> array and save on memory outside of the pll_28nm_register function.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Nit: we can use clk_name instead of vco_name too.

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> index 943a7e847c90..554978fc434d 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> @@ -383,7 +383,7 @@ static int dsi_28nm_pll_restore_state(struct msm_dsi_phy *phy)
>
>  static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **provided_clocks)
>  {
> -       char *clk_name, *vco_name;
> +       char clk_name[32], vco_name[32];
>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref",
> @@ -404,14 +404,6 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
>         if (!bytediv)
>                 return -ENOMEM;
>
> -       vco_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -       if (!vco_name)
> -               return -ENOMEM;
> -
> -       clk_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -       if (!clk_name)
> -               return -ENOMEM;
> -
>         snprintf(vco_name, 32, "dsi%dvco_clk", pll_28nm->phy->id);
>         vco_init.name = vco_name;
>
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

[PATCH 7/9] drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 36 ++++++++++------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
index 8199c53567f4..574f95ab2f22 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
@@ -764,14 +764,14 @@ static int dsi_14nm_set_usecase(struct msm_dsi_phy *phy)

 static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
 						const char *name,
-						const char *parent_name,
+						const struct clk_hw *parent_hw,
 						unsigned long flags,
 						u8 shift)
 {
 	struct dsi_pll_14nm_postdiv *pll_postdiv;
 	struct device *dev = &pll_14nm->phy->pdev->dev;
 	struct clk_init_data postdiv_init = {
-		.parent_names = (const char *[]) { parent_name },
+		.parent_hws = (const struct clk_hw *[]) { parent_hw },
 		.num_parents = 1,
 		.name = name,
 		.flags = flags,
@@ -800,7 +800,7 @@ static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,

 static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent[32], vco_name[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -811,7 +811,7 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
 		.ops = &clk_ops_dsi_pll_14nm_vco,
 	};
 	struct device *dev = &pll_14nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *n1_postdiv, *n1_postdivby2;
 	int ret;

 	DBG("DSI%d", pll_14nm->phy->id);
@@ -824,48 +824,46 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
 		return ret;

 	snprintf(clk_name, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dvco_clk", pll_14nm->phy->id);

 	/* N1 postdiv, bits 0-3 in REG_DSI_14nm_PHY_CMN_CLK_CFG0 */
-	hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent,
-				       CLK_SET_RATE_PARENT, 0);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	n1_postdiv = pll_14nm_postdiv_register(pll_14nm, clk_name,
+			&pll_14nm->clk_hw, CLK_SET_RATE_PARENT, 0);
+	if (IS_ERR(n1_postdiv))
+		return PTR_ERR(n1_postdiv);

 	snprintf(clk_name, 32, "dsi%dpllbyte", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);

 	/* DSI Byte clock = VCO_CLK / N1 / 8 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  CLK_SET_RATE_PARENT, 1, 8);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			n1_postdiv, CLK_SET_RATE_PARENT, 1, 8);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);

 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);

 	/*
 	 * Skip the mux for now, force DSICLK_SEL to 1, Add a /2 divider
 	 * on the way. Don't let it set parent.
 	 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 1, 2);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	n1_postdivby2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, n1_postdiv, 0, 1, 2);
+	if (IS_ERR(n1_postdivby2))
+		return PTR_ERR(n1_postdivby2);

 	snprintf(clk_name, 32, "dsi%dpll", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);

 	/* DSI pixel clock = VCO_CLK / N1 / 2 / N2
 	 * This is the output of N2 post-divider, bits 4-7 in
 	 * REG_DSI_14nm_PHY_CMN_CLK_CFG0. Don't let it set parent.
 	 */
-	hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent, 0, 4);
+	hw = pll_14nm_postdiv_register(pll_14nm, clk_name, n1_postdivby2,
+			0, 4);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);

-	provided_clocks[DSI_PIXEL_PLL_CLK]	= hw;
+	provided_clocks[DSI_PIXEL_PLL_CLK] = hw;

 	return 0;
 }
--
2.36.1

[PATCH 7/9] drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 36 ++++++++++------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
index 8199c53567f4..574f95ab2f22 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
@@ -764,14 +764,14 @@ static int dsi_14nm_set_usecase(struct msm_dsi_phy *phy)

 static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
 						const char *name,
-						const char *parent_name,
+						const struct clk_hw *parent_hw,
 						unsigned long flags,
 						u8 shift)
 {
 	struct dsi_pll_14nm_postdiv *pll_postdiv;
 	struct device *dev = &pll_14nm->phy->pdev->dev;
 	struct clk_init_data postdiv_init = {
-		.parent_names = (const char *[]) { parent_name },
+		.parent_hws = (const struct clk_hw *[]) { parent_hw },
 		.num_parents = 1,
 		.name = name,
 		.flags = flags,
@@ -800,7 +800,7 @@ static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,

 static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent[32], vco_name[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -811,7 +811,7 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
 		.ops = &clk_ops_dsi_pll_14nm_vco,
 	};
 	struct device *dev = &pll_14nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *n1_postdiv, *n1_postdivby2;
 	int ret;

 	DBG("DSI%d", pll_14nm->phy->id);
@@ -824,48 +824,46 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
 		return ret;

 	snprintf(clk_name, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dvco_clk", pll_14nm->phy->id);

 	/* N1 postdiv, bits 0-3 in REG_DSI_14nm_PHY_CMN_CLK_CFG0 */
-	hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent,
-				       CLK_SET_RATE_PARENT, 0);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	n1_postdiv = pll_14nm_postdiv_register(pll_14nm, clk_name,
+			&pll_14nm->clk_hw, CLK_SET_RATE_PARENT, 0);
+	if (IS_ERR(n1_postdiv))
+		return PTR_ERR(n1_postdiv);

 	snprintf(clk_name, 32, "dsi%dpllbyte", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);

 	/* DSI Byte clock = VCO_CLK / N1 / 8 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  CLK_SET_RATE_PARENT, 1, 8);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			n1_postdiv, CLK_SET_RATE_PARENT, 1, 8);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);

 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);

 	/*
 	 * Skip the mux for now, force DSICLK_SEL to 1, Add a /2 divider
 	 * on the way. Don't let it set parent.
 	 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 1, 2);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	n1_postdivby2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, n1_postdiv, 0, 1, 2);
+	if (IS_ERR(n1_postdivby2))
+		return PTR_ERR(n1_postdivby2);

 	snprintf(clk_name, 32, "dsi%dpll", pll_14nm->phy->id);
-	snprintf(parent, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);

 	/* DSI pixel clock = VCO_CLK / N1 / 2 / N2
 	 * This is the output of N2 post-divider, bits 4-7 in
 	 * REG_DSI_14nm_PHY_CMN_CLK_CFG0. Don't let it set parent.
 	 */
-	hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent, 0, 4);
+	hw = pll_14nm_postdiv_register(pll_14nm, clk_name, n1_postdivby2,
+			0, 4);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);

-	provided_clocks[DSI_PIXEL_PLL_CLK]	= hw;
+	provided_clocks[DSI_PIXEL_PLL_CLK] = hw;

 	return 0;
 }
--
2.36.1

Re: [PATCH 7/9] drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Nit: my rant regarding syntax changes applies here too.

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 36 ++++++++++------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index 8199c53567f4..574f95ab2f22 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -764,14 +764,14 @@ static int dsi_14nm_set_usecase(struct msm_dsi_phy *phy)
>
>  static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
>                                                 const char *name,
> -                                               const char *parent_name,
> +                                               const struct clk_hw *parent_hw,
>                                                 unsigned long flags,
>                                                 u8 shift)
>  {
>         struct dsi_pll_14nm_postdiv *pll_postdiv;
>         struct device *dev = &pll_14nm->phy->pdev->dev;
>         struct clk_init_data postdiv_init = {
> -               .parent_names = (const char *[]) { parent_name },
> +               .parent_hws = (const struct clk_hw *[]) { parent_hw },
>                 .num_parents = 1,
>                 .name = name,
>                 .flags = flags,
> @@ -800,7 +800,7 @@ static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
>
>  static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **provided_clocks)
>  {
> -       char clk_name[32], parent[32], vco_name[32];
> +       char clk_name[32], vco_name[32];
>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref",
> @@ -811,7 +811,7 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
>                 .ops = &clk_ops_dsi_pll_14nm_vco,
>         };
>         struct device *dev = &pll_14nm->phy->pdev->dev;
> -       struct clk_hw *hw;
> +       struct clk_hw *hw, *n1_postdiv, *n1_postdivby2;
>         int ret;
>
>         DBG("DSI%d", pll_14nm->phy->id);
> @@ -824,48 +824,46 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
>                 return ret;
>
>         snprintf(clk_name, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dvco_clk", pll_14nm->phy->id);
>
>         /* N1 postdiv, bits 0-3 in REG_DSI_14nm_PHY_CMN_CLK_CFG0 */
> -       hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent,
> -                                      CLK_SET_RATE_PARENT, 0);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       n1_postdiv = pll_14nm_postdiv_register(pll_14nm, clk_name,
> +                       &pll_14nm->clk_hw, CLK_SET_RATE_PARENT, 0);
> +       if (IS_ERR(n1_postdiv))
> +               return PTR_ERR(n1_postdiv);
>
>         snprintf(clk_name, 32, "dsi%dpllbyte", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
>
>         /* DSI Byte clock = VCO_CLK / N1 / 8 */
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
> -                                         CLK_SET_RATE_PARENT, 1, 8);
> +       hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
> +                       n1_postdiv, CLK_SET_RATE_PARENT, 1, 8);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>
>         provided_clocks[DSI_BYTE_PLL_CLK] = hw;
>
>         snprintf(clk_name, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
>
>         /*
>          * Skip the mux for now, force DSICLK_SEL to 1, Add a /2 divider
>          * on the way. Don't let it set parent.
>          */
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 1, 2);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       n1_postdivby2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
> +                       clk_name, n1_postdiv, 0, 1, 2);
> +       if (IS_ERR(n1_postdivby2))
> +               return PTR_ERR(n1_postdivby2);
>
>         snprintf(clk_name, 32, "dsi%dpll", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);
>
>         /* DSI pixel clock = VCO_CLK / N1 / 2 / N2
>          * This is the output of N2 post-divider, bits 4-7 in
>          * REG_DSI_14nm_PHY_CMN_CLK_CFG0. Don't let it set parent.
>          */
> -       hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent, 0, 4);
> +       hw = pll_14nm_postdiv_register(pll_14nm, clk_name, n1_postdivby2,
> +                       0, 4);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>
> -       provided_clocks[DSI_PIXEL_PLL_CLK]      = hw;
> +       provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
>
>         return 0;
>  }
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 7/9] drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Nit: my rant regarding syntax changes applies here too.

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 36 ++++++++++------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index 8199c53567f4..574f95ab2f22 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -764,14 +764,14 @@ static int dsi_14nm_set_usecase(struct msm_dsi_phy *phy)
>
>  static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
>                                                 const char *name,
> -                                               const char *parent_name,
> +                                               const struct clk_hw *parent_hw,
>                                                 unsigned long flags,
>                                                 u8 shift)
>  {
>         struct dsi_pll_14nm_postdiv *pll_postdiv;
>         struct device *dev = &pll_14nm->phy->pdev->dev;
>         struct clk_init_data postdiv_init = {
> -               .parent_names = (const char *[]) { parent_name },
> +               .parent_hws = (const struct clk_hw *[]) { parent_hw },
>                 .num_parents = 1,
>                 .name = name,
>                 .flags = flags,
> @@ -800,7 +800,7 @@ static struct clk_hw *pll_14nm_postdiv_register(struct dsi_pll_14nm *pll_14nm,
>
>  static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **provided_clocks)
>  {
> -       char clk_name[32], parent[32], vco_name[32];
> +       char clk_name[32], vco_name[32];
>         struct clk_init_data vco_init = {
>                 .parent_data = &(const struct clk_parent_data) {
>                         .fw_name = "ref",
> @@ -811,7 +811,7 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
>                 .ops = &clk_ops_dsi_pll_14nm_vco,
>         };
>         struct device *dev = &pll_14nm->phy->pdev->dev;
> -       struct clk_hw *hw;
> +       struct clk_hw *hw, *n1_postdiv, *n1_postdivby2;
>         int ret;
>
>         DBG("DSI%d", pll_14nm->phy->id);
> @@ -824,48 +824,46 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
>                 return ret;
>
>         snprintf(clk_name, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dvco_clk", pll_14nm->phy->id);
>
>         /* N1 postdiv, bits 0-3 in REG_DSI_14nm_PHY_CMN_CLK_CFG0 */
> -       hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent,
> -                                      CLK_SET_RATE_PARENT, 0);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       n1_postdiv = pll_14nm_postdiv_register(pll_14nm, clk_name,
> +                       &pll_14nm->clk_hw, CLK_SET_RATE_PARENT, 0);
> +       if (IS_ERR(n1_postdiv))
> +               return PTR_ERR(n1_postdiv);
>
>         snprintf(clk_name, 32, "dsi%dpllbyte", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
>
>         /* DSI Byte clock = VCO_CLK / N1 / 8 */
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
> -                                         CLK_SET_RATE_PARENT, 1, 8);
> +       hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
> +                       n1_postdiv, CLK_SET_RATE_PARENT, 1, 8);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>
>         provided_clocks[DSI_BYTE_PLL_CLK] = hw;
>
>         snprintf(clk_name, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dn1_postdiv_clk", pll_14nm->phy->id);
>
>         /*
>          * Skip the mux for now, force DSICLK_SEL to 1, Add a /2 divider
>          * on the way. Don't let it set parent.
>          */
> -       hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 1, 2);
> -       if (IS_ERR(hw))
> -               return PTR_ERR(hw);
> +       n1_postdivby2 = devm_clk_hw_register_fixed_factor_parent_hw(dev,
> +                       clk_name, n1_postdiv, 0, 1, 2);
> +       if (IS_ERR(n1_postdivby2))
> +               return PTR_ERR(n1_postdivby2);
>
>         snprintf(clk_name, 32, "dsi%dpll", pll_14nm->phy->id);
> -       snprintf(parent, 32, "dsi%dn1_postdivby2_clk", pll_14nm->phy->id);
>
>         /* DSI pixel clock = VCO_CLK / N1 / 2 / N2
>          * This is the output of N2 post-divider, bits 4-7 in
>          * REG_DSI_14nm_PHY_CMN_CLK_CFG0. Don't let it set parent.
>          */
> -       hw = pll_14nm_postdiv_register(pll_14nm, clk_name, parent, 0, 4);
> +       hw = pll_14nm_postdiv_register(pll_14nm, clk_name, n1_postdivby2,
> +                       0, 4);
>         if (IS_ERR(hw))
>                 return PTR_ERR(hw);
>
> -       provided_clocks[DSI_PIXEL_PLL_CLK]      = hw;
> +       provided_clocks[DSI_PIXEL_PLL_CLK] = hw;
>
>         return 0;
>  }
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

[PATCH 8/9] drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 92 ++++++++++------------
 1 file changed, 40 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index 08b015ea1b1e..178c3f70a7b2 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -571,8 +571,7 @@ static int dsi_10nm_set_usecase(struct msm_dsi_phy *phy)
  */
 static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent[32], vco_name[32];
-	char parent2[32], parent3[32], parent4[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -583,7 +582,8 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
 		.ops = &clk_ops_dsi_pll_10nm_vco,
 	};
 	struct device *dev = &pll_10nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *pll_out_div, *pll_bit, *pll_by_2_bit;
+	struct clk_hw *pll_post_out_div, *pclk_mux;
 	int ret;

 	DBG("DSI%d", pll_10nm->phy->id);
@@ -596,39 +596,34 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
 		return ret;

 	snprintf(clk_name, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%dvco_clk", pll_10nm->phy->id);

-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				     parent, CLK_SET_RATE_PARENT,
-				     pll_10nm->phy->pll_base +
-				     REG_DSI_10nm_PHY_PLL_PLL_OUTDIV_RATE,
-				     0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_out_div = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_10nm->clk_hw, CLK_SET_RATE_PARENT,
+			pll_10nm->phy->pll_base +
+			REG_DSI_10nm_PHY_PLL_PLL_OUTDIV_RATE,
+			0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
+	if (IS_ERR(pll_out_div)) {
+		ret = PTR_ERR(pll_out_div);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);

 	/* BIT CLK: DIV_CTRL_3_0 */
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     CLK_SET_RATE_PARENT,
-				     pll_10nm->phy->base +
-				     REG_DSI_10nm_PHY_CMN_CLK_CFG0,
-				     0, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_10nm->postdiv_lock);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_bit = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			pll_out_div, CLK_SET_RATE_PARENT,
+			pll_10nm->phy->base + REG_DSI_10nm_PHY_CMN_CLK_CFG0,
+			0, 4, CLK_DIVIDER_ONE_BASED, &pll_10nm->postdiv_lock);
+	if (IS_ERR(pll_bit)) {
+		ret = PTR_ERR(pll_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_byteclk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);

 	/* DSI Byte clock = VCO_CLK / OUT_DIV / BIT_DIV / 8 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  CLK_SET_RATE_PARENT, 1, 8);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			pll_bit, CLK_SET_RATE_PARENT, 1, 8);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
@@ -637,51 +632,44 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%d_pll_by_2_bit_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);

-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  0, 1, 2);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_by_2_bit = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, pll_bit, 0, 1, 2);
+	if (IS_ERR(pll_by_2_bit)) {
+		ret = PTR_ERR(pll_by_2_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_post_out_div_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);

-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  0, 1, 4);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_post_out_div = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, pll_out_div, 0, 1, 4);
+	if (IS_ERR(pll_post_out_div)) {
+		ret = PTR_ERR(pll_post_out_div);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pclk_mux", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);
-	snprintf(parent2, 32, "dsi%d_pll_by_2_bit_clk", pll_10nm->phy->id);
-	snprintf(parent3, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);
-	snprintf(parent4, 32, "dsi%d_pll_post_out_div_clk", pll_10nm->phy->id);
-
-	hw = devm_clk_hw_register_mux(dev, clk_name,
-				 ((const char *[]){
-				 parent, parent2, parent3, parent4
-				 }), 4, 0, pll_10nm->phy->base +
-				 REG_DSI_10nm_PHY_CMN_CLK_CFG1,
-				 0, 2, 0, NULL);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+
+	pclk_mux = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
+			((const struct clk_hw *[]){
+				pll_bit,
+				pll_by_2_bit,
+				pll_out_div,
+				pll_post_out_div,
+			}), 4, 0, pll_10nm->phy->base +
+			REG_DSI_10nm_PHY_CMN_CLK_CFG1, 0, 2, 0, NULL);
+	if (IS_ERR(pclk_mux)) {
+		ret = PTR_ERR(pclk_mux);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_dsiclk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pclk_mux", pll_10nm->phy->id);

 	/* PIX CLK DIV : DIV_CTRL_7_4*/
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     0, pll_10nm->phy->base +
-					REG_DSI_10nm_PHY_CMN_CLK_CFG0,
-				     4, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_10nm->postdiv_lock);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name, pclk_mux,
+			0, pll_10nm->phy->base + REG_DSI_10nm_PHY_CMN_CLK_CFG0,
+			4, 4, CLK_DIVIDER_ONE_BASED, &pll_10nm->postdiv_lock);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
--
2.36.1

[PATCH 8/9] drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 92 ++++++++++------------
 1 file changed, 40 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index 08b015ea1b1e..178c3f70a7b2 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -571,8 +571,7 @@ static int dsi_10nm_set_usecase(struct msm_dsi_phy *phy)
  */
 static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent[32], vco_name[32];
-	char parent2[32], parent3[32], parent4[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -583,7 +582,8 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
 		.ops = &clk_ops_dsi_pll_10nm_vco,
 	};
 	struct device *dev = &pll_10nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *pll_out_div, *pll_bit, *pll_by_2_bit;
+	struct clk_hw *pll_post_out_div, *pclk_mux;
 	int ret;

 	DBG("DSI%d", pll_10nm->phy->id);
@@ -596,39 +596,34 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
 		return ret;

 	snprintf(clk_name, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%dvco_clk", pll_10nm->phy->id);

-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				     parent, CLK_SET_RATE_PARENT,
-				     pll_10nm->phy->pll_base +
-				     REG_DSI_10nm_PHY_PLL_PLL_OUTDIV_RATE,
-				     0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_out_div = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_10nm->clk_hw, CLK_SET_RATE_PARENT,
+			pll_10nm->phy->pll_base +
+			REG_DSI_10nm_PHY_PLL_PLL_OUTDIV_RATE,
+			0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
+	if (IS_ERR(pll_out_div)) {
+		ret = PTR_ERR(pll_out_div);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);

 	/* BIT CLK: DIV_CTRL_3_0 */
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     CLK_SET_RATE_PARENT,
-				     pll_10nm->phy->base +
-				     REG_DSI_10nm_PHY_CMN_CLK_CFG0,
-				     0, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_10nm->postdiv_lock);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_bit = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			pll_out_div, CLK_SET_RATE_PARENT,
+			pll_10nm->phy->base + REG_DSI_10nm_PHY_CMN_CLK_CFG0,
+			0, 4, CLK_DIVIDER_ONE_BASED, &pll_10nm->postdiv_lock);
+	if (IS_ERR(pll_bit)) {
+		ret = PTR_ERR(pll_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_byteclk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);

 	/* DSI Byte clock = VCO_CLK / OUT_DIV / BIT_DIV / 8 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  CLK_SET_RATE_PARENT, 1, 8);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			pll_bit, CLK_SET_RATE_PARENT, 1, 8);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
@@ -637,51 +632,44 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%d_pll_by_2_bit_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);

-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  0, 1, 2);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_by_2_bit = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, pll_bit, 0, 1, 2);
+	if (IS_ERR(pll_by_2_bit)) {
+		ret = PTR_ERR(pll_by_2_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_post_out_div_clk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);

-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  0, 1, 4);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_post_out_div = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, pll_out_div, 0, 1, 4);
+	if (IS_ERR(pll_post_out_div)) {
+		ret = PTR_ERR(pll_post_out_div);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pclk_mux", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_10nm->phy->id);
-	snprintf(parent2, 32, "dsi%d_pll_by_2_bit_clk", pll_10nm->phy->id);
-	snprintf(parent3, 32, "dsi%d_pll_out_div_clk", pll_10nm->phy->id);
-	snprintf(parent4, 32, "dsi%d_pll_post_out_div_clk", pll_10nm->phy->id);
-
-	hw = devm_clk_hw_register_mux(dev, clk_name,
-				 ((const char *[]){
-				 parent, parent2, parent3, parent4
-				 }), 4, 0, pll_10nm->phy->base +
-				 REG_DSI_10nm_PHY_CMN_CLK_CFG1,
-				 0, 2, 0, NULL);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+
+	pclk_mux = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
+			((const struct clk_hw *[]){
+				pll_bit,
+				pll_by_2_bit,
+				pll_out_div,
+				pll_post_out_div,
+			}), 4, 0, pll_10nm->phy->base +
+			REG_DSI_10nm_PHY_CMN_CLK_CFG1, 0, 2, 0, NULL);
+	if (IS_ERR(pclk_mux)) {
+		ret = PTR_ERR(pclk_mux);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_dsiclk", pll_10nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pclk_mux", pll_10nm->phy->id);

 	/* PIX CLK DIV : DIV_CTRL_7_4*/
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     0, pll_10nm->phy->base +
-					REG_DSI_10nm_PHY_CMN_CLK_CFG0,
-				     4, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_10nm->postdiv_lock);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name, pclk_mux,
+			0, pll_10nm->phy->base + REG_DSI_10nm_PHY_CMN_CLK_CFG0,
+			4, 4, CLK_DIVIDER_ONE_BASED, &pll_10nm->postdiv_lock);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
--
2.36.1

Re: [PATCH 8/9] drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 92 ++++++++++------------
>  1 file changed, 40 insertions(+), 52 deletions(-)
>

-- 
With best wishes
Dmitry

Re: [PATCH 8/9] drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 92 ++++++++++------------
>  1 file changed, 40 insertions(+), 52 deletions(-)
>

-- 
With best wishes
Dmitry

[PATCH 9/9] drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 92 +++++++++++------------
 1 file changed, 42 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 66ed1919a1db..76a9d5094e1b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -585,8 +585,7 @@ static int dsi_7nm_set_usecase(struct msm_dsi_phy *phy)
  */
 static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent[32], vco_name[32];
-	char parent2[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -597,7 +596,8 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 		.ops = &clk_ops_dsi_pll_7nm_vco,
 	};
 	struct device *dev = &pll_7nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *pll_out_div, *pll_bit, *pll_by_2_bit;
+	struct clk_hw *pll_post_out_div, *phy_pll_out_dsi_parent;
 	int ret;

 	DBG("DSI%d", pll_7nm->phy->id);
@@ -610,40 +610,35 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 		return ret;

 	snprintf(clk_name, 32, "dsi%d_pll_out_div_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%dvco_clk", pll_7nm->phy->id);

-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				     parent, CLK_SET_RATE_PARENT,
-				     pll_7nm->phy->pll_base +
-				     REG_DSI_7nm_PHY_PLL_PLL_OUTDIV_RATE,
-				     0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_out_div = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_7nm->clk_hw, CLK_SET_RATE_PARENT,
+			pll_7nm->phy->pll_base +
+			REG_DSI_7nm_PHY_PLL_PLL_OUTDIV_RATE,
+			0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
+	if (IS_ERR(pll_out_div)) {
+		ret = PTR_ERR(pll_out_div);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_7nm->phy->id);

 	/* BIT CLK: DIV_CTRL_3_0 */
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     CLK_SET_RATE_PARENT,
-				     pll_7nm->phy->base +
-				     REG_DSI_7nm_PHY_CMN_CLK_CFG0,
-				     0, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_7nm->postdiv_lock);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_bit = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			pll_out_div, CLK_SET_RATE_PARENT,
+			pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG0,
+			0, 4, CLK_DIVIDER_ONE_BASED, &pll_7nm->postdiv_lock);
+	if (IS_ERR(pll_bit)) {
+		ret = PTR_ERR(pll_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_byteclk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);

 	/* DSI Byte clock = VCO_CLK / OUT_DIV / BIT_DIV / 8 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  CLK_SET_RATE_PARENT, 1,
-					  pll_7nm->phy->cphy_mode ? 7 : 8);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			pll_bit, CLK_SET_RATE_PARENT, 1,
+			pll_7nm->phy->cphy_mode ? 7 : 8);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
@@ -652,24 +647,24 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%d_pll_by_2_bit_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);

-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  0, 1, 2);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_by_2_bit = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, pll_bit, 0, 1, 2);
+	if (IS_ERR(pll_by_2_bit)) {
+		ret = PTR_ERR(pll_by_2_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_post_out_div_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_7nm->phy->id);

 	if (pll_7nm->phy->cphy_mode)
-		hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 2, 7);
+		pll_post_out_div = devm_clk_hw_register_fixed_factor_parent_hw(
+				dev, clk_name, pll_out_div, 0, 2, 7);
 	else
-		hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 1, 4);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+		pll_post_out_div = devm_clk_hw_register_fixed_factor_parent_hw(
+				dev, clk_name, pll_out_div, 0, 1, 4);
+	if (IS_ERR(pll_post_out_div)) {
+		ret = PTR_ERR(pll_post_out_div);
 		goto fail;
 	}

@@ -682,34 +677,31 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 		data = dsi_phy_read(pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
 		dsi_phy_write(pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1, data | 3);

-		snprintf(parent, 32, "dsi%d_pll_post_out_div_clk", pll_7nm->phy->id);
+		phy_pll_out_dsi_parent = pll_post_out_div;
 	} else {
 		snprintf(clk_name, 32, "dsi%d_pclk_mux", pll_7nm->phy->id);
-		snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);
-		snprintf(parent2, 32, "dsi%d_pll_by_2_bit_clk", pll_7nm->phy->id);
-
-		hw = devm_clk_hw_register_mux(dev, clk_name,
-					((const char *[]){
-					parent, parent2,
-					}), 2, 0, pll_7nm->phy->base +
-					REG_DSI_7nm_PHY_CMN_CLK_CFG1,
-					0, 1, 0, NULL);
+
+		hw = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
+				((const struct clk_hw *[]){
+					pll_bit,
+					pll_by_2_bit,
+				}), 2, 0, pll_7nm->phy->base +
+				REG_DSI_7nm_PHY_CMN_CLK_CFG1, 0, 1, 0, NULL);
 		if (IS_ERR(hw)) {
 			ret = PTR_ERR(hw);
 			goto fail;
 		}

-		snprintf(parent, 32, "dsi%d_pclk_mux", pll_7nm->phy->id);
+		phy_pll_out_dsi_parent = hw;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_dsiclk", pll_7nm->phy->id);

 	/* PIX CLK DIV : DIV_CTRL_7_4*/
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     0, pll_7nm->phy->base +
-					REG_DSI_7nm_PHY_CMN_CLK_CFG0,
-				     4, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_7nm->postdiv_lock);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			phy_pll_out_dsi_parent,
+			0, pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG0,
+			4, 4, CLK_DIVIDER_ONE_BASED, &pll_7nm->postdiv_lock);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
--
2.36.1

[PATCH 9/9] drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers

[Marijn Suijten]

parent_hw pointers are easier to manage and cheaper to use than
repeatedly formatting the parent name and subsequently leaving the clk
framework to perform lookups based on that name.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 92 +++++++++++------------
 1 file changed, 42 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 66ed1919a1db..76a9d5094e1b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -585,8 +585,7 @@ static int dsi_7nm_set_usecase(struct msm_dsi_phy *phy)
  */
 static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provided_clocks)
 {
-	char clk_name[32], parent[32], vco_name[32];
-	char parent2[32];
+	char clk_name[32], vco_name[32];
 	struct clk_init_data vco_init = {
 		.parent_data = &(const struct clk_parent_data) {
 			.fw_name = "ref",
@@ -597,7 +596,8 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 		.ops = &clk_ops_dsi_pll_7nm_vco,
 	};
 	struct device *dev = &pll_7nm->phy->pdev->dev;
-	struct clk_hw *hw;
+	struct clk_hw *hw, *pll_out_div, *pll_bit, *pll_by_2_bit;
+	struct clk_hw *pll_post_out_div, *phy_pll_out_dsi_parent;
 	int ret;

 	DBG("DSI%d", pll_7nm->phy->id);
@@ -610,40 +610,35 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 		return ret;

 	snprintf(clk_name, 32, "dsi%d_pll_out_div_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%dvco_clk", pll_7nm->phy->id);

-	hw = devm_clk_hw_register_divider(dev, clk_name,
-				     parent, CLK_SET_RATE_PARENT,
-				     pll_7nm->phy->pll_base +
-				     REG_DSI_7nm_PHY_PLL_PLL_OUTDIV_RATE,
-				     0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_out_div = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			&pll_7nm->clk_hw, CLK_SET_RATE_PARENT,
+			pll_7nm->phy->pll_base +
+			REG_DSI_7nm_PHY_PLL_PLL_OUTDIV_RATE,
+			0, 2, CLK_DIVIDER_POWER_OF_TWO, NULL);
+	if (IS_ERR(pll_out_div)) {
+		ret = PTR_ERR(pll_out_div);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_7nm->phy->id);

 	/* BIT CLK: DIV_CTRL_3_0 */
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     CLK_SET_RATE_PARENT,
-				     pll_7nm->phy->base +
-				     REG_DSI_7nm_PHY_CMN_CLK_CFG0,
-				     0, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_7nm->postdiv_lock);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_bit = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			pll_out_div, CLK_SET_RATE_PARENT,
+			pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG0,
+			0, 4, CLK_DIVIDER_ONE_BASED, &pll_7nm->postdiv_lock);
+	if (IS_ERR(pll_bit)) {
+		ret = PTR_ERR(pll_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_byteclk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);

 	/* DSI Byte clock = VCO_CLK / OUT_DIV / BIT_DIV / 8 */
-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  CLK_SET_RATE_PARENT, 1,
-					  pll_7nm->phy->cphy_mode ? 7 : 8);
+	hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, clk_name,
+			pll_bit, CLK_SET_RATE_PARENT, 1,
+			pll_7nm->phy->cphy_mode ? 7 : 8);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
@@ -652,24 +647,24 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 	provided_clocks[DSI_BYTE_PLL_CLK] = hw;

 	snprintf(clk_name, 32, "dsi%d_pll_by_2_bit_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);

-	hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent,
-					  0, 1, 2);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+	pll_by_2_bit = devm_clk_hw_register_fixed_factor_parent_hw(dev,
+			clk_name, pll_bit, 0, 1, 2);
+	if (IS_ERR(pll_by_2_bit)) {
+		ret = PTR_ERR(pll_by_2_bit);
 		goto fail;
 	}

 	snprintf(clk_name, 32, "dsi%d_pll_post_out_div_clk", pll_7nm->phy->id);
-	snprintf(parent, 32, "dsi%d_pll_out_div_clk", pll_7nm->phy->id);

 	if (pll_7nm->phy->cphy_mode)
-		hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 2, 7);
+		pll_post_out_div = devm_clk_hw_register_fixed_factor_parent_hw(
+				dev, clk_name, pll_out_div, 0, 2, 7);
 	else
-		hw = devm_clk_hw_register_fixed_factor(dev, clk_name, parent, 0, 1, 4);
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
+		pll_post_out_div = devm_clk_hw_register_fixed_factor_parent_hw(
+				dev, clk_name, pll_out_div, 0, 1, 4);
+	if (IS_ERR(pll_post_out_div)) {
+		ret = PTR_ERR(pll_post_out_div);
 		goto fail;
 	}

@@ -682,34 +677,31 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
 		data = dsi_phy_read(pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
 		dsi_phy_write(pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1, data | 3);

-		snprintf(parent, 32, "dsi%d_pll_post_out_div_clk", pll_7nm->phy->id);
+		phy_pll_out_dsi_parent = pll_post_out_div;
 	} else {
 		snprintf(clk_name, 32, "dsi%d_pclk_mux", pll_7nm->phy->id);
-		snprintf(parent, 32, "dsi%d_pll_bit_clk", pll_7nm->phy->id);
-		snprintf(parent2, 32, "dsi%d_pll_by_2_bit_clk", pll_7nm->phy->id);
-
-		hw = devm_clk_hw_register_mux(dev, clk_name,
-					((const char *[]){
-					parent, parent2,
-					}), 2, 0, pll_7nm->phy->base +
-					REG_DSI_7nm_PHY_CMN_CLK_CFG1,
-					0, 1, 0, NULL);
+
+		hw = devm_clk_hw_register_mux_parent_hws(dev, clk_name,
+				((const struct clk_hw *[]){
+					pll_bit,
+					pll_by_2_bit,
+				}), 2, 0, pll_7nm->phy->base +
+				REG_DSI_7nm_PHY_CMN_CLK_CFG1, 0, 1, 0, NULL);
 		if (IS_ERR(hw)) {
 			ret = PTR_ERR(hw);
 			goto fail;
 		}

-		snprintf(parent, 32, "dsi%d_pclk_mux", pll_7nm->phy->id);
+		phy_pll_out_dsi_parent = hw;
 	}

 	snprintf(clk_name, 32, "dsi%d_phy_pll_out_dsiclk", pll_7nm->phy->id);

 	/* PIX CLK DIV : DIV_CTRL_7_4*/
-	hw = devm_clk_hw_register_divider(dev, clk_name, parent,
-				     0, pll_7nm->phy->base +
-					REG_DSI_7nm_PHY_CMN_CLK_CFG0,
-				     4, 4, CLK_DIVIDER_ONE_BASED,
-				     &pll_7nm->postdiv_lock);
+	hw = devm_clk_hw_register_divider_parent_hw(dev, clk_name,
+			phy_pll_out_dsi_parent,
+			0, pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG0,
+			4, 4, CLK_DIVIDER_ONE_BASED, &pll_7nm->postdiv_lock);
 	if (IS_ERR(hw)) {
 		ret = PTR_ERR(hw);
 		goto fail;
--
2.36.1

Re: [PATCH 9/9] drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:39, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 92 +++++++++++------------
>  1 file changed, 42 insertions(+), 50 deletions(-)
>

-- 
With best wishes
Dmitry

Re: [PATCH 9/9] drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Tue, 24 May 2022 at 00:39, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 92 +++++++++++------------
>  1 file changed, 42 insertions(+), 50 deletions(-)
>

-- 
With best wishes
Dmitry

Re: [PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

Hi,

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> As stated in [1] I promised to tackle and send this series.
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> This series starts out by adding extra constructors for divider, mux and
> fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
> of some DT index or name.  Followed by individual patches performing the
> conversion, one DSI PHY at a time.
>
> dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
> devm_kzalloc allocations (for the lifetime of the device) with
> stack-local char arrays, like all the other DSI PHY drivers.
>
> I couldn't help but notice that clock names are wildly varying:
>
> - Some use underscores in the _clk suffix where others have nothing;
> - Some have an _ after the %d, others have not;
> - Some use a _pll suffix after dsi%d or even _phy_pll suffix.
>
> Are there any thoughts or feelings towards unifying these?
> Theoretically no clock names are used anywhere in the kernel, and
> everything is based on a phandle + index in DT (I have yet to validate
> this).  Obviously no .name/.fw_name will be updated to not break DT.

I'd say, leave them as is. Even if they are historical, we don't have
a strong pressure to change them.

Significant number of older platforms still use names to identify the
clock. And moreover apq8096/msm8960 uses dsi1/dsi2 instead of
dsi0/dsi1.

Probably we should call the next cycle "The Cycle of clocks cleaning".
I can volunteer to take care of 8960/8064/8016/8996, as at least I can
test them. But if you wish, you (or anybody else of course) can take
any of these platforms too, just ping me, so that I won't spend time
duplicating somebody's efforts.

> Which, by the way, is there a particular reason for:
>
>   #define DSI_BYTE_PLL_CLK              0
>   #define DSI_PIXEL_PLL_CLK             1
>
> To not be in the dt-bindings and used in the DT?

Before my restructure of the DSI PHY subsys, each driver defined them
separately. And the idea of moving them to a dt-bindings header didn't
come to my mind. Feel free to do so, it looks like a good idea.
Just as a note, DP PHY also uses 0 for the link clock and 1 for the
pixel clock. What do you think about having a single header for these
names?

>
> And with enough future improvements out of the way, let's round out this
> patch-series by stating that it has been successfully tested on:
>
> - Sony Nile Discovery (Xperia XA2 Ultra): 14nm;
> - Sony Seine PDX201 (Xperia 10II): 14nm;
> - Sony Loire Suzu (Xperia X): 28nm.
>
> And no diff is observed in debugfs's clk_summary.
>
> Unfortunately all other devices in my collection with a 7/10nm DSI PHY
> have a DSC panel which we have yet to get working.

I will test it on RB3 (10nm) and RB5 (7nm) during one of the next few days.

>
> [1]: https://lore.kernel.org/linux-arm-msm/20220502214235.s5plebunh4ttjhge@SoMainline.org/
>
> Marijn Suijten (9):
>   clk: divider: Introduce devm_clk_hw_register_divider_parent_hw()
>   clk: mux: Introduce devm_clk_hw_register_mux_parent_hws()
>   clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw()
>   drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names
>   drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers
>
>  drivers/clk/clk-fixed-factor.c                | 57 ++++++++++--
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c    | 92 ++++++++-----------
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    | 36 ++++----
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c    | 52 +++++------
>  .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c   | 26 ++----
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     | 92 +++++++++----------
>  include/linux/clk-provider.h                  | 34 +++++++
>  7 files changed, 209 insertions(+), 180 deletions(-)
>
> --
> 2.36.1



-- 
With best wishes
Dmitry

Re: [PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

Hi,

On Tue, 24 May 2022 at 00:38, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> As stated in [1] I promised to tackle and send this series.
>
> parent_hw pointers are easier to manage and cheaper to use than
> repeatedly formatting the parent name and subsequently leaving the clk
> framework to perform lookups based on that name.
>
> This series starts out by adding extra constructors for divider, mux and
> fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
> of some DT index or name.  Followed by individual patches performing the
> conversion, one DSI PHY at a time.
>
> dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
> devm_kzalloc allocations (for the lifetime of the device) with
> stack-local char arrays, like all the other DSI PHY drivers.
>
> I couldn't help but notice that clock names are wildly varying:
>
> - Some use underscores in the _clk suffix where others have nothing;
> - Some have an _ after the %d, others have not;
> - Some use a _pll suffix after dsi%d or even _phy_pll suffix.
>
> Are there any thoughts or feelings towards unifying these?
> Theoretically no clock names are used anywhere in the kernel, and
> everything is based on a phandle + index in DT (I have yet to validate
> this).  Obviously no .name/.fw_name will be updated to not break DT.

I'd say, leave them as is. Even if they are historical, we don't have
a strong pressure to change them.

Significant number of older platforms still use names to identify the
clock. And moreover apq8096/msm8960 uses dsi1/dsi2 instead of
dsi0/dsi1.

Probably we should call the next cycle "The Cycle of clocks cleaning".
I can volunteer to take care of 8960/8064/8016/8996, as at least I can
test them. But if you wish, you (or anybody else of course) can take
any of these platforms too, just ping me, so that I won't spend time
duplicating somebody's efforts.

> Which, by the way, is there a particular reason for:
>
>   #define DSI_BYTE_PLL_CLK              0
>   #define DSI_PIXEL_PLL_CLK             1
>
> To not be in the dt-bindings and used in the DT?

Before my restructure of the DSI PHY subsys, each driver defined them
separately. And the idea of moving them to a dt-bindings header didn't
come to my mind. Feel free to do so, it looks like a good idea.
Just as a note, DP PHY also uses 0 for the link clock and 1 for the
pixel clock. What do you think about having a single header for these
names?

>
> And with enough future improvements out of the way, let's round out this
> patch-series by stating that it has been successfully tested on:
>
> - Sony Nile Discovery (Xperia XA2 Ultra): 14nm;
> - Sony Seine PDX201 (Xperia 10II): 14nm;
> - Sony Loire Suzu (Xperia X): 28nm.
>
> And no diff is observed in debugfs's clk_summary.
>
> Unfortunately all other devices in my collection with a 7/10nm DSI PHY
> have a DSC panel which we have yet to get working.

I will test it on RB3 (10nm) and RB5 (7nm) during one of the next few days.

>
> [1]: https://lore.kernel.org/linux-arm-msm/20220502214235.s5plebunh4ttjhge@SoMainline.org/
>
> Marijn Suijten (9):
>   clk: divider: Introduce devm_clk_hw_register_divider_parent_hw()
>   clk: mux: Introduce devm_clk_hw_register_mux_parent_hws()
>   clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw()
>   drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names
>   drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers
>   drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers
>
>  drivers/clk/clk-fixed-factor.c                | 57 ++++++++++--
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c    | 92 ++++++++-----------
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    | 36 ++++----
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c    | 52 +++++------
>  .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c   | 26 ++----
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     | 92 +++++++++----------
>  include/linux/clk-provider.h                  | 34 +++++++
>  7 files changed, 209 insertions(+), 180 deletions(-)
>
> --
> 2.36.1



-- 
With best wishes
Dmitry

Re: [PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Marijn Suijten]

On 2022-05-24 02:43:01, Dmitry Baryshkov wrote:
> Hi,
> 
> On Tue, 24 May 2022 at 00:38, Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > As stated in [1] I promised to tackle and send this series.
> >
> > parent_hw pointers are easier to manage and cheaper to use than
> > repeatedly formatting the parent name and subsequently leaving the clk
> > framework to perform lookups based on that name.
> >
> > This series starts out by adding extra constructors for divider, mux and
> > fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
> > of some DT index or name.  Followed by individual patches performing the
> > conversion, one DSI PHY at a time.
> >
> > dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
> > devm_kzalloc allocations (for the lifetime of the device) with
> > stack-local char arrays, like all the other DSI PHY drivers.
> >
> > I couldn't help but notice that clock names are wildly varying:
> >
> > - Some use underscores in the _clk suffix where others have nothing;
> > - Some have an _ after the %d, others have not;
> > - Some use a _pll suffix after dsi%d or even _phy_pll suffix.
> >
> > Are there any thoughts or feelings towards unifying these?
> > Theoretically no clock names are used anywhere in the kernel, and
> > everything is based on a phandle + index in DT (I have yet to validate
> > this).  Obviously no .name/.fw_name will be updated to not break DT.
> 
> I'd say, leave them as is. Even if they are historical, we don't have
> a strong pressure to change them.

Leave them as it is, or - as suggested below - clean them up?

> Significant number of older platforms still use names to identify the
> clock. And moreover apq8096/msm8960 uses dsi1/dsi2 instead of
> dsi0/dsi1.
> 
> Probably we should call the next cycle "The Cycle of clocks cleaning".
> I can volunteer to take care of 8960/8064/8016/8996, as at least I can
> test them. But if you wish, you (or anybody else of course) can take
> any of these platforms too, just ping me, so that I won't spend time
> duplicating somebody's efforts.

We can at least clean up the names of clocks that are not "exported" by
the drivers.  However, we should also convert all other clk drivers to
utilize DT to define clk dependencies instead of depending on global
names, and already got quite some platforms tackled.  At that point we
can just convert all names (give or take the often discussed "backwards
compatbility" between the kernel and some ancient DT someone may still
be running on their device).

I don't own any device for the SoCs you mentioned, all good from my
side if you take them.  We should probably note down all clock drivers
that still need conversion and split them across devs with physical
access, then I can check what I still have lying around here as well.

> > Which, by the way, is there a particular reason for:
> >
> >   #define DSI_BYTE_PLL_CLK              0
> >   #define DSI_PIXEL_PLL_CLK             1
> >
> > To not be in the dt-bindings and used in the DT?
> 
> Before my restructure of the DSI PHY subsys, each driver defined them
> separately. And the idea of moving them to a dt-bindings header didn't
> come to my mind. Feel free to do so, it looks like a good idea.
> Just as a note, DP PHY also uses 0 for the link clock and 1 for the
> pixel clock. What do you think about having a single header for these
> names?

No worries, it's already much better to have them defined once :), now
we can just go one step further and move it to dt-bindings.  Great to
clean up the "magic constant indices" for the DP PHY as well
(phy-qcom-qmp.c is the only one defining these clocks, right?) and I
think we're fine having them in one header, pending someone suggesting a
name as I have no idea what to call it nor where to put it.  Under
dt-bindings/clock most likely, but what common name would we choose?
Something including qcom and mdss?

> 
> >
> > And with enough future improvements out of the way, let's round out this
> > patch-series by stating that it has been successfully tested on:
> >
> > - Sony Nile Discovery (Xperia XA2 Ultra): 14nm;
> > - Sony Seine PDX201 (Xperia 10II): 14nm;
> > - Sony Loire Suzu (Xperia X): 28nm.
> >
> > And no diff is observed in debugfs's clk_summary.
> >
> > Unfortunately all other devices in my collection with a 7/10nm DSI PHY
> > have a DSC panel which we have yet to get working.
> 
> I will test it on RB3 (10nm) and RB5 (7nm) during one of the next few days.

Lovely, thanks a ton - also for reviewing this so quickly!

- Marijn

Re: [PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Marijn Suijten]

On 2022-05-24 02:43:01, Dmitry Baryshkov wrote:
> Hi,
> 
> On Tue, 24 May 2022 at 00:38, Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > As stated in [1] I promised to tackle and send this series.
> >
> > parent_hw pointers are easier to manage and cheaper to use than
> > repeatedly formatting the parent name and subsequently leaving the clk
> > framework to perform lookups based on that name.
> >
> > This series starts out by adding extra constructors for divider, mux and
> > fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
> > of some DT index or name.  Followed by individual patches performing the
> > conversion, one DSI PHY at a time.
> >
> > dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
> > devm_kzalloc allocations (for the lifetime of the device) with
> > stack-local char arrays, like all the other DSI PHY drivers.
> >
> > I couldn't help but notice that clock names are wildly varying:
> >
> > - Some use underscores in the _clk suffix where others have nothing;
> > - Some have an _ after the %d, others have not;
> > - Some use a _pll suffix after dsi%d or even _phy_pll suffix.
> >
> > Are there any thoughts or feelings towards unifying these?
> > Theoretically no clock names are used anywhere in the kernel, and
> > everything is based on a phandle + index in DT (I have yet to validate
> > this).  Obviously no .name/.fw_name will be updated to not break DT.
> 
> I'd say, leave them as is. Even if they are historical, we don't have
> a strong pressure to change them.

Leave them as it is, or - as suggested below - clean them up?

> Significant number of older platforms still use names to identify the
> clock. And moreover apq8096/msm8960 uses dsi1/dsi2 instead of
> dsi0/dsi1.
> 
> Probably we should call the next cycle "The Cycle of clocks cleaning".
> I can volunteer to take care of 8960/8064/8016/8996, as at least I can
> test them. But if you wish, you (or anybody else of course) can take
> any of these platforms too, just ping me, so that I won't spend time
> duplicating somebody's efforts.

We can at least clean up the names of clocks that are not "exported" by
the drivers.  However, we should also convert all other clk drivers to
utilize DT to define clk dependencies instead of depending on global
names, and already got quite some platforms tackled.  At that point we
can just convert all names (give or take the often discussed "backwards
compatbility" between the kernel and some ancient DT someone may still
be running on their device).

I don't own any device for the SoCs you mentioned, all good from my
side if you take them.  We should probably note down all clock drivers
that still need conversion and split them across devs with physical
access, then I can check what I still have lying around here as well.

> > Which, by the way, is there a particular reason for:
> >
> >   #define DSI_BYTE_PLL_CLK              0
> >   #define DSI_PIXEL_PLL_CLK             1
> >
> > To not be in the dt-bindings and used in the DT?
> 
> Before my restructure of the DSI PHY subsys, each driver defined them
> separately. And the idea of moving them to a dt-bindings header didn't
> come to my mind. Feel free to do so, it looks like a good idea.
> Just as a note, DP PHY also uses 0 for the link clock and 1 for the
> pixel clock. What do you think about having a single header for these
> names?

No worries, it's already much better to have them defined once :), now
we can just go one step further and move it to dt-bindings.  Great to
clean up the "magic constant indices" for the DP PHY as well
(phy-qcom-qmp.c is the only one defining these clocks, right?) and I
think we're fine having them in one header, pending someone suggesting a
name as I have no idea what to call it nor where to put it.  Under
dt-bindings/clock most likely, but what common name would we choose?
Something including qcom and mdss?

> 
> >
> > And with enough future improvements out of the way, let's round out this
> > patch-series by stating that it has been successfully tested on:
> >
> > - Sony Nile Discovery (Xperia XA2 Ultra): 14nm;
> > - Sony Seine PDX201 (Xperia 10II): 14nm;
> > - Sony Loire Suzu (Xperia X): 28nm.
> >
> > And no diff is observed in debugfs's clk_summary.
> >
> > Unfortunately all other devices in my collection with a 7/10nm DSI PHY
> > have a DSC panel which we have yet to get working.
> 
> I will test it on RB3 (10nm) and RB5 (7nm) during one of the next few days.

Lovely, thanks a ton - also for reviewing this so quickly!

- Marijn

Re: [PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

[Dmitry Baryshkov]

On Wed, 25 May 2022 at 01:03, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2022-05-24 02:43:01, Dmitry Baryshkov wrote:
> > Hi,
> >
> > On Tue, 24 May 2022 at 00:38, Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > As stated in [1] I promised to tackle and send this series.
> > >
> > > parent_hw pointers are easier to manage and cheaper to use than
> > > repeatedly formatting the parent name and subsequently leaving the clk
> > > framework to perform lookups based on that name.
> > >
> > > This series starts out by adding extra constructors for divider, mux and
> > > fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
> > > of some DT index or name.  Followed by individual patches performing the
> > > conversion, one DSI PHY at a time.
> > >
> > > dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
> > > devm_kzalloc allocations (for the lifetime of the device) with
> > > stack-local char arrays, like all the other DSI PHY drivers.
> > >
> > > I couldn't help but notice that clock names are wildly varying:
> > >
> > > - Some use underscores in the _clk suffix where others have nothing;
> > > - Some have an _ after the %d, others have not;
> > > - Some use a _pll suffix after dsi%d or even _phy_pll suffix.
> > >
> > > Are there any thoughts or feelings towards unifying these?
> > > Theoretically no clock names are used anywhere in the kernel, and
> > > everything is based on a phandle + index in DT (I have yet to validate
> > > this).  Obviously no .name/.fw_name will be updated to not break DT.
> >
> > I'd say, leave them as is. Even if they are historical, we don't have
> > a strong pressure to change them.
>
> Leave them as it is, or - as suggested below - clean them up?

Let's leave the names as is for now, convert all clock drivers to
fetch clocks from DT and decide how to continue with clock names
afterwards.

> > Significant number of older platforms still use names to identify the
> > clock. And moreover apq8096/msm8960 uses dsi1/dsi2 instead of
> > dsi0/dsi1.
> >
> > Probably we should call the next cycle "The Cycle of clocks cleaning".
> > I can volunteer to take care of 8960/8064/8016/8996, as at least I can
> > test them. But if you wish, you (or anybody else of course) can take
> > any of these platforms too, just ping me, so that I won't spend time
> > duplicating somebody's efforts.
>
> We can at least clean up the names of clocks that are not "exported" by
> the drivers.  However, we should also convert all other clk drivers to
> utilize DT to define clk dependencies instead of depending on global
> names, and already got quite some platforms tackled.  At that point we
> can just convert all names (give or take the often discussed "backwards
> compatbility" between the kernel and some ancient DT someone may still
> be running on their device).
>
> I don't own any device for the SoCs you mentioned, all good from my
> side if you take them.  We should probably note down all clock drivers
> that still need conversion and split them across devs with physical
> access, then I can check what I still have lying around here as well.

Can you please make a google spreadsheet? Then anybody can take a look
and volunteer (or check that the platform is being taken care of).
I have 8064 (and thus I can cover 8960 too), 8016, 8096 on my desk and
qcs404 and 8998 in the remote lab (but I can leave them to somebody
else).

> > > Which, by the way, is there a particular reason for:
> > >
> > >   #define DSI_BYTE_PLL_CLK              0
> > >   #define DSI_PIXEL_PLL_CLK             1
> > >
> > > To not be in the dt-bindings and used in the DT?
> >
> > Before my restructure of the DSI PHY subsys, each driver defined them
> > separately. And the idea of moving them to a dt-bindings header didn't
> > come to my mind. Feel free to do so, it looks like a good idea.
> > Just as a note, DP PHY also uses 0 for the link clock and 1 for the
> > pixel clock. What do you think about having a single header for these
> > names?
>
> No worries, it's already much better to have them defined once :), now
> we can just go one step further and move it to dt-bindings.  Great to
> clean up the "magic constant indices" for the DP PHY as well
> (phy-qcom-qmp.c is the only one defining these clocks, right?)

No, phy-qcom-edp.c also uses these magic numbers.

> and I
> think we're fine having them in one header, pending someone suggesting a
> name as I have no idea what to call it nor where to put it.  Under
> dt-bindings/clock most likely, but what common name would we choose?
> Something including qcom and mdss?

dt-bindings/phy/phy-qcom-dsi.h and dt-bindings/phy/phy-qcom-dp.h?


-- 
With best wishes
Dmitry