Re: [PATCH V2 6/7] coresight: stm: Move ACPI support from AMBA driver to platform driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/12/2023 05:20, Anshuman Khandual wrote:


On 12/4/23 18:47, James Clark wrote:


On 04/12/2023 11:50, Anshuman Khandual wrote:


On 12/4/23 15:53, James Clark wrote:


On 01/12/2023 06:20, Anshuman Khandual wrote:
Add support for the stm 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@xxxxxxxxxx>
Cc: Sudeep Holla <sudeep.holla@xxxxxxx>
Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
Cc: Mike Leach <mike.leach@xxxxxxxxxx>
Cc: James Clark <james.clark@xxxxxxx>
Cc: Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>
Cc: Alexandre Torgue <alexandre.torgue@xxxxxxxxxxx>
Cc: linux-acpi@xxxxxxxxxxxxxxx
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: coresight@xxxxxxxxxxxxxxxx
Cc: linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
---
[...]
-module_amba_driver(stm_driver);
+static int stm_platform_probe(struct platform_device *pdev)
+{
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	int ret = 0;
+
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	ret = __stm_probe(&pdev->dev, res, NULL);

Very minor nit, but this used to print this:

   coresight stm0: STM500 initialized

And now it prints this:

   coresight stm0: (null) initialized

(null) kind of makes it look a little bit like something has gone wrong.
Maybe we could just put "initialised" if you don't have a string from ACPI?

__stm_probe() gets called from both AMBA and platform driver paths. Even though
a NULL check inside dev_info(..."%s initialized\n",...) could be added, but how
to differentiate it from a scenario when coresight_get_uci_data() returns NULL ?

Sudeep's suggestion seems ok, just add a hard coded string instead of
the NULL. And keep the coresight_get_uci_data() the same.

Something like this works ?

diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 3387ebc9d8ab..2b6834c4cac6 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -906,7 +906,7 @@ static int __stm_probe(struct device *dev, struct resource *res, void *dev_caps)
         pm_runtime_put(dev);
dev_info(&drvdata->csdev->dev, "%s initialized\n",
-                (char *)dev_caps);
+                dev_caps ? (char *)dev_caps: "STM");
         return 0;

Please could stop abusing the private data for storing the name ? Instead, we could read the PID of the device and use a table to map
it to the name and use that instead ?

Suzuki


  cs_unregister:






[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux