Mailing List Archive

[PATCH v7 05/19] clk: mediatek: reset: Merge and revise reset register function
There are two versions for clock reset register control for MediaTek
SoCs. The old hardware is one bit per reset control, and does not
have separate registers for bit set, clear and read-back operations.
This matches the scheme supported by the simple reset driver.

However, because we need to use different data structure from
reset_simple_data, we can not use the operation of simple reset
driver.
For this reason, we keep the original functions and name this version
as "MTK_RST_SIMPLE".

In this patch:
- Add a version enumeration to separate different reset hardware.
- Merge the reset register function of simple and set_clr into one
function "mtk_register_reset_controller".
- Rename input variable "num_regs" to "rst_bank_nr" to avoid
confusion. This variable is used to define the quantity of reset bank.
- Document mtk_reset_version and mtk_register_reset_controller.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/clk/mediatek/clk-mt2701-eth.c | 2 +-
drivers/clk/mediatek/clk-mt2701-g3d.c | 2 +-
drivers/clk/mediatek/clk-mt2701-hif.c | 2 +-
drivers/clk/mediatek/clk-mt2701.c | 4 +--
drivers/clk/mediatek/clk-mt2712.c | 4 +--
drivers/clk/mediatek/clk-mt7622-eth.c | 2 +-
drivers/clk/mediatek/clk-mt7622-hif.c | 4 +--
drivers/clk/mediatek/clk-mt7622.c | 4 +--
drivers/clk/mediatek/clk-mt7629-eth.c | 2 +-
drivers/clk/mediatek/clk-mt7629-hif.c | 4 +--
drivers/clk/mediatek/clk-mt8135.c | 4 +--
drivers/clk/mediatek/clk-mt8173.c | 4 +--
drivers/clk/mediatek/clk-mt8183.c | 3 +-
drivers/clk/mediatek/reset.c | 40 ++++++++++++---------------
drivers/clk/mediatek/reset.h | 24 +++++++++++++---
15 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt2701-eth.c b/drivers/clk/mediatek/clk-mt2701-eth.c
index 47c2289f3d1d..579343e0bba6 100644
--- a/drivers/clk/mediatek/clk-mt2701-eth.c
+++ b/drivers/clk/mediatek/clk-mt2701-eth.c
@@ -58,7 +58,7 @@ static int clk_mt2701_eth_probe(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt2701-g3d.c b/drivers/clk/mediatek/clk-mt2701-g3d.c
index 79929ed37f83..bd77ed25b101 100644
--- a/drivers/clk/mediatek/clk-mt2701-g3d.c
+++ b/drivers/clk/mediatek/clk-mt2701-g3d.c
@@ -52,7 +52,7 @@ static int clk_mt2701_g3dsys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0xc);
+ mtk_register_reset_controller(node, 1, 0xc, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt2701-hif.c b/drivers/clk/mediatek/clk-mt2701-hif.c
index 1aa36cb93ad0..ae3849b58db8 100644
--- a/drivers/clk/mediatek/clk-mt2701-hif.c
+++ b/drivers/clk/mediatek/clk-mt2701-hif.c
@@ -57,7 +57,7 @@ static int clk_mt2701_hif_probe(struct platform_device *pdev)
return r;
}

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return 0;
}
diff --git a/drivers/clk/mediatek/clk-mt2701.c b/drivers/clk/mediatek/clk-mt2701.c
index 7a4331057adc..081ccdd30bf2 100644
--- a/drivers/clk/mediatek/clk-mt2701.c
+++ b/drivers/clk/mediatek/clk-mt2701.c
@@ -787,7 +787,7 @@ static int mtk_infrasys_init(struct platform_device *pdev)
if (r)
return r;

- mtk_register_reset_controller(node, 2, 0x30);
+ mtk_register_reset_controller(node, 2, 0x30, MTK_RST_SIMPLE);

return 0;
}
@@ -910,7 +910,7 @@ static int mtk_pericfg_init(struct platform_device *pdev)
if (r)
return r;

- mtk_register_reset_controller(node, 2, 0x0);
+ mtk_register_reset_controller(node, 2, 0x0, MTK_RST_SIMPLE);

