Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/9] Make clk_hw::init NULL after clk registration
@ 2019-07-31 19:35 Stephen Boyd
  2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
                   ` (8 more replies)
  0 siblings, 9 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Andy Gross, Baolin Wang, Barry Song,
	Bjorn Andersson, Charles Keepax, Chunyan Zhang, Dinh Nguyen,
	Doug Anderson, Guo Zeng, Jerome Brunet, Kishon Vijay Abraham I,
	Manivannan Sadhasivam, Neil Armstrong, Richard Fitzgerald,
	Roger Quadros, Taniya Das

We don't want clk provider drivers using the init member of struct
clk_hw after the clk is registered. It isn't guaranteed to be a valid
pointer and all the necessary information inside the structure is copied
over into struct clk_core anyway. This patch series fixes up the handful
of users that do this and then overwrites the pointer to NULL within the
clk registration path.

Stephen Boyd (9):
  clk: actions: Don't reference clk_init_data after registration
  clk: lochnagar: Don't reference clk_init_data after registration
  clk: meson: axg-audio: Don't reference clk_init_data after
    registration
  clk: qcom: Don't reference clk_init_data after registration
  clk: sirf: Don't reference clk_init_data after registration
  clk: socfpga: Don't reference clk_init_data after registration
  clk: sprd: Don't reference clk_init_data after registration
  phy: ti: am654-serdes: Don't reference clk_init_data after
    registration
  clk: Overwrite clk_hw::init with NULL during clk_register()

Cc: Andy Gross <agross@kernel.org>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Barry Song <Baohua.Song@csr.com>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Guo Zeng <Guo.Zeng@csr.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Roger Quadros <rogerq@ti.com>
Cc: Taniya Das <tdas@codeaurora.org>

 drivers/clk/actions/owl-common.c     |  3 ++-
 drivers/clk/clk-lochnagar.c          |  2 +-
 drivers/clk/clk.c                    | 24 ++++++++++++++++--------
 drivers/clk/meson/axg-audio.c        |  7 +++++--
 drivers/clk/qcom/clk-rpmh.c          |  4 ++--
 drivers/clk/sirf/clk-common.c        | 12 ++++++++----
 drivers/clk/socfpga/clk-gate.c       | 21 +++++++++++----------
 drivers/clk/socfpga/clk-periph-a10.c |  7 ++++---
 drivers/clk/sprd/common.c            |  5 +++--
 drivers/phy/ti/phy-am654-serdes.c    |  4 ++--
 include/linux/clk-provider.h         |  3 ++-
 11 files changed, 56 insertions(+), 36 deletions(-)


base-commit: 5f9e832c137075045d15cd6899ab0505cfb2ca4b
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
@ 2019-07-31 19:35 ` Stephen Boyd
  2019-08-03 12:48   ` Manivannan Sadhasivam
                     ` (2 more replies)
  2019-07-31 19:35 ` [PATCH 2/9] clk: lochnagar: " Stephen Boyd
                   ` (7 subsequent siblings)
  8 siblings, 3 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Manivannan Sadhasivam

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/actions/owl-common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
index 32dd29e0a37e..71b683c4e643 100644
--- a/drivers/clk/actions/owl-common.c
+++ b/drivers/clk/actions/owl-common.c
@@ -68,6 +68,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
 	struct clk_hw *hw;
 
 	for (i = 0; i < hw_clks->num; i++) {
+		const char *name = hw->init->name;
 
 		hw = hw_clks->hws[i];
 
@@ -77,7 +78,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
 		ret = devm_clk_hw_register(dev, hw);
 		if (ret) {
 			dev_err(dev, "Couldn't register clock %d - %s\n",
-				i, hw->init->name);
+				i, name);
 			return ret;
 		}
 	}
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/9] clk: lochnagar: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
  2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-08-01  8:07   ` Charles Keepax
  2019-08-14  0:24   ` Stephen Boyd
  2019-07-31 19:35 ` [PATCH 3/9] clk: meson: axg-audio: " Stephen Boyd
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Charles Keepax, Richard Fitzgerald

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/clk-lochnagar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-lochnagar.c b/drivers/clk/clk-lochnagar.c
index fa8c91758b1d..565bcd0cdde9 100644
--- a/drivers/clk/clk-lochnagar.c
+++ b/drivers/clk/clk-lochnagar.c
@@ -198,7 +198,7 @@ static u8 lochnagar_clk_get_parent(struct clk_hw *hw)
 	if (ret < 0) {
 		dev_dbg(priv->dev, "Failed to read parent of %s: %d\n",
 			lclk->name, ret);
-		return hw->init->num_parents;
+		return clk_hw_get_num_parents(hw);
 	}
 
 	val &= lclk->src_mask;
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
  2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
  2019-07-31 19:35 ` [PATCH 2/9] clk: lochnagar: " Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-07-31 19:53   ` Neil Armstrong
                     ` (2 more replies)
  2019-07-31 19:35 ` [PATCH 4/9] clk: qcom: " Stephen Boyd
                   ` (5 subsequent siblings)
  8 siblings, 3 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Neil Armstrong, Jerome Brunet

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/meson/axg-audio.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
index 8028ff6f6610..db0b73d53551 100644
--- a/drivers/clk/meson/axg-audio.c
+++ b/drivers/clk/meson/axg-audio.c
@@ -992,15 +992,18 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
 
 	/* Take care to skip the registered input clocks */
 	for (i = AUD_CLKID_DDR_ARB; i < data->hw_onecell_data->num; i++) {
+		const char *name;
+
 		hw = data->hw_onecell_data->hws[i];
 		/* array might be sparse */
 		if (!hw)
 			continue;
 
+		name = hw->init->name;
+
 		ret = devm_clk_hw_register(dev, hw);
 		if (ret) {
-			dev_err(dev, "failed to register clock %s\n",
-				hw->init->name);
+			dev_err(dev, "failed to register clock %s\n", name);
 			return ret;
 		}
 	}
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 4/9] clk: qcom: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
                   ` (2 preceding siblings ...)
  2019-07-31 19:35 ` [PATCH 3/9] clk: meson: axg-audio: " Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-08-04 17:49   ` Taniya Das
  2019-08-14  0:24   ` Stephen Boyd
  2019-07-31 19:35 ` [PATCH 5/9] clk: sirf: " Stephen Boyd
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Taniya Das, Andy Gross

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Taniya Das <tdas@codeaurora.org>
Cc: Andy Gross <agross@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/qcom/clk-rpmh.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
index c3fd632af119..7a8a84dcb70d 100644
--- a/drivers/clk/qcom/clk-rpmh.c
+++ b/drivers/clk/qcom/clk-rpmh.c
@@ -396,6 +396,7 @@ static int clk_rpmh_probe(struct platform_device *pdev)
 	hw_clks = desc->clks;
 
 	for (i = 0; i < desc->num_clks; i++) {
+		const char *name = hw_clks[i]->init->name;
 		u32 res_addr;
 		size_t aux_data_len;
 		const struct bcm_db *data;
@@ -426,8 +427,7 @@ static int clk_rpmh_probe(struct platform_device *pdev)
 
 		ret = devm_clk_hw_register(&pdev->dev, hw_clks[i]);
 		if (ret) {
-			dev_err(&pdev->dev, "failed to register %s\n",
-				hw_clks[i]->init->name);
+			dev_err(&pdev->dev, "failed to register %s\n", name);
 			return ret;
 		}
 	}
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 5/9] clk: sirf: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
                   ` (3 preceding siblings ...)
  2019-07-31 19:35 ` [PATCH 4/9] clk: qcom: " Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-08-14  0:25   ` Stephen Boyd
  2019-07-31 19:35 ` [PATCH 6/9] clk: socfpga: " Stephen Boyd
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Guo Zeng, Barry Song

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Guo Zeng <Guo.Zeng@csr.com>
Cc: Barry Song <Baohua.Song@csr.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/sirf/clk-common.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/sirf/clk-common.c b/drivers/clk/sirf/clk-common.c
index ad7951b6b285..dcf4e25a0216 100644
--- a/drivers/clk/sirf/clk-common.c
+++ b/drivers/clk/sirf/clk-common.c
@@ -297,9 +297,10 @@ static u8 dmn_clk_get_parent(struct clk_hw *hw)
 {
 	struct clk_dmn *clk = to_dmnclk(hw);
 	u32 cfg = clkc_readl(clk->regofs);
+	const char *name = clk_hw_get_name(hw);
 
 	/* parent of io domain can only be pll3 */
-	if (strcmp(hw->init->name, "io") == 0)
+	if (strcmp(name, "io") == 0)
 		return 4;
 
 	WARN_ON((cfg & (BIT(3) - 1)) > 4);
@@ -311,9 +312,10 @@ static int dmn_clk_set_parent(struct clk_hw *hw, u8 parent)
 {
 	struct clk_dmn *clk = to_dmnclk(hw);
 	u32 cfg = clkc_readl(clk->regofs);
+	const char *name = clk_hw_get_name(hw);
 
 	/* parent of io domain can only be pll3 */
-	if (strcmp(hw->init->name, "io") == 0)
+	if (strcmp(name, "io") == 0)
 		return -EINVAL;
 
 	cfg &= ~(BIT(3) - 1);
@@ -353,7 +355,8 @@ static long dmn_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 {
 	unsigned long fin;
 	unsigned ratio, wait, hold;
-	unsigned bits = (strcmp(hw->init->name, "mem") == 0) ? 3 : 4;
+	const char *name = clk_hw_get_name(hw);
+	unsigned bits = (strcmp(name, "mem") == 0) ? 3 : 4;
 
 	fin = *parent_rate;
 	ratio = fin / rate;
@@ -375,7 +378,8 @@ static int dmn_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct clk_dmn *clk = to_dmnclk(hw);
 	unsigned long fin;
 	unsigned ratio, wait, hold, reg;
-	unsigned bits = (strcmp(hw->init->name, "mem") == 0) ? 3 : 4;
+	const char *name = clk_hw_get_name(hw);
+	unsigned bits = (strcmp(name, "mem") == 0) ? 3 : 4;
 
 	fin = parent_rate;
 	ratio = fin / rate;
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 6/9] clk: socfpga: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
                   ` (4 preceding siblings ...)
  2019-07-31 19:35 ` [PATCH 5/9] clk: sirf: " Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-08-01 15:12   ` Dinh Nguyen
  2019-08-14  0:25   ` Stephen Boyd
  2019-07-31 19:35 ` [PATCH 7/9] clk: sprd: " Stephen Boyd
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd; +Cc: linux-kernel, linux-clk, Dinh Nguyen

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Dinh Nguyen <dinguyen@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/socfpga/clk-gate.c       | 21 +++++++++++----------
 drivers/clk/socfpga/clk-periph-a10.c |  7 ++++---
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c
index 3966cd43b552..b3c8143909dc 100644
--- a/drivers/clk/socfpga/clk-gate.c
+++ b/drivers/clk/socfpga/clk-gate.c
@@ -31,20 +31,20 @@ static u8 socfpga_clk_get_parent(struct clk_hw *hwclk)
 	u32 l4_src;
 	u32 perpll_src;
 
