On 12/4/23 16:24, 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.
[...]
-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);
- if (ret) {
pm_runtime_put_noidle(&pdev->dev);pm_runtime_disable(&pdev->dev);- }
I'm not sure if these pm_runtime()s are right because there is already a put inside of __tmc_probe() if it fails. If you unload and then reload
Actually there is a pm_runtime_put() on the success path, not when it fails. So pm_runtime_put() gets called when __tmc_probe() returns 0.
__tmc_probe() { .... ret = misc_register(&drvdata->miscdev); if (ret) coresight_unregister(drvdata->csdev); else pm_runtime_put(dev); out: return ret; }
tmc_platform_probe() { .... pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev);
ret = __tmc_probe(&pdev->dev, res, NULL); if (ret) { pm_runtime_put_noidle(&pdev->dev); pm_runtime_disable(&pdev->dev); } return ret; }
tmc_probe() { .... return __tmc_probe(&adev->dev, &adev->res, coresight_get_uci_data(id)); }
Currently pm_runtime_put() gets called
- In success path both for AMBA and platform drivers - In error path only for platform driver
Although the problem might be with pm_runtime_disable() instead
- pm_runtime_disable() is not required in the platform driver probe() path - But might be required in tmc_platform_remove() along with a clk_put()
all the coresight modules with these patches you get these errors which are new:
coresight-tpiu-platform ARMHC979:00: Unbalanced pm_runtime_enable!
The code is similar in TPIU platform driver as well.
CSCFG registered etm0 coresight etm0: CPU0: etm v4.2 initialized CSCFG registered etm1 coresight etm1: CPU1: etm v4.2 initialized CSCFG registered etm2 coresight etm2: CPU2: etm v4.2 initialized CSCFG registered etm3 coresight etm3: CPU3: etm v4.2 initialized coresight-tmc-platform ARMHC97C:00: Unbalanced pm_runtime_enable! coresight-tmc-platform ARMHC97C:01: Unbalanced pm_runtime_enable! coresight-tmc-platform ARMHC97C:02: Unbalanced pm_runtime_enable! coresight-tmc-platform ARMHC97C:03: Unbalanced pm_runtime_enable!
It might be worth testing all of these pm_runtime()s, including the error case ones, because loading and unloading the modules doesn't even include the error scenarios, so there are probably more bad ones in there too.
The code is very similar in CATU, STM as well but debug_platform_remove() seems to be doing this right.
I am not very familiar with all the power management aspects in coresight, please do let me know if I missing something here.