return 0;
}
diff --git a/drivers/clk/mediatek/clk-mt2712.c b/drivers/clk/mediatek/clk-mt2712.c
index 1dd90d9a70ac..60eadf78e79c 100644
--- a/drivers/clk/mediatek/clk-mt2712.c
+++ b/drivers/clk/mediatek/clk-mt2712.c
@@ -1361,7 +1361,7 @@ static int clk_mt2712_infra_probe(struct platform_device *pdev)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);

- mtk_register_reset_controller(node, 2, 0x30);
+ mtk_register_reset_controller(node, 2, 0x30, MTK_RST_SIMPLE);

return r;
}
@@ -1383,7 +1383,7 @@ static int clk_mt2712_peri_probe(struct platform_device *pdev)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);

- mtk_register_reset_controller(node, 2, 0);
+ mtk_register_reset_controller(node, 2, 0, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt7622-eth.c b/drivers/clk/mediatek/clk-mt7622-eth.c
index b12d48705496..424105b91b7d 100644
--- a/drivers/clk/mediatek/clk-mt7622-eth.c
+++ b/drivers/clk/mediatek/clk-mt7622-eth.c
@@ -82,7 +82,7 @@ static int clk_mt7622_ethsys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt7622-hif.c b/drivers/clk/mediatek/clk-mt7622-hif.c
index 58728e35e80a..88bccf595ee4 100644
--- a/drivers/clk/mediatek/clk-mt7622-hif.c
+++ b/drivers/clk/mediatek/clk-mt7622-hif.c
@@ -93,7 +93,7 @@ static int clk_mt7622_ssusbsys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
@@ -115,7 +115,7 @@ static int clk_mt7622_pciesys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt7622.c b/drivers/clk/mediatek/clk-mt7622.c
index a110ee2b5ea6..2880cddf0722 100644
--- a/drivers/clk/mediatek/clk-mt7622.c
+++ b/drivers/clk/mediatek/clk-mt7622.c
@@ -663,7 +663,7 @@ static int mtk_infrasys_init(struct platform_device *pdev)
if (r)
return r;

- mtk_register_reset_controller(node, 1, 0x30);
+ mtk_register_reset_controller(node, 1, 0x30, MTK_RST_SIMPLE);

return 0;
}
@@ -714,7 +714,7 @@ static int mtk_pericfg_init(struct platform_device *pdev)

clk_prepare_enable(clk_data->hws[CLK_PERI_UART0_PD]->clk);

- mtk_register_reset_controller(node, 2, 0x0);
+ mtk_register_reset_controller(node, 2, 0x0, MTK_RST_SIMPLE);

return 0;
}
diff --git a/drivers/clk/mediatek/clk-mt7629-eth.c b/drivers/clk/mediatek/clk-mt7629-eth.c
index c49fd732c9b2..ba2548ef4031 100644
--- a/drivers/clk/mediatek/clk-mt7629-eth.c
+++ b/drivers/clk/mediatek/clk-mt7629-eth.c
@@ -92,7 +92,7 @@ static int clk_mt7629_ethsys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt7629-hif.c b/drivers/clk/mediatek/clk-mt7629-hif.c
index acaa97fda331..5863ac799b70 100644
--- a/drivers/clk/mediatek/clk-mt7629-hif.c
+++ b/drivers/clk/mediatek/clk-mt7629-hif.c
@@ -88,7 +88,7 @@ static int clk_mt7629_ssusbsys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
@@ -110,7 +110,7 @@ static int clk_mt7629_pciesys_init(struct platform_device *pdev)
"could not register clock provider: %s: %d\n",
pdev->name, r);

- mtk_register_reset_controller(node, 1, 0x34);
+ mtk_register_reset_controller(node, 1, 0x34, MTK_RST_SIMPLE);

return r;
}
diff --git a/drivers/clk/mediatek/clk-mt8135.c b/drivers/clk/mediatek/clk-mt8135.c
index 19827037b6c0..fd6201f059b6 100644
--- a/drivers/clk/mediatek/clk-mt8135.c
+++ b/drivers/clk/mediatek/clk-mt8135.c
@@ -559,7 +559,7 @@ static void __init mtk_infrasys_init(struct device_node *node)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);

