[PATCH 1/1 v2] ARM: coresight: fix system halt when etm enable and add nosmp to command line

[PATCH 1/1 v2] ARM: coresight: fix system halt when etm enable and add nosmp to command line

[Frank.Li at freescale.com]

From: Frank Li <Frank.Li@freescale.com>

when nosmp set, only 1 core enabled. try to access disable core will halt.
dts need added arm,primecell-periphid =  <id> for none-boot core
otherwise amba_device_add will halt when read peripheral id.

Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
Change from v1 to v2
	- fixed typo

 drivers/coresight/coresight-etm3x.c | 4 ++++
 drivers/coresight/of_coresight.c    | 3 +--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/coresight/coresight-etm3x.c b/drivers/coresight/coresight-etm3x.c
index 73c3669..d312952 100644
--- a/drivers/coresight/coresight-etm3x.c
+++ b/drivers/coresight/coresight-etm3x.c
@@ -1829,6 +1829,10 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
 		return ret;
 
 	drvdata->cpu = pdata ? pdata->cpu : 0;
+	if (drvdata->cpu < 0) {
+		ret = -EINVAL;
+		goto err_arch_supported;
+	}
 
 	get_online_cpus();
 	etmdrvdata[drvdata->cpu] = drvdata;
diff --git a/drivers/coresight/of_coresight.c b/drivers/coresight/of_coresight.c
index c3efa41..6804bbd 100644
--- a/drivers/coresight/of_coresight.c
+++ b/drivers/coresight/of_coresight.c
@@ -196,8 +196,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
 		if (cell) {
 			hwid = of_read_number(cell, of_n_addr_cells(dn));
 			index = get_logical_index(hwid);
-			if (index != -EINVAL)
-				pdata->cpu = index;
+			pdata->cpu = index;
 		}
 	}
 
-- 
1.9.1

[PATCH 1/1 v2] ARM: coresight: fix system halt when etm enable and add nosmp to command line

[Dirk Behme]

Hi Frank,

On 19.02.2015 14:57, Frank.Li at freescale.com wrote:
> From: Frank Li <Frank.Li@freescale.com>
>
> when nosmp set, only 1 core enabled. try to access disable core will halt.
> dts need added arm,primecell-periphid =  <id> for none-boot core
> otherwise amba_device_add will halt when read peripheral id.
>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>


Just out of interest: Do you have the coresight patches working on 
i.MX6, now?

I.e. is

http://www.spinics.net/lists/arm-kernel/msg397864.html

clarified, now?

Best regards

Dirk

[PATCH 1/1 v2] ARM: coresight: fix system halt when etm enable and add nosmp to command line

[Zhi Li]

On Fri, Feb 20, 2015 at 1:18 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> Hi Frank,
>
> On 19.02.2015 14:57, Frank.Li at freescale.com wrote:
>>
>> From: Frank Li <Frank.Li@freescale.com>
>>
>> when nosmp set, only 1 core enabled. try to access disable core will halt.
>> dts need added arm,primecell-periphid =  <id> for none-boot core
>> otherwise amba_device_add will halt when read peripheral id.
>>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>
>
>
> Just out of interest: Do you have the coresight patches working on i.MX6,
> now?

No,  i.MX6 have bugs about coresight.
I worked at MX7.

best regards
Frank Li

>
> I.e. is
>
> http://www.spinics.net/lists/arm-kernel/msg397864.html
>
> clarified, now?
>
> Best regards
>
> Dirk

[PATCH 1/1 v2] ARM: coresight: fix system halt when etm enable and add nosmp to command line

[Mathieu Poirier]

On 19 February 2015 at 06:57,  <Frank.Li@freescale.com> wrote:
> From: Frank Li <Frank.Li@freescale.com>
>
> when nosmp set, only 1 core enabled. try to access disable core will halt.
> dts need added arm,primecell-periphid =  <id> for none-boot core
> otherwise amba_device_add will halt when read peripheral id.
>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
> Change from v1 to v2
>         - fixed typo
>
>  drivers/coresight/coresight-etm3x.c | 4 ++++
>  drivers/coresight/of_coresight.c    | 3 +--
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/coresight/coresight-etm3x.c b/drivers/coresight/coresight-etm3x.c
> index 73c3669..d312952 100644
> --- a/drivers/coresight/coresight-etm3x.c
> +++ b/drivers/coresight/coresight-etm3x.c
> @@ -1829,6 +1829,10 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
>                 return ret;
>
>         drvdata->cpu = pdata ? pdata->cpu : 0;
> +       if (drvdata->cpu < 0) {
> +               ret = -EINVAL;
> +               goto err_arch_supported;
> +       }
>
>         get_online_cpus();
>         etmdrvdata[drvdata->cpu] = drvdata;
> diff --git a/drivers/coresight/of_coresight.c b/drivers/coresight/of_coresight.c
> index c3efa41..6804bbd 100644
> --- a/drivers/coresight/of_coresight.c
> +++ b/drivers/coresight/of_coresight.c
> @@ -196,8 +196,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
>                 if (cell) {
>                         hwid = of_read_number(cell, of_n_addr_cells(dn));
>                         index = get_logical_index(hwid);
> -                       if (index != -EINVAL)
> -                               pdata->cpu = index;
> +                       pdata->cpu = index;
>                 }
>         }
>
> --
> 1.9.1
>

