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.
