[PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[Yassine Oudjana]

This series started as part of an earlier series adding support for the main
clock controllers on MediaTek MT6735[1]. It has since been split off and
expanded. It adds a new function to unregister a reset controller and expands
the mtk_clk_simple_probe/remove functions to support the main 5 types of clocks:
- PLLs		(new)
- Fixed clocks	(new)
- Fixed factors	(new)
- Muxes		(new)
- Gates		(supported previously)
This should allow it to be used in most clock drivers, resulting in reduced
code duplication. It will be used in MT6735 clock drivers in the upcoming v2
of the MT6735 main clock controller series.

Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

Yassine Oudjana (6):
  clk: mediatek: gate: Export mtk_clk_register_gates_with_dev
  clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe
  clk: mediatek: reset: Return reset data pointer on register
  clk: mediatek: reset: Implement mtk_unregister_reset_controller() API
  clk: mediatek: Unregister reset controller on simple remove
  clk: mediatek: Add support for other clock types in simple
    probe/remove

 drivers/clk/mediatek/clk-gate.c   |   1 +
 drivers/clk/mediatek/clk-mt8192.c |   7 +-
 drivers/clk/mediatek/clk-mtk.c    | 123 +++++++++++++++++++++++++-----
 drivers/clk/mediatek/clk-mtk.h    |  22 +++++-
 drivers/clk/mediatek/reset.c      |  41 ++++++----
 drivers/clk/mediatek/reset.h      |  20 +++--
 6 files changed, 167 insertions(+), 47 deletions(-)

-- 
2.36.1

[PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[Yassine Oudjana]

This series started as part of an earlier series adding support for the main
clock controllers on MediaTek MT6735[1]. It has since been split off and
expanded. It adds a new function to unregister a reset controller and expands
the mtk_clk_simple_probe/remove functions to support the main 5 types of clocks:
- PLLs		(new)
- Fixed clocks	(new)
- Fixed factors	(new)
- Muxes		(new)
- Gates		(supported previously)
This should allow it to be used in most clock drivers, resulting in reduced
code duplication. It will be used in MT6735 clock drivers in the upcoming v2
of the MT6735 main clock controller series.

Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

Yassine Oudjana (6):
  clk: mediatek: gate: Export mtk_clk_register_gates_with_dev
  clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe
  clk: mediatek: reset: Return reset data pointer on register
  clk: mediatek: reset: Implement mtk_unregister_reset_controller() API
  clk: mediatek: Unregister reset controller on simple remove
  clk: mediatek: Add support for other clock types in simple
    probe/remove

 drivers/clk/mediatek/clk-gate.c   |   1 +
 drivers/clk/mediatek/clk-mt8192.c |   7 +-
 drivers/clk/mediatek/clk-mtk.c    | 123 +++++++++++++++++++++++++-----
 drivers/clk/mediatek/clk-mtk.h    |  22 +++++-
 drivers/clk/mediatek/reset.c      |  41 ++++++----
 drivers/clk/mediatek/reset.h      |  20 +++--
 6 files changed, 167 insertions(+), 47 deletions(-)

-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[Yassine Oudjana]

This series started as part of an earlier series adding support for the main
clock controllers on MediaTek MT6735[1]. It has since been split off and
expanded. It adds a new function to unregister a reset controller and expands
the mtk_clk_simple_probe/remove functions to support the main 5 types of clocks:
- PLLs		(new)
- Fixed clocks	(new)
- Fixed factors	(new)
- Muxes		(new)
- Gates		(supported previously)
This should allow it to be used in most clock drivers, resulting in reduced
code duplication. It will be used in MT6735 clock drivers in the upcoming v2
of the MT6735 main clock controller series.

Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

Yassine Oudjana (6):
  clk: mediatek: gate: Export mtk_clk_register_gates_with_dev
  clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe
  clk: mediatek: reset: Return reset data pointer on register
  clk: mediatek: reset: Implement mtk_unregister_reset_controller() API
  clk: mediatek: Unregister reset controller on simple remove
  clk: mediatek: Add support for other clock types in simple
    probe/remove

 drivers/clk/mediatek/clk-gate.c   |   1 +
 drivers/clk/mediatek/clk-mt8192.c |   7 +-
 drivers/clk/mediatek/clk-mtk.c    | 123 +++++++++++++++++++++++++-----
 drivers/clk/mediatek/clk-mtk.h    |  22 +++++-
 drivers/clk/mediatek/reset.c      |  41 ++++++----
 drivers/clk/mediatek/reset.h      |  20 +++--
 6 files changed, 167 insertions(+), 47 deletions(-)

-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

This allows it to be used in drivers built as modules.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-gate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
index 421806236228..0c867136e49d 100644
--- a/drivers/clk/mediatek/clk-gate.c
+++ b/drivers/clk/mediatek/clk-gate.c
@@ -261,6 +261,7 @@ int mtk_clk_register_gates_with_dev(struct device_node *node,
 
 	return PTR_ERR(hw);
 }
+EXPORT_SYMBOL_GPL(mtk_clk_register_gates_with_dev);
 
 int mtk_clk_register_gates(struct device_node *node,
 			   const struct mtk_gate *clks, int num,
-- 
2.36.1

[PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

This allows it to be used in drivers built as modules.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-gate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
index 421806236228..0c867136e49d 100644
--- a/drivers/clk/mediatek/clk-gate.c
+++ b/drivers/clk/mediatek/clk-gate.c
@@ -261,6 +261,7 @@ int mtk_clk_register_gates_with_dev(struct device_node *node,
 
 	return PTR_ERR(hw);
 }
+EXPORT_SYMBOL_GPL(mtk_clk_register_gates_with_dev);
 
 int mtk_clk_register_gates(struct device_node *node,
 			   const struct mtk_gate *clks, int num,
-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

This allows it to be used in drivers built as modules.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-gate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
index 421806236228..0c867136e49d 100644
--- a/drivers/clk/mediatek/clk-gate.c
+++ b/drivers/clk/mediatek/clk-gate.c
@@ -261,6 +261,7 @@ int mtk_clk_register_gates_with_dev(struct device_node *node,
 
 	return PTR_ERR(hw);
 }
+EXPORT_SYMBOL_GPL(mtk_clk_register_gates_with_dev);
 
 int mtk_clk_register_gates(struct device_node *node,
 			   const struct mtk_gate *clks, int num,
-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Register gates with dev in mtk_clk_simple_probe.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 41e60a7e8ff9..3a8875b6c37f 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -434,7 +434,8 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	if (!clk_data)
 		return -ENOMEM;
 
-	r = mtk_clk_register_gates(node, mcd->clks, mcd->num_clks, clk_data);
+	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
+					    clk_data, &pdev->dev);
 	if (r)
 		goto free_data;
 
-- 
2.36.1

[PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Register gates with dev in mtk_clk_simple_probe.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 41e60a7e8ff9..3a8875b6c37f 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -434,7 +434,8 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	if (!clk_data)
 		return -ENOMEM;
 
-	r = mtk_clk_register_gates(node, mcd->clks, mcd->num_clks, clk_data);
+	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
+					    clk_data, &pdev->dev);
 	if (r)
 		goto free_data;
 
-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Register gates with dev in mtk_clk_simple_probe.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 41e60a7e8ff9..3a8875b6c37f 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -434,7 +434,8 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	if (!clk_data)
 		return -ENOMEM;
 
-	r = mtk_clk_register_gates(node, mcd->clks, mcd->num_clks, clk_data);
+	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
+					    clk_data, &pdev->dev);
 	if (r)
 		goto free_data;
 
-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Return a struct mtk_clk_rst_data * when registering a reset
controller in preparation for adding an unregister helper
that will take it as an argument. Make the necessary changes
in drivers that do not currently discard the return value
of register functions.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mt8192.c |  7 +++++--
 drivers/clk/mediatek/clk-mtk.c    |  9 +++++---
 drivers/clk/mediatek/reset.c      | 34 ++++++++++++++++---------------
 drivers/clk/mediatek/reset.h      | 14 +++++++------
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index ebbd2798d9a3..a658a74644de 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1255,6 +1255,7 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 {
 	struct clk_hw_onecell_data *clk_data;
 	struct device_node *node = pdev->dev.of_node;
+	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	clk_data = mtk_alloc_clk_data(CLK_INFRA_NR_CLK);
@@ -1265,9 +1266,11 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 	if (r)
 		goto free_clk_data;
 
-	r = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
-	if (r)
+	rst_data = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
+	if (IS_ERR(rst_data)) {
+		r = PTR_ERR(rst_data);
 		goto free_clk_data;
+	}
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3a8875b6c37f..1b5591733e2b 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -424,6 +424,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	const struct mtk_clk_desc *mcd;
 	struct clk_hw_onecell_data *clk_data;
 	struct device_node *node = pdev->dev.of_node;
+	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	mcd = of_device_get_match_data(&pdev->dev);
@@ -446,10 +447,12 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, clk_data);
 
 	if (mcd->rst_desc) {
-		r = mtk_register_reset_controller_with_dev(&pdev->dev,
-							   mcd->rst_desc);
-		if (r)
+		rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
+							   	  mcd->rst_desc);
+		if (IS_ERR(rst_data)) {
+			r = PTR_ERR(rst_data);
 			goto unregister_clks;
+		}
 	}
 
 	return r;
diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 290ceda84ce4..09862baf1d57 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -110,8 +110,9 @@ static int reset_xlate(struct reset_controller_dev *rcdev,
 	return data->desc->rst_idx_map[reset_spec->args[0]];
 }
 
-int mtk_register_reset_controller(struct device_node *np,
-				  const struct mtk_clk_rst_desc *desc)
+struct mtk_clk_rst_data
+*mtk_register_reset_controller(struct device_node *np,
+			       const struct mtk_clk_rst_desc *desc)
 {
 	struct regmap *regmap;
 	const struct reset_control_ops *rcops = NULL;
@@ -120,7 +121,7 @@ int mtk_register_reset_controller(struct device_node *np,
 
 	if (!desc) {
 		pr_err("mtk clock reset desc is NULL\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	switch (desc->version) {
@@ -132,18 +133,18 @@ int mtk_register_reset_controller(struct device_node *np,
 		break;
 	default:
 		pr_err("Unknown reset version %d\n", desc->version);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	regmap = device_node_to_regmap(np);
 	if (IS_ERR(regmap)) {
 		pr_err("Cannot find regmap for %pOF: %pe\n", np, regmap);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	data->desc = desc;
 	data->regmap = regmap;
@@ -163,14 +164,15 @@ int mtk_register_reset_controller(struct device_node *np,
 	if (ret) {
 		pr_err("could not register reset controller: %d\n", ret);
 		kfree(data);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return data;
 }
 
-int mtk_register_reset_controller_with_dev(struct device *dev,
-					   const struct mtk_clk_rst_desc *desc)
+struct mtk_clk_rst_data
+*mtk_register_reset_controller_with_dev(struct device *dev,
+					const struct mtk_clk_rst_desc *desc)
 {
 	struct device_node *np = dev->of_node;
 	struct regmap *regmap;
@@ -180,7 +182,7 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 
 	if (!desc) {
 		dev_err(dev, "mtk clock reset desc is NULL\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	switch (desc->version) {
@@ -192,18 +194,18 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 		break;
 	default:
 		dev_err(dev, "Unknown reset version %d\n", desc->version);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	regmap = device_node_to_regmap(np);
 	if (IS_ERR(regmap)) {
 		dev_err(dev, "Cannot find regmap %pe\n", regmap);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	data->desc = desc;
 	data->regmap = regmap;
@@ -223,10 +225,10 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 	ret = devm_reset_controller_register(dev, &data->rcdev);
 	if (ret) {
 		dev_err(dev, "could not register reset controller: %d\n", ret);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return data;
 }
 EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 913fe676cba7..7418dd0d046f 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -64,19 +64,21 @@ struct mtk_clk_rst_data {
  * @np: Pointer to device node.
  * @desc: Constant pointer to description of clock reset.
  *
- * Return: 0 on success and errorno otherwise.
+ * Return: Pointer to struct mtk_clk_rst_data on success and error pointer otherwise.
  */
-int mtk_register_reset_controller(struct device_node *np,
-				  const struct mtk_clk_rst_desc *desc);
+struct mtk_clk_rst_data
+*mtk_register_reset_controller(struct device_node *np,
+			       const struct mtk_clk_rst_desc *desc);
 
 /**
  * mtk_register_reset_controller - Register mediatek clock reset controller with device
  * @np: Pointer to device.
  * @desc: Constant pointer to description of clock reset.
  *
- * Return: 0 on success and errorno otherwise.
+ * Return: Pointer to struct mtk_clk_rst_data on success and error pointer otherwise.
  */
-int mtk_register_reset_controller_with_dev(struct device *dev,
-					   const struct mtk_clk_rst_desc *desc);
+struct mtk_clk_rst_data
+*mtk_register_reset_controller_with_dev(struct device *dev,
+					const struct mtk_clk_rst_desc *desc);
 
 #endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.36.1

[PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Return a struct mtk_clk_rst_data * when registering a reset
controller in preparation for adding an unregister helper
that will take it as an argument. Make the necessary changes
in drivers that do not currently discard the return value
of register functions.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mt8192.c |  7 +++++--
 drivers/clk/mediatek/clk-mtk.c    |  9 +++++---
 drivers/clk/mediatek/reset.c      | 34 ++++++++++++++++---------------
 drivers/clk/mediatek/reset.h      | 14 +++++++------
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index ebbd2798d9a3..a658a74644de 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1255,6 +1255,7 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 {
 	struct clk_hw_onecell_data *clk_data;
 	struct device_node *node = pdev->dev.of_node;
+	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	clk_data = mtk_alloc_clk_data(CLK_INFRA_NR_CLK);
@@ -1265,9 +1266,11 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 	if (r)
 		goto free_clk_data;
 
-	r = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
-	if (r)
+	rst_data = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
+	if (IS_ERR(rst_data)) {
+		r = PTR_ERR(rst_data);
 		goto free_clk_data;
+	}
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3a8875b6c37f..1b5591733e2b 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -424,6 +424,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	const struct mtk_clk_desc *mcd;
 	struct clk_hw_onecell_data *clk_data;
 	struct device_node *node = pdev->dev.of_node;
+	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	mcd = of_device_get_match_data(&pdev->dev);
@@ -446,10 +447,12 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, clk_data);
 
 	if (mcd->rst_desc) {
-		r = mtk_register_reset_controller_with_dev(&pdev->dev,
-							   mcd->rst_desc);
-		if (r)
+		rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
+							   	  mcd->rst_desc);
+		if (IS_ERR(rst_data)) {
+			r = PTR_ERR(rst_data);
 			goto unregister_clks;
+		}
 	}
 
 	return r;
diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 290ceda84ce4..09862baf1d57 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -110,8 +110,9 @@ static int reset_xlate(struct reset_controller_dev *rcdev,
 	return data->desc->rst_idx_map[reset_spec->args[0]];
 }
 
-int mtk_register_reset_controller(struct device_node *np,
-				  const struct mtk_clk_rst_desc *desc)
+struct mtk_clk_rst_data
+*mtk_register_reset_controller(struct device_node *np,
+			       const struct mtk_clk_rst_desc *desc)
 {
 	struct regmap *regmap;
 	const struct reset_control_ops *rcops = NULL;
@@ -120,7 +121,7 @@ int mtk_register_reset_controller(struct device_node *np,
 
 	if (!desc) {
 		pr_err("mtk clock reset desc is NULL\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	switch (desc->version) {
@@ -132,18 +133,18 @@ int mtk_register_reset_controller(struct device_node *np,
 		break;
 	default:
 		pr_err("Unknown reset version %d\n", desc->version);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	regmap = device_node_to_regmap(np);
 	if (IS_ERR(regmap)) {
 		pr_err("Cannot find regmap for %pOF: %pe\n", np, regmap);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	data->desc = desc;
 	data->regmap = regmap;
@@ -163,14 +164,15 @@ int mtk_register_reset_controller(struct device_node *np,
 	if (ret) {
 		pr_err("could not register reset controller: %d\n", ret);
 		kfree(data);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return data;
 }
 
-int mtk_register_reset_controller_with_dev(struct device *dev,
-					   const struct mtk_clk_rst_desc *desc)
+struct mtk_clk_rst_data
+*mtk_register_reset_controller_with_dev(struct device *dev,
+					const struct mtk_clk_rst_desc *desc)
 {
 	struct device_node *np = dev->of_node;
 	struct regmap *regmap;
@@ -180,7 +182,7 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 
 	if (!desc) {
 		dev_err(dev, "mtk clock reset desc is NULL\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	switch (desc->version) {
@@ -192,18 +194,18 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 		break;
 	default:
 		dev_err(dev, "Unknown reset version %d\n", desc->version);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	regmap = device_node_to_regmap(np);
 	if (IS_ERR(regmap)) {
 		dev_err(dev, "Cannot find regmap %pe\n", regmap);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	data->desc = desc;
 	data->regmap = regmap;
@@ -223,10 +225,10 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 	ret = devm_reset_controller_register(dev, &data->rcdev);
 	if (ret) {
 		dev_err(dev, "could not register reset controller: %d\n", ret);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return data;
 }
 EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 913fe676cba7..7418dd0d046f 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -64,19 +64,21 @@ struct mtk_clk_rst_data {
  * @np: Pointer to device node.
  * @desc: Constant pointer to description of clock reset.
  *
- * Return: 0 on success and errorno otherwise.
+ * Return: Pointer to struct mtk_clk_rst_data on success and error pointer otherwise.
  */
-int mtk_register_reset_controller(struct device_node *np,
-				  const struct mtk_clk_rst_desc *desc);
+struct mtk_clk_rst_data
+*mtk_register_reset_controller(struct device_node *np,
+			       const struct mtk_clk_rst_desc *desc);
 
 /**
  * mtk_register_reset_controller - Register mediatek clock reset controller with device
  * @np: Pointer to device.
  * @desc: Constant pointer to description of clock reset.
  *
- * Return: 0 on success and errorno otherwise.
+ * Return: Pointer to struct mtk_clk_rst_data on success and error pointer otherwise.
  */
-int mtk_register_reset_controller_with_dev(struct device *dev,
-					   const struct mtk_clk_rst_desc *desc);
+struct mtk_clk_rst_data
+*mtk_register_reset_controller_with_dev(struct device *dev,
+					const struct mtk_clk_rst_desc *desc);
 
 #endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Return a struct mtk_clk_rst_data * when registering a reset
controller in preparation for adding an unregister helper
that will take it as an argument. Make the necessary changes
in drivers that do not currently discard the return value
of register functions.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mt8192.c |  7 +++++--
 drivers/clk/mediatek/clk-mtk.c    |  9 +++++---
 drivers/clk/mediatek/reset.c      | 34 ++++++++++++++++---------------
 drivers/clk/mediatek/reset.h      | 14 +++++++------
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index ebbd2798d9a3..a658a74644de 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1255,6 +1255,7 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 {
 	struct clk_hw_onecell_data *clk_data;
 	struct device_node *node = pdev->dev.of_node;
+	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	clk_data = mtk_alloc_clk_data(CLK_INFRA_NR_CLK);
@@ -1265,9 +1266,11 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 	if (r)
 		goto free_clk_data;
 
-	r = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
-	if (r)
+	rst_data = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
+	if (IS_ERR(rst_data)) {
+		r = PTR_ERR(rst_data);
 		goto free_clk_data;
+	}
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3a8875b6c37f..1b5591733e2b 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -424,6 +424,7 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	const struct mtk_clk_desc *mcd;
 	struct clk_hw_onecell_data *clk_data;
 	struct device_node *node = pdev->dev.of_node;
+	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	mcd = of_device_get_match_data(&pdev->dev);
@@ -446,10 +447,12 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, clk_data);
 
 	if (mcd->rst_desc) {
-		r = mtk_register_reset_controller_with_dev(&pdev->dev,
-							   mcd->rst_desc);
-		if (r)
+		rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
+							   	  mcd->rst_desc);
+		if (IS_ERR(rst_data)) {
+			r = PTR_ERR(rst_data);
 			goto unregister_clks;
+		}
 	}
 
 	return r;
diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 290ceda84ce4..09862baf1d57 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -110,8 +110,9 @@ static int reset_xlate(struct reset_controller_dev *rcdev,
 	return data->desc->rst_idx_map[reset_spec->args[0]];
 }
 
-int mtk_register_reset_controller(struct device_node *np,
-				  const struct mtk_clk_rst_desc *desc)
+struct mtk_clk_rst_data
+*mtk_register_reset_controller(struct device_node *np,
+			       const struct mtk_clk_rst_desc *desc)
 {
 	struct regmap *regmap;
 	const struct reset_control_ops *rcops = NULL;
@@ -120,7 +121,7 @@ int mtk_register_reset_controller(struct device_node *np,
 
 	if (!desc) {
 		pr_err("mtk clock reset desc is NULL\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	switch (desc->version) {
@@ -132,18 +133,18 @@ int mtk_register_reset_controller(struct device_node *np,
 		break;
 	default:
 		pr_err("Unknown reset version %d\n", desc->version);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	regmap = device_node_to_regmap(np);
 	if (IS_ERR(regmap)) {
 		pr_err("Cannot find regmap for %pOF: %pe\n", np, regmap);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	data->desc = desc;
 	data->regmap = regmap;
@@ -163,14 +164,15 @@ int mtk_register_reset_controller(struct device_node *np,
 	if (ret) {
 		pr_err("could not register reset controller: %d\n", ret);
 		kfree(data);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return data;
 }
 
-int mtk_register_reset_controller_with_dev(struct device *dev,
-					   const struct mtk_clk_rst_desc *desc)
+struct mtk_clk_rst_data
+*mtk_register_reset_controller_with_dev(struct device *dev,
+					const struct mtk_clk_rst_desc *desc)
 {
 	struct device_node *np = dev->of_node;
 	struct regmap *regmap;
@@ -180,7 +182,7 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 
 	if (!desc) {
 		dev_err(dev, "mtk clock reset desc is NULL\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	switch (desc->version) {
@@ -192,18 +194,18 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 		break;
 	default:
 		dev_err(dev, "Unknown reset version %d\n", desc->version);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	regmap = device_node_to_regmap(np);
 	if (IS_ERR(regmap)) {
 		dev_err(dev, "Cannot find regmap %pe\n", regmap);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	data->desc = desc;
 	data->regmap = regmap;
@@ -223,10 +225,10 @@ int mtk_register_reset_controller_with_dev(struct device *dev,
 	ret = devm_reset_controller_register(dev, &data->rcdev);
 	if (ret) {
 		dev_err(dev, "could not register reset controller: %d\n", ret);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return data;
 }
 EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 913fe676cba7..7418dd0d046f 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -64,19 +64,21 @@ struct mtk_clk_rst_data {
  * @np: Pointer to device node.
  * @desc: Constant pointer to description of clock reset.
  *
- * Return: 0 on success and errorno otherwise.
+ * Return: Pointer to struct mtk_clk_rst_data on success and error pointer otherwise.
  */
-int mtk_register_reset_controller(struct device_node *np,
-				  const struct mtk_clk_rst_desc *desc);
+struct mtk_clk_rst_data
+*mtk_register_reset_controller(struct device_node *np,
+			       const struct mtk_clk_rst_desc *desc);
 
 /**
  * mtk_register_reset_controller - Register mediatek clock reset controller with device
  * @np: Pointer to device.
  * @desc: Constant pointer to description of clock reset.
  *
- * Return: 0 on success and errorno otherwise.
+ * Return: Pointer to struct mtk_clk_rst_data on success and error pointer otherwise.
  */
-int mtk_register_reset_controller_with_dev(struct device *dev,
-					   const struct mtk_clk_rst_desc *desc);
+struct mtk_clk_rst_data
+*mtk_register_reset_controller_with_dev(struct device *dev,
+					const struct mtk_clk_rst_desc *desc);
 
 #endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] clk: mediatek: reset: Implement mtk_unregister_reset_controller() API

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add a function to unregister a reset controller previously
registered with mtk_register_reset_controller() or
mtk_register_reset_controller_with_dev(), and do the
necessary cleanup.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/reset.c | 7 +++++++
 drivers/clk/mediatek/reset.h | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 09862baf1d57..c1ab8c87ec27 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -232,4 +232,11 @@ struct mtk_clk_rst_data
 }
 EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
+void mtk_unregister_reset_controller(struct mtk_clk_rst_data *data)
+{
+	reset_controller_unregister(&data->rcdev);
+	kfree(data);
+}
+EXPORT_SYMBOL_GPL(mtk_unregister_reset_controller);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 7418dd0d046f..0feaea4115a8 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -81,4 +81,10 @@ struct mtk_clk_rst_data
 *mtk_register_reset_controller_with_dev(struct device *dev,
 					const struct mtk_clk_rst_desc *desc);
 
+/**
+ * mtk_unregister_reset_controller - Unregister mediatek clock reset controller
+ * @data: Pointer to previously registered struct mtk_clk_rst_data.
+ */
+void mtk_unregister_reset_controller(struct mtk_clk_rst_data *data);
+
 #endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.36.1

[PATCH 4/6] clk: mediatek: reset: Implement mtk_unregister_reset_controller() API

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add a function to unregister a reset controller previously
registered with mtk_register_reset_controller() or
mtk_register_reset_controller_with_dev(), and do the
necessary cleanup.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/reset.c | 7 +++++++
 drivers/clk/mediatek/reset.h | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 09862baf1d57..c1ab8c87ec27 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -232,4 +232,11 @@ struct mtk_clk_rst_data
 }
 EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
+void mtk_unregister_reset_controller(struct mtk_clk_rst_data *data)
+{
+	reset_controller_unregister(&data->rcdev);
+	kfree(data);
+}
+EXPORT_SYMBOL_GPL(mtk_unregister_reset_controller);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 7418dd0d046f..0feaea4115a8 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -81,4 +81,10 @@ struct mtk_clk_rst_data
 *mtk_register_reset_controller_with_dev(struct device *dev,
 					const struct mtk_clk_rst_desc *desc);
 
+/**
+ * mtk_unregister_reset_controller - Unregister mediatek clock reset controller
+ * @data: Pointer to previously registered struct mtk_clk_rst_data.
+ */
+void mtk_unregister_reset_controller(struct mtk_clk_rst_data *data);
+
 #endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 4/6] clk: mediatek: reset: Implement mtk_unregister_reset_controller() API

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add a function to unregister a reset controller previously
registered with mtk_register_reset_controller() or
mtk_register_reset_controller_with_dev(), and do the
necessary cleanup.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/reset.c | 7 +++++++
 drivers/clk/mediatek/reset.h | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 09862baf1d57..c1ab8c87ec27 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -232,4 +232,11 @@ struct mtk_clk_rst_data
 }
 EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
 
+void mtk_unregister_reset_controller(struct mtk_clk_rst_data *data)
+{
+	reset_controller_unregister(&data->rcdev);
+	kfree(data);
+}
+EXPORT_SYMBOL_GPL(mtk_unregister_reset_controller);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 7418dd0d046f..0feaea4115a8 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -81,4 +81,10 @@ struct mtk_clk_rst_data
 *mtk_register_reset_controller_with_dev(struct device *dev,
 					const struct mtk_clk_rst_desc *desc);
 
+/**
+ * mtk_unregister_reset_controller - Unregister mediatek clock reset controller
+ * @data: Pointer to previously registered struct mtk_clk_rst_data.
+ */
+void mtk_unregister_reset_controller(struct mtk_clk_rst_data *data);
+
 #endif /* __DRV_CLK_MTK_RESET_H */
-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] clk: mediatek: Unregister reset controller on simple remove

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Store clk_data and rst_data pointers in a new wrapper struct,
set it as platform driver data, then use it in mtk_clk_simple_remove
to unregister the reset controller.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 47 +++++++++++++++++++++-------------
 drivers/clk/mediatek/clk-mtk.h |  5 ++++
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 1b5591733e2b..3382802663f4 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -422,35 +422,41 @@ void mtk_clk_unregister_dividers(const struct mtk_clk_divider *mcds, int num,
 int mtk_clk_simple_probe(struct platform_device *pdev)
 {
 	const struct mtk_clk_desc *mcd;
-	struct clk_hw_onecell_data *clk_data;
+	struct mtk_simple_clk_controller *clk_ctrl;
 	struct device_node *node = pdev->dev.of_node;
-	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	mcd = of_device_get_match_data(&pdev->dev);
 	if (!mcd)
 		return -EINVAL;
 
-	clk_data = mtk_alloc_clk_data(mcd->num_clks);
-	if (!clk_data)
+	clk_ctrl = kzalloc(sizeof(*clk_ctrl), GFP_KERNEL);
+	if (!clk_ctrl)
 		return -ENOMEM;
 
+	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
+	if (!clk_ctrl->clk_data) {
+		r = -ENOMEM;
+		goto free_clk_ctrl;
+	}
+
 	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
-					    clk_data, &pdev->dev);
+					    clk_ctrl->clk_data, &pdev->dev);
 	if (r)
-		goto free_data;
+		goto free_clk_data;
 
-	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
 	if (r)
 		goto unregister_clks;
 
-	platform_set_drvdata(pdev, clk_data);
+	platform_set_drvdata(pdev, clk_ctrl);
 
 	if (mcd->rst_desc) {
-		rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
-							   	  mcd->rst_desc);
-		if (IS_ERR(rst_data)) {
-			r = PTR_ERR(rst_data);
+		clk_ctrl->rst_data =
+			mtk_register_reset_controller_with_dev(&pdev->dev,
+							       mcd->rst_desc);
+		if (IS_ERR(clk_ctrl->rst_data)) {
+			r = PTR_ERR(clk_ctrl->rst_data);
 			goto unregister_clks;
 		}
 	}
@@ -458,9 +464,11 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	return r;
 
 unregister_clks:
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
-free_data:
-	mtk_free_clk_data(clk_data);
+	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+free_clk_data:
+	mtk_free_clk_data(clk_ctrl->clk_data);
+free_clk_ctrl:
+	kfree(clk_ctrl);
 	return r;
 }
 EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
@@ -468,12 +476,15 @@ EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
 int mtk_clk_simple_remove(struct platform_device *pdev)
 {
 	const struct mtk_clk_desc *mcd = of_device_get_match_data(&pdev->dev);
-	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
+	struct mtk_simple_clk_controller *clk_ctrl = platform_get_drvdata(pdev);
 	struct device_node *node = pdev->dev.of_node;
 
 	of_clk_del_provider(node);
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
-	mtk_free_clk_data(clk_data);
+	if (clk_ctrl->rst_data)
+		mtk_unregister_reset_controller(clk_ctrl->rst_data);
+	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+	mtk_free_clk_data(clk_ctrl->clk_data);
+	kfree(clk_ctrl);
 
 	return 0;
 }
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 1b95c484d5aa..fa092bca97c8 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -189,6 +189,11 @@ void mtk_free_clk_data(struct clk_hw_onecell_data *clk_data);
 struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 			const char *parent_name, void __iomem *reg);
 
+struct mtk_simple_clk_controller {
+	struct clk_hw_onecell_data *clk_data;
+	struct mtk_clk_rst_data *rst_data;
+};
+
 struct mtk_clk_desc {
 	const struct mtk_gate *clks;
 	size_t num_clks;
-- 
2.36.1

[PATCH 5/6] clk: mediatek: Unregister reset controller on simple remove

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Store clk_data and rst_data pointers in a new wrapper struct,
set it as platform driver data, then use it in mtk_clk_simple_remove
to unregister the reset controller.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 47 +++++++++++++++++++++-------------
 drivers/clk/mediatek/clk-mtk.h |  5 ++++
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 1b5591733e2b..3382802663f4 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -422,35 +422,41 @@ void mtk_clk_unregister_dividers(const struct mtk_clk_divider *mcds, int num,
 int mtk_clk_simple_probe(struct platform_device *pdev)
 {
 	const struct mtk_clk_desc *mcd;
-	struct clk_hw_onecell_data *clk_data;
+	struct mtk_simple_clk_controller *clk_ctrl;
 	struct device_node *node = pdev->dev.of_node;
-	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	mcd = of_device_get_match_data(&pdev->dev);
 	if (!mcd)
 		return -EINVAL;
 
-	clk_data = mtk_alloc_clk_data(mcd->num_clks);
-	if (!clk_data)
+	clk_ctrl = kzalloc(sizeof(*clk_ctrl), GFP_KERNEL);
+	if (!clk_ctrl)
 		return -ENOMEM;
 
+	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
+	if (!clk_ctrl->clk_data) {
+		r = -ENOMEM;
+		goto free_clk_ctrl;
+	}
+
 	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
-					    clk_data, &pdev->dev);
+					    clk_ctrl->clk_data, &pdev->dev);
 	if (r)
-		goto free_data;
+		goto free_clk_data;
 
-	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
 	if (r)
 		goto unregister_clks;
 
-	platform_set_drvdata(pdev, clk_data);
+	platform_set_drvdata(pdev, clk_ctrl);
 
 	if (mcd->rst_desc) {
-		rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
-							   	  mcd->rst_desc);
-		if (IS_ERR(rst_data)) {
-			r = PTR_ERR(rst_data);
+		clk_ctrl->rst_data =
+			mtk_register_reset_controller_with_dev(&pdev->dev,
+							       mcd->rst_desc);
+		if (IS_ERR(clk_ctrl->rst_data)) {
+			r = PTR_ERR(clk_ctrl->rst_data);
 			goto unregister_clks;
 		}
 	}
@@ -458,9 +464,11 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	return r;
 
 unregister_clks:
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
-free_data:
-	mtk_free_clk_data(clk_data);
+	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+free_clk_data:
+	mtk_free_clk_data(clk_ctrl->clk_data);
+free_clk_ctrl:
+	kfree(clk_ctrl);
 	return r;
 }
 EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
@@ -468,12 +476,15 @@ EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
 int mtk_clk_simple_remove(struct platform_device *pdev)
 {
 	const struct mtk_clk_desc *mcd = of_device_get_match_data(&pdev->dev);
-	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
+	struct mtk_simple_clk_controller *clk_ctrl = platform_get_drvdata(pdev);
 	struct device_node *node = pdev->dev.of_node;
 
 	of_clk_del_provider(node);
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
-	mtk_free_clk_data(clk_data);
+	if (clk_ctrl->rst_data)
+		mtk_unregister_reset_controller(clk_ctrl->rst_data);
+	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+	mtk_free_clk_data(clk_ctrl->clk_data);
+	kfree(clk_ctrl);
 
 	return 0;
 }
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 1b95c484d5aa..fa092bca97c8 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -189,6 +189,11 @@ void mtk_free_clk_data(struct clk_hw_onecell_data *clk_data);
 struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 			const char *parent_name, void __iomem *reg);
 
+struct mtk_simple_clk_controller {
+	struct clk_hw_onecell_data *clk_data;
+	struct mtk_clk_rst_data *rst_data;
+};
+
 struct mtk_clk_desc {
 	const struct mtk_gate *clks;
 	size_t num_clks;
-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 5/6] clk: mediatek: Unregister reset controller on simple remove

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Store clk_data and rst_data pointers in a new wrapper struct,
set it as platform driver data, then use it in mtk_clk_simple_remove
to unregister the reset controller.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 47 +++++++++++++++++++++-------------
 drivers/clk/mediatek/clk-mtk.h |  5 ++++
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 1b5591733e2b..3382802663f4 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -422,35 +422,41 @@ void mtk_clk_unregister_dividers(const struct mtk_clk_divider *mcds, int num,
 int mtk_clk_simple_probe(struct platform_device *pdev)
 {
 	const struct mtk_clk_desc *mcd;
-	struct clk_hw_onecell_data *clk_data;
+	struct mtk_simple_clk_controller *clk_ctrl;
 	struct device_node *node = pdev->dev.of_node;
-	struct mtk_clk_rst_data *rst_data;
 	int r;
 
 	mcd = of_device_get_match_data(&pdev->dev);
 	if (!mcd)
 		return -EINVAL;
 
-	clk_data = mtk_alloc_clk_data(mcd->num_clks);
-	if (!clk_data)
+	clk_ctrl = kzalloc(sizeof(*clk_ctrl), GFP_KERNEL);
+	if (!clk_ctrl)
 		return -ENOMEM;
 
+	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
+	if (!clk_ctrl->clk_data) {
+		r = -ENOMEM;
+		goto free_clk_ctrl;
+	}
+
 	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
-					    clk_data, &pdev->dev);
+					    clk_ctrl->clk_data, &pdev->dev);
 	if (r)
-		goto free_data;
+		goto free_clk_data;
 
-	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
 	if (r)
 		goto unregister_clks;
 
-	platform_set_drvdata(pdev, clk_data);
+	platform_set_drvdata(pdev, clk_ctrl);
 
 	if (mcd->rst_desc) {
-		rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
-							   	  mcd->rst_desc);
-		if (IS_ERR(rst_data)) {
-			r = PTR_ERR(rst_data);
+		clk_ctrl->rst_data =
+			mtk_register_reset_controller_with_dev(&pdev->dev,
+							       mcd->rst_desc);
+		if (IS_ERR(clk_ctrl->rst_data)) {
+			r = PTR_ERR(clk_ctrl->rst_data);
 			goto unregister_clks;
 		}
 	}
@@ -458,9 +464,11 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	return r;
 
 unregister_clks:
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
-free_data:
-	mtk_free_clk_data(clk_data);
+	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+free_clk_data:
+	mtk_free_clk_data(clk_ctrl->clk_data);
+free_clk_ctrl:
+	kfree(clk_ctrl);
 	return r;
 }
 EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
@@ -468,12 +476,15 @@ EXPORT_SYMBOL_GPL(mtk_clk_simple_probe);
 int mtk_clk_simple_remove(struct platform_device *pdev)
 {
 	const struct mtk_clk_desc *mcd = of_device_get_match_data(&pdev->dev);
-	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
+	struct mtk_simple_clk_controller *clk_ctrl = platform_get_drvdata(pdev);
 	struct device_node *node = pdev->dev.of_node;
 
 	of_clk_del_provider(node);
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_data);
-	mtk_free_clk_data(clk_data);
+	if (clk_ctrl->rst_data)
+		mtk_unregister_reset_controller(clk_ctrl->rst_data);
+	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+	mtk_free_clk_data(clk_ctrl->clk_data);
+	kfree(clk_ctrl);
 
 	return 0;
 }
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 1b95c484d5aa..fa092bca97c8 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -189,6 +189,11 @@ void mtk_free_clk_data(struct clk_hw_onecell_data *clk_data);
 struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 			const char *parent_name, void __iomem *reg);
 
+struct mtk_simple_clk_controller {
+	struct clk_hw_onecell_data *clk_data;
+	struct mtk_clk_rst_data *rst_data;
+};
+
 struct mtk_clk_desc {
 	const struct mtk_gate *clks;
 	size_t num_clks;
-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] clk: mediatek: Add support for other clock types in simple probe/remove

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Simple probe/remove functions currently support gates only. Add
PLLs, fixed clocks, fixed factors and muxes to support most
clock controllers. struct mtk_clk_desc now takes descriptions
for all of these clocks, and only the ones set will be registered.
Most clock controllers will only use a subset of the types
supported.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 88 +++++++++++++++++++++++++++++-----
 drivers/clk/mediatek/clk-mtk.h | 17 ++++++-
 2 files changed, 92 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3382802663f4..df1209d5b6fb 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -15,8 +15,10 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-#include "clk-mtk.h"
 #include "clk-gate.h"
+#include "clk-mtk.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
 
 struct clk_hw_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
 {
@@ -434,20 +436,55 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	if (!clk_ctrl)
 		return -ENOMEM;
 
-	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
+	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_plls
+					      + mcd->num_fixed_clks
+					      + mcd->num_factors
+					      + mcd->num_muxes
+					      + mcd->num_gates);
 	if (!clk_ctrl->clk_data) {
 		r = -ENOMEM;
 		goto free_clk_ctrl;
 	}
 
-	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
-					    clk_ctrl->clk_data, &pdev->dev);
-	if (r)
-		goto free_clk_data;
+	if (mcd->plls) {
+		r = mtk_clk_register_plls(node, mcd->plls, mcd->num_plls,
+					  clk_ctrl->clk_data);
+		if (r)
+			goto free_clk_data;
+	}
+
+	if (mcd->fixed_clks) {
+		r = mtk_clk_register_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+						clk_ctrl->clk_data);
+		if (r)
+			goto unregister_plls;
+	}
+
+	if (mcd->factors) {
+		r = mtk_clk_register_factors(mcd->factors, mcd->num_factors,
+					     clk_ctrl->clk_data);
+		if (r)
+			goto unregister_fixed_clks;
+	}
+
+	if (mcd->muxes) {
+		spin_lock_init(&clk_ctrl->mux_lock);
+		r = mtk_clk_register_muxes(mcd->muxes, mcd->num_muxes, node,
+					   &clk_ctrl->mux_lock, clk_ctrl->clk_data);
+		if (r)
+			goto unregister_factors;
+	}
+
+	if (mcd->gates) {
+		r = mtk_clk_register_gates_with_dev(node, mcd->gates, mcd->num_gates,
+						    clk_ctrl->clk_data, &pdev->dev);
+		if (r)
+			goto unregister_muxes;
+	}
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
 	if (r)
-		goto unregister_clks;
+		goto unregister_gates;
 
 	platform_set_drvdata(pdev, clk_ctrl);
 
@@ -457,14 +494,30 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 							       mcd->rst_desc);
 		if (IS_ERR(clk_ctrl->rst_data)) {
 			r = PTR_ERR(clk_ctrl->rst_data);
-			goto unregister_clks;
+			goto unregister_clk_provider;
 		}
 	}
 
 	return r;
 
-unregister_clks:
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+unregister_clk_provider:
+	of_clk_del_provider(node);
+unregister_gates:
+	if (mcd->gates)
+		mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
+unregister_muxes:
+	if (mcd->muxes)
+		mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
+unregister_factors:
+	if (mcd->factors)
+		mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
+unregister_fixed_clks:
+	if (mcd->fixed_clks)
+		mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+					      clk_ctrl->clk_data);
+unregister_plls:
+	if (mcd->plls)
+		mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
 free_clk_data:
 	mtk_free_clk_data(clk_ctrl->clk_data);
 free_clk_ctrl:
@@ -480,9 +533,22 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
 	struct device_node *node = pdev->dev.of_node;
 
 	of_clk_del_provider(node);
+
 	if (clk_ctrl->rst_data)
 		mtk_unregister_reset_controller(clk_ctrl->rst_data);
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+
+	if (mcd->gates)
+		mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
+	if (mcd->muxes)
+		mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
+	if (mcd->factors)
+		mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
+	if (mcd->fixed_clks)
+		mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+					      clk_ctrl->clk_data);
+	if (mcd->plls)
+		mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
+
 	mtk_free_clk_data(clk_ctrl->clk_data);
 	kfree(clk_ctrl);
 
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index fa092bca97c8..23bce98bca20 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -13,6 +13,9 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+#include "clk-gate.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
 #include "reset.h"
 
 #define MAX_MUX_GATE_BIT	31
@@ -191,12 +194,22 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 
 struct mtk_simple_clk_controller {
 	struct clk_hw_onecell_data *clk_data;
+	spinlock_t mux_lock;
 	struct mtk_clk_rst_data *rst_data;
 };
 
 struct mtk_clk_desc {
-	const struct mtk_gate *clks;
-	size_t num_clks;
+	const struct mtk_pll_data *plls;
+	size_t num_plls;
+	const struct mtk_fixed_clk *fixed_clks;
+	size_t num_fixed_clks;
+	const struct mtk_fixed_factor *factors;
+	size_t num_factors;
+	const struct mtk_mux *muxes;
+	size_t num_muxes;
+	const struct mtk_gate *gates;
+	size_t num_gates;
+
 	const struct mtk_clk_rst_desc *rst_desc;
 };
 
-- 
2.36.1

[PATCH 6/6] clk: mediatek: Add support for other clock types in simple probe/remove

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Simple probe/remove functions currently support gates only. Add
PLLs, fixed clocks, fixed factors and muxes to support most
clock controllers. struct mtk_clk_desc now takes descriptions
for all of these clocks, and only the ones set will be registered.
Most clock controllers will only use a subset of the types
supported.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 88 +++++++++++++++++++++++++++++-----
 drivers/clk/mediatek/clk-mtk.h | 17 ++++++-
 2 files changed, 92 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3382802663f4..df1209d5b6fb 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -15,8 +15,10 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-#include "clk-mtk.h"
 #include "clk-gate.h"
+#include "clk-mtk.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
 
 struct clk_hw_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
 {
@@ -434,20 +436,55 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	if (!clk_ctrl)
 		return -ENOMEM;
 
-	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
+	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_plls
+					      + mcd->num_fixed_clks
+					      + mcd->num_factors
+					      + mcd->num_muxes
+					      + mcd->num_gates);
 	if (!clk_ctrl->clk_data) {
 		r = -ENOMEM;
 		goto free_clk_ctrl;
 	}
 
-	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
-					    clk_ctrl->clk_data, &pdev->dev);
-	if (r)
-		goto free_clk_data;
+	if (mcd->plls) {
+		r = mtk_clk_register_plls(node, mcd->plls, mcd->num_plls,
+					  clk_ctrl->clk_data);
+		if (r)
+			goto free_clk_data;
+	}
+
+	if (mcd->fixed_clks) {
+		r = mtk_clk_register_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+						clk_ctrl->clk_data);
+		if (r)
+			goto unregister_plls;
+	}
+
+	if (mcd->factors) {
+		r = mtk_clk_register_factors(mcd->factors, mcd->num_factors,
+					     clk_ctrl->clk_data);
+		if (r)
+			goto unregister_fixed_clks;
+	}
+
+	if (mcd->muxes) {
+		spin_lock_init(&clk_ctrl->mux_lock);
+		r = mtk_clk_register_muxes(mcd->muxes, mcd->num_muxes, node,
+					   &clk_ctrl->mux_lock, clk_ctrl->clk_data);
+		if (r)
+			goto unregister_factors;
+	}
+
+	if (mcd->gates) {
+		r = mtk_clk_register_gates_with_dev(node, mcd->gates, mcd->num_gates,
+						    clk_ctrl->clk_data, &pdev->dev);
+		if (r)
+			goto unregister_muxes;
+	}
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
 	if (r)
-		goto unregister_clks;
+		goto unregister_gates;
 
 	platform_set_drvdata(pdev, clk_ctrl);
 
@@ -457,14 +494,30 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 							       mcd->rst_desc);
 		if (IS_ERR(clk_ctrl->rst_data)) {
 			r = PTR_ERR(clk_ctrl->rst_data);
-			goto unregister_clks;
+			goto unregister_clk_provider;
 		}
 	}
 
 	return r;
 
-unregister_clks:
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+unregister_clk_provider:
+	of_clk_del_provider(node);
+unregister_gates:
+	if (mcd->gates)
+		mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
+unregister_muxes:
+	if (mcd->muxes)
+		mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
+unregister_factors:
+	if (mcd->factors)
+		mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
+unregister_fixed_clks:
+	if (mcd->fixed_clks)
+		mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+					      clk_ctrl->clk_data);
+unregister_plls:
+	if (mcd->plls)
+		mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
 free_clk_data:
 	mtk_free_clk_data(clk_ctrl->clk_data);
 free_clk_ctrl:
@@ -480,9 +533,22 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
 	struct device_node *node = pdev->dev.of_node;
 
 	of_clk_del_provider(node);
+
 	if (clk_ctrl->rst_data)
 		mtk_unregister_reset_controller(clk_ctrl->rst_data);
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+
+	if (mcd->gates)
+		mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
+	if (mcd->muxes)
+		mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
+	if (mcd->factors)
+		mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
+	if (mcd->fixed_clks)
+		mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+					      clk_ctrl->clk_data);
+	if (mcd->plls)
+		mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
+
 	mtk_free_clk_data(clk_ctrl->clk_data);
 	kfree(clk_ctrl);
 
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index fa092bca97c8..23bce98bca20 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -13,6 +13,9 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+#include "clk-gate.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
 #include "reset.h"
 
 #define MAX_MUX_GATE_BIT	31
@@ -191,12 +194,22 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 
 struct mtk_simple_clk_controller {
 	struct clk_hw_onecell_data *clk_data;
+	spinlock_t mux_lock;
 	struct mtk_clk_rst_data *rst_data;
 };
 
 struct mtk_clk_desc {
-	const struct mtk_gate *clks;
-	size_t num_clks;
+	const struct mtk_pll_data *plls;
+	size_t num_plls;
+	const struct mtk_fixed_clk *fixed_clks;
+	size_t num_fixed_clks;
+	const struct mtk_fixed_factor *factors;
+	size_t num_factors;
+	const struct mtk_mux *muxes;
+	size_t num_muxes;
+	const struct mtk_gate *gates;
+	size_t num_gates;
+
 	const struct mtk_clk_rst_desc *rst_desc;
 };
 
-- 
2.36.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 6/6] clk: mediatek: Add support for other clock types in simple probe/remove

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Simple probe/remove functions currently support gates only. Add
PLLs, fixed clocks, fixed factors and muxes to support most
clock controllers. struct mtk_clk_desc now takes descriptions
for all of these clocks, and only the ones set will be registered.
Most clock controllers will only use a subset of the types
supported.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/ 
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
  https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
- Export required symbols to compile clk drivers as module (single patch)
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/

 drivers/clk/mediatek/clk-mtk.c | 88 +++++++++++++++++++++++++++++-----
 drivers/clk/mediatek/clk-mtk.h | 17 ++++++-
 2 files changed, 92 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3382802663f4..df1209d5b6fb 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -15,8 +15,10 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-#include "clk-mtk.h"
 #include "clk-gate.h"
+#include "clk-mtk.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
 
 struct clk_hw_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
 {
@@ -434,20 +436,55 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 	if (!clk_ctrl)
 		return -ENOMEM;
 
-	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
+	clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_plls
+					      + mcd->num_fixed_clks
+					      + mcd->num_factors
+					      + mcd->num_muxes
+					      + mcd->num_gates);
 	if (!clk_ctrl->clk_data) {
 		r = -ENOMEM;
 		goto free_clk_ctrl;
 	}
 
-	r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
-					    clk_ctrl->clk_data, &pdev->dev);
-	if (r)
-		goto free_clk_data;
+	if (mcd->plls) {
+		r = mtk_clk_register_plls(node, mcd->plls, mcd->num_plls,
+					  clk_ctrl->clk_data);
+		if (r)
+			goto free_clk_data;
+	}
+
+	if (mcd->fixed_clks) {
+		r = mtk_clk_register_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+						clk_ctrl->clk_data);
+		if (r)
+			goto unregister_plls;
+	}
+
+	if (mcd->factors) {
+		r = mtk_clk_register_factors(mcd->factors, mcd->num_factors,
+					     clk_ctrl->clk_data);
+		if (r)
+			goto unregister_fixed_clks;
+	}
+
+	if (mcd->muxes) {
+		spin_lock_init(&clk_ctrl->mux_lock);
+		r = mtk_clk_register_muxes(mcd->muxes, mcd->num_muxes, node,
+					   &clk_ctrl->mux_lock, clk_ctrl->clk_data);
+		if (r)
+			goto unregister_factors;
+	}
+
+	if (mcd->gates) {
+		r = mtk_clk_register_gates_with_dev(node, mcd->gates, mcd->num_gates,
+						    clk_ctrl->clk_data, &pdev->dev);
+		if (r)
+			goto unregister_muxes;
+	}
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
 	if (r)
-		goto unregister_clks;
+		goto unregister_gates;
 
 	platform_set_drvdata(pdev, clk_ctrl);
 
@@ -457,14 +494,30 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
 							       mcd->rst_desc);
 		if (IS_ERR(clk_ctrl->rst_data)) {
 			r = PTR_ERR(clk_ctrl->rst_data);
-			goto unregister_clks;
+			goto unregister_clk_provider;
 		}
 	}
 
 	return r;
 
-unregister_clks:
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+unregister_clk_provider:
+	of_clk_del_provider(node);
+unregister_gates:
+	if (mcd->gates)
+		mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
+unregister_muxes:
+	if (mcd->muxes)
+		mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
+unregister_factors:
+	if (mcd->factors)
+		mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
+unregister_fixed_clks:
+	if (mcd->fixed_clks)
+		mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+					      clk_ctrl->clk_data);
+unregister_plls:
+	if (mcd->plls)
+		mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
 free_clk_data:
 	mtk_free_clk_data(clk_ctrl->clk_data);
 free_clk_ctrl:
@@ -480,9 +533,22 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
 	struct device_node *node = pdev->dev.of_node;
 
 	of_clk_del_provider(node);
+
 	if (clk_ctrl->rst_data)
 		mtk_unregister_reset_controller(clk_ctrl->rst_data);
-	mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
+
+	if (mcd->gates)
+		mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
+	if (mcd->muxes)
+		mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
+	if (mcd->factors)
+		mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
+	if (mcd->fixed_clks)
+		mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
+					      clk_ctrl->clk_data);
+	if (mcd->plls)
+		mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
+
 	mtk_free_clk_data(clk_ctrl->clk_data);
 	kfree(clk_ctrl);
 
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index fa092bca97c8..23bce98bca20 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -13,6 +13,9 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+#include "clk-gate.h"
+#include "clk-mux.h"
+#include "clk-pll.h"
 #include "reset.h"
 
 #define MAX_MUX_GATE_BIT	31
@@ -191,12 +194,22 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
 
 struct mtk_simple_clk_controller {
 	struct clk_hw_onecell_data *clk_data;
+	spinlock_t mux_lock;
 	struct mtk_clk_rst_data *rst_data;
 };
 
 struct mtk_clk_desc {
-	const struct mtk_gate *clks;
-	size_t num_clks;
+	const struct mtk_pll_data *plls;
+	size_t num_plls;
+	const struct mtk_fixed_clk *fixed_clks;
+	size_t num_fixed_clks;
+	const struct mtk_fixed_factor *factors;
+	size_t num_factors;
+	const struct mtk_mux *muxes;
+	size_t num_muxes;
+	const struct mtk_gate *gates;
+	size_t num_gates;
+
 	const struct mtk_clk_rst_desc *rst_desc;
 };
 
-- 
2.36.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[Yassine Oudjana]

On Thu, May 19 2022 at 17:47:22 +0400, Yassine Oudjana 
<yassine.oudjana@gmail.com> wrote:
> This series started as part of an earlier series adding support for 
> the main
> clock controllers on MediaTek MT6735[1]. It has since been split off 
> and
> expanded. It adds a new function to unregister a reset controller and 
> expands
> the mtk_clk_simple_probe/remove functions to support the main 5 types 
> of clocks:
> - PLLs		(new)
> - Fixed clocks	(new)
> - Fixed factors	(new)
> - Muxes		(new)
> - Gates		(supported previously)
> This should allow it to be used in most clock drivers, resulting in 
> reduced
> code duplication. It will be used in MT6735 clock drivers in the 
> upcoming v2
> of the MT6735 main clock controller series.
> 
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   
> https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 
> (series)
>   
> https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single 
> patch)
>   
> https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
> 
> Yassine Oudjana (6):
>   clk: mediatek: gate: Export mtk_clk_register_gates_with_dev
>   clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe
>   clk: mediatek: reset: Return reset data pointer on register
>   clk: mediatek: reset: Implement mtk_unregister_reset_controller() 
> API
>   clk: mediatek: Unregister reset controller on simple remove
>   clk: mediatek: Add support for other clock types in simple
>     probe/remove
> 
>  drivers/clk/mediatek/clk-gate.c   |   1 +
>  drivers/clk/mediatek/clk-mt8192.c |   7 +-
>  drivers/clk/mediatek/clk-mtk.c    | 123 
> +++++++++++++++++++++++++-----
>  drivers/clk/mediatek/clk-mtk.h    |  22 +++++-
>  drivers/clk/mediatek/reset.c      |  41 ++++++----
>  drivers/clk/mediatek/reset.h      |  20 +++--
>  6 files changed, 167 insertions(+), 47 deletions(-)
> 
> --
> 2.36.1
> 

Replying since there is a missing reference:

[1] 
https://patchwork.kernel.org/project/linux-clk/cover/20220504122601.335495-1-y.oudjana@protonmail.com/

Re: [PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[Yassine Oudjana]

On Thu, May 19 2022 at 17:47:22 +0400, Yassine Oudjana 
<yassine.oudjana@gmail.com> wrote:
> This series started as part of an earlier series adding support for 
> the main
> clock controllers on MediaTek MT6735[1]. It has since been split off 
> and
> expanded. It adds a new function to unregister a reset controller and 
> expands
> the mtk_clk_simple_probe/remove functions to support the main 5 types 
> of clocks:
> - PLLs		(new)
> - Fixed clocks	(new)
> - Fixed factors	(new)
> - Muxes		(new)
> - Gates		(supported previously)
> This should allow it to be used in most clock drivers, resulting in 
> reduced
> code duplication. It will be used in MT6735 clock drivers in the 
> upcoming v2
> of the MT6735 main clock controller series.
> 
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   
> https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 
> (series)
>   
> https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single 
> patch)
>   
> https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
> 
> Yassine Oudjana (6):
>   clk: mediatek: gate: Export mtk_clk_register_gates_with_dev
>   clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe
>   clk: mediatek: reset: Return reset data pointer on register
>   clk: mediatek: reset: Implement mtk_unregister_reset_controller() 
> API
>   clk: mediatek: Unregister reset controller on simple remove
>   clk: mediatek: Add support for other clock types in simple
>     probe/remove
> 
>  drivers/clk/mediatek/clk-gate.c   |   1 +
>  drivers/clk/mediatek/clk-mt8192.c |   7 +-
>  drivers/clk/mediatek/clk-mtk.c    | 123 
> +++++++++++++++++++++++++-----
>  drivers/clk/mediatek/clk-mtk.h    |  22 +++++-
>  drivers/clk/mediatek/reset.c      |  41 ++++++----
>  drivers/clk/mediatek/reset.h      |  20 +++--
>  6 files changed, 167 insertions(+), 47 deletions(-)
> 
> --
> 2.36.1
> 

Replying since there is a missing reference:

[1] 
https://patchwork.kernel.org/project/linux-clk/cover/20220504122601.335495-1-y.oudjana@protonmail.com/



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 0/6] clk: mediatek: Improvements to simple probe/remove and reset controller unregistration

[Yassine Oudjana]

On Thu, May 19 2022 at 17:47:22 +0400, Yassine Oudjana 
<yassine.oudjana@gmail.com> wrote:
> This series started as part of an earlier series adding support for 
> the main
> clock controllers on MediaTek MT6735[1]. It has since been split off 
> and
> expanded. It adds a new function to unregister a reset controller and 
> expands
> the mtk_clk_simple_probe/remove functions to support the main 5 types 
> of clocks:
> - PLLs		(new)
> - Fixed clocks	(new)
> - Fixed factors	(new)
> - Muxes		(new)
> - Gates		(supported previously)
> This should allow it to be used in most clock drivers, resulting in 
> reduced
> code duplication. It will be used in MT6735 clock drivers in the 
> upcoming v2
> of the MT6735 main clock controller series.
> 
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   
> https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 
> (series)
>   
> https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single 
> patch)
>   
> https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
> 
> Yassine Oudjana (6):
>   clk: mediatek: gate: Export mtk_clk_register_gates_with_dev
>   clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe
>   clk: mediatek: reset: Return reset data pointer on register
>   clk: mediatek: reset: Implement mtk_unregister_reset_controller() 
> API
>   clk: mediatek: Unregister reset controller on simple remove
>   clk: mediatek: Add support for other clock types in simple
>     probe/remove
> 
>  drivers/clk/mediatek/clk-gate.c   |   1 +
>  drivers/clk/mediatek/clk-mt8192.c |   7 +-
>  drivers/clk/mediatek/clk-mtk.c    | 123 
> +++++++++++++++++++++++++-----
>  drivers/clk/mediatek/clk-mtk.h    |  22 +++++-
>  drivers/clk/mediatek/reset.c      |  41 ++++++----
>  drivers/clk/mediatek/reset.h      |  20 +++--
>  6 files changed, 167 insertions(+), 47 deletions(-)
> 
> --
> 2.36.1
> 

Replying since there is a missing reference:

[1] 
https://patchwork.kernel.org/project/linux-clk/cover/20220504122601.335495-1-y.oudjana@protonmail.com/



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Register gates with dev in mtk_clk_simple_probe.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Register gates with dev in mtk_clk_simple_probe.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Register gates with dev in mtk_clk_simple_probe.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Chen-Yu Tsai]

On Fri, May 20, 2022 at 12:49 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
> <yassine.oudjana@gmail.com> wrote:
> >
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Register gates with dev in mtk_clk_simple_probe.
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Side note: one day I would like to rename all the common functions so
that the follow the style of the CCF. One example would be

  - mtk_XXX_register() takes |struct device *|
  - of_mtk_XXX_register() takes |struct device_node *|

I don't know if I'll ever get to it.

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Chen-Yu Tsai]

On Fri, May 20, 2022 at 12:49 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
> <yassine.oudjana@gmail.com> wrote:
> >
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Register gates with dev in mtk_clk_simple_probe.
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Side note: one day I would like to rename all the common functions so
that the follow the style of the CCF. One example would be

  - mtk_XXX_register() takes |struct device *|
  - of_mtk_XXX_register() takes |struct device_node *|

I don't know if I'll ever get to it.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[Chen-Yu Tsai]

On Fri, May 20, 2022 at 12:49 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
> <yassine.oudjana@gmail.com> wrote:
> >
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Register gates with dev in mtk_clk_simple_probe.
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Side note: one day I would like to rename all the common functions so
that the follow the style of the CCF. One example would be

  - mtk_XXX_register() takes |struct device *|
  - of_mtk_XXX_register() takes |struct device_node *|

I don't know if I'll ever get to it.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> This allows it to be used in drivers built as modules.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Re: [PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> This allows it to be used in drivers built as modules.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> This allows it to be used in drivers built as modules.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Rex-BC Chen]

On Thu, 2022-05-19 at 17:47 +0400, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Return a struct mtk_clk_rst_data * when registering a reset
> controller in preparation for adding an unregister helper
> that will take it as an argument. Make the necessary changes
> in drivers that do not currently discard the return value
> of register functions.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyN1YkE77G$ 
>  
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195
> (series)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyNwSqs3wS$
>  
> - Export required symbols to compile clk drivers as module (single
> patch)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyNwGDWf68$
>  
> 
>  drivers/clk/mediatek/clk-mt8192.c |  7 +++++--
>  drivers/clk/mediatek/clk-mtk.c    |  9 +++++---
>  drivers/clk/mediatek/reset.c      | 34 ++++++++++++++++-------------
> --
>  drivers/clk/mediatek/reset.h      | 14 +++++++------
>  4 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8192.c
> b/drivers/clk/mediatek/clk-mt8192.c
> index ebbd2798d9a3..a658a74644de 100644
> --- a/drivers/clk/mediatek/clk-mt8192.c
> +++ b/drivers/clk/mediatek/clk-mt8192.c
> @@ -1255,6 +1255,7 @@ static int clk_mt8192_infra_probe(struct
> platform_device *pdev)
>  {
>  	struct clk_hw_onecell_data *clk_data;
>  	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_clk_rst_data *rst_data;
>  	int r;
>  
>  	clk_data = mtk_alloc_clk_data(CLK_INFRA_NR_CLK);
> @@ -1265,9 +1266,11 @@ static int clk_mt8192_infra_probe(struct
> platform_device *pdev)
>  	if (r)
>  		goto free_clk_data;
>  
> -	r = mtk_register_reset_controller_with_dev(&pdev->dev,
> &clk_rst_desc);
> -	if (r)
> +	rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
> &clk_rst_desc);
> +	if (IS_ERR(rst_data)) {
> +		r = PTR_ERR(rst_data);
>  		goto free_clk_data;
> +	}
>  
>  	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> clk_data);
>  	if (r)
> diff --git a/drivers/clk/mediatek/clk-mtk.c
> b/drivers/clk/mediatek/clk-mtk.c
> index 3a8875b6c37f..1b5591733e2b 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -424,6 +424,7 @@ int mtk_clk_simple_probe(struct platform_device
> *pdev)
>  	const struct mtk_clk_desc *mcd;
>  	struct clk_hw_onecell_data *clk_data;
>  	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_clk_rst_data *rst_data;
>  	int r;
>  
>  	mcd = of_device_get_match_data(&pdev->dev);
> @@ -446,10 +447,12 @@ int mtk_clk_simple_probe(struct platform_device
> *pdev)
>  	platform_set_drvdata(pdev, clk_data);
>  
>  	if (mcd->rst_desc) {
> -		r = mtk_register_reset_controller_with_dev(&pdev->dev,
> -							   mcd-
> >rst_desc);
> -		if (r)
> +		rst_data =
> mtk_register_reset_controller_with_dev(&pdev->dev,
> +							   	  mcd
> ->rst_desc);
> +		if (IS_ERR(rst_data)) {
> +			r = PTR_ERR(rst_data);
>  			goto unregister_clks;
> +		}
>  	}
>  
>  	return r;
> diff --git a/drivers/clk/mediatek/reset.c
> b/drivers/clk/mediatek/reset.c
> index 290ceda84ce4..09862baf1d57 100644
> --- a/drivers/clk/mediatek/reset.c
> +++ b/drivers/clk/mediatek/reset.c
> @@ -110,8 +110,9 @@ static int reset_xlate(struct
> reset_controller_dev *rcdev,
>  	return data->desc->rst_idx_map[reset_spec->args[0]];
>  }
>  
> -int mtk_register_reset_controller(struct device_node *np,
> -				  const struct mtk_clk_rst_desc *desc)
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller(struct device_node *np,
> +			       const struct mtk_clk_rst_desc *desc)
>  {
>  	struct regmap *regmap;
>  	const struct reset_control_ops *rcops = NULL;
> @@ -120,7 +121,7 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  
>  	if (!desc) {
>  		pr_err("mtk clock reset desc is NULL\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	switch (desc->version) {
> @@ -132,18 +133,18 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  		break;
>  	default:
>  		pr_err("Unknown reset version %d\n", desc->version);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	regmap = device_node_to_regmap(np);
>  	if (IS_ERR(regmap)) {
>  		pr_err("Cannot find regmap for %pOF: %pe\n", np,
> regmap);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	data = kzalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->desc = desc;
>  	data->regmap = regmap;
> @@ -163,14 +164,15 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  	if (ret) {
>  		pr_err("could not register reset controller: %d\n",
> ret);
>  		kfree(data);
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return data;
>  }
>  
> -int mtk_register_reset_controller_with_dev(struct device *dev,
> -					   const struct
> mtk_clk_rst_desc *desc)
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller_with_dev(struct device *dev,
> +					const struct mtk_clk_rst_desc
> *desc)
>  {
>  	struct device_node *np = dev->of_node;
>  	struct regmap *regmap;
> @@ -180,7 +182,7 @@ int mtk_register_reset_controller_with_dev(struct
> device *dev,
>  
>  	if (!desc) {
>  		dev_err(dev, "mtk clock reset desc is NULL\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	switch (desc->version) {
> @@ -192,18 +194,18 @@ int
> mtk_register_reset_controller_with_dev(struct device *dev,
>  		break;
>  	default:
>  		dev_err(dev, "Unknown reset version %d\n", desc-
> >version);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	regmap = device_node_to_regmap(np);
>  	if (IS_ERR(regmap)) {
>  		dev_err(dev, "Cannot find regmap %pe\n", regmap);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->desc = desc;
>  	data->regmap = regmap;
> @@ -223,10 +225,10 @@ int
> mtk_register_reset_controller_with_dev(struct device *dev,
>  	ret = devm_reset_controller_register(dev, &data->rcdev);
>  	if (ret) {
>  		dev_err(dev, "could not register reset controller:
> %d\n", ret);
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return data;
>  }
>  EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
>  
> diff --git a/drivers/clk/mediatek/reset.h
> b/drivers/clk/mediatek/reset.h
> index 913fe676cba7..7418dd0d046f 100644
> --- a/drivers/clk/mediatek/reset.h
> +++ b/drivers/clk/mediatek/reset.h
> @@ -64,19 +64,21 @@ struct mtk_clk_rst_data {
>   * @np: Pointer to device node.
>   * @desc: Constant pointer to description of clock reset.
>   *
> - * Return: 0 on success and errorno otherwise.
> + * Return: Pointer to struct mtk_clk_rst_data on success and error
> pointer otherwise.
>   */
> -int mtk_register_reset_controller(struct device_node *np,
> -				  const struct mtk_clk_rst_desc *desc);
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller(struct device_node *np,
> +			       const struct mtk_clk_rst_desc *desc);
>  
>  /**
>   * mtk_register_reset_controller - Register mediatek clock reset
> controller with device
>   * @np: Pointer to device.
>   * @desc: Constant pointer to description of clock reset.
>   *
> - * Return: 0 on success and errorno otherwise.
> + * Return: Pointer to struct mtk_clk_rst_data on success and error
> pointer otherwise.
>   */
> -int mtk_register_reset_controller_with_dev(struct device *dev,
> -					   const struct
> mtk_clk_rst_desc *desc);
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller_with_dev(struct device *dev,
> +					const struct mtk_clk_rst_desc
> *desc);
>  
>  #endif /* __DRV_CLK_MTK_RESET_H */
> -- 
> 2.36.1
> 

Hello,

Stephen wants me to use  "auxiliary bus" in [1].
I am not sure why it didn't appear in lore, so I add the message.
I said I will find some time to do this after my reset cleanup series.
If so, I think we don't need to modify this in this time?

-----
Quoting Rex-BC Chen (2022-05-08 22:35:55)
> 
> The drivers of this series are reviewed.
> The binding of this series are also acked.
> Could you spare some time and give us some suggestion?

Have you considered using the auxiliary bus to split the Mediatek clk
and reset device up into a clk device and a reset device? The idea
would be to move the reset related code into drivers/reset and have the
clk code in drivers/clk. It's purely an organizational thing and it can
certainly be done later but it may be a good idea to do this to clearly
split out the two different functionalities.
-----

[1]:
https://lore.kernel.org/all/20220503093856.22250-1-rex-bc.chen@mediatek.com/

BRs,
Rex

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Rex-BC Chen]

On Thu, 2022-05-19 at 17:47 +0400, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Return a struct mtk_clk_rst_data * when registering a reset
> controller in preparation for adding an unregister helper
> that will take it as an argument. Make the necessary changes
> in drivers that do not currently discard the return value
> of register functions.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyN1YkE77G$ 
>  
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195
> (series)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyNwSqs3wS$
>  
> - Export required symbols to compile clk drivers as module (single
> patch)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyNwGDWf68$
>  
> 
>  drivers/clk/mediatek/clk-mt8192.c |  7 +++++--
>  drivers/clk/mediatek/clk-mtk.c    |  9 +++++---
>  drivers/clk/mediatek/reset.c      | 34 ++++++++++++++++-------------
> --
>  drivers/clk/mediatek/reset.h      | 14 +++++++------
>  4 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8192.c
> b/drivers/clk/mediatek/clk-mt8192.c
> index ebbd2798d9a3..a658a74644de 100644
> --- a/drivers/clk/mediatek/clk-mt8192.c
> +++ b/drivers/clk/mediatek/clk-mt8192.c
> @@ -1255,6 +1255,7 @@ static int clk_mt8192_infra_probe(struct
> platform_device *pdev)
>  {
>  	struct clk_hw_onecell_data *clk_data;
>  	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_clk_rst_data *rst_data;
>  	int r;
>  
>  	clk_data = mtk_alloc_clk_data(CLK_INFRA_NR_CLK);
> @@ -1265,9 +1266,11 @@ static int clk_mt8192_infra_probe(struct
> platform_device *pdev)
>  	if (r)
>  		goto free_clk_data;
>  
> -	r = mtk_register_reset_controller_with_dev(&pdev->dev,
> &clk_rst_desc);
> -	if (r)
> +	rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
> &clk_rst_desc);
> +	if (IS_ERR(rst_data)) {
> +		r = PTR_ERR(rst_data);
>  		goto free_clk_data;
> +	}
>  
>  	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> clk_data);
>  	if (r)
> diff --git a/drivers/clk/mediatek/clk-mtk.c
> b/drivers/clk/mediatek/clk-mtk.c
> index 3a8875b6c37f..1b5591733e2b 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -424,6 +424,7 @@ int mtk_clk_simple_probe(struct platform_device
> *pdev)
>  	const struct mtk_clk_desc *mcd;
>  	struct clk_hw_onecell_data *clk_data;
>  	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_clk_rst_data *rst_data;
>  	int r;
>  
>  	mcd = of_device_get_match_data(&pdev->dev);
> @@ -446,10 +447,12 @@ int mtk_clk_simple_probe(struct platform_device
> *pdev)
>  	platform_set_drvdata(pdev, clk_data);
>  
>  	if (mcd->rst_desc) {
> -		r = mtk_register_reset_controller_with_dev(&pdev->dev,
> -							   mcd-
> >rst_desc);
> -		if (r)
> +		rst_data =
> mtk_register_reset_controller_with_dev(&pdev->dev,
> +							   	  mcd
> ->rst_desc);
> +		if (IS_ERR(rst_data)) {
> +			r = PTR_ERR(rst_data);
>  			goto unregister_clks;
> +		}
>  	}
>  
>  	return r;
> diff --git a/drivers/clk/mediatek/reset.c
> b/drivers/clk/mediatek/reset.c
> index 290ceda84ce4..09862baf1d57 100644
> --- a/drivers/clk/mediatek/reset.c
> +++ b/drivers/clk/mediatek/reset.c
> @@ -110,8 +110,9 @@ static int reset_xlate(struct
> reset_controller_dev *rcdev,
>  	return data->desc->rst_idx_map[reset_spec->args[0]];
>  }
>  
> -int mtk_register_reset_controller(struct device_node *np,
> -				  const struct mtk_clk_rst_desc *desc)
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller(struct device_node *np,
> +			       const struct mtk_clk_rst_desc *desc)
>  {
>  	struct regmap *regmap;
>  	const struct reset_control_ops *rcops = NULL;
> @@ -120,7 +121,7 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  
>  	if (!desc) {
>  		pr_err("mtk clock reset desc is NULL\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	switch (desc->version) {
> @@ -132,18 +133,18 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  		break;
>  	default:
>  		pr_err("Unknown reset version %d\n", desc->version);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	regmap = device_node_to_regmap(np);
>  	if (IS_ERR(regmap)) {
>  		pr_err("Cannot find regmap for %pOF: %pe\n", np,
> regmap);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	data = kzalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->desc = desc;
>  	data->regmap = regmap;
> @@ -163,14 +164,15 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  	if (ret) {
>  		pr_err("could not register reset controller: %d\n",
> ret);
>  		kfree(data);
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return data;
>  }
>  
> -int mtk_register_reset_controller_with_dev(struct device *dev,
> -					   const struct
> mtk_clk_rst_desc *desc)
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller_with_dev(struct device *dev,
> +					const struct mtk_clk_rst_desc
> *desc)
>  {
>  	struct device_node *np = dev->of_node;
>  	struct regmap *regmap;
> @@ -180,7 +182,7 @@ int mtk_register_reset_controller_with_dev(struct
> device *dev,
>  
>  	if (!desc) {
>  		dev_err(dev, "mtk clock reset desc is NULL\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	switch (desc->version) {
> @@ -192,18 +194,18 @@ int
> mtk_register_reset_controller_with_dev(struct device *dev,
>  		break;
>  	default:
>  		dev_err(dev, "Unknown reset version %d\n", desc-
> >version);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	regmap = device_node_to_regmap(np);
>  	if (IS_ERR(regmap)) {
>  		dev_err(dev, "Cannot find regmap %pe\n", regmap);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->desc = desc;
>  	data->regmap = regmap;
> @@ -223,10 +225,10 @@ int
> mtk_register_reset_controller_with_dev(struct device *dev,
>  	ret = devm_reset_controller_register(dev, &data->rcdev);
>  	if (ret) {
>  		dev_err(dev, "could not register reset controller:
> %d\n", ret);
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return data;
>  }
>  EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
>  
> diff --git a/drivers/clk/mediatek/reset.h
> b/drivers/clk/mediatek/reset.h
> index 913fe676cba7..7418dd0d046f 100644
> --- a/drivers/clk/mediatek/reset.h
> +++ b/drivers/clk/mediatek/reset.h
> @@ -64,19 +64,21 @@ struct mtk_clk_rst_data {
>   * @np: Pointer to device node.
>   * @desc: Constant pointer to description of clock reset.
>   *
> - * Return: 0 on success and errorno otherwise.
> + * Return: Pointer to struct mtk_clk_rst_data on success and error
> pointer otherwise.
>   */
> -int mtk_register_reset_controller(struct device_node *np,
> -				  const struct mtk_clk_rst_desc *desc);
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller(struct device_node *np,
> +			       const struct mtk_clk_rst_desc *desc);
>  
>  /**
>   * mtk_register_reset_controller - Register mediatek clock reset
> controller with device
>   * @np: Pointer to device.
>   * @desc: Constant pointer to description of clock reset.
>   *
> - * Return: 0 on success and errorno otherwise.
> + * Return: Pointer to struct mtk_clk_rst_data on success and error
> pointer otherwise.
>   */
> -int mtk_register_reset_controller_with_dev(struct device *dev,
> -					   const struct
> mtk_clk_rst_desc *desc);
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller_with_dev(struct device *dev,
> +					const struct mtk_clk_rst_desc
> *desc);
>  
>  #endif /* __DRV_CLK_MTK_RESET_H */
> -- 
> 2.36.1
> 

Hello,

Stephen wants me to use  "auxiliary bus" in [1].
I am not sure why it didn't appear in lore, so I add the message.
I said I will find some time to do this after my reset cleanup series.
If so, I think we don't need to modify this in this time?

-----
Quoting Rex-BC Chen (2022-05-08 22:35:55)
> 
> The drivers of this series are reviewed.
> The binding of this series are also acked.
> Could you spare some time and give us some suggestion?

Have you considered using the auxiliary bus to split the Mediatek clk
and reset device up into a clk device and a reset device? The idea
would be to move the reset related code into drivers/reset and have the
clk code in drivers/clk. It's purely an organizational thing and it can
certainly be done later but it may be a good idea to do this to clearly
split out the two different functionalities.
-----

[1]:
https://lore.kernel.org/all/20220503093856.22250-1-rex-bc.chen@mediatek.com/

BRs,
Rex


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Rex-BC Chen]

On Thu, 2022-05-19 at 17:47 +0400, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Return a struct mtk_clk_rst_data * when registering a reset
> controller in preparation for adding an unregister helper
> that will take it as an argument. Make the necessary changes
> in drivers that do not currently discard the return value
> of register functions.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyN1YkE77G$ 
>  
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195
> (series)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyNwSqs3wS$
>  
> - Export required symbols to compile clk drivers as module (single
> patch)
>   
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/__;!!CTRNKA9wMg0ARbw!1TS-6hbS7UPn08ETCuNFzymINPNyp_PlQ22cQbJVNp6vDRjgREzDVlLjvsmyNwGDWf68$
>  
> 
>  drivers/clk/mediatek/clk-mt8192.c |  7 +++++--
>  drivers/clk/mediatek/clk-mtk.c    |  9 +++++---
>  drivers/clk/mediatek/reset.c      | 34 ++++++++++++++++-------------
> --
>  drivers/clk/mediatek/reset.h      | 14 +++++++------
>  4 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8192.c
> b/drivers/clk/mediatek/clk-mt8192.c
> index ebbd2798d9a3..a658a74644de 100644
> --- a/drivers/clk/mediatek/clk-mt8192.c
> +++ b/drivers/clk/mediatek/clk-mt8192.c
> @@ -1255,6 +1255,7 @@ static int clk_mt8192_infra_probe(struct
> platform_device *pdev)
>  {
>  	struct clk_hw_onecell_data *clk_data;
>  	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_clk_rst_data *rst_data;
>  	int r;
>  
>  	clk_data = mtk_alloc_clk_data(CLK_INFRA_NR_CLK);
> @@ -1265,9 +1266,11 @@ static int clk_mt8192_infra_probe(struct
> platform_device *pdev)
>  	if (r)
>  		goto free_clk_data;
>  
> -	r = mtk_register_reset_controller_with_dev(&pdev->dev,
> &clk_rst_desc);
> -	if (r)
> +	rst_data = mtk_register_reset_controller_with_dev(&pdev->dev,
> &clk_rst_desc);
> +	if (IS_ERR(rst_data)) {
> +		r = PTR_ERR(rst_data);
>  		goto free_clk_data;
> +	}
>  
>  	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> clk_data);
>  	if (r)
> diff --git a/drivers/clk/mediatek/clk-mtk.c
> b/drivers/clk/mediatek/clk-mtk.c
> index 3a8875b6c37f..1b5591733e2b 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -424,6 +424,7 @@ int mtk_clk_simple_probe(struct platform_device
> *pdev)
>  	const struct mtk_clk_desc *mcd;
>  	struct clk_hw_onecell_data *clk_data;
>  	struct device_node *node = pdev->dev.of_node;
> +	struct mtk_clk_rst_data *rst_data;
>  	int r;
>  
>  	mcd = of_device_get_match_data(&pdev->dev);
> @@ -446,10 +447,12 @@ int mtk_clk_simple_probe(struct platform_device
> *pdev)
>  	platform_set_drvdata(pdev, clk_data);
>  
>  	if (mcd->rst_desc) {
> -		r = mtk_register_reset_controller_with_dev(&pdev->dev,
> -							   mcd-
> >rst_desc);
> -		if (r)
> +		rst_data =
> mtk_register_reset_controller_with_dev(&pdev->dev,
> +							   	  mcd
> ->rst_desc);
> +		if (IS_ERR(rst_data)) {
> +			r = PTR_ERR(rst_data);
>  			goto unregister_clks;
> +		}
>  	}
>  
>  	return r;
> diff --git a/drivers/clk/mediatek/reset.c
> b/drivers/clk/mediatek/reset.c
> index 290ceda84ce4..09862baf1d57 100644
> --- a/drivers/clk/mediatek/reset.c
> +++ b/drivers/clk/mediatek/reset.c
> @@ -110,8 +110,9 @@ static int reset_xlate(struct
> reset_controller_dev *rcdev,
>  	return data->desc->rst_idx_map[reset_spec->args[0]];
>  }
>  
> -int mtk_register_reset_controller(struct device_node *np,
> -				  const struct mtk_clk_rst_desc *desc)
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller(struct device_node *np,
> +			       const struct mtk_clk_rst_desc *desc)
>  {
>  	struct regmap *regmap;
>  	const struct reset_control_ops *rcops = NULL;
> @@ -120,7 +121,7 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  
>  	if (!desc) {
>  		pr_err("mtk clock reset desc is NULL\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	switch (desc->version) {
> @@ -132,18 +133,18 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  		break;
>  	default:
>  		pr_err("Unknown reset version %d\n", desc->version);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	regmap = device_node_to_regmap(np);
>  	if (IS_ERR(regmap)) {
>  		pr_err("Cannot find regmap for %pOF: %pe\n", np,
> regmap);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	data = kzalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->desc = desc;
>  	data->regmap = regmap;
> @@ -163,14 +164,15 @@ int mtk_register_reset_controller(struct
> device_node *np,
>  	if (ret) {
>  		pr_err("could not register reset controller: %d\n",
> ret);
>  		kfree(data);
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return data;
>  }
>  
> -int mtk_register_reset_controller_with_dev(struct device *dev,
> -					   const struct
> mtk_clk_rst_desc *desc)
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller_with_dev(struct device *dev,
> +					const struct mtk_clk_rst_desc
> *desc)
>  {
>  	struct device_node *np = dev->of_node;
>  	struct regmap *regmap;
> @@ -180,7 +182,7 @@ int mtk_register_reset_controller_with_dev(struct
> device *dev,
>  
>  	if (!desc) {
>  		dev_err(dev, "mtk clock reset desc is NULL\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	switch (desc->version) {
> @@ -192,18 +194,18 @@ int
> mtk_register_reset_controller_with_dev(struct device *dev,
>  		break;
>  	default:
>  		dev_err(dev, "Unknown reset version %d\n", desc-
> >version);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	regmap = device_node_to_regmap(np);
>  	if (IS_ERR(regmap)) {
>  		dev_err(dev, "Cannot find regmap %pe\n", regmap);
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->desc = desc;
>  	data->regmap = regmap;
> @@ -223,10 +225,10 @@ int
> mtk_register_reset_controller_with_dev(struct device *dev,
>  	ret = devm_reset_controller_register(dev, &data->rcdev);
>  	if (ret) {
>  		dev_err(dev, "could not register reset controller:
> %d\n", ret);
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return data;
>  }
>  EXPORT_SYMBOL_GPL(mtk_register_reset_controller_with_dev);
>  
> diff --git a/drivers/clk/mediatek/reset.h
> b/drivers/clk/mediatek/reset.h
> index 913fe676cba7..7418dd0d046f 100644
> --- a/drivers/clk/mediatek/reset.h
> +++ b/drivers/clk/mediatek/reset.h
> @@ -64,19 +64,21 @@ struct mtk_clk_rst_data {
>   * @np: Pointer to device node.
>   * @desc: Constant pointer to description of clock reset.
>   *
> - * Return: 0 on success and errorno otherwise.
> + * Return: Pointer to struct mtk_clk_rst_data on success and error
> pointer otherwise.
>   */
> -int mtk_register_reset_controller(struct device_node *np,
> -				  const struct mtk_clk_rst_desc *desc);
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller(struct device_node *np,
> +			       const struct mtk_clk_rst_desc *desc);
>  
>  /**
>   * mtk_register_reset_controller - Register mediatek clock reset
> controller with device
>   * @np: Pointer to device.
>   * @desc: Constant pointer to description of clock reset.
>   *
> - * Return: 0 on success and errorno otherwise.
> + * Return: Pointer to struct mtk_clk_rst_data on success and error
> pointer otherwise.
>   */
> -int mtk_register_reset_controller_with_dev(struct device *dev,
> -					   const struct
> mtk_clk_rst_desc *desc);
> +struct mtk_clk_rst_data
> +*mtk_register_reset_controller_with_dev(struct device *dev,
> +					const struct mtk_clk_rst_desc
> *desc);
>  
>  #endif /* __DRV_CLK_MTK_RESET_H */
> -- 
> 2.36.1
> 

Hello,

Stephen wants me to use  "auxiliary bus" in [1].
I am not sure why it didn't appear in lore, so I add the message.
I said I will find some time to do this after my reset cleanup series.
If so, I think we don't need to modify this in this time?

-----
Quoting Rex-BC Chen (2022-05-08 22:35:55)
> 
> The drivers of this series are reviewed.
> The binding of this series are also acked.
> Could you spare some time and give us some suggestion?

Have you considered using the auxiliary bus to split the Mediatek clk
and reset device up into a clk device and a reset device? The idea
would be to move the reset related code into drivers/reset and have the
clk code in drivers/clk. It's purely an organizational thing and it can
certainly be done later but it may be a good idea to do this to clearly
split out the two different functionalities.
-----

[1]:
https://lore.kernel.org/all/20220503093856.22250-1-rex-bc.chen@mediatek.com/

BRs,
Rex


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> This allows it to be used in drivers built as modules.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Thumbs up!

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> This allows it to be used in drivers built as modules.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Thumbs up!

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/6] clk: mediatek: gate: Export mtk_clk_register_gates_with_dev

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> This allows it to be used in drivers built as modules.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Thumbs up!

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Register gates with dev in mtk_clk_simple_probe.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Register gates with dev in mtk_clk_simple_probe.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 2/6] clk: mediatek: Use mtk_clk_register_gates_with_dev in simple probe

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Register gates with dev in mtk_clk_simple_probe.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Return a struct mtk_clk_rst_data * when registering a reset
> controller in preparation for adding an unregister helper
> that will take it as an argument. Make the necessary changes
> in drivers that do not currently discard the return value
> of register functions.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Hello Yassine,

Thanks for your efforts on helping to make the MediaTek clocks better - I agree
(and I'm not the only one..) that there's a lot of work to do on this side.

Though... I don't think that this is the right direction: you're right about
properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
but I'm not sure that this kind of noise brings any benefit.

Explaining:
You definitely saw that there's a new register _with_dev, which uses devm ops
and that's going to automatically cleanup in case of removal/failure.
This is what we should do.

Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
means that we can eventually change them to tristate!), so that we slowly remove
all users of all functions that are not "_with_dev", and that we finally remove
all of these then-unused functions as well.

Making sure that I don't get misunderstood:
      I'm not implying that this huge migration work is on your shoulders!

P.S.: Chen-Yu, Miles: do you also agree? :-)

Cheers,
Angelo

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Return a struct mtk_clk_rst_data * when registering a reset
> controller in preparation for adding an unregister helper
> that will take it as an argument. Make the necessary changes
> in drivers that do not currently discard the return value
> of register functions.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Hello Yassine,

Thanks for your efforts on helping to make the MediaTek clocks better - I agree
(and I'm not the only one..) that there's a lot of work to do on this side.

Though... I don't think that this is the right direction: you're right about
properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
but I'm not sure that this kind of noise brings any benefit.

Explaining:
You definitely saw that there's a new register _with_dev, which uses devm ops
and that's going to automatically cleanup in case of removal/failure.
This is what we should do.

Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
means that we can eventually change them to tristate!), so that we slowly remove
all users of all functions that are not "_with_dev", and that we finally remove
all of these then-unused functions as well.

Making sure that I don't get misunderstood:
      I'm not implying that this huge migration work is on your shoulders!

P.S.: Chen-Yu, Miles: do you also agree? :-)

Cheers,
Angelo


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[AngeloGioacchino Del Regno]

Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Return a struct mtk_clk_rst_data * when registering a reset
> controller in preparation for adding an unregister helper
> that will take it as an argument. Make the necessary changes
> in drivers that do not currently discard the return value
> of register functions.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Hello Yassine,

Thanks for your efforts on helping to make the MediaTek clocks better - I agree
(and I'm not the only one..) that there's a lot of work to do on this side.

Though... I don't think that this is the right direction: you're right about
properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
but I'm not sure that this kind of noise brings any benefit.

Explaining:
You definitely saw that there's a new register _with_dev, which uses devm ops
and that's going to automatically cleanup in case of removal/failure.
This is what we should do.

Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
means that we can eventually change them to tristate!), so that we slowly remove
all users of all functions that are not "_with_dev", and that we finally remove
all of these then-unused functions as well.

Making sure that I don't get misunderstood:
      I'm not implying that this huge migration work is on your shoulders!

P.S.: Chen-Yu, Miles: do you also agree? :-)

Cheers,
Angelo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Chen-Yu Tsai]

On Fri, May 20, 2022 at 4:42 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Return a struct mtk_clk_rst_data * when registering a reset
> > controller in preparation for adding an unregister helper
> > that will take it as an argument. Make the necessary changes
> > in drivers that do not currently discard the return value
> > of register functions.
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Hello Yassine,
>
> Thanks for your efforts on helping to make the MediaTek clocks better - I agree
> (and I'm not the only one..) that there's a lot of work to do on this side.
>
> Though... I don't think that this is the right direction: you're right about
> properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.

Since Rex is working on cleaning up the reset bits, maybe also coordinate
with him?

> Explaining:
> You definitely saw that there's a new register _with_dev, which uses devm ops
> and that's going to automatically cleanup in case of removal/failure.

I don't think they use devres at the moment. They should, but I haven't
gotten that far with my improvements. *wink*

> This is what we should do.
>
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
> means that we can eventually change them to tristate!), so that we slowly remove
> all users of all functions that are not "_with_dev", and that we finally remove
> all of these then-unused functions as well.

I agree with moving all the drivers off of CLK_OF_DECLARE() if possible.
There are definitely going to be a few ARM32 ones that can't be converted,
likely because they don't have the ARM arch timer, and need a clock
registered before the timers. Been there.

And there's a whole lot of work to be done for all the old drivers so that
they clean up after themselves.

My desire is to clean up the API (the naming and the parameters) so that
they more closely match up with the underlying CCF APIs they call. We can
then add devres variants (and of_ ones for the unfortunate platforms that
need them).

> Making sure that I don't get misunderstood:
>       I'm not implying that this huge migration work is on your shoulders!

Yeah. I'll likely take up quite a bit of it after all my currently planned
cleanup work is done. Unless of course someone beats me to it.

> P.S.: Chen-Yu, Miles: do you also agree? :-)

Yes.


Regards
ChenYu

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Chen-Yu Tsai]

On Fri, May 20, 2022 at 4:42 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Return a struct mtk_clk_rst_data * when registering a reset
> > controller in preparation for adding an unregister helper
> > that will take it as an argument. Make the necessary changes
> > in drivers that do not currently discard the return value
> > of register functions.
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Hello Yassine,
>
> Thanks for your efforts on helping to make the MediaTek clocks better - I agree
> (and I'm not the only one..) that there's a lot of work to do on this side.
>
> Though... I don't think that this is the right direction: you're right about
> properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.

Since Rex is working on cleaning up the reset bits, maybe also coordinate
with him?

> Explaining:
> You definitely saw that there's a new register _with_dev, which uses devm ops
> and that's going to automatically cleanup in case of removal/failure.

I don't think they use devres at the moment. They should, but I haven't
gotten that far with my improvements. *wink*

> This is what we should do.
>
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
> means that we can eventually change them to tristate!), so that we slowly remove
> all users of all functions that are not "_with_dev", and that we finally remove
> all of these then-unused functions as well.

I agree with moving all the drivers off of CLK_OF_DECLARE() if possible.
There are definitely going to be a few ARM32 ones that can't be converted,
likely because they don't have the ARM arch timer, and need a clock
registered before the timers. Been there.

And there's a whole lot of work to be done for all the old drivers so that
they clean up after themselves.

My desire is to clean up the API (the naming and the parameters) so that
they more closely match up with the underlying CCF APIs they call. We can
then add devres variants (and of_ ones for the unfortunate platforms that
need them).

> Making sure that I don't get misunderstood:
>       I'm not implying that this huge migration work is on your shoulders!

Yeah. I'll likely take up quite a bit of it after all my currently planned
cleanup work is done. Unless of course someone beats me to it.

> P.S.: Chen-Yu, Miles: do you also agree? :-)

Yes.


Regards
ChenYu

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Chen-Yu Tsai]

On Fri, May 20, 2022 at 4:42 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> >
> > Return a struct mtk_clk_rst_data * when registering a reset
> > controller in preparation for adding an unregister helper
> > that will take it as an argument. Make the necessary changes
> > in drivers that do not currently discard the return value
> > of register functions.
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Hello Yassine,
>
> Thanks for your efforts on helping to make the MediaTek clocks better - I agree
> (and I'm not the only one..) that there's a lot of work to do on this side.
>
> Though... I don't think that this is the right direction: you're right about
> properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.

Since Rex is working on cleaning up the reset bits, maybe also coordinate
with him?

> Explaining:
> You definitely saw that there's a new register _with_dev, which uses devm ops
> and that's going to automatically cleanup in case of removal/failure.

I don't think they use devres at the moment. They should, but I haven't
gotten that far with my improvements. *wink*

> This is what we should do.
>
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
> means that we can eventually change them to tristate!), so that we slowly remove
> all users of all functions that are not "_with_dev", and that we finally remove
> all of these then-unused functions as well.

I agree with moving all the drivers off of CLK_OF_DECLARE() if possible.
There are definitely going to be a few ARM32 ones that can't be converted,
likely because they don't have the ARM arch timer, and need a clock
registered before the timers. Been there.

And there's a whole lot of work to be done for all the old drivers so that
they clean up after themselves.

My desire is to clean up the API (the naming and the parameters) so that
they more closely match up with the underlying CCF APIs they call. We can
then add devres variants (and of_ ones for the unfortunate platforms that
need them).

> Making sure that I don't get misunderstood:
>       I'm not implying that this huge migration work is on your shoulders!

Yeah. I'll likely take up quite a bit of it after all my currently planned
cleanup work is done. Unless of course someone beats me to it.

> P.S.: Chen-Yu, Miles: do you also agree? :-)

Yes.


Regards
ChenYu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Miles Chen]

> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > 
> > Return a struct mtk_clk_rst_data * when registering a reset
> > controller in preparation for adding an unregister helper
> > that will take it as an argument. Make the necessary changes
> > in drivers that do not currently discard the return value
> > of register functions.
> > 
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Hello Yassine,
> 
> Thanks for your efforts on helping to make the MediaTek clocks better - I agree
> (and I'm not the only one..) that there's a lot of work to do on this side.
> 
> Though... I don't think that this is the right direction: you're right about
> properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.
> 
> Explaining:
> You definitely saw that there's a new register _with_dev, which uses devm ops
> and that's going to automatically cleanup in case of removal/failure.
> This is what we should do.
> 
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
> means that we can eventually change them to tristate!), so that we slowly remove
> all users of all functions that are not "_with_dev", and that we finally remove
> all of these then-unused functions as well.
> 
> Making sure that I don't get misunderstood:
>       I'm not implying that this huge migration work is on your shoulders!
> 
> P.S.: Chen-Yu, Miles: do you also agree? :-)

Agreed. 3/6, 4/6, 5/6 introduce mtk_simple_clk_controller, but we just got
ChenYu's patch merged to clk-next to switch to clk_hw_onecell_data, and
hoplefully we can test them in v5.18 or v5.19.
So I think we should wait for ChenYu's cleanup.

-	struct clk_hw_onecell_data *clk_data;
+	struct mtk_simple_clk_controller *clk_ctrl;

thanks,
Miles

> 
> Cheers,
> Angelo
> 
> 

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Miles Chen]

> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > 
> > Return a struct mtk_clk_rst_data * when registering a reset
> > controller in preparation for adding an unregister helper
> > that will take it as an argument. Make the necessary changes
> > in drivers that do not currently discard the return value
> > of register functions.
> > 
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Hello Yassine,
> 
> Thanks for your efforts on helping to make the MediaTek clocks better - I agree
> (and I'm not the only one..) that there's a lot of work to do on this side.
> 
> Though... I don't think that this is the right direction: you're right about
> properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.
> 
> Explaining:
> You definitely saw that there's a new register _with_dev, which uses devm ops
> and that's going to automatically cleanup in case of removal/failure.
> This is what we should do.
> 
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
> means that we can eventually change them to tristate!), so that we slowly remove
> all users of all functions that are not "_with_dev", and that we finally remove
> all of these then-unused functions as well.
> 
> Making sure that I don't get misunderstood:
>       I'm not implying that this huge migration work is on your shoulders!
> 
> P.S.: Chen-Yu, Miles: do you also agree? :-)

Agreed. 3/6, 4/6, 5/6 introduce mtk_simple_clk_controller, but we just got
ChenYu's patch merged to clk-next to switch to clk_hw_onecell_data, and
hoplefully we can test them in v5.18 or v5.19.
So I think we should wait for ChenYu's cleanup.

-	struct clk_hw_onecell_data *clk_data;
+	struct mtk_simple_clk_controller *clk_ctrl;

thanks,
Miles

> 
> Cheers,
> Angelo
> 
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Miles Chen]

> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > 
> > Return a struct mtk_clk_rst_data * when registering a reset
> > controller in preparation for adding an unregister helper
> > that will take it as an argument. Make the necessary changes
> > in drivers that do not currently discard the return value
> > of register functions.
> > 
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Hello Yassine,
> 
> Thanks for your efforts on helping to make the MediaTek clocks better - I agree
> (and I'm not the only one..) that there's a lot of work to do on this side.
> 
> Though... I don't think that this is the right direction: you're right about
> properly unregistering (in patch 4/6) the reset controllers on rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.
> 
> Explaining:
> You definitely saw that there's a new register _with_dev, which uses devm ops
> and that's going to automatically cleanup in case of removal/failure.
> This is what we should do.
> 
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also
> means that we can eventually change them to tristate!), so that we slowly remove
> all users of all functions that are not "_with_dev", and that we finally remove
> all of these then-unused functions as well.
> 
> Making sure that I don't get misunderstood:
>       I'm not implying that this huge migration work is on your shoulders!
> 
> P.S.: Chen-Yu, Miles: do you also agree? :-)

Agreed. 3/6, 4/6, 5/6 introduce mtk_simple_clk_controller, but we just got
ChenYu's patch merged to clk-next to switch to clk_hw_onecell_data, and
hoplefully we can test them in v5.18 or v5.19.
So I think we should wait for ChenYu's cleanup.

-	struct clk_hw_onecell_data *clk_data;
+	struct mtk_simple_clk_controller *clk_ctrl;

thanks,
Miles

> 
> Cheers,
> Angelo
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] clk: mediatek: Add support for other clock types in simple probe/remove

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:50 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Simple probe/remove functions currently support gates only. Add
> PLLs, fixed clocks, fixed factors and muxes to support most
> clock controllers. struct mtk_clk_desc now takes descriptions
> for all of these clocks, and only the ones set will be registered.
> Most clock controllers will only use a subset of the types
> supported.

I assume this mostly benefits the apmixedsys (PLL) and topckgen
(mix of dividers and muxes and gates) drivers.

I plan on introducing |struct clk_hw *| based clk_parent_data for internal
clock parents. This will require parent clocks be registered before child
clocks. Depending on the hardware, registration of different clock types
would need to be interweaved, and we would end up losing any benefits this
patch brings.

I would hold off on this patch until we have a clearer picture of what
needs to be done. At least two or three clock drivers that can use this
should be introduced or converted

> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single patch)
>   https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
>
>  drivers/clk/mediatek/clk-mtk.c | 88 +++++++++++++++++++++++++++++-----
>  drivers/clk/mediatek/clk-mtk.h | 17 ++++++-
>  2 files changed, 92 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 3382802663f4..df1209d5b6fb 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -15,8 +15,10 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>
> -#include "clk-mtk.h"
>  #include "clk-gate.h"
> +#include "clk-mtk.h"
> +#include "clk-mux.h"
> +#include "clk-pll.h"
>
>  struct clk_hw_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
>  {
> @@ -434,20 +436,55 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>         if (!clk_ctrl)
>                 return -ENOMEM;
>
> -       clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
> +       clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_plls
> +                                             + mcd->num_fixed_clks
> +                                             + mcd->num_factors
> +                                             + mcd->num_muxes
> +                                             + mcd->num_gates);

There are also dividers and composite clocks.

ChenYu

>         if (!clk_ctrl->clk_data) {
>                 r = -ENOMEM;
>                 goto free_clk_ctrl;
>         }
>
> -       r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
> -                                           clk_ctrl->clk_data, &pdev->dev);
> -       if (r)
> -               goto free_clk_data;
> +       if (mcd->plls) {
> +               r = mtk_clk_register_plls(node, mcd->plls, mcd->num_plls,
> +                                         clk_ctrl->clk_data);
> +               if (r)
> +                       goto free_clk_data;
> +       }
> +
> +       if (mcd->fixed_clks) {
> +               r = mtk_clk_register_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                               clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_plls;
> +       }
> +
> +       if (mcd->factors) {
> +               r = mtk_clk_register_factors(mcd->factors, mcd->num_factors,
> +                                            clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_fixed_clks;
> +       }
> +
> +       if (mcd->muxes) {
> +               spin_lock_init(&clk_ctrl->mux_lock);
> +               r = mtk_clk_register_muxes(mcd->muxes, mcd->num_muxes, node,
> +                                          &clk_ctrl->mux_lock, clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_factors;
> +       }
> +
> +       if (mcd->gates) {
> +               r = mtk_clk_register_gates_with_dev(node, mcd->gates, mcd->num_gates,
> +                                                   clk_ctrl->clk_data, &pdev->dev);
> +               if (r)
> +                       goto unregister_muxes;
> +       }
>
>         r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
>         if (r)
> -               goto unregister_clks;
> +               goto unregister_gates;
>
>         platform_set_drvdata(pdev, clk_ctrl);
>
> @@ -457,14 +494,30 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>                                                                mcd->rst_desc);
>                 if (IS_ERR(clk_ctrl->rst_data)) {
>                         r = PTR_ERR(clk_ctrl->rst_data);
> -                       goto unregister_clks;
> +                       goto unregister_clk_provider;
>                 }
>         }
>
>         return r;
>
> -unregister_clks:
> -       mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
> +unregister_clk_provider:
> +       of_clk_del_provider(node);
> +unregister_gates:
> +       if (mcd->gates)
> +               mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
> +unregister_muxes:
> +       if (mcd->muxes)
> +               mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
> +unregister_factors:
> +       if (mcd->factors)
> +               mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
> +unregister_fixed_clks:
> +       if (mcd->fixed_clks)
> +               mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                             clk_ctrl->clk_data);
> +unregister_plls:
> +       if (mcd->plls)
> +               mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
>  free_clk_data:
>         mtk_free_clk_data(clk_ctrl->clk_data);
>  free_clk_ctrl:
> @@ -480,9 +533,22 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
>         struct device_node *node = pdev->dev.of_node;
>
>         of_clk_del_provider(node);
> +
>         if (clk_ctrl->rst_data)
>                 mtk_unregister_reset_controller(clk_ctrl->rst_data);
> -       mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
> +
> +       if (mcd->gates)
> +               mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
> +       if (mcd->muxes)
> +               mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
> +       if (mcd->factors)
> +               mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
> +       if (mcd->fixed_clks)
> +               mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                             clk_ctrl->clk_data);
> +       if (mcd->plls)
> +               mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
> +
>         mtk_free_clk_data(clk_ctrl->clk_data);
>         kfree(clk_ctrl);
>
> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> index fa092bca97c8..23bce98bca20 100644
> --- a/drivers/clk/mediatek/clk-mtk.h
> +++ b/drivers/clk/mediatek/clk-mtk.h
> @@ -13,6 +13,9 @@
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>
> +#include "clk-gate.h"
> +#include "clk-mux.h"
> +#include "clk-pll.h"
>  #include "reset.h"
>
>  #define MAX_MUX_GATE_BIT       31
> @@ -191,12 +194,22 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
>
>  struct mtk_simple_clk_controller {
>         struct clk_hw_onecell_data *clk_data;
> +       spinlock_t mux_lock;
>         struct mtk_clk_rst_data *rst_data;
>  };
>
>  struct mtk_clk_desc {
> -       const struct mtk_gate *clks;
> -       size_t num_clks;
> +       const struct mtk_pll_data *plls;
> +       size_t num_plls;
> +       const struct mtk_fixed_clk *fixed_clks;
> +       size_t num_fixed_clks;
> +       const struct mtk_fixed_factor *factors;
> +       size_t num_factors;
> +       const struct mtk_mux *muxes;
> +       size_t num_muxes;
> +       const struct mtk_gate *gates;
> +       size_t num_gates;
> +
>         const struct mtk_clk_rst_desc *rst_desc;
>  };
>
> --
> 2.36.1
>

Re: [PATCH 6/6] clk: mediatek: Add support for other clock types in simple probe/remove

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:50 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Simple probe/remove functions currently support gates only. Add
> PLLs, fixed clocks, fixed factors and muxes to support most
> clock controllers. struct mtk_clk_desc now takes descriptions
> for all of these clocks, and only the ones set will be registered.
> Most clock controllers will only use a subset of the types
> supported.

I assume this mostly benefits the apmixedsys (PLL) and topckgen
(mix of dividers and muxes and gates) drivers.

I plan on introducing |struct clk_hw *| based clk_parent_data for internal
clock parents. This will require parent clocks be registered before child
clocks. Depending on the hardware, registration of different clock types
would need to be interweaved, and we would end up losing any benefits this
patch brings.

I would hold off on this patch until we have a clearer picture of what
needs to be done. At least two or three clock drivers that can use this
should be introduced or converted

> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single patch)
>   https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
>
>  drivers/clk/mediatek/clk-mtk.c | 88 +++++++++++++++++++++++++++++-----
>  drivers/clk/mediatek/clk-mtk.h | 17 ++++++-
>  2 files changed, 92 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 3382802663f4..df1209d5b6fb 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -15,8 +15,10 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>
> -#include "clk-mtk.h"
>  #include "clk-gate.h"
> +#include "clk-mtk.h"
> +#include "clk-mux.h"
> +#include "clk-pll.h"
>
>  struct clk_hw_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
>  {
> @@ -434,20 +436,55 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>         if (!clk_ctrl)
>                 return -ENOMEM;
>
> -       clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
> +       clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_plls
> +                                             + mcd->num_fixed_clks
> +                                             + mcd->num_factors
> +                                             + mcd->num_muxes
> +                                             + mcd->num_gates);

There are also dividers and composite clocks.

ChenYu

>         if (!clk_ctrl->clk_data) {
>                 r = -ENOMEM;
>                 goto free_clk_ctrl;
>         }
>
> -       r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
> -                                           clk_ctrl->clk_data, &pdev->dev);
> -       if (r)
> -               goto free_clk_data;
> +       if (mcd->plls) {
> +               r = mtk_clk_register_plls(node, mcd->plls, mcd->num_plls,
> +                                         clk_ctrl->clk_data);
> +               if (r)
> +                       goto free_clk_data;
> +       }
> +
> +       if (mcd->fixed_clks) {
> +               r = mtk_clk_register_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                               clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_plls;
> +       }
> +
> +       if (mcd->factors) {
> +               r = mtk_clk_register_factors(mcd->factors, mcd->num_factors,
> +                                            clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_fixed_clks;
> +       }
> +
> +       if (mcd->muxes) {
> +               spin_lock_init(&clk_ctrl->mux_lock);
> +               r = mtk_clk_register_muxes(mcd->muxes, mcd->num_muxes, node,
> +                                          &clk_ctrl->mux_lock, clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_factors;
> +       }
> +
> +       if (mcd->gates) {
> +               r = mtk_clk_register_gates_with_dev(node, mcd->gates, mcd->num_gates,
> +                                                   clk_ctrl->clk_data, &pdev->dev);
> +               if (r)
> +                       goto unregister_muxes;
> +       }
>
>         r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
>         if (r)
> -               goto unregister_clks;
> +               goto unregister_gates;
>
>         platform_set_drvdata(pdev, clk_ctrl);
>
> @@ -457,14 +494,30 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>                                                                mcd->rst_desc);
>                 if (IS_ERR(clk_ctrl->rst_data)) {
>                         r = PTR_ERR(clk_ctrl->rst_data);
> -                       goto unregister_clks;
> +                       goto unregister_clk_provider;
>                 }
>         }
>
>         return r;
>
> -unregister_clks:
> -       mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
> +unregister_clk_provider:
> +       of_clk_del_provider(node);
> +unregister_gates:
> +       if (mcd->gates)
> +               mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
> +unregister_muxes:
> +       if (mcd->muxes)
> +               mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
> +unregister_factors:
> +       if (mcd->factors)
> +               mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
> +unregister_fixed_clks:
> +       if (mcd->fixed_clks)
> +               mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                             clk_ctrl->clk_data);
> +unregister_plls:
> +       if (mcd->plls)
> +               mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
>  free_clk_data:
>         mtk_free_clk_data(clk_ctrl->clk_data);
>  free_clk_ctrl:
> @@ -480,9 +533,22 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
>         struct device_node *node = pdev->dev.of_node;
>
>         of_clk_del_provider(node);
> +
>         if (clk_ctrl->rst_data)
>                 mtk_unregister_reset_controller(clk_ctrl->rst_data);
> -       mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
> +
> +       if (mcd->gates)
> +               mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
> +       if (mcd->muxes)
> +               mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
> +       if (mcd->factors)
> +               mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
> +       if (mcd->fixed_clks)
> +               mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                             clk_ctrl->clk_data);
> +       if (mcd->plls)
> +               mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
> +
>         mtk_free_clk_data(clk_ctrl->clk_data);
>         kfree(clk_ctrl);
>
> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> index fa092bca97c8..23bce98bca20 100644
> --- a/drivers/clk/mediatek/clk-mtk.h
> +++ b/drivers/clk/mediatek/clk-mtk.h
> @@ -13,6 +13,9 @@
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>
> +#include "clk-gate.h"
> +#include "clk-mux.h"
> +#include "clk-pll.h"
>  #include "reset.h"
>
>  #define MAX_MUX_GATE_BIT       31
> @@ -191,12 +194,22 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
>
>  struct mtk_simple_clk_controller {
>         struct clk_hw_onecell_data *clk_data;
> +       spinlock_t mux_lock;
>         struct mtk_clk_rst_data *rst_data;
>  };
>
>  struct mtk_clk_desc {
> -       const struct mtk_gate *clks;
> -       size_t num_clks;
> +       const struct mtk_pll_data *plls;
> +       size_t num_plls;
> +       const struct mtk_fixed_clk *fixed_clks;
> +       size_t num_fixed_clks;
> +       const struct mtk_fixed_factor *factors;
> +       size_t num_factors;
> +       const struct mtk_mux *muxes;
> +       size_t num_muxes;
> +       const struct mtk_gate *gates;
> +       size_t num_gates;
> +
>         const struct mtk_clk_rst_desc *rst_desc;
>  };
>
> --
> 2.36.1
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 6/6] clk: mediatek: Add support for other clock types in simple probe/remove

[Chen-Yu Tsai]

On Thu, May 19, 2022 at 9:50 PM Yassine Oudjana
<yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Simple probe/remove functions currently support gates only. Add
> PLLs, fixed clocks, fixed factors and muxes to support most
> clock controllers. struct mtk_clk_desc now takes descriptions
> for all of these clocks, and only the ones set will be registered.
> Most clock controllers will only use a subset of the types
> supported.

I assume this mostly benefits the apmixedsys (PLL) and topckgen
(mix of dividers and muxes and gates) drivers.

I plan on introducing |struct clk_hw *| based clk_parent_data for internal
clock parents. This will require parent clocks be registered before child
clocks. Depending on the hardware, registration of different clock types
would need to be interweaved, and we would end up losing any benefits this
patch brings.

I would hold off on this patch until we have a clearer picture of what
needs to be done. At least two or three clock drivers that can use this
should be introduced or converted

> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220510104804.544597-1-wenst@chromium.org/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
>   https://patchwork.kernel.org/project/linux-mediatek/cover/20220503093856.22250-1-rex-bc.chen@mediatek.com/
> - Export required symbols to compile clk drivers as module (single patch)
>   https://patchwork.kernel.org/project/linux-mediatek/patch/20220518111652.223727-7-angelogioacchino.delregno@collabora.com/
>
>  drivers/clk/mediatek/clk-mtk.c | 88 +++++++++++++++++++++++++++++-----
>  drivers/clk/mediatek/clk-mtk.h | 17 ++++++-
>  2 files changed, 92 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 3382802663f4..df1209d5b6fb 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -15,8 +15,10 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>
> -#include "clk-mtk.h"
>  #include "clk-gate.h"
> +#include "clk-mtk.h"
> +#include "clk-mux.h"
> +#include "clk-pll.h"
>
>  struct clk_hw_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
>  {
> @@ -434,20 +436,55 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>         if (!clk_ctrl)
>                 return -ENOMEM;
>
> -       clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_clks);
> +       clk_ctrl->clk_data = mtk_alloc_clk_data(mcd->num_plls
> +                                             + mcd->num_fixed_clks
> +                                             + mcd->num_factors
> +                                             + mcd->num_muxes
> +                                             + mcd->num_gates);

There are also dividers and composite clocks.

ChenYu

>         if (!clk_ctrl->clk_data) {
>                 r = -ENOMEM;
>                 goto free_clk_ctrl;
>         }
>
> -       r = mtk_clk_register_gates_with_dev(node, mcd->clks, mcd->num_clks,
> -                                           clk_ctrl->clk_data, &pdev->dev);
> -       if (r)
> -               goto free_clk_data;
> +       if (mcd->plls) {
> +               r = mtk_clk_register_plls(node, mcd->plls, mcd->num_plls,
> +                                         clk_ctrl->clk_data);
> +               if (r)
> +                       goto free_clk_data;
> +       }
> +
> +       if (mcd->fixed_clks) {
> +               r = mtk_clk_register_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                               clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_plls;
> +       }
> +
> +       if (mcd->factors) {
> +               r = mtk_clk_register_factors(mcd->factors, mcd->num_factors,
> +                                            clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_fixed_clks;
> +       }
> +
> +       if (mcd->muxes) {
> +               spin_lock_init(&clk_ctrl->mux_lock);
> +               r = mtk_clk_register_muxes(mcd->muxes, mcd->num_muxes, node,
> +                                          &clk_ctrl->mux_lock, clk_ctrl->clk_data);
> +               if (r)
> +                       goto unregister_factors;
> +       }
> +
> +       if (mcd->gates) {
> +               r = mtk_clk_register_gates_with_dev(node, mcd->gates, mcd->num_gates,
> +                                                   clk_ctrl->clk_data, &pdev->dev);
> +               if (r)
> +                       goto unregister_muxes;
> +       }
>
>         r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_ctrl->clk_data);
>         if (r)
> -               goto unregister_clks;
> +               goto unregister_gates;
>
>         platform_set_drvdata(pdev, clk_ctrl);
>
> @@ -457,14 +494,30 @@ int mtk_clk_simple_probe(struct platform_device *pdev)
>                                                                mcd->rst_desc);
>                 if (IS_ERR(clk_ctrl->rst_data)) {
>                         r = PTR_ERR(clk_ctrl->rst_data);
> -                       goto unregister_clks;
> +                       goto unregister_clk_provider;
>                 }
>         }
>
>         return r;
>
> -unregister_clks:
> -       mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
> +unregister_clk_provider:
> +       of_clk_del_provider(node);
> +unregister_gates:
> +       if (mcd->gates)
> +               mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
> +unregister_muxes:
> +       if (mcd->muxes)
> +               mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
> +unregister_factors:
> +       if (mcd->factors)
> +               mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
> +unregister_fixed_clks:
> +       if (mcd->fixed_clks)
> +               mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                             clk_ctrl->clk_data);
> +unregister_plls:
> +       if (mcd->plls)
> +               mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
>  free_clk_data:
>         mtk_free_clk_data(clk_ctrl->clk_data);
>  free_clk_ctrl:
> @@ -480,9 +533,22 @@ int mtk_clk_simple_remove(struct platform_device *pdev)
>         struct device_node *node = pdev->dev.of_node;
>
>         of_clk_del_provider(node);
> +
>         if (clk_ctrl->rst_data)
>                 mtk_unregister_reset_controller(clk_ctrl->rst_data);
> -       mtk_clk_unregister_gates(mcd->clks, mcd->num_clks, clk_ctrl->clk_data);
> +
> +       if (mcd->gates)
> +               mtk_clk_unregister_gates(mcd->gates, mcd->num_gates, clk_ctrl->clk_data);
> +       if (mcd->muxes)
> +               mtk_clk_unregister_muxes(mcd->muxes, mcd->num_muxes, clk_ctrl->clk_data);
> +       if (mcd->factors)
> +               mtk_clk_unregister_factors(mcd->factors, mcd->num_factors, clk_ctrl->clk_data);
> +       if (mcd->fixed_clks)
> +               mtk_clk_unregister_fixed_clks(mcd->fixed_clks, mcd->num_fixed_clks,
> +                                             clk_ctrl->clk_data);
> +       if (mcd->plls)
> +               mtk_clk_unregister_plls(mcd->plls, mcd->num_plls, clk_ctrl->clk_data);
> +
>         mtk_free_clk_data(clk_ctrl->clk_data);
>         kfree(clk_ctrl);
>
> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> index fa092bca97c8..23bce98bca20 100644
> --- a/drivers/clk/mediatek/clk-mtk.h
> +++ b/drivers/clk/mediatek/clk-mtk.h
> @@ -13,6 +13,9 @@
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>
> +#include "clk-gate.h"
> +#include "clk-mux.h"
> +#include "clk-pll.h"
>  #include "reset.h"
>
>  #define MAX_MUX_GATE_BIT       31
> @@ -191,12 +194,22 @@ struct clk_hw *mtk_clk_register_ref2usb_tx(const char *name,
>
>  struct mtk_simple_clk_controller {
>         struct clk_hw_onecell_data *clk_data;
> +       spinlock_t mux_lock;
>         struct mtk_clk_rst_data *rst_data;
>  };
>
>  struct mtk_clk_desc {
> -       const struct mtk_gate *clks;
> -       size_t num_clks;
> +       const struct mtk_pll_data *plls;
> +       size_t num_plls;
> +       const struct mtk_fixed_clk *fixed_clks;
> +       size_t num_fixed_clks;
> +       const struct mtk_fixed_factor *factors;
> +       size_t num_factors;
> +       const struct mtk_mux *muxes;
> +       size_t num_muxes;
> +       const struct mtk_gate *gates;
> +       size_t num_gates;
> +
>         const struct mtk_clk_rst_desc *rst_desc;
>  };
>
> --
> 2.36.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Yassine Oudjana]

On Fri, May 20 2022 at 10:42:40 +0200, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>> Return a struct mtk_clk_rst_data * when registering a reset
>> controller in preparation for adding an unregister helper
>> that will take it as an argument. Make the necessary changes
>> in drivers that do not currently discard the return value
>> of register functions.
>> 
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Hello Yassine,
> 
> Thanks for your efforts on helping to make the MediaTek clocks better 
> - I agree
> (and I'm not the only one..) that there's a lot of work to do on this 
> side.
> 
> Though... I don't think that this is the right direction: you're 
> right about
> properly unregistering (in patch 4/6) the reset controllers on 
> rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.
> 
> Explaining:
> You definitely saw that there's a new register _with_dev, which uses 
> devm ops
> and that's going to automatically cleanup in case of removal/failure.
> This is what we should do.
> 
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, 
> steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers 
> (which also
> means that we can eventually change them to tristate!), so that we 
> slowly remove
> all users of all functions that are not "_with_dev", and that we 
> finally remove
> all of these then-unused functions as well.

I've tried to make small (but hopefully not too small) steps with
little improvements. Originally in MT6735 clock drivers v1, I only
added reset controller unregister, and while rebasing on Rex-BC's
reset cleanup series I found mtk_clk_simple_probe/remove while
looking for references to mtk_register_reset_controller, so
I thought of using it for my drivers resulting in this series
adding support for the extra 4 clock types. I started finding
other things that could be improved such as the other clock types
not having register_*_with_dev(), but I had to avoid adding
anything else since that would only make me find more things to
improve and this series would've never been finished and sent.

With that said, if these patches could become an obstacle for
later more complete reworks, then by all means drop them.

> 
> Making sure that I don't get misunderstood:
>      I'm not implying that this huge migration work is on your 
> shoulders!
> 

Of course. I would never be able to handle such a large task.
Everyone currently helping with modernizing this common clock
framework has my full respect. You are doing amazing work.

Thanks,
Yassine

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Yassine Oudjana]

On Fri, May 20 2022 at 10:42:40 +0200, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>> Return a struct mtk_clk_rst_data * when registering a reset
>> controller in preparation for adding an unregister helper
>> that will take it as an argument. Make the necessary changes
>> in drivers that do not currently discard the return value
>> of register functions.
>> 
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Hello Yassine,
> 
> Thanks for your efforts on helping to make the MediaTek clocks better 
> - I agree
> (and I'm not the only one..) that there's a lot of work to do on this 
> side.
> 
> Though... I don't think that this is the right direction: you're 
> right about
> properly unregistering (in patch 4/6) the reset controllers on 
> rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.
> 
> Explaining:
> You definitely saw that there's a new register _with_dev, which uses 
> devm ops
> and that's going to automatically cleanup in case of removal/failure.
> This is what we should do.
> 
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, 
> steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers 
> (which also
> means that we can eventually change them to tristate!), so that we 
> slowly remove
> all users of all functions that are not "_with_dev", and that we 
> finally remove
> all of these then-unused functions as well.

I've tried to make small (but hopefully not too small) steps with
little improvements. Originally in MT6735 clock drivers v1, I only
added reset controller unregister, and while rebasing on Rex-BC's
reset cleanup series I found mtk_clk_simple_probe/remove while
looking for references to mtk_register_reset_controller, so
I thought of using it for my drivers resulting in this series
adding support for the extra 4 clock types. I started finding
other things that could be improved such as the other clock types
not having register_*_with_dev(), but I had to avoid adding
anything else since that would only make me find more things to
improve and this series would've never been finished and sent.

With that said, if these patches could become an obstacle for
later more complete reworks, then by all means drop them.

> 
> Making sure that I don't get misunderstood:
>      I'm not implying that this huge migration work is on your 
> shoulders!
> 

Of course. I would never be able to handle such a large task.
Everyone currently helping with modernizing this common clock
framework has my full respect. You are doing amazing work.

Thanks,
Yassine




_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register

[Yassine Oudjana]

On Fri, May 20 2022 at 10:42:40 +0200, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>> Return a struct mtk_clk_rst_data * when registering a reset
>> controller in preparation for adding an unregister helper
>> that will take it as an argument. Make the necessary changes
>> in drivers that do not currently discard the return value
>> of register functions.
>> 
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Hello Yassine,
> 
> Thanks for your efforts on helping to make the MediaTek clocks better 
> - I agree
> (and I'm not the only one..) that there's a lot of work to do on this 
> side.
> 
> Though... I don't think that this is the right direction: you're 
> right about
> properly unregistering (in patch 4/6) the reset controllers on 
> rmmod/failure
> but I'm not sure that this kind of noise brings any benefit.
> 
> Explaining:
> You definitely saw that there's a new register _with_dev, which uses 
> devm ops
> and that's going to automatically cleanup in case of removal/failure.
> This is what we should do.
> 
> Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, 
> steadily) migrate
> all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers 
> (which also
> means that we can eventually change them to tristate!), so that we 
> slowly remove
> all users of all functions that are not "_with_dev", and that we 
> finally remove
> all of these then-unused functions as well.

I've tried to make small (but hopefully not too small) steps with
little improvements. Originally in MT6735 clock drivers v1, I only
added reset controller unregister, and while rebasing on Rex-BC's
reset cleanup series I found mtk_clk_simple_probe/remove while
looking for references to mtk_register_reset_controller, so
I thought of using it for my drivers resulting in this series
adding support for the extra 4 clock types. I started finding
other things that could be improved such as the other clock types
not having register_*_with_dev(), but I had to avoid adding
anything else since that would only make me find more things to
improve and this series would've never been finished and sent.

With that said, if these patches could become an obstacle for
later more complete reworks, then by all means drop them.

> 
> Making sure that I don't get misunderstood:
>      I'm not implying that this huge migration work is on your 
> shoulders!
> 

Of course. I would never be able to handle such a large task.
Everyone currently helping with modernizing this common clock
framework has my full respect. You are doing amazing work.

Thanks,
Yassine




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel