| Message ID | 20240312102318.2285165-8-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | coresight: Move remaining AMBA ACPI devices into platform driver | expand |
On 12/03/2024 10:23, Anshuman Khandual wrote: > Add support for the catu devices in a new platform driver, which can then > be used on ACPI based platforms. This change would now allow runtime power > management for ACPI based systems. The driver would try to enable the APB > clock if available. But first this renames and then refactors catu_probe() > and catu_remove(), making sure it can be used both for platform and AMBA > drivers. This also moves pm_runtime_put() from catu_probe() to the callers. > > Cc: Lorenzo Pieralisi <lpieralisi@kernel.org> > Cc: Sudeep Holla <sudeep.holla@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: James Clark <james.clark@arm.com> > Cc: linux-acpi@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Cc: coresight@lists.linaro.org > Acked-by: Sudeep Holla <sudeep.holla@arm.com> # For ACPI related changes > Reviewed-by: James Clark <james.clark@arm.com> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > Changes in V6: > > - Added clk_put() for pclk in catu_platform_probe() error path > - Added WARN_ON(!drvdata) check in catu_platform_remove() > - Added additional elements for acpi_device_id[] > > drivers/acpi/arm64/amba.c | 1 - > drivers/hwtracing/coresight/coresight-catu.c | 146 ++++++++++++++++--- > drivers/hwtracing/coresight/coresight-catu.h | 1 + > 3 files changed, 125 insertions(+), 23 deletions(-) > > diff --git a/drivers/acpi/arm64/amba.c b/drivers/acpi/arm64/amba.c > index afb6afb66967..587061b0fd2f 100644 > --- a/drivers/acpi/arm64/amba.c > +++ b/drivers/acpi/arm64/amba.c > @@ -27,7 +27,6 @@ static const struct acpi_device_id amba_id_list[] = { > {"ARMHC503", 0}, /* ARM CoreSight Debug */ > {"ARMHC979", 0}, /* ARM CoreSight TPIU */ > {"ARMHC97C", 0}, /* ARM CoreSight SoC-400 TMC, SoC-600 ETF/ETB */ > - {"ARMHC9CA", 0}, /* ARM CoreSight CATU */ > {"", 0}, > }; > > diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c > index 3949ded0d4fa..8fa035e5a0e8 100644 > --- a/drivers/hwtracing/coresight/coresight-catu.c > +++ b/drivers/hwtracing/coresight/coresight-catu.c > @@ -7,6 +7,8 @@ > * Author: Suzuki K Poulose <suzuki.poulose@arm.com> > */ > > +#include <linux/platform_device.h> > +#include <linux/acpi.h> minor nit: Please retain the alphabetic order. > #include <linux/amba/bus.h> > #include <linux/device.h> > #include <linux/dma-mapping.h> > @@ -502,31 +504,25 @@ static const struct coresight_ops catu_ops = { > .helper_ops = &catu_helper_ops, > }; > > -static int catu_probe(struct amba_device *adev, const struct amba_id *id) > +static int __catu_probe(struct device *dev, struct resource *res) > { > int ret = 0; > u32 dma_mask; > - struct catu_drvdata *drvdata; > + struct catu_drvdata *drvdata = dev_get_drvdata(dev); > struct coresight_desc catu_desc; > struct coresight_platform_data *pdata = NULL; > - struct device *dev = &adev->dev; > void __iomem *base; > > catu_desc.name = coresight_alloc_device_name(&catu_devs, dev); > if (!catu_desc.name) > return -ENOMEM; > > - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > - if (!drvdata) { > - ret = -ENOMEM; > - goto out; > - } > - > - dev_set_drvdata(dev, drvdata); > - base = devm_ioremap_resource(dev, &adev->res); > - if (IS_ERR(base)) { > - ret = PTR_ERR(base); > - goto out; > + if (res) { We don't have a case where res == NULL and we shouldn't support that. > + base = devm_ioremap_resource(dev, res); > + if (IS_ERR(base)) { > + ret = PTR_ERR(base); > + goto out; > + } > } > > /* Setup dma mask for the device */ > @@ -567,19 +563,39 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) > drvdata->csdev = coresight_register(&catu_desc); > if (IS_ERR(drvdata->csdev)) > ret = PTR_ERR(drvdata->csdev); > - else > - pm_runtime_put(&adev->dev); > out: > return ret; > } > > -static void catu_remove(struct amba_device *adev) > +static int catu_probe(struct amba_device *adev, const struct amba_id *id) > +{ > + struct catu_drvdata *drvdata; > + int ret; > + > + drvdata = devm_kzalloc(&adev->dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + amba_set_drvdata(adev, drvdata); > + ret = __catu_probe(&adev->dev, &adev->res); > + if (!ret) > + pm_runtime_put(&adev->dev); > + > + return ret; > +} > + > +static void __catu_remove(struct device *dev) > { > - struct catu_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + struct catu_drvdata *drvdata = dev_get_drvdata(dev); > > coresight_unregister(drvdata->csdev); > } > > +static void catu_remove(struct amba_device *adev) > +{ > + __catu_remove(&adev->dev); > +} > + > static struct amba_id catu_ids[] = { > CS_AMBA_ID(0x000bb9ee), > {}, > @@ -598,13 +614,99 @@ static struct amba_driver catu_driver = { > .id_table = catu_ids, > }; > > +static int catu_platform_probe(struct platform_device *pdev) > +{ > + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + struct catu_drvdata *drvdata; > + int ret = 0; > + > + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); > + if (IS_ERR(drvdata->pclk)) > + return -ENODEV; > + > + pm_runtime_get_noresume(&pdev->dev); > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + > + dev_set_drvdata(&pdev->dev, drvdata); > + ret = __catu_probe(&pdev->dev, res); > + pm_runtime_put(&pdev->dev); > + if (ret) { > + pm_runtime_disable(&pdev->dev); > + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) Here drvdata is guaranteed to be valid and the check is not required. > + clk_put(drvdata->pclk); > + } > + > + return ret; > +} > + > +static int catu_platform_remove(struct platform_device *pdev) > +{ > + struct catu_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > + > + if (WARN_ON(!drvdata)) > + return -ENODEV; > + > + __catu_remove(&pdev->dev); > + pm_runtime_disable(&pdev->dev); > + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) Same here. Rest looks fine. Suzuki
On 3/12/24 20:35, Suzuki K Poulose wrote: > On 12/03/2024 10:23, Anshuman Khandual wrote: >> Add support for the catu devices in a new platform driver, which can then >> be used on ACPI based platforms. This change would now allow runtime power >> management for ACPI based systems. The driver would try to enable the APB >> clock if available. But first this renames and then refactors catu_probe() >> and catu_remove(), making sure it can be used both for platform and AMBA >> drivers. This also moves pm_runtime_put() from catu_probe() to the callers. >> >> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org> >> Cc: Sudeep Holla <sudeep.holla@arm.com> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >> Cc: Mike Leach <mike.leach@linaro.org> >> Cc: James Clark <james.clark@arm.com> >> Cc: linux-acpi@vger.kernel.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Cc: coresight@lists.linaro.org >> Acked-by: Sudeep Holla <sudeep.holla@arm.com> # For ACPI related changes >> Reviewed-by: James Clark <james.clark@arm.com> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> Changes in V6: >> >> - Added clk_put() for pclk in catu_platform_probe() error path >> - Added WARN_ON(!drvdata) check in catu_platform_remove() >> - Added additional elements for acpi_device_id[] >> >> drivers/acpi/arm64/amba.c | 1 - >> drivers/hwtracing/coresight/coresight-catu.c | 146 ++++++++++++++++--- >> drivers/hwtracing/coresight/coresight-catu.h | 1 + >> 3 files changed, 125 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/acpi/arm64/amba.c b/drivers/acpi/arm64/amba.c >> index afb6afb66967..587061b0fd2f 100644 >> --- a/drivers/acpi/arm64/amba.c >> +++ b/drivers/acpi/arm64/amba.c >> @@ -27,7 +27,6 @@ static const struct acpi_device_id amba_id_list[] = { >> {"ARMHC503", 0}, /* ARM CoreSight Debug */ >> {"ARMHC979", 0}, /* ARM CoreSight TPIU */ >> {"ARMHC97C", 0}, /* ARM CoreSight SoC-400 TMC, SoC-600 ETF/ETB */ >> - {"ARMHC9CA", 0}, /* ARM CoreSight CATU */ >> {"", 0}, >> }; >> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c >> index 3949ded0d4fa..8fa035e5a0e8 100644 >> --- a/drivers/hwtracing/coresight/coresight-catu.c >> +++ b/drivers/hwtracing/coresight/coresight-catu.c >> @@ -7,6 +7,8 @@ >> * Author: Suzuki K Poulose <suzuki.poulose@arm.com> >> */ >> +#include <linux/platform_device.h> >> +#include <linux/acpi.h> > > minor nit: Please retain the alphabetic order. Sure, will do that. > >> #include <linux/amba/bus.h> >> #include <linux/device.h> >> #include <linux/dma-mapping.h> >> @@ -502,31 +504,25 @@ static const struct coresight_ops catu_ops = { >> .helper_ops = &catu_helper_ops, >> }; >> -static int catu_probe(struct amba_device *adev, const struct amba_id *id) >> +static int __catu_probe(struct device *dev, struct resource *res) >> { >> int ret = 0; >> u32 dma_mask; >> - struct catu_drvdata *drvdata; >> + struct catu_drvdata *drvdata = dev_get_drvdata(dev); >> struct coresight_desc catu_desc; >> struct coresight_platform_data *pdata = NULL; >> - struct device *dev = &adev->dev; >> void __iomem *base; >> catu_desc.name = coresight_alloc_device_name(&catu_devs, dev); >> if (!catu_desc.name) >> return -ENOMEM; >> - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >> - if (!drvdata) { >> - ret = -ENOMEM; >> - goto out; >> - } >> - >> - dev_set_drvdata(dev, drvdata); >> - base = devm_ioremap_resource(dev, &adev->res); >> - if (IS_ERR(base)) { >> - ret = PTR_ERR(base); >> - goto out; >> + if (res) { > > We don't have a case where res == NULL and we shouldn't support that. Sure, will drop that here and in cpu debug driver as well. > >> + base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(base)) { >> + ret = PTR_ERR(base); >> + goto out; >> + } >> } >> /* Setup dma mask for the device */ >> @@ -567,19 +563,39 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) >> drvdata->csdev = coresight_register(&catu_desc); >> if (IS_ERR(drvdata->csdev)) >> ret = PTR_ERR(drvdata->csdev); >> - else >> - pm_runtime_put(&adev->dev); >> out: >> return ret; >> } >> -static void catu_remove(struct amba_device *adev) >> +static int catu_probe(struct amba_device *adev, const struct amba_id *id) >> +{ >> + struct catu_drvdata *drvdata; >> + int ret; >> + >> + drvdata = devm_kzalloc(&adev->dev, sizeof(*drvdata), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + >> + amba_set_drvdata(adev, drvdata); >> + ret = __catu_probe(&adev->dev, &adev->res); >> + if (!ret) >> + pm_runtime_put(&adev->dev); >> + >> + return ret; >> +} >> + >> +static void __catu_remove(struct device *dev) >> { >> - struct catu_drvdata *drvdata = dev_get_drvdata(&adev->dev); >> + struct catu_drvdata *drvdata = dev_get_drvdata(dev); >> coresight_unregister(drvdata->csdev); >> } >> +static void catu_remove(struct amba_device *adev) >> +{ >> + __catu_remove(&adev->dev); >> +} >> + >> static struct amba_id catu_ids[] = { >> CS_AMBA_ID(0x000bb9ee), >> {}, >> @@ -598,13 +614,99 @@ static struct amba_driver catu_driver = { >> .id_table = catu_ids, >> }; >> +static int catu_platform_probe(struct platform_device *pdev) >> +{ >> + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + struct catu_drvdata *drvdata; >> + int ret = 0; >> + >> + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL); >> + if (!drvdata) >> + return -ENOMEM; >> + >> + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); >> + if (IS_ERR(drvdata->pclk)) >> + return -ENODEV; >> + >> + pm_runtime_get_noresume(&pdev->dev); >> + pm_runtime_set_active(&pdev->dev); >> + pm_runtime_enable(&pdev->dev); >> + >> + dev_set_drvdata(&pdev->dev, drvdata); >> + ret = __catu_probe(&pdev->dev, res); >> + pm_runtime_put(&pdev->dev); >> + if (ret) { >> + pm_runtime_disable(&pdev->dev); >> + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) > > Here drvdata is guaranteed to be valid and the check is not required. Sure, will drop. > >> + clk_put(drvdata->pclk); >> + } >> + >> + return ret; >> +} >> + >> +static int catu_platform_remove(struct platform_device *pdev) >> +{ >> + struct catu_drvdata *drvdata = dev_get_drvdata(&pdev->dev); >> + >> + if (WARN_ON(!drvdata)) >> + return -ENODEV; >> + >> + __catu_remove(&pdev->dev); >> + pm_runtime_disable(&pdev->dev); >> + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) > > Same here. Rest looks fine. Sure, will drop.
diff --git a/drivers/acpi/arm64/amba.c b/drivers/acpi/arm64/amba.c index afb6afb66967..587061b0fd2f 100644 --- a/drivers/acpi/arm64/amba.c +++ b/drivers/acpi/arm64/amba.c @@ -27,7 +27,6 @@ static const struct acpi_device_id amba_id_list[] = { {"ARMHC503", 0}, /* ARM CoreSight Debug */ {"ARMHC979", 0}, /* ARM CoreSight TPIU */ {"ARMHC97C", 0}, /* ARM CoreSight SoC-400 TMC, SoC-600 ETF/ETB */ - {"ARMHC9CA", 0}, /* ARM CoreSight CATU */ {"", 0}, }; diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 3949ded0d4fa..8fa035e5a0e8 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -7,6 +7,8 @@ * Author: Suzuki K Poulose <suzuki.poulose@arm.com> */ +#include <linux/platform_device.h> +#include <linux/acpi.h> #include <linux/amba/bus.h> #include <linux/device.h> #include <linux/dma-mapping.h> @@ -502,31 +504,25 @@ static const struct coresight_ops catu_ops = { .helper_ops = &catu_helper_ops, }; -static int catu_probe(struct amba_device *adev, const struct amba_id *id) +static int __catu_probe(struct device *dev, struct resource *res) { int ret = 0; u32 dma_mask; - struct catu_drvdata *drvdata; + struct catu_drvdata *drvdata = dev_get_drvdata(dev); struct coresight_desc catu_desc; struct coresight_platform_data *pdata = NULL; - struct device *dev = &adev->dev; void __iomem *base; catu_desc.name = coresight_alloc_device_name(&catu_devs, dev); if (!catu_desc.name) return -ENOMEM; - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); - if (!drvdata) { - ret = -ENOMEM; - goto out; - } - - dev_set_drvdata(dev, drvdata); - base = devm_ioremap_resource(dev, &adev->res); - if (IS_ERR(base)) { - ret = PTR_ERR(base); - goto out; + if (res) { + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) { + ret = PTR_ERR(base); + goto out; + } } /* Setup dma mask for the device */ @@ -567,19 +563,39 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) drvdata->csdev = coresight_register(&catu_desc); if (IS_ERR(drvdata->csdev)) ret = PTR_ERR(drvdata->csdev); - else - pm_runtime_put(&adev->dev); out: return ret; } -static void catu_remove(struct amba_device *adev) +static int catu_probe(struct amba_device *adev, const struct amba_id *id) +{ + struct catu_drvdata *drvdata; + int ret; + + drvdata = devm_kzalloc(&adev->dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + amba_set_drvdata(adev, drvdata); + ret = __catu_probe(&adev->dev, &adev->res); + if (!ret) + pm_runtime_put(&adev->dev); + + return ret; +} + +static void __catu_remove(struct device *dev) { - struct catu_drvdata *drvdata = dev_get_drvdata(&adev->dev); + struct catu_drvdata *drvdata = dev_get_drvdata(dev); coresight_unregister(drvdata->csdev); } +static void catu_remove(struct amba_device *adev) +{ + __catu_remove(&adev->dev); +} + static struct amba_id catu_ids[] = { CS_AMBA_ID(0x000bb9ee), {}, @@ -598,13 +614,99 @@ static struct amba_driver catu_driver = { .id_table = catu_ids, }; +static int catu_platform_probe(struct platform_device *pdev) +{ + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + struct catu_drvdata *drvdata; + int ret = 0; + + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); + if (IS_ERR(drvdata->pclk)) + return -ENODEV; + + pm_runtime_get_noresume(&pdev->dev); + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + + dev_set_drvdata(&pdev->dev, drvdata); + ret = __catu_probe(&pdev->dev, res); + pm_runtime_put(&pdev->dev); + if (ret) { + pm_runtime_disable(&pdev->dev); + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) + clk_put(drvdata->pclk); + } + + return ret; +} + +static int catu_platform_remove(struct platform_device *pdev) +{ + struct catu_drvdata *drvdata = dev_get_drvdata(&pdev->dev); + + if (WARN_ON(!drvdata)) + return -ENODEV; + + __catu_remove(&pdev->dev); + pm_runtime_disable(&pdev->dev); + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) + clk_put(drvdata->pclk); + return 0; +} + +#ifdef CONFIG_PM +static int catu_runtime_suspend(struct device *dev) +{ + struct catu_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) + clk_disable_unprepare(drvdata->pclk); + return 0; +} + +static int catu_runtime_resume(struct device *dev) +{ + struct catu_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) + clk_prepare_enable(drvdata->pclk); + return 0; +} +#endif + +static const struct dev_pm_ops catu_dev_pm_ops = { + SET_RUNTIME_PM_OPS(catu_runtime_suspend, catu_runtime_resume, NULL) +}; + +#ifdef CONFIG_ACPI +static const struct acpi_device_id catu_acpi_ids[] = { + {"ARMHC9CA", 0, 0, 0}, /* ARM CoreSight CATU */ + {}, +}; + +MODULE_DEVICE_TABLE(acpi, catu_acpi_ids); +#endif + +static struct platform_driver catu_platform_driver = { + .probe = catu_platform_probe, + .remove = catu_platform_remove, + .driver = { + .name = "coresight-catu-platform", + .acpi_match_table = ACPI_PTR(catu_acpi_ids), + .suppress_bind_attrs = true, + .pm = &catu_dev_pm_ops, + }, +}; + static int __init catu_init(void) { int ret; - ret = amba_driver_register(&catu_driver); - if (ret) - pr_info("Error registering catu driver\n"); + ret = coresight_init_driver("catu", &catu_driver, &catu_platform_driver); tmc_etr_set_catu_ops(&etr_catu_buf_ops); return ret; } @@ -612,7 +714,7 @@ static int __init catu_init(void) static void __exit catu_exit(void) { tmc_etr_remove_catu_ops(); - amba_driver_unregister(&catu_driver); + coresight_remove_driver(&catu_driver, &catu_platform_driver); } module_init(catu_init); diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h index 442e034bbfba..141feac1c14b 100644 --- a/drivers/hwtracing/coresight/coresight-catu.h +++ b/drivers/hwtracing/coresight/coresight-catu.h @@ -61,6 +61,7 @@ #define CATU_IRQEN_OFF 0x0 struct catu_drvdata { + struct clk *pclk; void __iomem *base; struct coresight_device *csdev; int irq;