linux-kernel - Re: [PATCH 23/25] coresight: stm: ACPI support for parsing stimulus base

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 4 Apr 2019 12:27:13 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     mathieu.poirier@...aro.org
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, coresight@...ts.linaro.org,
        mike.leach@...aro.org, robert.walker@....com, rjw@...ysocki.net
Subject: Re: [PATCH 23/25] coresight: stm: ACPI support for parsing stimulus
 base

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@...aro.org>
>> Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....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