-	if (streq(hwclk->init->name, SOCFPGA_L4_MP_CLK)) {
+	if (streq(name, SOCFPGA_L4_MP_CLK)) {
 		l4_src = readl(clk_mgr_base_addr + CLKMGR_L4SRC);
 		return l4_src &= 0x1;
 	}
-	if (streq(hwclk->init->name, SOCFPGA_L4_SP_CLK)) {
+	if (streq(name, SOCFPGA_L4_SP_CLK)) {
 		l4_src = readl(clk_mgr_base_addr + CLKMGR_L4SRC);
 		return !!(l4_src & 2);
 	}
 
 	perpll_src = readl(clk_mgr_base_addr + CLKMGR_PERPLL_SRC);
-	if (streq(hwclk->init->name, SOCFPGA_MMC_CLK))
+	if (streq(name, SOCFPGA_MMC_CLK))
 		return perpll_src &= 0x3;
-	if (streq(hwclk->init->name, SOCFPGA_NAND_CLK) ||
-			streq(hwclk->init->name, SOCFPGA_NAND_X_CLK))
+	if (streq(name, SOCFPGA_NAND_CLK) ||
+			streq(name, SOCFPGA_NAND_X_CLK))
 			return (perpll_src >> 2) & 3;
 
 	/* QSPI clock */
@@ -55,24 +55,25 @@ static u8 socfpga_clk_get_parent(struct clk_hw *hwclk)
 static int socfpga_clk_set_parent(struct clk_hw *hwclk, u8 parent)
 {
 	u32 src_reg;
+	const char *name = clk_hw_get_name(hwclk);
 
-	if (streq(hwclk->init->name, SOCFPGA_L4_MP_CLK)) {
+	if (streq(name, SOCFPGA_L4_MP_CLK)) {
 		src_reg = readl(clk_mgr_base_addr + CLKMGR_L4SRC);
 		src_reg &= ~0x1;
 		src_reg |= parent;
 		writel(src_reg, clk_mgr_base_addr + CLKMGR_L4SRC);
-	} else if (streq(hwclk->init->name, SOCFPGA_L4_SP_CLK)) {
+	} else if (streq(name, SOCFPGA_L4_SP_CLK)) {
 		src_reg = readl(clk_mgr_base_addr + CLKMGR_L4SRC);
 		src_reg &= ~0x2;
 		src_reg |= (parent << 1);
 		writel(src_reg, clk_mgr_base_addr + CLKMGR_L4SRC);
 	} else {
 		src_reg = readl(clk_mgr_base_addr + CLKMGR_PERPLL_SRC);
-		if (streq(hwclk->init->name, SOCFPGA_MMC_CLK)) {
+		if (streq(name, SOCFPGA_MMC_CLK)) {
 			src_reg &= ~0x3;
 			src_reg |= parent;
-		} else if (streq(hwclk->init->name, SOCFPGA_NAND_CLK) ||
-			streq(hwclk->init->name, SOCFPGA_NAND_X_CLK)) {
+		} else if (streq(name, SOCFPGA_NAND_CLK) ||
+			streq(name, SOCFPGA_NAND_X_CLK)) {
 			src_reg &= ~0xC;
 			src_reg |= (parent << 2);
 		} else {/* QSPI clock */
diff --git a/drivers/clk/socfpga/clk-periph-a10.c b/drivers/clk/socfpga/clk-periph-a10.c
index a8ff7229611d..3e0c55727b89 100644
--- a/drivers/clk/socfpga/clk-periph-a10.c
+++ b/drivers/clk/socfpga/clk-periph-a10.c
@@ -40,11 +40,12 @@ static u8 clk_periclk_get_parent(struct clk_hw *hwclk)
 {
 	struct socfpga_periph_clk *socfpgaclk = to_socfpga_periph_clk(hwclk);
 	u32 clk_src;
+	const char *name = clk_hw_get_name(hwclk);
 
 	clk_src = readl(socfpgaclk->hw.reg);
-	if (streq(hwclk->init->name, SOCFPGA_MPU_FREE_CLK) ||
-	    streq(hwclk->init->name, SOCFPGA_NOC_FREE_CLK) ||
-	    streq(hwclk->init->name, SOCFPGA_SDMMC_FREE_CLK))
+	if (streq(name, SOCFPGA_MPU_FREE_CLK) ||
+	    streq(name, SOCFPGA_NOC_FREE_CLK) ||
+	    streq(name, SOCFPGA_SDMMC_FREE_CLK))
 		return (clk_src >> CLK_MGR_FREE_SHIFT) &
 			CLK_MGR_FREE_MASK;
 	else
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 7/9] clk: sprd: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
                   ` (5 preceding siblings ...)
  2019-07-31 19:35 ` [PATCH 6/9] clk: socfpga: " Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-08-01  7:41   ` Baolin Wang
                     ` (2 more replies)
  2019-07-31 19:35 ` [PATCH 8/9] phy: ti: am654-serdes: " Stephen Boyd
  2019-07-31 19:35 ` [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register() Stephen Boyd
  8 siblings, 3 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Chunyan Zhang, Baolin Wang

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
Cc: Baolin Wang <baolin.wang@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/sprd/common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
index a5bdca1de5d0..9d56eac43832 100644
--- a/drivers/clk/sprd/common.c
+++ b/drivers/clk/sprd/common.c
@@ -76,16 +76,17 @@ int sprd_clk_probe(struct device *dev, struct clk_hw_onecell_data *clkhw)
 	struct clk_hw *hw;
 
 	for (i = 0; i < clkhw->num; i++) {
+		const char *name;
 
 		hw = clkhw->hws[i];
-
 		if (!hw)
 			continue;
 
+		name = hw->init->name;
 		ret = devm_clk_hw_register(dev, hw);
 		if (ret) {
 			dev_err(dev, "Couldn't register clock %d - %s\n",
-				i, hw->init->name);
+				i, name);
 			return ret;
 		}
 	}
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 8/9] phy: ti: am654-serdes: Don't reference clk_init_data after registration
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
                   ` (6 preceding siblings ...)
  2019-07-31 19:35 ` [PATCH 7/9] clk: sprd: " Stephen Boyd
@ 2019-07-31 19:35 ` " Stephen Boyd
  2019-08-02 12:45   ` Kishon Vijay Abraham I
  2019-08-14  0:25   ` Stephen Boyd
  2019-07-31 19:35 ` [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register() Stephen Boyd
  8 siblings, 2 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Roger Quadros, Kishon Vijay Abraham I

A future patch is going to change semantics of clk_register() so that
clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
referencing this member here so that we don't run into NULL pointer
exceptions.

Cc: Roger Quadros <rogerq@ti.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/phy/ti/phy-am654-serdes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/ti/phy-am654-serdes.c b/drivers/phy/ti/phy-am654-serdes.c
index f8edd0840fa2..398a8c9b675a 100644
--- a/drivers/phy/ti/phy-am654-serdes.c
+++ b/drivers/phy/ti/phy-am654-serdes.c
@@ -335,6 +335,7 @@ static int serdes_am654_clk_mux_set_parent(struct clk_hw *hw, u8 index)
 {
 	struct serdes_am654_clk_mux *mux = to_serdes_am654_clk_mux(hw);
 	struct regmap *regmap = mux->regmap;
+	const char *name = clk_hw_get_name(hw);
 	unsigned int reg = mux->reg;
 	int clk_id = mux->clk_id;
 	int parents[SERDES_NUM_CLOCKS];
@@ -374,8 +375,7 @@ static int serdes_am654_clk_mux_set_parent(struct clk_hw *hw, u8 index)
 		 * This can never happen, unless we missed
 		 * a valid combination in serdes_am654_mux_table.
 		 */
-		WARN(1, "Failed to find the parent of %s clock\n",
-		     hw->init->name);
+		WARN(1, "Failed to find the parent of %s clock\n", name);
 		return -EINVAL;
 	}
 
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register()
  2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
                   ` (7 preceding siblings ...)
  2019-07-31 19:35 ` [PATCH 8/9] phy: ti: am654-serdes: " Stephen Boyd
@ 2019-07-31 19:35 ` Stephen Boyd
  2019-08-08 16:14   ` Sylwester Nawrocki
  2019-08-14  0:25   ` Stephen Boyd
  8 siblings, 2 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-07-31 19:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Bjorn Andersson, Doug Anderson

We don't want clk provider drivers to use the init structure after clk
registration time, but we leave a dangling reference to it by means of
clk_hw::init. Let's overwrite the member with NULL during clk_register()
so that this can't be used anymore after registration time.

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack so I can take this through clk tree

 drivers/clk/clk.c            | 24 ++++++++++++++++--------
 include/linux/clk-provider.h |  3 ++-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c0990703ce54..efac620264a2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3484,9 +3484,9 @@ static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
 	return 0;
 }
 
-static int clk_core_populate_parent_map(struct clk_core *core)
+static int clk_core_populate_parent_map(struct clk_core *core,
+					const struct clk_init_data *init)
 {
-	const struct clk_init_data *init = core->hw->init;
 	u8 num_parents = init->num_parents;
 	const char * const *parent_names = init->parent_names;
 	const struct clk_hw **parent_hws = init->parent_hws;
@@ -3566,6 +3566,14 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 {
 	int ret;
 	struct clk_core *core;
+	const struct clk_init_data *init = hw->init;
+
+	/*
+	 * The init data is not supposed to be used outside of registration path.
+	 * Set it to NULL so that provider drivers can't use it either and so that
+	 * we catch use of hw->init early on in the core.
+	 */
+	hw->init = NULL;
 
 	core = kzalloc(sizeof(*core), GFP_KERNEL);
 	if (!core) {
@@ -3573,17 +3581,17 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 		goto fail_out;
 	}
 
-	core->name = kstrdup_const(hw->init->name, GFP_KERNEL);
+	core->name = kstrdup_const(init->name, GFP_KERNEL);
 	if (!core->name) {
 		ret = -ENOMEM;
 		goto fail_name;
 	}
 
-	if (WARN_ON(!hw->init->ops)) {
+	if (WARN_ON(!init->ops)) {
 		ret = -EINVAL;
 		goto fail_ops;
 	}
-	core->ops = hw->init->ops;
+	core->ops = init->ops;
 
 	if (dev && pm_runtime_enabled(dev))
 		core->rpm_enabled = true;
@@ -3592,13 +3600,13 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	if (dev && dev->driver)
 		core->owner = dev->driver->owner;
 	core->hw = hw;
-	core->flags = hw->init->flags;
-	core->num_parents = hw->init->num_parents;
+	core->flags = init->flags;
+	core->num_parents = init->num_parents;
 	core->min_rate = 0;
 	core->max_rate = ULONG_MAX;
 	hw->core = core;
 
-	ret = clk_core_populate_parent_map(core);
+	ret = clk_core_populate_parent_map(core, init);
 	if (ret)
 		goto fail_parents;
 
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 2ae7604783dd..214c75ed62ae 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -299,7 +299,8 @@ struct clk_init_data {
  * into the clk API
  *
  * @init: pointer to struct clk_init_data that contains the init data shared
- * with the common clock framework.
+ * with the common clock framework. This pointer will be set to NULL once
+ * a clk_register() variant is called on this clk_hw pointer.
  */
 struct clk_hw {
 	struct clk_core *core;
-- 
Sent by a computer through tubes


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 3/9] clk: meson: axg-audio: " Stephen Boyd
@ 2019-07-31 19:53   ` Neil Armstrong
  2019-08-06  8:49   ` Jerome Brunet
  2019-08-14  0:24   ` Stephen Boyd
  2 siblings, 0 replies; 34+ messages in thread
From: Neil Armstrong @ 2019-07-31 19:53 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette; +Cc: linux-kernel, linux-clk, Jerome Brunet

Hi Stephen,

Le 31/07/2019 à 21:35, Stephen Boyd a écrit :
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> Please ack so I can take this through clk tree
> 
>  drivers/clk/meson/axg-audio.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
> index 8028ff6f6610..db0b73d53551 100644
> --- a/drivers/clk/meson/axg-audio.c
> +++ b/drivers/clk/meson/axg-audio.c
> @@ -992,15 +992,18 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
>  
>  	/* Take care to skip the registered input clocks */
>  	for (i = AUD_CLKID_DDR_ARB; i < data->hw_onecell_data->num; i++) {
> +		const char *name;
> +
>  		hw = data->hw_onecell_data->hws[i];
>  		/* array might be sparse */
>  		if (!hw)
>  			continue;
>  
> +		name = hw->init->name;
> +
>  		ret = devm_clk_hw_register(dev, hw);
>  		if (ret) {
> -			dev_err(dev, "failed to register clock %s\n",
> -				hw->init->name);
> +			dev_err(dev, "failed to register clock %s\n", name);
>  			return ret;
>  		}
>  	}
> 

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 7/9] clk: sprd: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 7/9] clk: sprd: " Stephen Boyd
@ 2019-08-01  7:41   ` Baolin Wang
  2019-08-02  1:57   ` Chunyan Zhang
  2019-08-14  0:25   ` Stephen Boyd
  2 siblings, 0 replies; 34+ messages in thread
From: Baolin Wang @ 2019-08-01  7:41 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Michael Turquette, LKML, linux-clk, Chunyan Zhang

Hi,

On Thu, 1 Aug 2019 at 03:35, Stephen Boyd <sboyd@kernel.org> wrote:
>
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>
> Please ack so I can take this through clk tree

Thanks.
Acked-by: Baolin Wang <baolin.wang@linaro.org>

-- 
Baolin Wang
Best Regards

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/9] clk: lochnagar: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 2/9] clk: lochnagar: " Stephen Boyd
@ 2019-08-01  8:07   ` Charles Keepax
  2019-08-14  0:24   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Charles Keepax @ 2019-08-01  8:07 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, linux-clk, Richard Fitzgerald

On Wed, Jul 31, 2019 at 12:35:10PM -0700, Stephen Boyd wrote:
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
> Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/9] clk: socfpga: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 6/9] clk: socfpga: " Stephen Boyd
@ 2019-08-01 15:12   ` Dinh Nguyen
  2019-08-01 16:13     ` Stephen Boyd
  2019-08-14  0:25   ` Stephen Boyd
  1 sibling, 1 reply; 34+ messages in thread
From: Dinh Nguyen @ 2019-08-01 15:12 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette; +Cc: linux-kernel, linux-clk

Hi Stephen,

On 7/31/19 2:35 PM, Stephen Boyd wrote:
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> Please ack so I can take this through clk tree
> 
>  drivers/clk/socfpga/clk-gate.c       | 21 +++++++++++----------
>  drivers/clk/socfpga/clk-periph-a10.c |  7 ++++---
>  2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c
> index 3966cd43b552..b3c8143909dc 100644
> --- a/drivers/clk/socfpga/clk-gate.c
> +++ b/drivers/clk/socfpga/clk-gate.c
> @@ -31,20 +31,20 @@ static u8 socfpga_clk_get_parent(struct clk_hw *hwclk)
>  	u32 l4_src;
>  	u32 perpll_src;

You need this line here:

	const char *name = clk_hw_get_name(hwclk);

Otherwise, it fails to build. With the above change:

Acked-by: Dinh Nguyen <dinguyen@kernel.org>

Thanks,
Dinh

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/9] clk: socfpga: Don't reference clk_init_data after registration
  2019-08-01 15:12   ` Dinh Nguyen
@ 2019-08-01 16:13     ` Stephen Boyd
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-01 16:13 UTC (permalink / raw)
  To: Dinh Nguyen, Michael Turquette; +Cc: linux-kernel, linux-clk

Quoting Dinh Nguyen (2019-08-01 08:12:58)
> Hi Stephen,
> 
> On 7/31/19 2:35 PM, Stephen Boyd wrote:
> > A future patch is going to change semantics of clk_register() so that
> > clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> > referencing this member here so that we don't run into NULL pointer
> > exceptions.
> > 
> > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > 
> > Please ack so I can take this through clk tree
> > 
> >  drivers/clk/socfpga/clk-gate.c       | 21 +++++++++++----------
> >  drivers/clk/socfpga/clk-periph-a10.c |  7 ++++---
> >  2 files changed, 15 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c
> > index 3966cd43b552..b3c8143909dc 100644
> > --- a/drivers/clk/socfpga/clk-gate.c
> > +++ b/drivers/clk/socfpga/clk-gate.c
> > @@ -31,20 +31,20 @@ static u8 socfpga_clk_get_parent(struct clk_hw *hwclk)
> >       u32 l4_src;
> >       u32 perpll_src;
> 
> You need this line here:
> 
>         const char *name = clk_hw_get_name(hwclk);
> 
> Otherwise, it fails to build. With the above change:
> 
> Acked-by: Dinh Nguyen <dinguyen@kernel.org>

Awesome thanks! 


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 7/9] clk: sprd: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 7/9] clk: sprd: " Stephen Boyd
  2019-08-01  7:41   ` Baolin Wang
@ 2019-08-02  1:57   ` Chunyan Zhang
  2019-08-14  0:25   ` Stephen Boyd
  2 siblings, 0 replies; 34+ messages in thread
From: Chunyan Zhang @ 2019-08-02  1:57 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, Linux Kernel Mailing List, linux-clk, Baolin Wang

On Thu, 1 Aug 2019 at 03:35, Stephen Boyd <sboyd@kernel.org> wrote:
>
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>
> Please ack so I can take this through clk tree

Acked-by: Chunyan Zhang <zhang.chunyan@linaro.org>

Thanks.

>
>  drivers/clk/sprd/common.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
> index a5bdca1de5d0..9d56eac43832 100644
> --- a/drivers/clk/sprd/common.c
> +++ b/drivers/clk/sprd/common.c
> @@ -76,16 +76,17 @@ int sprd_clk_probe(struct device *dev, struct clk_hw_onecell_data *clkhw)
>         struct clk_hw *hw;
>
>         for (i = 0; i < clkhw->num; i++) {
> +               const char *name;
>
>                 hw = clkhw->hws[i];
> -
>                 if (!hw)
>                         continue;
>
> +               name = hw->init->name;
>                 ret = devm_clk_hw_register(dev, hw);
>                 if (ret) {
>                         dev_err(dev, "Couldn't register clock %d - %s\n",
> -                               i, hw->init->name);
> +                               i, name);
>                         return ret;
>                 }
>         }
> --
> Sent by a computer through tubes
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 8/9] phy: ti: am654-serdes: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 8/9] phy: ti: am654-serdes: " Stephen Boyd
@ 2019-08-02 12:45   ` Kishon Vijay Abraham I
  2019-08-14  0:25   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Kishon Vijay Abraham I @ 2019-08-02 12:45 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette; +Cc: linux-kernel, linux-clk, Roger Quadros



On 01/08/19 1:05 AM, Stephen Boyd wrote:
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Roger Quadros <rogerq@ti.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> Please ack so I can take this through clk tree

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> 
>  drivers/phy/ti/phy-am654-serdes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-am654-serdes.c b/drivers/phy/ti/phy-am654-serdes.c
> index f8edd0840fa2..398a8c9b675a 100644
> --- a/drivers/phy/ti/phy-am654-serdes.c
> +++ b/drivers/phy/ti/phy-am654-serdes.c
> @@ -335,6 +335,7 @@ static int serdes_am654_clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  {
>  	struct serdes_am654_clk_mux *mux = to_serdes_am654_clk_mux(hw);
>  	struct regmap *regmap = mux->regmap;
> +	const char *name = clk_hw_get_name(hw);
>  	unsigned int reg = mux->reg;
>  	int clk_id = mux->clk_id;
>  	int parents[SERDES_NUM_CLOCKS];
> @@ -374,8 +375,7 @@ static int serdes_am654_clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  		 * This can never happen, unless we missed
>  		 * a valid combination in serdes_am654_mux_table.
>  		 */
> -		WARN(1, "Failed to find the parent of %s clock\n",
> -		     hw->init->name);
> +		WARN(1, "Failed to find the parent of %s clock\n", name);
>  		return -EINVAL;
>  	}
>  
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
@ 2019-08-03 12:48   ` Manivannan Sadhasivam
  2019-08-14  0:24   ` Stephen Boyd
  2019-08-16 11:22   ` Manivannan Sadhasivam
  2 siblings, 0 replies; 34+ messages in thread
From: Manivannan Sadhasivam @ 2019-08-03 12:48 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Michael Turquette, linux-kernel, linux-clk

On Wed, Jul 31, 2019 at 12:35:09PM -0700, Stephen Boyd wrote:
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
> 
> Please ack so I can take this through clk tree
> 
>  drivers/clk/actions/owl-common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
> index 32dd29e0a37e..71b683c4e643 100644
> --- a/drivers/clk/actions/owl-common.c
> +++ b/drivers/clk/actions/owl-common.c
> @@ -68,6 +68,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
>  	struct clk_hw *hw;
>  
>  	for (i = 0; i < hw_clks->num; i++) {
> +		const char *name = hw->init->name;
>  
>  		hw = hw_clks->hws[i];
>  
> @@ -77,7 +78,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
>  		ret = devm_clk_hw_register(dev, hw);
>  		if (ret) {
>  			dev_err(dev, "Couldn't register clock %d - %s\n",
> -				i, hw->init->name);
> +				i, name);
>  			return ret;
>  		}
>  	}
> -- 
> Sent by a computer through tubes
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/9] clk: qcom: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 4/9] clk: qcom: " Stephen Boyd
@ 2019-08-04 17:49   ` Taniya Das
  2019-08-14  0:24   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Taniya Das @ 2019-08-04 17:49 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette; +Cc: linux-kernel, linux-clk, Andy Gross



On 8/1/2019 1:05 AM, Stephen Boyd wrote:
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Taniya Das <tdas@codeaurora.org>
> Cc: Andy Gross <agross@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>


Acked-by: Taniya Das <tdas@codeaurora.org>

> ---
> 
> Please ack so I can take this through clk tree
> 
>   drivers/clk/qcom/clk-rpmh.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> index c3fd632af119..7a8a84dcb70d 100644
> --- a/drivers/clk/qcom/clk-rpmh.c
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -396,6 +396,7 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>   	hw_clks = desc->clks;
>   
>   	for (i = 0; i < desc->num_clks; i++) {
> +		const char *name = hw_clks[i]->init->name;
>   		u32 res_addr;
>   		size_t aux_data_len;
>   		const struct bcm_db *data;
> @@ -426,8 +427,7 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>   
>   		ret = devm_clk_hw_register(&pdev->dev, hw_clks[i]);
>   		if (ret) {
> -			dev_err(&pdev->dev, "failed to register %s\n",
> -				hw_clks[i]->init->name);
> +			dev_err(&pdev->dev, "failed to register %s\n", name);
>   			return ret;
>   		}
>   	}
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 3/9] clk: meson: axg-audio: " Stephen Boyd
  2019-07-31 19:53   ` Neil Armstrong
@ 2019-08-06  8:49   ` Jerome Brunet
  2019-08-06 21:48     ` Stephen Boyd
  2019-08-14  0:24   ` Stephen Boyd
  2 siblings, 1 reply; 34+ messages in thread
From: Jerome Brunet @ 2019-08-06  8:49 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Neil Armstrong

On Wed 31 Jul 2019 at 12:35, Stephen Boyd <sboyd@kernel.org> wrote:

> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.

Hi Stephen,

What to do you indend to do with this one ? Will you apply directly or
should we take it ?

We have several changes for the controller which may conflict with this
one. It is nothing major but the sooner I know how this changes goes in,
the sooner I can rebase the rest.

Also, We were (re)using the init_data only on register failures.
I understand that you want to guarantee .init is NULL when the clock is
registered, but it this particular case, the registeration failed so the
clock is not registered.

IMO, it would be better if devm_clk_hw_register() left the init_data
untouched if the registration fails.

>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>
> Please ack so I can take this through clk tree
>
>  drivers/clk/meson/axg-audio.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
> index 8028ff6f6610..db0b73d53551 100644
> --- a/drivers/clk/meson/axg-audio.c
> +++ b/drivers/clk/meson/axg-audio.c
> @@ -992,15 +992,18 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
>  
>  	/* Take care to skip the registered input clocks */
>  	for (i = AUD_CLKID_DDR_ARB; i < data->hw_onecell_data->num; i++) {
> +		const char *name;
> +
>  		hw = data->hw_onecell_data->hws[i];
>  		/* array might be sparse */
>  		if (!hw)
>  			continue;
>  
> +		name = hw->init->name;
> +
>  		ret = devm_clk_hw_register(dev, hw);
>  		if (ret) {
> -			dev_err(dev, "failed to register clock %s\n",
> -				hw->init->name);
> +			dev_err(dev, "failed to register clock %s\n", name);
>  			return ret;
>  		}
>  	}
> -- 
> Sent by a computer through tubes

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
  2019-08-06  8:49   ` Jerome Brunet
@ 2019-08-06 21:48     ` Stephen Boyd
  2019-08-07 13:57       ` Jerome Brunet
  0 siblings, 1 reply; 34+ messages in thread
From: Stephen Boyd @ 2019-08-06 21:48 UTC (permalink / raw)
  To: Jerome Brunet, Michael Turquette; +Cc: linux-kernel, linux-clk, Neil Armstrong

Quoting Jerome Brunet (2019-08-06 01:49:47)
> On Wed 31 Jul 2019 at 12:35, Stephen Boyd <sboyd@kernel.org> wrote:
> 
> > A future patch is going to change semantics of clk_register() so that
> > clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> > referencing this member here so that we don't run into NULL pointer
> > exceptions.
> 
> Hi Stephen,
> 
> What to do you indend to do with this one ? Will you apply directly or
> should we take it ?

I said below:

 Please ack so I can take this through clk tree

> 
> We have several changes for the controller which may conflict with this
> one. It is nothing major but the sooner I know how this changes goes in,
> the sooner I can rebase the rest.

Will it conflict? I can deal with conflicts.

> 
> Also, We were (re)using the init_data only on register failures.
> I understand that you want to guarantee .init is NULL when the clock is
> registered, but it this particular case, the registeration failed so the
> clock is not registered.
> 
> IMO, it would be better if devm_clk_hw_register() left the init_data
> untouched if the registration fails.

Do you have other usage of the init_data besides printing out the name?
I think we could have devm_clk_hw_register() print out the name of the
clk that failed to register instead, and get rid of more code in drivers
that way. Unless of course there are other uses of the init struct?

> 
> >
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Cc: Jerome Brunet <jbrunet@baylibre.com>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> >
> > Please ack so I can take this through clk tree
> >
> >  drivers/clk/meson/axg-audio.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
  2019-08-06 21:48     ` Stephen Boyd
@ 2019-08-07 13:57       ` Jerome Brunet
  0 siblings, 0 replies; 34+ messages in thread
From: Jerome Brunet @ 2019-08-07 13:57 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette; +Cc: linux-kernel, linux-clk, Neil Armstrong

On Tue 06 Aug 2019 at 14:48, Stephen Boyd <sboyd@kernel.org> wrote:

> Quoting Jerome Brunet (2019-08-06 01:49:47)
>> On Wed 31 Jul 2019 at 12:35, Stephen Boyd <sboyd@kernel.org> wrote:
>> 
>> > A future patch is going to change semantics of clk_register() so that
>> > clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
>> > referencing this member here so that we don't run into NULL pointer
>> > exceptions.
>> 
>> Hi Stephen,
>> 
>> What to do you indend to do with this one ? Will you apply directly or
>> should we take it ?
>
> I said below:
>
>  Please ack so I can take this through clk tree
>

Missed it, sorry.

>> 
>> We have several changes for the controller which may conflict with this
>> one. It is nothing major but the sooner I know how this changes goes in,
>> the sooner I can rebase the rest.
>
> Will it conflict? I can deal with conflicts.

I'll check to be sure and notify you in the PR if necessary

>
>> 
>> Also, We were (re)using the init_data only on register failures.
>> I understand that you want to guarantee .init is NULL when the clock is
>> registered, but it this particular case, the registeration failed so the
>> clock is not registered.
>> 
>> IMO, it would be better if devm_clk_hw_register() left the init_data
>> untouched if the registration fails.
>
> Do you have other usage of the init_data besides printing out the
> name?

No other use

> I think we could have devm_clk_hw_register() print out the name of the
> clk that failed to register instead, and get rid of more code in drivers
> that way.

Sure, why not.

> Unless of course there are other uses of the init struct?

I was just commenting on the fact that initialization function tends to
rollback what they can in case of failures, usually.

>
>> 
>> >
>> > Cc: Neil Armstrong <narmstrong@baylibre.com>
>> > Cc: Jerome Brunet <jbrunet@baylibre.com>
>> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
>> > ---
>> >
>> > Please ack so I can take this through clk tree
>> >
>> >  drivers/clk/meson/axg-audio.c | 7 +++++--
>> >  1 file changed, 5 insertions(+), 2 deletions(-)
>> >

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register()
  2019-07-31 19:35 ` [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register() Stephen Boyd
@ 2019-08-08 16:14   ` Sylwester Nawrocki
  2019-08-14  0:25   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Sylwester Nawrocki @ 2019-08-08 16:14 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: linux-kernel, linux-clk, Bjorn Andersson, Doug Anderson

On 7/31/19 21:35, Stephen Boyd wrote:
> We don't want clk provider drivers to use the init structure after clk
> registration time, but we leave a dangling reference to it by means of
> clk_hw::init. Let's overwrite the member with NULL during clk_register()
> so that this can't be used anymore after registration time.
> 
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

>  drivers/clk/clk.c            | 24 ++++++++++++++++--------
>  include/linux/clk-provider.h |  3 ++-
>  2 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..efac620264a2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3484,9 +3484,9 @@ static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
>  	return 0;
>  }
>  
> -static int clk_core_populate_parent_map(struct clk_core *core)
> +static int clk_core_populate_parent_map(struct clk_core *core,
> +					const struct clk_init_data *init)
>  {
> -	const struct clk_init_data *init = core->hw->init;
>  	u8 num_parents = init->num_parents;
>  	const char * const *parent_names = init->parent_names;
>  	const struct clk_hw **parent_hws = init->parent_hws;
> @@ -3566,6 +3566,14 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>  {
>  	int ret;
>  	struct clk_core *core;
> +	const struct clk_init_data *init = hw->init;
> +
> +	/*
> +	 * The init data is not supposed to be used outside of registration path.
> +	 * Set it to NULL so that provider drivers can't use it either and so that
> +	 * we catch use of hw->init early on in the core.
> +	 */

nit: This comment could be re-edited to not exceed 80 columns limit.

> +	hw->init = NULL;

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
  2019-08-03 12:48   ` Manivannan Sadhasivam
@ 2019-08-14  0:24   ` Stephen Boyd
  2019-08-16 11:22   ` Manivannan Sadhasivam
  2 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:24 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Manivannan Sadhasivam

Quoting Stephen Boyd (2019-07-31 12:35:09)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/9] clk: lochnagar: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 2/9] clk: lochnagar: " Stephen Boyd
  2019-08-01  8:07   ` Charles Keepax
@ 2019-08-14  0:24   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:24 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Charles Keepax, Richard Fitzgerald

Quoting Stephen Boyd (2019-07-31 12:35:10)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
> Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/9] clk: meson: axg-audio: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 3/9] clk: meson: axg-audio: " Stephen Boyd
  2019-07-31 19:53   ` Neil Armstrong
  2019-08-06  8:49   ` Jerome Brunet
@ 2019-08-14  0:24   ` Stephen Boyd
  2 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:24 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Neil Armstrong, Jerome Brunet

Quoting Stephen Boyd (2019-07-31 12:35:11)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/9] clk: qcom: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 4/9] clk: qcom: " Stephen Boyd
  2019-08-04 17:49   ` Taniya Das
@ 2019-08-14  0:24   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:24 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Taniya Das, Andy Gross

Quoting Stephen Boyd (2019-07-31 12:35:12)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Taniya Das <tdas@codeaurora.org>
> Cc: Andy Gross <agross@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/9] clk: sirf: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 5/9] clk: sirf: " Stephen Boyd
@ 2019-08-14  0:25   ` Stephen Boyd
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Guo Zeng, Barry Song

Quoting Stephen Boyd (2019-07-31 12:35:13)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Guo Zeng <Guo.Zeng@csr.com>
> Cc: Barry Song <Baohua.Song@csr.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/9] clk: socfpga: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 6/9] clk: socfpga: " Stephen Boyd
  2019-08-01 15:12   ` Dinh Nguyen
@ 2019-08-14  0:25   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd; +Cc: linux-kernel, linux-clk, Dinh Nguyen

Quoting Stephen Boyd (2019-07-31 12:35:14)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 7/9] clk: sprd: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 7/9] clk: sprd: " Stephen Boyd
  2019-08-01  7:41   ` Baolin Wang
  2019-08-02  1:57   ` Chunyan Zhang
@ 2019-08-14  0:25   ` Stephen Boyd
  2 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Chunyan Zhang, Baolin Wang

