diff mbox series

[23/25] coresight: stm: ACPI support for parsing stimulus base

Message ID 1553107783-3340-24-git-send-email-suzuki.poulose@arm.com (mailing list archive)
State Not Applicable, archived
Headers show
Series coresight: Support for ACPI bindings | expand

Commit Message

Suzuki K Poulose March 20, 2019, 6:49 p.m. UTC
The stimulus base for STM device must be listed as the second memory
resource, followed by the programming base address. 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 March 28, 2019, 8:41 p.m. UTC | #1
On Wed, Mar 20, 2019 at 06:49:40PM +0000, Suzuki K Poulose wrote:
> The stimulus base for STM device must be listed as the second memory
> resource, followed by the programming base address. 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 d94ae22..995443a 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>
> @@ -717,10 +718,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;

Is the ACPI binding crystal clear on the fact that the second resource region
has to be for stimulus ports?

> +	}
> +
> +	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)

Wouldn't it be better to use is_of_node()?

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

is_acpi_device_node()?

> +		return acpi_stm_get_stimulus_area(dev, res);
>  	return -ENOENT;
>  }
>  
> -- 
> 2.7.4
>
Suzuki K Poulose April 4, 2019, 11:27 a.m. UTC | #2
Hi Mathieu,

On 28/03/2019 20:41, Mathieu Poirier wrote:
> On Wed, Mar 20, 2019 at 06:49:40PM +0000, Suzuki K Poulose wrote:
>> The stimulus base for STM device must be listed as the second memory
>> resource, followed by the programming base address. 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>
>> ---

>> +#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;
> 
> Is the ACPI binding crystal clear on the fact that the second resource region
> has to be for stimulus ports?

Yes. Section 2.3 Resources in ACPI for CoreSightTM 1.0 (DEN0067) :

"Each CoresSight component needs to declare the resources it owns using the _CRS
method. This must include base address and span covering the MMIO interface of
the device. In addition those that can raise interrupts must describe the
interrupts they consume.

For STM two base addresses must be presented, these must be provided in order.
First the configuration base address, and then external stimuli memory region
base address"

>>   static int stm_get_stimulus_area(struct device *dev, struct resource *res)
>>   {
>>   	if (dev->of_node)
> 
> Wouldn't it be better to use is_of_node()?


> 
>>   		return of_stm_get_stimulus_area(dev, res);
>> +	else if (is_acpi_node(dev->fwnode)
> 
> is_acpi_device_node()?
> 

Yes, to both the above.

Cheers
Suzuki
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index d94ae22..995443a 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>
@@ -717,10 +718,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;
 }