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

On 04/12/2023 10:42, James Clark wrote:

On 01/12/2023 06:20, Anshuman Khandual wrote:

quoted

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 <redacted>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <redacted>
Cc: James Clark <redacted>
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 <redacted>
---
  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(-)

[...]

quoted

-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));
+		ret = tmc_etr_setup_caps(dev, devid, dev_caps);
  		if (ret)
  			goto out;

[...]

quoted

-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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel