On 04/12/2023 10:42, James Clark wrote:
On 01/12/2023 06:20, Anshuman Khandual wrote:
Add support for the tmc devices in the 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.
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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/acpi/arm64/amba.c | 2 - .../hwtracing/coresight/coresight-tmc-core.c | 130 +++++++++++++++--- drivers/hwtracing/coresight/coresight-tmc.h | 1 + 3 files changed, 115 insertions(+), 18 deletions(-)
[...]
-static int tmc_probe(struct amba_device *adev, const struct amba_id *id) +static int __tmc_probe(struct device *dev, struct resource *res, void *dev_caps) { int ret = 0; u32 devid; void __iomem *base;
- struct device *dev = &adev->dev; struct coresight_platform_data *pdata = NULL;
- struct tmc_drvdata *drvdata;
- struct resource *res = &adev->res;
- struct tmc_drvdata *drvdata = dev_get_drvdata(dev); struct coresight_desc desc = { 0 }; struct coresight_dev_list *dev_list = NULL;
ret = -ENOMEM;
- drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
- if (!drvdata)
goto out;
- dev_set_drvdata(dev, drvdata);
/* Validity for the resource is already checked by the AMBA core */ base = devm_ioremap_resource(dev, res); @@ -487,8 +482,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) desc.type = CORESIGHT_DEV_TYPE_SINK; desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM; desc.ops = &tmc_etr_cs_ops;
ret = tmc_etr_setup_caps(dev, devid,
coresight_get_uci_data(id));
if (ret) goto out;ret = tmc_etr_setup_caps(dev, devid, dev_caps);
[...]
-module_amba_driver(tmc_driver); +static int tmc_platform_probe(struct platform_device *pdev) +{
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- struct tmc_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;
- dev_set_drvdata(&pdev->dev, drvdata);
- pm_runtime_get_noresume(&pdev->dev);
- pm_runtime_set_active(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
- ret = __tmc_probe(&pdev->dev, res, NULL);
What effect does passing NULL here have? It seems like the dev_caps did something useful when it's set to CORESIGHT_SOC_600_ETR_CAPS. Maybe Suzuki knows as he added it?
That was supposed to initialise the capabilities of the TMC-ETR, which are not discoverable by ID registers. (e.g., SAVE restore of RRP/RWP). I guess, we could get rid of that cap and use the "PID" register match to figure out the capabilities.
Suzuki