Message ID | 20220519125527.18544-6-rex-bc.chen@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Cleanup MediaTek clk reset drivers and support SoCs | expand |
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
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
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 */