- mtk_register_reset_controller(node, 2, 0x30);
+ mtk_register_reset_controller(node, 2, 0x30, MTK_RST_SIMPLE);
}
CLK_OF_DECLARE(mtk_infrasys, "mediatek,mt8135-infracfg", mtk_infrasys_init);

@@ -587,7 +587,7 @@ static void __init mtk_pericfg_init(struct device_node *node)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);

- mtk_register_reset_controller(node, 2, 0);
+ mtk_register_reset_controller(node, 2, 0, MTK_RST_SIMPLE);
}
CLK_OF_DECLARE(mtk_pericfg, "mediatek,mt8135-pericfg", mtk_pericfg_init);

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index d34b248c42ca..cf46df2d1fdd 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -882,7 +882,7 @@ static void __init mtk_infrasys_init(struct device_node *node)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);

- mtk_register_reset_controller(node, 2, 0x30);
+ mtk_register_reset_controller(node, 2, 0x30, MTK_RST_SIMPLE);
}
CLK_OF_DECLARE(mtk_infrasys, "mediatek,mt8173-infracfg", mtk_infrasys_init);

@@ -910,7 +910,7 @@ static void __init mtk_pericfg_init(struct device_node *node)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);

- mtk_register_reset_controller(node, 2, 0);
+ mtk_register_reset_controller(node, 2, 0, MTK_RST_SIMPLE);
}
CLK_OF_DECLARE(mtk_pericfg, "mediatek,mt8173-pericfg", mtk_pericfg_init);

diff --git a/drivers/clk/mediatek/clk-mt8183.c b/drivers/clk/mediatek/clk-mt8183.c
index 8a755fadebb5..7fc78ed4e569 100644
--- a/drivers/clk/mediatek/clk-mt8183.c
+++ b/drivers/clk/mediatek/clk-mt8183.c
@@ -1240,7 +1240,8 @@ static int clk_mt8183_infra_probe(struct platform_device *pdev)
return r;
}

- mtk_register_reset_controller_set_clr(node, 4, INFRA_RST0_SET_OFFSET);
+ mtk_register_reset_controller(node, 4,
+ INFRA_RST0_SET_OFFSET, MTK_RST_SET_CLR);

return r;
}
diff --git a/drivers/clk/mediatek/reset.c b/drivers/clk/mediatek/reset.c
index 22fa9f09752c..a54a835c1d47 100644
--- a/drivers/clk/mediatek/reset.c
+++ b/drivers/clk/mediatek/reset.c
@@ -92,14 +92,25 @@ static const struct reset_control_ops mtk_reset_ops_set_clr = {
.reset = mtk_reset_set_clr,
};