Quoting Stephen Boyd (2019-07-31 12:35:15)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 8/9] phy: ti: am654-serdes: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 8/9] phy: ti: am654-serdes: " Stephen Boyd
  2019-08-02 12:45   ` Kishon Vijay Abraham I
@ 2019-08-14  0:25   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Roger Quadros, Kishon Vijay Abraham I

Quoting Stephen Boyd (2019-07-31 12:35:16)
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Roger Quadros <rogerq@ti.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register()
  2019-07-31 19:35 ` [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register() Stephen Boyd
  2019-08-08 16:14   ` Sylwester Nawrocki
@ 2019-08-14  0:25   ` Stephen Boyd
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-14  0:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Bjorn Andersson, Doug Anderson

Quoting Stephen Boyd (2019-07-31 12:35:17)
> We don't want clk provider drivers to use the init structure after clk
> registration time, but we leave a dangling reference to it by means of
> clk_hw::init. Let's overwrite the member with NULL during clk_register()
> so that this can't be used anymore after registration time.
> 
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration
  2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
  2019-08-03 12:48   ` Manivannan Sadhasivam
  2019-08-14  0:24   ` Stephen Boyd
@ 2019-08-16 11:22   ` Manivannan Sadhasivam
  2019-08-16 14:50     ` Stephen Boyd
  2 siblings, 1 reply; 34+ messages in thread
From: Manivannan Sadhasivam @ 2019-08-16 11:22 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Michael Turquette, linux-kernel, linux-clk

On Wed, Jul 31, 2019 at 12:35:09PM -0700, Stephen Boyd wrote:
> A future patch is going to change semantics of clk_register() so that
> clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> referencing this member here so that we don't run into NULL pointer
> exceptions.
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> Please ack so I can take this through clk tree
> 
>  drivers/clk/actions/owl-common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
> index 32dd29e0a37e..71b683c4e643 100644
> --- a/drivers/clk/actions/owl-common.c
> +++ b/drivers/clk/actions/owl-common.c
> @@ -68,6 +68,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
>  	struct clk_hw *hw;
>  
>  	for (i = 0; i < hw_clks->num; i++) {
> +		const char *name = hw->init->name;
>  

This should come after below statement and hence the warning is generated
in linux-next. Sorry for missing!

Thanks,
Mani

>  		hw = hw_clks->hws[i];
>  
> @@ -77,7 +78,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
>  		ret = devm_clk_hw_register(dev, hw);
>  		if (ret) {
>  			dev_err(dev, "Couldn't register clock %d - %s\n",
> -				i, hw->init->name);
> +				i, name);
>  			return ret;
>  		}
>  	}
> -- 
> Sent by a computer through tubes
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration
  2019-08-16 11:22   ` Manivannan Sadhasivam
@ 2019-08-16 14:50     ` Stephen Boyd
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-08-16 14:50 UTC (permalink / raw)
  To: Manivannan Sadhasivam; +Cc: Michael Turquette, linux-kernel, linux-clk

