| Message ID | 1546318276-18993-12-git-send-email-yong.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up "mediatek,larb" after adding device_link | expand |
On Mon, Dec 31, 2018 at 8:53 PM Yong Wu <yong.wu@mediatek.com> wrote: > > MediaTek IOMMU should wait for smi larb which need wait for the > power domain(mtk-scpsys.c) and the multimedia ccf who both are > module init. Thus, subsys_initcall for MediaTek IOMMU is not helpful. > Switch to builtin_platform_driver. > > Meanwhile, the ".remove" can be removed. Move its content to > ".shutdown". > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 23 ++--------------------- > drivers/iommu/mtk_iommu_v1.c | 16 ++-------------- > 2 files changed, 4 insertions(+), 35 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 735ae8d..2798b12 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -690,7 +690,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); > } > > -static int mtk_iommu_remove(struct platform_device *pdev) > +static void mtk_iommu_shutdown(struct platform_device *pdev) > { > struct mtk_iommu_data *data = platform_get_drvdata(pdev); > > @@ -703,12 +703,6 @@ static int mtk_iommu_remove(struct platform_device *pdev) > clk_disable_unprepare(data->bclk); > devm_free_irq(&pdev->dev, data->irq, data); > component_master_del(&pdev->dev, &mtk_iommu_com_ops); > - return 0; > -} > - > -static void mtk_iommu_shutdown(struct platform_device *pdev) > -{ > - mtk_iommu_remove(pdev); Is there a reason all these things are happening in shutdown()? Don't we normally just not clean things up and let the machine turn off? Normally I'm a big advocate of proper symmetric teardown, so it hurts a little to ask.
On Mon, 2019-02-25 at 15:56 -0800, Evan Green wrote: > On Mon, Dec 31, 2018 at 8:53 PM Yong Wu <yong.wu@mediatek.com> wrote: > > > > MediaTek IOMMU should wait for smi larb which need wait for the > > power domain(mtk-scpsys.c) and the multimedia ccf who both are > > module init. Thus, subsys_initcall for MediaTek IOMMU is not helpful. > > Switch to builtin_platform_driver. > > > > Meanwhile, the ".remove" can be removed. Move its content to > > ".shutdown". > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/iommu/mtk_iommu.c | 23 ++--------------------- > > drivers/iommu/mtk_iommu_v1.c | 16 ++-------------- > > 2 files changed, 4 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index 735ae8d..2798b12 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -690,7 +690,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); > > } > > > > -static int mtk_iommu_remove(struct platform_device *pdev) > > +static void mtk_iommu_shutdown(struct platform_device *pdev) > > { > > struct mtk_iommu_data *data = platform_get_drvdata(pdev); > > > > @@ -703,12 +703,6 @@ static int mtk_iommu_remove(struct platform_device *pdev) > > clk_disable_unprepare(data->bclk); > > devm_free_irq(&pdev->dev, data->irq, data); > > component_master_del(&pdev->dev, &mtk_iommu_com_ops); > > - return 0; > > -} > > - > > -static void mtk_iommu_shutdown(struct platform_device *pdev) > > -{ > > - mtk_iommu_remove(pdev); > > Is there a reason all these things are happening in shutdown()? Don't > we normally just not clean things up and let the machine turn off? > Normally I'm a big advocate of proper symmetric teardown, so it hurts > a little to ask. In this patch I change the driver to builtin driver. I guess the "_remove" is not helpful now. thus I remove it. About the shutdown, I don't have a special reason for it. I only want to mute the m4u log when something go wrong in the shutdown flow. > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Wed, Feb 27, 2019 at 6:33 AM Yong Wu <yong.wu@mediatek.com> wrote: > > On Mon, 2019-02-25 at 15:56 -0800, Evan Green wrote: > > On Mon, Dec 31, 2018 at 8:53 PM Yong Wu <yong.wu@mediatek.com> wrote: > > > > > > MediaTek IOMMU should wait for smi larb which need wait for the > > > power domain(mtk-scpsys.c) and the multimedia ccf who both are > > > module init. Thus, subsys_initcall for MediaTek IOMMU is not helpful. > > > Switch to builtin_platform_driver. > > > > > > Meanwhile, the ".remove" can be removed. Move its content to > > > ".shutdown". > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > --- > > > drivers/iommu/mtk_iommu.c | 23 ++--------------------- > > > drivers/iommu/mtk_iommu_v1.c | 16 ++-------------- > > > 2 files changed, 4 insertions(+), 35 deletions(-) > > > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > > index 735ae8d..2798b12 100644 > > > --- a/drivers/iommu/mtk_iommu.c > > > +++ b/drivers/iommu/mtk_iommu.c > > > @@ -690,7 +690,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > > return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); > > > } > > > > > > -static int mtk_iommu_remove(struct platform_device *pdev) > > > +static void mtk_iommu_shutdown(struct platform_device *pdev) > > > { > > > struct mtk_iommu_data *data = platform_get_drvdata(pdev); > > > > > > @@ -703,12 +703,6 @@ static int mtk_iommu_remove(struct platform_device *pdev) > > > clk_disable_unprepare(data->bclk); > > > devm_free_irq(&pdev->dev, data->irq, data); > > > component_master_del(&pdev->dev, &mtk_iommu_com_ops); > > > - return 0; > > > -} > > > - > > > -static void mtk_iommu_shutdown(struct platform_device *pdev) > > > -{ > > > - mtk_iommu_remove(pdev); > > > > Is there a reason all these things are happening in shutdown()? Don't > > we normally just not clean things up and let the machine turn off? > > Normally I'm a big advocate of proper symmetric teardown, so it hurts > > a little to ask. > > In this patch I change the driver to builtin driver. I guess the > "_remove" is not helpful now. thus I remove it. > > About the shutdown, I don't have a special reason for it. I only want to > mute the m4u log when something go wrong in the shutdown flow. > Wouldn't you want to see the logs when something goes wrong in shutdown? Or is there a lot of noise that gets generated if you don't do this during a graceful shutdown? I guess if there's some real reason you're doing all this stuff in shutdown then it's fine, but if it was just a default shuttling of code from remove to shutdown because remove was going away then it could be deleted. -Evan
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index 735ae8d..2798b12 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -690,7 +690,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); } -static int mtk_iommu_remove(struct platform_device *pdev) +static void mtk_iommu_shutdown(struct platform_device *pdev) { struct mtk_iommu_data *data = platform_get_drvdata(pdev); @@ -703,12 +703,6 @@ static int mtk_iommu_remove(struct platform_device *pdev) clk_disable_unprepare(data->bclk); devm_free_irq(&pdev->dev, data->irq, data); component_master_del(&pdev->dev, &mtk_iommu_com_ops); - return 0; -} - -static void mtk_iommu_shutdown(struct platform_device *pdev) -{ - mtk_iommu_remove(pdev); } static int __maybe_unused mtk_iommu_suspend(struct device *dev) @@ -791,7 +785,6 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev) static struct platform_driver mtk_iommu_driver = { .probe = mtk_iommu_probe, - .remove = mtk_iommu_remove, .shutdown = mtk_iommu_shutdown, .driver = { .name = "mtk-iommu", @@ -799,16 +792,4 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev) .pm = &mtk_iommu_pm_ops, } }; - -static int __init mtk_iommu_init(void) -{ - int ret; - - ret = platform_driver_register(&mtk_iommu_driver); - if (ret != 0) - pr_err("Failed to register MTK IOMMU driver\n"); - - return ret; -} - -subsys_initcall(mtk_iommu_init) +builtin_platform_driver(mtk_iommu_driver); diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c index 022bad9..5464918 100644 --- a/drivers/iommu/mtk_iommu_v1.c +++ b/drivers/iommu/mtk_iommu_v1.c @@ -648,7 +648,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); } -static int mtk_iommu_remove(struct platform_device *pdev) +static void mtk_iommu_shutdown(struct platform_device *pdev) { struct mtk_iommu_data *data = platform_get_drvdata(pdev); @@ -661,12 +661,6 @@ static int mtk_iommu_remove(struct platform_device *pdev) clk_disable_unprepare(data->bclk); devm_free_irq(&pdev->dev, data->irq, data); component_master_del(&pdev->dev, &mtk_iommu_com_ops); - return 0; -} - -static void mtk_iommu_shutdown(struct platform_device *pdev) -{ - mtk_iommu_remove(pdev); } static int __maybe_unused mtk_iommu_suspend(struct device *dev) @@ -705,7 +699,6 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev) static struct platform_driver mtk_iommu_driver = { .probe = mtk_iommu_probe, - .remove = mtk_iommu_remove, .shutdown = mtk_iommu_shutdown, .driver = { .name = "mtk-iommu-v1", @@ -713,9 +706,4 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev) .pm = &mtk_iommu_pm_ops, } }; - -static int __init m4u_init(void) -{ - return platform_driver_register(&mtk_iommu_driver); -} -subsys_initcall(m4u_init); +builtin_platform_driver(mtk_iommu_driver);
MediaTek IOMMU should wait for smi larb which need wait for the power domain(mtk-scpsys.c) and the multimedia ccf who both are module init. Thus, subsys_initcall for MediaTek IOMMU is not helpful. Switch to builtin_platform_driver. Meanwhile, the ".remove" can be removed. Move its content to ".shutdown". Signed-off-by: Yong Wu <yong.wu@mediatek.com> --- drivers/iommu/mtk_iommu.c | 23 ++--------------------- drivers/iommu/mtk_iommu_v1.c | 16 ++-------------- 2 files changed, 4 insertions(+), 35 deletions(-)