-static void mtk_register_reset_controller_common(struct device_node *np,
- unsigned int num_regs,
- int regofs,
- const struct reset_control_ops *reset_ops)
+void mtk_register_reset_controller(struct device_node *np,
+ u32 rst_bank_nr, u16 reg_ofs, u8 version)
{
struct mtk_reset *data;
int ret;
struct regmap *regmap;
+ const struct reset_control_ops *rcops = NULL;
+
+ switch (version) {
+ case MTK_RST_SIMPLE:
+ rcops = &mtk_reset_ops;
+ break;
+ case MTK_RST_SET_CLR:
+ rcops = &mtk_reset_ops_set_clr;
+ break;
+ default:
+ pr_err("Unknown reset version %d\n", version);
+ return;
+ }

regmap = device_node_to_regmap(np);
if (IS_ERR(regmap)) {
@@ -112,32 +123,17 @@ static void mtk_register_reset_controller_common(struct device_node *np,
return;

data->regmap = regmap;
- data->regofs = regofs;
+ data->regofs = reg_ofs;
data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = num_regs * 32;
- data->rcdev.ops = reset_ops;
+ data->rcdev.nr_resets = rst_bank_nr * 32;
+ data->rcdev.ops = rcops;
data->rcdev.of_node = np;

ret = reset_controller_register(&data->rcdev);
if (ret) {
pr_err("could not register reset controller: %d\n", ret);
kfree(data);
- return;
}
}

-void mtk_register_reset_controller(struct device_node *np,
- unsigned int num_regs, int regofs)
-{
- mtk_register_reset_controller_common(np, num_regs, regofs,
- &mtk_reset_ops);
-}
-
-void mtk_register_reset_controller_set_clr(struct device_node *np,
- unsigned int num_regs, int regofs)
-{
- mtk_register_reset_controller_common(np, num_regs, regofs,
- &mtk_reset_ops_set_clr);
-}
-
MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/reset.h b/drivers/clk/mediatek/reset.h
index 764a8affe206..2a39eec9cff7 100644
--- a/drivers/clk/mediatek/reset.h
+++ b/drivers/clk/mediatek/reset.h
@@ -9,16 +9,32 @@
#include <linux/reset-controller.h>
#include <linux/types.h>

+/**
+ * enum mtk_reset_version - Version of MediaTek clock reset controller.
+ * @MTK_RST_SIMPLE: Use the same registers for bit set and clear.
+ * @MTK_RST_SET_CLR: Use separate registers for bit set and clear.
+ * @MTK_RST_MAX: Total quantity of version for MediaTek clock reset controller.
+ */
+enum mtk_reset_version {
+ MTK_RST_SIMPLE = 0,
+ MTK_RST_SET_CLR,
+ MTK_RST_MAX,
+};
+
struct mtk_reset {
struct regmap *regmap;
int regofs;
struct reset_controller_dev rcdev;
};

+/**
+ * mtk_register_reset_controller - Register MediaTek clock reset controller
+ * @np: Pointer to device node.
+ * @rst_bank_nr: Quantity of reset bank.
+ * @reg_ofs: Base offset of the reset register.
+ * @version: Version of MediaTek clock reset controller.
+ */
void mtk_register_reset_controller(struct device_node *np,
- unsigned int num_regs, int regofs);
-
-void mtk_register_reset_controller_set_clr(struct device_node *np,
- unsigned int num_regs, int regofs);
+ u32 rst_bank_nr, u16 reg_ofs, u8 version);

#endif /* __DRV_CLK_MTK_RESET_H */
--
2.18.0
Re: [PATCH v7 05/19] clk: mediatek: reset: Merge and revise reset register function [ In reply to ]
Hi Rex,

On Thu, May 19, 2022 at 08:55:13PM +0800, Rex-BC Chen wrote:
> There are two versions for clock reset register control for MediaTek
> SoCs. The old hardware is one bit per reset control, and does not
> have separate registers for bit set, clear and read-back operations.
> This matches the scheme supported by the simple reset driver.
>
> However, because we need to use different data structure from
> reset_simple_data, we can not use the operation of simple reset
> driver.
> For this reason, we keep the original functions and name this version
> as "MTK_RST_SIMPLE".
>
> In this patch:
> - Add a version enumeration to separate different reset hardware.
> - Merge the reset register function of simple and set_clr into one
> function "mtk_register_reset_controller".
> - Rename input variable "num_regs" to "rst_bank_nr" to avoid
> confusion. This variable is used to define the quantity of reset bank.
> - Document mtk_reset_version and mtk_register_reset_controller.
>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---

<snip>

> index 764a8affe206..2a39eec9cff7 100644
> --- a/drivers/clk/mediatek/reset.h
> +++ b/drivers/clk/mediatek/reset.h
> @@ -9,16 +9,32 @@
> #include <linux/reset-controller.h>
> #include <linux/types.h>
>
> +/**
> + * enum mtk_reset_version - Version of MediaTek clock reset controller.
> + * @MTK_RST_SIMPLE: Use the same registers for bit set and clear.
> + * @MTK_RST_SET_CLR: Use separate registers for bit set and clear.
> + * @MTK_RST_MAX: Total quantity of version for MediaTek clock reset controller.
> + */
> +enum mtk_reset_version {
> + MTK_RST_SIMPLE = 0,
> + MTK_RST_SET_CLR,
> + MTK_RST_MAX,
> +};
> +
> struct mtk_reset {
> struct regmap *regmap;
> int regofs;
> struct reset_controller_dev rcdev;
> };
>
> +/**
> + * mtk_register_reset_controller - Register MediaTek clock reset controller
> + * @np: Pointer to device node.
> + * @rst_bank_nr: Quantity of reset bank.
> + * @reg_ofs: Base offset of the reset register.
> + * @version: Version of MediaTek clock reset controller.
> + */
> void mtk_register_reset_controller(struct device_node *np,
> - unsigned int num_regs, int regofs);
> -
> -void mtk_register_reset_controller_set_clr(struct device_node *np,
> - unsigned int num_regs, int regofs);
> + u32 rst_bank_nr, u16 reg_ofs, u8 version);

