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/[email protected]/
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Export required symbols to compile clk drivers as module (single patch)
https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
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
From: Yassine Oudjana <[email protected]>
This allows it to be used in drivers built as modules.
Signed-off-by: Yassine Oudjana <[email protected]>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Export required symbols to compile clk drivers as module (single patch)
https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
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
From: Yassine Oudjana <[email protected]>
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 <[email protected]>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Export required symbols to compile clk drivers as module (single patch)
https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
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
From: Yassine Oudjana <[email protected]>
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 <[email protected]>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Export required symbols to compile clk drivers as module (single patch)
https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
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
On Thu, May 19, 2022 at 9:50 PM Yassine Oudjana
<[email protected]> wrote:
>
> From: Yassine Oudjana <[email protected]>
>
> 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 <[email protected]>
> ---
> Dependencies:
> - clk: mediatek: Move to struct clk_hw provider APIs (series)
> https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
> https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
> - Export required symbols to compile clk drivers as module (single patch)
> https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
>
> 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
>
From: Yassine Oudjana <[email protected]>
Register gates with dev in mtk_clk_simple_probe.
Signed-off-by: Yassine Oudjana <[email protected]>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Export required symbols to compile clk drivers as module (single patch)
https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
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
From: Yassine Oudjana <[email protected]>
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 <[email protected]>
---
Dependencies:
- clk: mediatek: Move to struct clk_hw provider APIs (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Cleanup MediaTek clk reset drivers and support MT8192/MT8195 (series)
https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
- Export required symbols to compile clk drivers as module (single patch)
https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
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
Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <[email protected]>
>
> 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 <[email protected]>
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
On Fri, May 20 2022 at 10:42:40 +0200, AngeloGioacchino Del Regno
<[email protected]> wrote:
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <[email protected]>
>>
>> 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 <[email protected]>
>
> 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
On Thu, May 19 2022 at 17:47:22 +0400, Yassine Oudjana
<[email protected]> 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/[email protected]/
> - Cleanup MediaTek clk reset drivers and support MT8192/MT8195
> (series)
>
> https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
> - Export required symbols to compile clk drivers as module (single
> patch)
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
>
> 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/[email protected]/
Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <[email protected]>
>
> This allows it to be used in drivers built as modules.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
Thumbs up!
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
On Thu, May 19, 2022 at 9:49 PM Yassine Oudjana
<[email protected]> wrote:
>
> From: Yassine Oudjana <[email protected]>
>
> This allows it to be used in drivers built as modules.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Fri, May 20, 2022 at 4:42 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 19/05/22 15:47, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <[email protected]>
> >
> > 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 <[email protected]>
>
> 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