Quoting Manivannan Sadhasivam (2019-08-16 04:22:10)
> On Wed, Jul 31, 2019 at 12:35:09PM -0700, Stephen Boyd wrote:
> > A future patch is going to change semantics of clk_register() so that
> > clk_hw::init is guaranteed to be NULL after a clk is registered. Avoid
> > referencing this member here so that we don't run into NULL pointer
> > exceptions.
> > 
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > 
> > Please ack so I can take this through clk tree
> > 
> >  drivers/clk/actions/owl-common.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
> > index 32dd29e0a37e..71b683c4e643 100644
> > --- a/drivers/clk/actions/owl-common.c
> > +++ b/drivers/clk/actions/owl-common.c
> > @@ -68,6 +68,7 @@ int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
> >       struct clk_hw *hw;
> >  
> >       for (i = 0; i < hw_clks->num; i++) {
> > +             const char *name = hw->init->name;
> >  
> 
> This should come after below statement and hence the warning is generated
> in linux-next. Sorry for missing!
> 

Oh right. Will fix it.


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, back to index

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 19:35 [PATCH 0/9] Make clk_hw::init NULL after clk registration Stephen Boyd
2019-07-31 19:35 ` [PATCH 1/9] clk: actions: Don't reference clk_init_data after registration Stephen Boyd
2019-08-03 12:48   ` Manivannan Sadhasivam
2019-08-14  0:24   ` Stephen Boyd
2019-08-16 11:22   ` Manivannan Sadhasivam
2019-08-16 14:50     ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 2/9] clk: lochnagar: " Stephen Boyd
2019-08-01  8:07   ` Charles Keepax
2019-08-14  0:24   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 3/9] clk: meson: axg-audio: " Stephen Boyd
2019-07-31 19:53   ` Neil Armstrong
2019-08-06  8:49   ` Jerome Brunet
2019-08-06 21:48     ` Stephen Boyd
2019-08-07 13:57       ` Jerome Brunet
2019-08-14  0:24   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 4/9] clk: qcom: " Stephen Boyd
2019-08-04 17:49   ` Taniya Das
2019-08-14  0:24   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 5/9] clk: sirf: " Stephen Boyd
2019-08-14  0:25   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 6/9] clk: socfpga: " Stephen Boyd
2019-08-01 15:12   ` Dinh Nguyen
2019-08-01 16:13     ` Stephen Boyd
2019-08-14  0:25   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 7/9] clk: sprd: " Stephen Boyd
2019-08-01  7:41   ` Baolin Wang
2019-08-02  1:57   ` Chunyan Zhang
2019-08-14  0:25   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 8/9] phy: ti: am654-serdes: " Stephen Boyd
2019-08-02 12:45   ` Kishon Vijay Abraham I
2019-08-14  0:25   ` Stephen Boyd
2019-07-31 19:35 ` [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during clk_register() Stephen Boyd
2019-08-08 16:14   ` Sylwester Nawrocki
2019-08-14  0:25   ` Stephen Boyd

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org linux-clk@archiver.kernel.org
	public-inbox-index linux-clk


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/ public-inbox