Why not use 'enum mtk_reset_version' instead of a generic u8? Same thing when
you move it to a struct in patch 6.

Thanks,
N?colas
Re: [PATCH v7 05/19] clk: mediatek: reset: Merge and revise reset register function [ In reply to ]
On Fri, 2022-05-20 at 11:12 -0400, Nícolas F. R. A. Prado wrote:
> Hi Rex,
>
> On Thu, May 19, 2022 at 08:55:13PM +0800, Rex-BC Chen wrote:
> > There are two versions for clock reset register control for
> > MediaTek
> > SoCs. The old hardware is one bit per reset control, and does not
> > have separate registers for bit set, clear and read-back
> > operations.
> > This matches the scheme supported by the simple reset driver.
> >
> > However, because we need to use different data structure from
> > reset_simple_data, we can not use the operation of simple reset
> > driver.
> > For this reason, we keep the original functions and name this
> > version
> > as "MTK_RST_SIMPLE".
> >
> > In this patch:
> > - Add a version enumeration to separate different reset hardware.
> > - Merge the reset register function of simple and set_clr into one
> > function "mtk_register_reset_controller".
> > - Rename input variable "num_regs" to "rst_bank_nr" to avoid
> > confusion. This variable is used to define the quantity of reset
> > bank.
> > - Document mtk_reset_version and mtk_register_reset_controller.
> >
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
>
> <snip>
>
> > index 764a8affe206..2a39eec9cff7 100644
> > --- a/drivers/clk/mediatek/reset.h
> > +++ b/drivers/clk/mediatek/reset.h
> > @@ -9,16 +9,32 @@
> > #include <linux/reset-controller.h>
> > #include <linux/types.h>
> >
> > +/**
> > + * enum mtk_reset_version - Version of MediaTek clock reset
> > controller.
> > + * @MTK_RST_SIMPLE: Use the same registers for bit set and clear.
> > + * @MTK_RST_SET_CLR: Use separate registers for bit set and clear.
> > + * @MTK_RST_MAX: Total quantity of version for MediaTek clock
> > reset controller.
> > + */
> > +enum mtk_reset_version {
> > + MTK_RST_SIMPLE = 0,
> > + MTK_RST_SET_CLR,
> > + MTK_RST_MAX,
> > +};
> > +
> > struct mtk_reset {
> > struct regmap *regmap;
> > int regofs;
> > struct reset_controller_dev rcdev;
> > };
> >
> > +/**
> > + * mtk_register_reset_controller - Register MediaTek clock reset
> > controller
> > + * @np: Pointer to device node.
> > + * @rst_bank_nr: Quantity of reset bank.
> > + * @reg_ofs: Base offset of the reset register.
> > + * @version: Version of MediaTek clock reset controller.
> > + */
> > void mtk_register_reset_controller(struct device_node *np,
> > - unsigned int num_regs, int regofs);
> > -
> > -void mtk_register_reset_controller_set_clr(struct device_node *np,
> > - unsigned int num_regs, int
> > regofs);
> > + u32 rst_bank_nr, u16 reg_ofs, u8
> > version);
>
> Why not use 'enum mtk_reset_version' instead of a generic u8? Same
> thing when
> you move it to a struct in patch 6.
>
> Thanks,
> Nícolas

HEllo Nícolas,

Thanks for the review.
I will modify for both patch 5 and 6 for this in next version.

BRs,
Rex