Messages in this thread Patch in this message | | | From | Rex-BC Chen (陳柏辰) <> | Subject | Re: [PATCH v5] reset: mediatek: Move mediatek system clock reset to reset folder | Date | Fri, 21 Oct 2022 02:55:19 +0000 |
| |
On Wed, 2022-10-12 at 16:45 -0700, Stephen Boyd wrote: > Quoting Bo-Chen Chen (2022-10-07 03:48:42) > > To manager mediatek system clock reset easier, we move the driver > > to > > drivers/reset. > > > > - Create reset/mediatek folder. > > - Move clk/mediatek/reset.c to reset/mediatek/reset-mediatek- > > sysclk.c > > - Because we don't want to build in unsed static variable (For > > example, > > if we use mt8186, we don't want to build in the variable of > > MT8195.), > > we use clk KConfig to do this. > > - Move reset data which are scattered around the mediatek drivers > > to > > reset-mtxxxx.c. > > - For mtk clk drivers which support device, we can ues > > mtk_reset_controller_register() to register reset controller > > using > > auxiliary bus. > > - For mtk clk drivers which do not support device (only support > > device_node), we use mtk_reset_{init/remove}_with_node to > > register > > reset controller. > > > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com> > > [...] > > diff --git a/drivers/clk/mediatek/Kconfig > > b/drivers/clk/mediatek/Kconfig > > index 843cea0c7a44..bcd073ada0e9 100644 > > --- a/drivers/clk/mediatek/Kconfig > > +++ b/drivers/clk/mediatek/Kconfig > > @@ -8,6 +8,8 @@ menu "Clock driver for MediaTek SoC" > > config COMMON_CLK_MEDIATEK > > tristate > > select RESET_CONTROLLER > > + select RESET_MEDIATEK_SYSCLK > > Is this select necessary? >
Hello Stephen,
Thanks for your review. The system clock reset driver is original built with mediatek clk. Therefore, I think we should select this.
Please refer to drivers/clk/mediatek/Makefile
"obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o clk-mux.o"
> > + select AUXILIARY_BUS > > help > > MediaTek SoCs' clock support. > > > > diff --git a/drivers/clk/mediatek/clk-mtk.c > > b/drivers/clk/mediatek/clk-mtk.c > > index d31f01d0ba1c..61b7ee23738a 100644 > > --- a/drivers/clk/mediatek/clk-mtk.c > > +++ b/drivers/clk/mediatek/clk-mtk.c > > @@ -444,6 +444,63 @@ void mtk_clk_unregister_dividers(const struct > > mtk_clk_divider *mcds, int num, > > } > > EXPORT_SYMBOL_GPL(mtk_clk_unregister_dividers); > > > > +static void mtk_reset_unregister_adev(void *_adev) > > +{ > > + struct auxiliary_device *adev = _adev; > > + > > + auxiliary_device_delete(adev); > > +} > > + > > +static void mtk_reset_adev_release(struct device *dev) > > +{ > > + struct auxiliary_device *adev = to_auxiliary_dev(dev); > > + > > + auxiliary_device_uninit(adev); > > + > > + kfree(adev); > > +} > > + > > +static struct auxiliary_device *mtk_reset_adev_alloc(struct device > > *dev, const char *name) > > +{ > > + struct auxiliary_device *adev; > > + int ret; > > + > > + adev = kzalloc(sizeof(*adev), GFP_KERNEL); > > + if (!adev) > > + return ERR_PTR(-ENOMEM); > > + > > + adev->name = name; > > + adev->dev.parent = dev; > > + adev->dev.release = mtk_reset_adev_release; > > + > > + ret = auxiliary_device_init(adev); > > + if (ret) { > > + kfree(adev); > > + return ERR_PTR(ret); > > + } > > + > > + return adev; > > +} > > + > > +int mtk_reset_controller_register(struct device *dev, const char > > *name) > > +{ > > + struct auxiliary_device *adev; > > + int ret; > > + > > + adev = mtk_reset_adev_alloc(dev, name); > > + if (IS_ERR(adev)) > > + return PTR_ERR(adev); > > + > > + ret = auxiliary_device_add(adev); > > + if (ret) { > > + auxiliary_device_uninit(adev); > > + return ret; > > + } > > + > > + return devm_add_action_or_reset(dev, > > mtk_reset_unregister_adev, adev); > > +} > > +EXPORT_SYMBOL_GPL(mtk_reset_controller_register); > > + > > int mtk_clk_simple_probe(struct platform_device *pdev) > > { > > const struct mtk_clk_desc *mcd; > > @@ -470,9 +527,8 @@ 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 (mcd->rst_name) { > > + r = mtk_reset_controller_register(&pdev->dev, mcd- > > >rst_name); > > if (r) > > goto unregister_clks; > > } > > Can you split this part off to a different patch? It would make it > easier to review. And then you stack the reset part of the patch > after > this. >
OK, I will separate this part to another patch.
> > diff --git a/drivers/clk/mediatek/clk-mtk.h > > b/drivers/clk/mediatek/clk-mtk.h > > index 63ae7941aa92..9578643ef5a2 100644 > > --- a/drivers/clk/mediatek/clk-mtk.h > > +++ b/drivers/clk/mediatek/clk-mtk.h > > @@ -7,14 +7,14 @@ > > #ifndef __DRV_CLK_MTK_H > > #define __DRV_CLK_MTK_H > > > > +#include <linux/auxiliary_bus.h> > > #include <linux/clk-provider.h> > > #include <linux/io.h> > > #include <linux/kernel.h> > > +#include <linux/reset/reset-mediatek-sysclk.h> > > #include <linux/spinlock.h> > > #include <linux/types.h> > > > > -#include "reset.h" > > - > > #define MAX_MUX_GATE_BIT 31 > > #define INVALID_MUX_GATE_BIT (MAX_MUX_GATE_BIT + 1) > > > > @@ -195,10 +195,12 @@ void mtk_clk_unregister_ref2usb_tx(struct > > clk_hw *hw); > > struct mtk_clk_desc { > > const struct mtk_gate *clks; > > size_t num_clks; > > - const struct mtk_clk_rst_desc *rst_desc; > > + char *rst_name; > > }; > > > > int mtk_clk_simple_probe(struct platform_device *pdev); > > int mtk_clk_simple_remove(struct platform_device *pdev); > > > > +int mtk_reset_controller_register(struct device *dev, const char > > *name); > > + > > #endif /* __DRV_CLK_MTK_H */ > > diff --git a/drivers/clk/mediatek/reset.c > > b/drivers/clk/mediatek/reset.c > > deleted file mode 100644 > > index 290ceda84ce4..000000000000 > > --- a/drivers/clk/mediatek/reset.c > > +++ /dev/null > > This file could probably be deleted in yet another patch once nobody > is > using it.
I am not sure what do you mean. This series is for move reset.c to drivers/reset/mediatek. Should I not delete this file?
BRs, Bo-Chen
| |