Frank,

Thanks for your submission - this is a valid scenario.  On the flip
side things have changed in "of_get_coresight_platform_data()" and
your patch's logic doesn't apply.  Go look at my next branch [1] - we
are now using "of_get_cpu_node()" so that things work on 64-bit
implementations.

Keep what you've done in "etm_probe()" but you will have to re-work
the other part - meddling with "cpu_online()" would be my first
choice...

Lastly please improve the commit log.  Adding a peripheral ID in the
DTS will indeed prevent AMBA from proving a non-responsive memory area
but it is not the point of the patch itself.

Thanks,
Mathieu

[1]. https://git.linaro.org/kernel/coresight.git/shortlog/refs/heads/next

[PATCH 1/1 v2] ARM: coresight: fix system halt when etm enable and add nosmp to command line

[Zhi Li]

On Tue, Feb 24, 2015 at 4:00 PM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On 19 February 2015 at 06:57,  <Frank.Li@freescale.com> wrote:
>> From: Frank Li <Frank.Li@freescale.com>
>>
>> when nosmp set, only 1 core enabled. try to access disable core will halt.
>> dts need added arm,primecell-periphid =  <id> for none-boot core
>> otherwise amba_device_add will halt when read peripheral id.
>>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>> ---
>> Change from v1 to v2
>>         - fixed typo
>>
>>  drivers/coresight/coresight-etm3x.c | 4 ++++
>>  drivers/coresight/of_coresight.c    | 3 +--
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/coresight/coresight-etm3x.c b/drivers/coresight/coresight-etm3x.c
>> index 73c3669..d312952 100644
>> --- a/drivers/coresight/coresight-etm3x.c
>> +++ b/drivers/coresight/coresight-etm3x.c
>> @@ -1829,6 +1829,10 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
>>                 return ret;
>>
>>         drvdata->cpu = pdata ? pdata->cpu : 0;
>> +       if (drvdata->cpu < 0) {
>> +               ret = -EINVAL;
>> +               goto err_arch_supported;
>> +       }
>>
>>         get_online_cpus();
>>         etmdrvdata[drvdata->cpu] = drvdata;
>> diff --git a/drivers/coresight/of_coresight.c b/drivers/coresight/of_coresight.c
>> index c3efa41..6804bbd 100644
>> --- a/drivers/coresight/of_coresight.c
>> +++ b/drivers/coresight/of_coresight.c
>> @@ -196,8 +196,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
>>                 if (cell) {
>>                         hwid = of_read_number(cell, of_n_addr_cells(dn));
>>                         index = get_logical_index(hwid);
>> -                       if (index != -EINVAL)
>> -                               pdata->cpu = index;
>> +                       pdata->cpu = index;
>>                 }
>>         }
>>
>> --
>> 1.9.1
>>
>
> Frank,
>
> Thanks for your submission - this is a valid scenario.  On the flip
> side things have changed in "of_get_coresight_platform_data()" and
> your patch's logic doesn't apply.  Go look at my next branch [1] - we
> are now using "of_get_cpu_node()" so that things work on 64-bit
> implementations.

Okay I will rebase to you next branch.

>
> Keep what you've done in "etm_probe()" but you will have to re-work
> the other part - meddling with "cpu_online()" would be my first
> choice...

of_get_cpu_node may return NULL if core is not exist. I will test it
when I get hardware.
pdata->cpu will always return 0 if nosmp.

>
> Lastly please improve the commit log.  Adding a peripheral ID in the
> DTS will indeed prevent AMBA from proving a non-responsive memory area
> but it is not the point of the patch itself.

Our imx7 just come out.  dts part have not upstream yet.
So I just sent out coresight part change.

>
> Thanks,
> Mathieu
>
> [1]. https://git.linaro.org/kernel/coresight.git/shortlog/refs/heads/next