[v2,31/36] coresight: stm: ACPI support for parsing stimulus base
diff mbox series

Message ID 1555344260-12375-32-git-send-email-suzuki.poulose@arm.com
State Superseded
Headers show
Series
  • coresight: Support for ACPI bindings
Related show

Commit Message

Suzuki Kuruppassery Poulose April 15, 2019, 4:04 p.m. UTC
The stimulus base for STM device must be listed as the second memory
resource, followed by the programming base address as described in
"Section 2.3 Resources" in ACPI for CoreSightTM 1.0 Platform Design
documen (DEN0067).

Add support for parsing the information for ACPI.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-stm.c | 43 +++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Mathieu Poirier April 23, 2019, 5:59 p.m. UTC | #1
On Mon, Apr 15, 2019 at 05:04:14PM +0100, Suzuki K Poulose wrote:
> The stimulus base for STM device must be listed as the second memory
> resource, followed by the programming base address as described in
> "Section 2.3 Resources" in ACPI for CoreSightTM 1.0 Platform Design
> documen (DEN0067).

Thank you for adding more information.

> 
> Add support for parsing the information for ACPI.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-stm.c | 43 +++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 2d6ae00..7cf2fe4 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -16,6 +16,7 @@
>   * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org>
>   */
>  #include <asm/local.h>
> +#include <linux/acpi.h>
>  #include <linux/amba/bus.h>
>  #include <linux/bitmap.h>
>  #include <linux/clk.h>
> @@ -716,10 +717,52 @@ static inline int of_stm_get_stimulus_area(struct device *dev,
>  }
>  #endif
>  
> +#ifdef CONFIG_ACPI
> +static int acpi_stm_get_stimulus_area(struct device *dev, struct resource *res)
> +{
> +	int rc;
> +	bool found_base = false;
> +	struct resource_entry *rent;
> +	LIST_HEAD(res_list);
> +
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> +	if (!adev)
> +		return -ENODEV;
> +	rc = acpi_dev_get_resources(adev, &res_list, NULL, NULL);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = -ENOENT;
> +	list_for_each_entry(rent, &res_list, node) {
> +		if (resource_type(rent->res) != IORESOURCE_MEM)
> +			continue;
> +		if (found_base) {
> +			*res = *rent->res;
> +			rc = 0;
> +			break;
> +		}
> +
> +		found_base = true;
> +	}

I would copy the description in the changelog just before the
list_for_each_entry() loop.

> +
> +	acpi_dev_free_resource_list(&res_list);
> +	return rc;
> +}
> +#else
> +static inline int acpi_stm_get_stimulus_area(struct device *dev,
> +					     struct resource *res)
> +{
> +	return -ENOENT;
> +}
> +#endif
> +
>  static int stm_get_stimulus_area(struct device *dev, struct resource *res)
>  {
>  	if (dev->of_node)

I noticed the direct access of ->of_node in patch 16 and thought it was a little
odd that is_of_node() wasn't used.

>  		return of_stm_get_stimulus_area(dev, res);
> +	else if (is_acpi_node(dev->fwnode))
> +		return acpi_stm_get_stimulus_area(dev, res);
>  	return -ENOENT;

With the above:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  }
>  
> -- 
> 2.7.4
>
Suzuki Kuruppassery Poulose April 25, 2019, 4:17 p.m. UTC | #2
On 23/04/2019 18:59, Mathieu Poirier wrote:
> On Mon, Apr 15, 2019 at 05:04:14PM +0100, Suzuki K Poulose wrote:
>> The stimulus base for STM device must be listed as the second memory
>> resource, followed by the programming base address as described in
>> "Section 2.3 Resources" in ACPI for CoreSightTM 1.0 Platform Design
>> documen (DEN0067).
> 
> Thank you for adding more information.
> 
>>
>> Add support for parsing the information for ACPI.


> 
> I would copy the description in the changelog just before the
> list_for_each_entry() loop.
> 


>>   static int stm_get_stimulus_area(struct device *dev, struct resource *res)
>>   {
>>   	if (dev->of_node)
> 
> I noticed the direct access of ->of_node in patch 16 and thought it was a little
> odd that is_of_node() wasn't used.
> 
>>   		return of_stm_get_stimulus_area(dev, res);
>> +	else if (is_acpi_node(dev->fwnode))
>> +		return acpi_stm_get_stimulus_area(dev, res);
>>   	return -ENOENT;
> 
> With the above:

Both addressed.

> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks !
Suzuki

Patch
diff mbox series

diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 2d6ae00..7cf2fe4 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -16,6 +16,7 @@ 
  * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org>
  */
 #include <asm/local.h>
+#include <linux/acpi.h>
 #include <linux/amba/bus.h>
 #include <linux/bitmap.h>
 #include <linux/clk.h>
@@ -716,10 +717,52 @@  static inline int of_stm_get_stimulus_area(struct device *dev,
 }
 #endif
 
+#ifdef CONFIG_ACPI
+static int acpi_stm_get_stimulus_area(struct device *dev, struct resource *res)
+{
+	int rc;
+	bool found_base = false;
+	struct resource_entry *rent;
+	LIST_HEAD(res_list);
+
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+
+	if (!adev)
+		return -ENODEV;
+	rc = acpi_dev_get_resources(adev, &res_list, NULL, NULL);
+	if (rc < 0)
+		return rc;
+
+	rc = -ENOENT;
+	list_for_each_entry(rent, &res_list, node) {
+		if (resource_type(rent->res) != IORESOURCE_MEM)
+			continue;
+		if (found_base) {
+			*res = *rent->res;
+			rc = 0;
+			break;
+		}
+
+		found_base = true;
+	}
+
+	acpi_dev_free_resource_list(&res_list);
+	return rc;
+}
+#else
+static inline int acpi_stm_get_stimulus_area(struct device *dev,
+					     struct resource *res)
+{
+	return -ENOENT;
+}
+#endif
+
 static int stm_get_stimulus_area(struct device *dev, struct resource *res)
 {
 	if (dev->of_node)
 		return of_stm_get_stimulus_area(dev, res);
+	else if (is_acpi_node(dev->fwnode))
+		return acpi_stm_get_stimulus_area(dev, res);
 	return -ENOENT;
 }