[PATCHv2 0/3] Detect UDIV/SDIV support from ISAR0

[PATCHv2 0/3] Detect UDIV/SDIV support from ISAR0

[Stephen Boyd]

While attempting to upstream a patch to add the IDIV hwcap for
Krait processors, Will suggested we move the code to read the
ISAR0 register. This patchset does that and also works around
the early Krait CPU designs that don't follow the latest ARM ARM
for the ISAR0 register.

Stepan Moskovchenko (1):
  ARM: Work around faulty ISAR0 register in some Krait CPUs

Stephen Boyd (2):
  ARM: Clear IDIVT hwcap if CONFIG_ARM_THUMB=n
  ARM: Detect support for SDIV/UDIV from ISAR0 register

 arch/arm/kernel/setup.c | 22 +++++++++++++++++++++-
 arch/arm/mm/proc-v7.S   | 19 +++++++++++++++++--
 2 files changed, 38 insertions(+), 3 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/3] ARM: Clear IDIVT hwcap if CONFIG_ARM_THUMB=n

[Stephen Boyd]

Don't advertise support for the SDIV/UDIV thumb instructions if
the kernel is not compiled with support for thumb userspace. This
is in line with how we remove the THUMB hwcap in these
configurations.

Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 3f6cbb2..e2c8bbf 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -484,7 +484,7 @@ static void __init setup_processor(void)
 		 list->elf_name, ENDIANNESS);
 	elf_hwcap = list->elf_hwcap;
 #ifndef CONFIG_ARM_THUMB
-	elf_hwcap &= ~HWCAP_THUMB;
+	elf_hwcap &= ~(HWCAP_THUMB | HWCAP_IDIVT);
 #endif
 
 	feat_v6_fixup();
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0 register

[Stephen Boyd]

The ISAR0 register indicates support for the SDIV and UDIV
instructions in both the Thumb and ARM instruction set. Read the
register to detect the supported instructions and update the
elf_hwcap mask as appropriate. This is better than adding more
and more cpuid checks in proc-v7.S for each new cpu variant that
supports these instructions.

Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/setup.c | 20 ++++++++++++++++++++
 arch/arm/mm/proc-v7.S   |  4 ++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index e2c8bbf..f3ac13f 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -353,6 +353,23 @@ void __init early_print(const char *str, ...)
 	printk("%s", buf);
 }
 
+static void __init cpuid_init_hwcaps(void)
+{
+	unsigned int divide_instrs;
+
+	if (cpu_architecture() < CPU_ARCH_ARMv7)
+		return;
+
+	divide_instrs = (read_cpuid_ext(CPUID_EXT_ISAR0) & 0x0f000000) >> 24;
+
+	switch (divide_instrs) {
+	case 2:
+		elf_hwcap |= HWCAP_IDIVA;
+	case 1:
+		elf_hwcap |= HWCAP_IDIVT;
+	}
+}
+
 static void __init feat_v6_fixup(void)
 {
 	int id = read_cpuid_id();
@@ -483,6 +500,9 @@ static void __init setup_processor(void)
 	snprintf(elf_platform, ELF_PLATFORM_SIZE, "%s%c",
 		 list->elf_name, ENDIANNESS);
 	elf_hwcap = list->elf_hwcap;
+
+	cpuid_init_hwcaps();
+
 #ifndef CONFIG_ARM_THUMB
 	elf_hwcap &= ~(HWCAP_THUMB | HWCAP_IDIVT);
 #endif
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 3a3c015..bcd3d48 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -420,7 +420,7 @@ __v7_pj4b_proc_info:
 __v7_ca7mp_proc_info:
 	.long	0x410fc070
 	.long	0xff0ffff0
-	__v7_proc __v7_ca7mp_setup, hwcaps = HWCAP_IDIV
+	__v7_proc __v7_ca7mp_setup
 	.size	__v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
 
 	/*
@@ -430,7 +430,7 @@ __v7_ca7mp_proc_info:
 __v7_ca15mp_proc_info:
 	.long	0x410fc0f0
 	.long	0xff0ffff0
-	__v7_proc __v7_ca15mp_setup, hwcaps = HWCAP_IDIV
+	__v7_proc __v7_ca15mp_setup
 	.size	__v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
 
 	/*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0 register

[Uwe Kleine-König]

Hello Stephen,

On Mon, Mar 18, 2013 at 11:28:56AM -0700, Stephen Boyd wrote:
> The ISAR0 register indicates support for the SDIV and UDIV
> instructions in both the Thumb and ARM instruction set. Read the
> register to detect the supported instructions and update the
> elf_hwcap mask as appropriate. This is better than adding more
> and more cpuid checks in proc-v7.S for each new cpu variant that
> supports these instructions.
you pointed out yesterday that this could work on v7-m, too. As I based
my patches on 3.9-rc1 this patch (8164f7af88) wasn't included. When
updating this results in a warning, because I have

	6ebd4d0 (ARM: stub out read_cpuid and read_cpuid_ext for CPU_CP15=n)

from rmk's devel-stable branch.
 
> Acked-by: Will Deacon <will.deacon@arm.com>
> Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm/kernel/setup.c | 20 ++++++++++++++++++++
>  arch/arm/mm/proc-v7.S   |  4 ++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index e2c8bbf..f3ac13f 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -353,6 +353,23 @@ void __init early_print(const char *str, ...)
>  	printk("%s", buf);
>  }
>  
> +static void __init cpuid_init_hwcaps(void)
> +{
> +	unsigned int divide_instrs;
> +
> +	if (cpu_architecture() < CPU_ARCH_ARMv7)
> +		return;
> +
> +	divide_instrs = (read_cpuid_ext(CPUID_EXT_ISAR0) & 0x0f000000) >> 24;
The problem is that read_cpuid_ext is called which doesn't map to
something useful for v7-m. So maybe add a check:

	if (!IS_ENABLED(CONFIG_CPU_CP15))
		return;

?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Uwe Kleine-König]

Hello,

On Thu, Apr 18, 2013 at 11:10:19AM +0200, Uwe Kleine-K?nig wrote:
> On Mon, Mar 18, 2013 at 11:28:56AM -0700, Stephen Boyd wrote:
> > The ISAR0 register indicates support for the SDIV and UDIV
> > instructions in both the Thumb and ARM instruction set. Read the
> > register to detect the supported instructions and update the
> > elf_hwcap mask as appropriate. This is better than adding more
> > and more cpuid checks in proc-v7.S for each new cpu variant that
> > supports these instructions.
> you pointed out yesterday that this could work on v7-m, too. As I based
> my patches on 3.9-rc1 this patch (8164f7af88) wasn't included. When
> updating this results in a warning, because I have
> 
> 	6ebd4d0 (ARM: stub out read_cpuid and read_cpuid_ext for CPU_CP15=n)
> 
> from rmk's devel-stable branch.
>  
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > ---
> >  arch/arm/kernel/setup.c | 20 ++++++++++++++++++++
> >  arch/arm/mm/proc-v7.S   |  4 ++--
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index e2c8bbf..f3ac13f 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -353,6 +353,23 @@ void __init early_print(const char *str, ...)
> >  	printk("%s", buf);
> >  }
> >  
> > +static void __init cpuid_init_hwcaps(void)
> > +{
> > +	unsigned int divide_instrs;
> > +
> > +	if (cpu_architecture() < CPU_ARCH_ARMv7)
> > +		return;
> > +
> > +	divide_instrs = (read_cpuid_ext(CPUID_EXT_ISAR0) & 0x0f000000) >> 24;
> The problem is that read_cpuid_ext is called which doesn't map to
> something useful for v7-m. So maybe add a check:
> 
> 	if (!IS_ENABLED(CONFIG_CPU_CP15))
> 		return;
> 
> ?
The patch below fixes the issue for me on V7-M. The only drawback is
that the list of registers isn't shared. Maybe someone has a nice idea?

BTW, there doesn't seem to be an equivalent register set for 
CPUID_{CACHETYPE,TCM,TLBTYPE,MPIDR} on v7-M.

Best regards
Uwe

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index 4eb94a3..ec635ff 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -10,6 +10,22 @@
 #define CPUID_TLBTYPE	3
 #define CPUID_MPIDR	5
 
+#ifdef CONFIG_CPU_V7M
+#define CPUID_EXT_PFR0	0x40
+#define CPUID_EXT_PFR1	0x44
+#define CPUID_EXT_DFR0	0x48
+#define CPUID_EXT_AFR0	0x4c
+#define CPUID_EXT_MMFR0	0x50
+#define CPUID_EXT_MMFR1	0x54
+#define CPUID_EXT_MMFR2	0x58
+#define CPUID_EXT_MMFR3	0x5c
+#define CPUID_EXT_ISAR0	0x60
+#define CPUID_EXT_ISAR1	0x64
+#define CPUID_EXT_ISAR2	0x68
+#define CPUID_EXT_ISAR3	0x6c
+#define CPUID_EXT_ISAR4	0x70
+#define CPUID_EXT_ISAR5	0x74
+#else
 #define CPUID_EXT_PFR0	"c1, 0"
 #define CPUID_EXT_PFR1	"c1, 1"
 #define CPUID_EXT_DFR0	"c1, 2"
@@ -24,6 +40,7 @@
 #define CPUID_EXT_ISAR3	"c2, 3"
 #define CPUID_EXT_ISAR4	"c2, 4"
 #define CPUID_EXT_ISAR5	"c2, 5"
+#endif
 
 #define MPIDR_SMP_BITMASK (0x3 << 30)
 #define MPIDR_SMP_VALUE (0x2 << 30)
@@ -79,7 +96,23 @@ extern unsigned int processor_id;
 		__val;							\
 	})
 
-#else /* ifdef CONFIG_CPU_CP15 */
+#elif defined(CONFIG_CPU_V7M)
+
+#include <asm/io.h>
+#include <asm/v7m.h>
+
+#define read_cpuid(reg)							\
+	({								\
+		WARN_ON_ONCE(1);					\
+		0;							\
+	})
+
+static inline unsigned int __attribute_const__ read_cpuid_ext(unsigned offset)
+{
+	return readl(BASEADDR_V7M_SCB + offset);
+}
+
+#else /* ifdef CONFIG_CPU_CP15 / elif defined (CONFIG_CPU_V7M) */
 
 /*
  * read_cpuid and read_cpuid_ext should only ever be called on machines that
@@ -108,9 +141,6 @@ static inline unsigned int __attribute_const__ read_cpuid_id(void)
 
 #elif defined(CONFIG_CPU_V7M)
 
-#include <asm/io.h>
-#include <asm/v7m.h>
-
 static inline unsigned int __attribute_const__ read_cpuid_id(void)
 {
 	return readl(BASEADDR_V7M_SCB + V7M_SCB_CPUID);
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index 000499c..0c93588 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -144,7 +144,7 @@ __v7m_proc_info:
 	b	__v7m_setup		@ proc_info_list.__cpu_flush
 	.long	cpu_arch_name
 	.long	cpu_elf_name
-	.long	HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_IDIVT
+	.long	HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT
 	.long	cpu_v7m_name
 	.long	v7m_processor_functions	@ proc_info_list.proc
 	.long	0			@ proc_info_list.tlb

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Stephen Boyd]

On 05/06/13 02:30, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Thu, Apr 18, 2013 at 11:10:19AM +0200, Uwe Kleine-K?nig wrote:
>> On Mon, Mar 18, 2013 at 11:28:56AM -0700, Stephen Boyd wrote:
>>> The ISAR0 register indicates support for the SDIV and UDIV
>>> instructions in both the Thumb and ARM instruction set. Read the
>>> register to detect the supported instructions and update the
>>> elf_hwcap mask as appropriate. This is better than adding more
>>> and more cpuid checks in proc-v7.S for each new cpu variant that
>>> supports these instructions.
>> you pointed out yesterday that this could work on v7-m, too. As I based
>> my patches on 3.9-rc1 this patch (8164f7af88) wasn't included. When
>> updating this results in a warning, because I have
>>
>> 	6ebd4d0 (ARM: stub out read_cpuid and read_cpuid_ext for CPU_CP15=n)
>>
>> from rmk's devel-stable branch.
>>  
>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>> Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>>> ---
>>>  arch/arm/kernel/setup.c | 20 ++++++++++++++++++++
>>>  arch/arm/mm/proc-v7.S   |  4 ++--
>>>  2 files changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>>> index e2c8bbf..f3ac13f 100644
>>> --- a/arch/arm/kernel/setup.c
>>> +++ b/arch/arm/kernel/setup.c
>>> @@ -353,6 +353,23 @@ void __init early_print(const char *str, ...)
>>>  	printk("%s", buf);
>>>  }
>>>  
>>> +static void __init cpuid_init_hwcaps(void)
>>> +{
>>> +	unsigned int divide_instrs;
>>> +
>>> +	if (cpu_architecture() < CPU_ARCH_ARMv7)
>>> +		return;
>>> +
>>> +	divide_instrs = (read_cpuid_ext(CPUID_EXT_ISAR0) & 0x0f000000) >> 24;
>> The problem is that read_cpuid_ext is called which doesn't map to
>> something useful for v7-m. So maybe add a check:
>>
>> 	if (!IS_ENABLED(CONFIG_CPU_CP15))
>> 		return;
>>
>> ?
> The patch below fixes the issue for me on V7-M. The only drawback is
> that the list of registers isn't shared. Maybe someone has a nice idea?

The patch looks ok to me.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Will Deacon]

On Mon, May 06, 2013 at 10:30:59AM +0100, Uwe Kleine-K?nig wrote:
> Hello,

Hi Uwe,

> On Thu, Apr 18, 2013 at 11:10:19AM +0200, Uwe Kleine-K?nig wrote:
> The patch below fixes the issue for me on V7-M. The only drawback is
> that the list of registers isn't shared. Maybe someone has a nice idea?
> 
> BTW, there doesn't seem to be an equivalent register set for 
> CPUID_{CACHETYPE,TCM,TLBTYPE,MPIDR} on v7-M.

That make sense: we don't have caches, MMUs or SMP.

> Best regards
> Uwe
> 
> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index 4eb94a3..ec635ff 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -10,6 +10,22 @@
>  #define CPUID_TLBTYPE	3
>  #define CPUID_MPIDR	5
>  
> +#ifdef CONFIG_CPU_V7M
> +#define CPUID_EXT_PFR0	0x40
> +#define CPUID_EXT_PFR1	0x44
> +#define CPUID_EXT_DFR0	0x48
> +#define CPUID_EXT_AFR0	0x4c
> +#define CPUID_EXT_MMFR0	0x50
> +#define CPUID_EXT_MMFR1	0x54
> +#define CPUID_EXT_MMFR2	0x58
> +#define CPUID_EXT_MMFR3	0x5c
> +#define CPUID_EXT_ISAR0	0x60
> +#define CPUID_EXT_ISAR1	0x64
> +#define CPUID_EXT_ISAR2	0x68
> +#define CPUID_EXT_ISAR3	0x6c
> +#define CPUID_EXT_ISAR4	0x70
> +#define CPUID_EXT_ISAR5	0x74
> +#else
>  #define CPUID_EXT_PFR0	"c1, 0"
>  #define CPUID_EXT_PFR1	"c1, 1"
>  #define CPUID_EXT_DFR0	"c1, 2"
> @@ -24,6 +40,7 @@
>  #define CPUID_EXT_ISAR3	"c2, 3"
>  #define CPUID_EXT_ISAR4	"c2, 4"
>  #define CPUID_EXT_ISAR5	"c2, 5"
> +#endif
>  
>  #define MPIDR_SMP_BITMASK (0x3 << 30)
>  #define MPIDR_SMP_VALUE (0x2 << 30)
> @@ -79,7 +96,23 @@ extern unsigned int processor_id;
>  		__val;							\
>  	})
>  
> -#else /* ifdef CONFIG_CPU_CP15 */
> +#elif defined(CONFIG_CPU_V7M)
> +
> +#include <asm/io.h>
> +#include <asm/v7m.h>
> +
> +#define read_cpuid(reg)							\
> +	({								\
> +		WARN_ON_ONCE(1);					\
> +		0;							\
> +	})

We do have an MIDR in PMSAv7 (not to be confused with MPIDR), so why not read
that here?

Will

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Uwe Kleine-König]

Hello Will,

On Tue, May 07, 2013 at 10:22:09AM +0100, Will Deacon wrote:
> On Mon, May 06, 2013 at 10:30:59AM +0100, Uwe Kleine-K?nig wrote:
> > On Thu, Apr 18, 2013 at 11:10:19AM +0200, Uwe Kleine-K?nig wrote:
> > The patch below fixes the issue for me on V7-M. The only drawback is
> > that the list of registers isn't shared. Maybe someone has a nice idea?
> > 
> > BTW, there doesn't seem to be an equivalent register set for 
> > CPUID_{CACHETYPE,TCM,TLBTYPE,MPIDR} on v7-M.
> 
> That make sense: we don't have caches, MMUs or SMP.
IMHO the sensible thing is to implement the registers then and let them
tell: I don't have these things.

> > diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> > index 4eb94a3..ec635ff 100644
> > --- a/arch/arm/include/asm/cputype.h
> > +++ b/arch/arm/include/asm/cputype.h
> > @@ -10,6 +10,22 @@
> >  #define CPUID_TLBTYPE	3
> >  #define CPUID_MPIDR	5
> >  
> > +#ifdef CONFIG_CPU_V7M
> > +#define CPUID_EXT_PFR0	0x40
> > +#define CPUID_EXT_PFR1	0x44
> > +#define CPUID_EXT_DFR0	0x48
> > +#define CPUID_EXT_AFR0	0x4c
> > +#define CPUID_EXT_MMFR0	0x50
> > +#define CPUID_EXT_MMFR1	0x54
> > +#define CPUID_EXT_MMFR2	0x58
> > +#define CPUID_EXT_MMFR3	0x5c
> > +#define CPUID_EXT_ISAR0	0x60
> > +#define CPUID_EXT_ISAR1	0x64
> > +#define CPUID_EXT_ISAR2	0x68
> > +#define CPUID_EXT_ISAR3	0x6c
> > +#define CPUID_EXT_ISAR4	0x70
> > +#define CPUID_EXT_ISAR5	0x74
> > +#else
> >  #define CPUID_EXT_PFR0	"c1, 0"
> >  #define CPUID_EXT_PFR1	"c1, 1"
> >  #define CPUID_EXT_DFR0	"c1, 2"
> > @@ -24,6 +40,7 @@
> >  #define CPUID_EXT_ISAR3	"c2, 3"
> >  #define CPUID_EXT_ISAR4	"c2, 4"
> >  #define CPUID_EXT_ISAR5	"c2, 5"
> > +#endif
> >  
> >  #define MPIDR_SMP_BITMASK (0x3 << 30)
> >  #define MPIDR_SMP_VALUE (0x2 << 30)
> > @@ -79,7 +96,23 @@ extern unsigned int processor_id;
> >  		__val;							\
> >  	})
> >  
> > -#else /* ifdef CONFIG_CPU_CP15 */
> > +#elif defined(CONFIG_CPU_V7M)
> > +
> > +#include <asm/io.h>
> > +#include <asm/v7m.h>
> > +
> > +#define read_cpuid(reg)							\
> > +	({								\
> > +		WARN_ON_ONCE(1);					\
> > +		0;							\
> > +	})
> 
> We do have an MIDR in PMSAv7 (not to be confused with MPIDR), so why not read
> that here?
I don't know it and it's unrelated to this patch. But I will note it and
check it. In the meantime I'm taking patches of course :-)

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Will Deacon]

On Tue, May 07, 2013 at 10:35:08AM +0100, Uwe Kleine-K?nig wrote:
> Hello Will,

Hello,

> On Tue, May 07, 2013 at 10:22:09AM +0100, Will Deacon wrote:
> > On Mon, May 06, 2013 at 10:30:59AM +0100, Uwe Kleine-K?nig wrote:
> > > +#elif defined(CONFIG_CPU_V7M)
> > > +
> > > +#include <asm/io.h>
> > > +#include <asm/v7m.h>
> > > +
> > > +#define read_cpuid(reg)							\
> > > +	({								\
> > > +		WARN_ON_ONCE(1);					\
> > > +		0;							\
> > > +	})
> > 
> > We do have an MIDR in PMSAv7 (not to be confused with MPIDR), so why not read
> > that here?
> I don't know it and it's unrelated to this patch. But I will note it and
> check it. In the meantime I'm taking patches of course :-)

I don't think it is unrelated to this patch: I'm suggesting that you add an
MIDR read to read_cpuid, instead of the unconditional WARN_ON_ONCE which
you're adding here.

Then you can remove your v7m-specific read_cpuid_id implementation, because
read_cpuid(CPUID_ID) will do the right thing.

...or am I missing something?

Will

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Jonathan Austin]

Hi Will,

On 07/05/13 10:22, Will Deacon wrote:
> On Mon, May 06, 2013 at 10:30:59AM +0100, Uwe Kleine-K?nig wrote:

>> +
>> +#define read_cpuid(reg)							\
>> +	({								\
>> +		WARN_ON_ONCE(1);					\
>> +		0;							\
>> +	})
>
> We do have an MIDR in PMSAv7 (not to be confused with MPIDR), so why not read
> that here?
We don't need to add/remove anything for PMSAv7 as that's already 
implemented...

For V7M we have something called CPUID Base Register, not MIDR - the 
layout looks the same though. That's the only one of the CPUID_X that we 
have, but it is never requested directly - only via read_cpuid_id (this 
is to support a hardcoded cpuid from kconfig) so what Uwe's got does 
work, though I agree that it'd be much nicer to implement the underlying 
function and have the accessor general for both AR and M.

Jonny

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Uwe Kleine-König]

On Tue, May 07, 2013 at 11:56:28AM +0100, Jonathan Austin wrote:
> Hi Will,
> 
> On 07/05/13 10:22, Will Deacon wrote:
> >On Mon, May 06, 2013 at 10:30:59AM +0100, Uwe Kleine-K?nig wrote:
> 
> >>+
> >>+#define read_cpuid(reg)							\
> >>+	({								\
> >>+		WARN_ON_ONCE(1);					\
> >>+		0;							\
> >>+	})
> >
> >We do have an MIDR in PMSAv7 (not to be confused with MPIDR), so why not read
> >that here?
> We don't need to add/remove anything for PMSAv7 as that's already
> implemented...
> 
> For V7M we have something called CPUID Base Register, not MIDR - the
> layout looks the same though. That's the only one of the CPUID_X
> that we have, but it is never requested directly - only via
> read_cpuid_id (this is to support a hardcoded cpuid from kconfig) so
> what Uwe's got does work, though I agree that it'd be much nicer to
> implement the underlying function and have the accessor general for
> both AR and M.
The current state (that is next + the patch from this thread) is:

#ifdef CONFIG_CPU_CP15
	define read_cpuid and read_cpuid_ext using mrc p15
	read_cpuid_id = read_cpuid(CPUID_ID)
#elif defined (CONFIG_CPU_V7M)
	read_cpuid returns 0 (with a warning)
	read_cpuid_ext reads from SCB
	read_cpuid_id reads from V7M_SCB_CPUID
#else
	read_cpuid and read_cpuid_ext return 0 (with a warning)
	read_cpuid_id returns CONFIG_PROCESSOR_ID (a bit simplified)
#endif

Now, what is the "underlying function" that should be used to implement
which accessor? I assume you want to have

	read_cpuid_id = read_cpuid(CPUID_ID)

on v7-M, too? So what is the suggestion for read_cpuid?:

	if (reg == CPUID_ID)
		return readl(V7M_SCB_CPUID);
	else
		return 0;
	
That doesn't make much sense in my eyes. If I got you wrong, please give
more details.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Will Deacon]

On Tue, May 07, 2013 at 01:40:13PM +0100, Uwe Kleine-K?nig wrote:
> On Tue, May 07, 2013 at 11:56:28AM +0100, Jonathan Austin wrote:
> > For V7M we have something called CPUID Base Register, not MIDR - the
> > layout looks the same though. That's the only one of the CPUID_X
> > that we have, but it is never requested directly - only via
> > read_cpuid_id (this is to support a hardcoded cpuid from kconfig) so
> > what Uwe's got does work, though I agree that it'd be much nicer to
> > implement the underlying function and have the accessor general for
> > both AR and M.
> The current state (that is next + the patch from this thread) is:
> 
> #ifdef CONFIG_CPU_CP15
> 	define read_cpuid and read_cpuid_ext using mrc p15
> 	read_cpuid_id = read_cpuid(CPUID_ID)
> #elif defined (CONFIG_CPU_V7M)
> 	read_cpuid returns 0 (with a warning)
> 	read_cpuid_ext reads from SCB
> 	read_cpuid_id reads from V7M_SCB_CPUID
> #else
> 	read_cpuid and read_cpuid_ext return 0 (with a warning)
> 	read_cpuid_id returns CONFIG_PROCESSOR_ID (a bit simplified)
> #endif
> 
> Now, what is the "underlying function" that should be used to implement
> which accessor? I assume you want to have
> 
> 	read_cpuid_id = read_cpuid(CPUID_ID)
> 
> on v7-M, too? So what is the suggestion for read_cpuid?:
> 
> 	if (reg == CPUID_ID)
> 		return readl(V7M_SCB_CPUID);
> 	else
> 		return 0;

Yes, either switch on the reg or define CPUID_ID to be V7M_SCB_CPUID for
v7m, then just do the readl in read_cpuid. I'm not sure the warnings add
much (we can do read_cpuid(random integer) with cp15 and it will blindly
construct the mrc).

> That doesn't make much sense in my eyes. If I got you wrong, please give
> more details.

I'm just suggesting that we try and keep the functions similar between v7m and
cp15 where there isn't a good reason not to.

Up to you.

Will

[RFC PATCH] implement read_cpuid_ext for v7-M (Was: Re: [PATCHv2 2/3] ARM: Detect support for SDIV/UDIV from ISAR0) register

[Jonathan Austin]

On 06/05/13 10:30, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Thu, Apr 18, 2013 at 11:10:19AM +0200, Uwe Kleine-K?nig wrote:
>> On Mon, Mar 18, 2013 at 11:28:56AM -0700, Stephen Boyd wrote:
>>> The ISAR0 register indicates support for the SDIV and UDIV
>>> instructions in both the Thumb and ARM instruction set. Read the
>>> register to detect the supported instructions and update the
>>> elf_hwcap mask as appropriate. This is better than adding more
>>> and more cpuid checks in proc-v7.S for each new cpu variant that
>>> supports these instructions.
>> you pointed out yesterday that this could work on v7-m, too. As I based
>> my patches on 3.9-rc1 this patch (8164f7af88) wasn't included. When
>> updating this results in a warning, because I have
>>
>> 	6ebd4d0 (ARM: stub out read_cpuid and read_cpuid_ext for CPU_CP15=n)
>>
>> from rmk's devel-stable branch.
>>
>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>> Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>>> ---
>>>   arch/arm/kernel/setup.c | 20 ++++++++++++++++++++
>>>   arch/arm/mm/proc-v7.S   |  4 ++--
>>>   2 files changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>>> index e2c8bbf..f3ac13f 100644
>>> --- a/arch/arm/kernel/setup.c
>>> +++ b/arch/arm/kernel/setup.c
>>> @@ -353,6 +353,23 @@ void __init early_print(const char *str, ...)
>>>   	printk("%s", buf);
>>>   }
>>>
>>> +static void __init cpuid_init_hwcaps(void)
>>> +{
>>> +	unsigned int divide_instrs;
>>> +
>>> +	if (cpu_architecture() < CPU_ARCH_ARMv7)
>>> +		return;
>>> +
>>> +	divide_instrs = (read_cpuid_ext(CPUID_EXT_ISAR0) & 0x0f000000) >> 24;
>> The problem is that read_cpuid_ext is called which doesn't map to
>> something useful for v7-m. So maybe add a check:
>>
>> 	if (!IS_ENABLED(CONFIG_CPU_CP15))
>> 		return;
>>
>> ?
> The patch below fixes the issue for me on V7-M. The only drawback is
> that the list of registers isn't shared. Maybe someone has a nice idea?

I looked at whether there was anything nice that we could do around this 
but it most things end up more ugly than a separate long list of 
defines... It also makes things hard to understand so I prefer having 
these extra defines.

On the other hand, I think that this is about the time to think about 
another file for V7M CPUID stuff...

This file is beginning to look a bit unruly with all the different cases 
(V7M, !CPU_CP15, as well as the functions like read_cpuid_part_number 
that have the same definition in all cases). Thoughts?

>
> BTW, there doesn't seem to be an equivalent register set for
> CPUID_{CACHETYPE,TCM,TLBTYPE,MPIDR} on v7-M.
>
> Best regards
> Uwe
>
> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index 4eb94a3..ec635ff 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -10,6 +10,22 @@
>   #define CPUID_TLBTYPE	3
>   #define CPUID_MPIDR	5
>
> +#ifdef CONFIG_CPU_V7M

Why not stick the

#include <asm/io.h>
#include <asm/v7m.h>

here?
> +#define CPUID_EXT_PFR0	0x40
> +#define CPUID_EXT_PFR1	0x44
> +#define CPUID_EXT_DFR0	0x48
> +#define CPUID_EXT_AFR0	0x4c
> +#define CPUID_EXT_MMFR0	0x50
> +#define CPUID_EXT_MMFR1	0x54
> +#define CPUID_EXT_MMFR2	0x58
> +#define CPUID_EXT_MMFR3	0x5c
> +#define CPUID_EXT_ISAR0	0x60
> +#define CPUID_EXT_ISAR1	0x64
> +#define CPUID_EXT_ISAR2	0x68
> +#define CPUID_EXT_ISAR3	0x6c
> +#define CPUID_EXT_ISAR4	0x70
> +#define CPUID_EXT_ISAR5	0x74
> +#else
>   #define CPUID_EXT_PFR0	"c1, 0"
>   #define CPUID_EXT_PFR1	"c1, 1"
>   #define CPUID_EXT_DFR0	"c1, 2"
> @@ -24,6 +40,7 @@
>   #define CPUID_EXT_ISAR3	"c2, 3"
>   #define CPUID_EXT_ISAR4	"c2, 4"
>   #define CPUID_EXT_ISAR5	"c2, 5"
> +#endif
>
>   #define MPIDR_SMP_BITMASK (0x3 << 30)
>   #define MPIDR_SMP_VALUE (0x2 << 30)
> @@ -79,7 +96,23 @@ extern unsigned int processor_id;
>   		__val;							\
>   	})
>
> -#else /* ifdef CONFIG_CPU_CP15 */
> +#elif defined(CONFIG_CPU_V7M)
> +
> +#include <asm/io.h>
> +#include <asm/v7m.h>
> +
> +#define read_cpuid(reg)							\
> +	({								\
> +		WARN_ON_ONCE(1);					\
> +		0;							\
> +	})
> +
> +static inline unsigned int __attribute_const__ read_cpuid_ext(unsigned offset)
> +{
> +	return readl(BASEADDR_V7M_SCB + offset);
> +}
> +
> +#else /* ifdef CONFIG_CPU_CP15 / elif defined (CONFIG_CPU_V7M) */
>
>   /*
>    * read_cpuid and read_cpuid_ext should only ever be called on machines that
> @@ -108,9 +141,6 @@ static inline unsigned int __attribute_const__ read_cpuid_id(void)
>
>   #elif defined(CONFIG_CPU_V7M)
>
> -#include <asm/io.h>
> -#include <asm/v7m.h>
> -
>   static inline unsigned int __attribute_const__ read_cpuid_id(void)
>   {
>   	return readl(BASEADDR_V7M_SCB + V7M_SCB_CPUID);
> diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
> index 000499c..0c93588 100644
> --- a/arch/arm/mm/proc-v7m.S
> +++ b/arch/arm/mm/proc-v7m.S
> @@ -144,7 +144,7 @@ __v7m_proc_info:
>   	b	__v7m_setup		@ proc_info_list.__cpu_flush
>   	.long	cpu_arch_name
>   	.long	cpu_elf_name
> -	.long	HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_IDIVT
> +	.long	HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT
>   	.long	cpu_v7m_name
>   	.long	v7m_processor_functions	@ proc_info_list.proc
>   	.long	0			@ proc_info_list.tlb
>

[PATCHv2 3/3] ARM: Work around faulty ISAR0 register in some Krait CPUs

[Stephen Boyd]

From: Stepan Moskovchenko <stepanm@codeaurora.org>

Some early versions of the Krait CPU design incorrectly indicate
that they only support the UDIV and SDIV instructions in Thumb
mode when they actually support them in ARM and Thumb mode. It
seems that these CPUs follow the DDI0406B ARM ARM which has two
possible values for the divide instructions field, instead of the
DDI0406C document which has three possible values.

Work around this problem by checking the MIDR against Krait CPUs
with this faulty ISAR0 register and force the hwcaps to indicate
support in both modes.

Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Stepan Moskovchenko <stepanm@codeaurora.org>
[sboyd: Rewrote commit text to reflect real reasoning now that
	we autodetect udiv/sdiv]
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/mm/proc-v7.S | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index bcd3d48..f584d3f 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -434,6 +434,21 @@ __v7_ca15mp_proc_info:
 	.size	__v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
 
 	/*
+	 * Qualcomm Inc. Krait processors.
+	 */
+	.type	__krait_proc_info, #object
+__krait_proc_info:
+	.long	0x510f0400		@ Required ID value
+	.long	0xff0ffc00		@ Mask for ID
+	/*
+	 * Some Krait processors don't indicate support for SDIV and UDIV
+	 * instructions in the ARM instruction set, even though they actually
+	 * do support them.
+	 */
+	__v7_proc __v7_setup, hwcaps = HWCAP_IDIV
+	.size	__krait_proc_info, . - __krait_proc_info
+
+	/*
 	 * Match any ARMv7 processor core.
 	 */
 	.type	__v7_proc_info, #object
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 3/3] ARM: Work around faulty ISAR0 register in some Krait CPUs

[Will Deacon]

On Mon, Mar 18, 2013 at 06:28:57PM +0000, Stephen Boyd wrote:
> From: Stepan Moskovchenko <stepanm@codeaurora.org>
> 
> Some early versions of the Krait CPU design incorrectly indicate
> that they only support the UDIV and SDIV instructions in Thumb
> mode when they actually support them in ARM and Thumb mode. It
> seems that these CPUs follow the DDI0406B ARM ARM which has two
> possible values for the divide instructions field, instead of the
> DDI0406C document which has three possible values.
> 
> Work around this problem by checking the MIDR against Krait CPUs
> with this faulty ISAR0 register and force the hwcaps to indicate
> support in both modes.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Stepan Moskovchenko <stepanm@codeaurora.org>
> [sboyd: Rewrote commit text to reflect real reasoning now that
> 	we autodetect udiv/sdiv]
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm/mm/proc-v7.S | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCHv2 3/3] ARM: Work around faulty ISAR0 register in some Krait CPUs

[Stephen Boyd]

On 03/18/13 11:34, Will Deacon wrote:
> On Mon, Mar 18, 2013 at 06:28:57PM +0000, Stephen Boyd wrote:
>> From: Stepan Moskovchenko <stepanm@codeaurora.org>
>>
>> Some early versions of the Krait CPU design incorrectly indicate
>> that they only support the UDIV and SDIV instructions in Thumb
>> mode when they actually support them in ARM and Thumb mode. It
>> seems that these CPUs follow the DDI0406B ARM ARM which has two
>> possible values for the divide instructions field, instead of the
>> DDI0406C document which has three possible values.
>>
>> Work around this problem by checking the MIDR against Krait CPUs
>> with this faulty ISAR0 register and force the hwcaps to indicate
>> support in both modes.
>>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Stepan Moskovchenko <stepanm@codeaurora.org>
>> [sboyd: Rewrote commit text to reflect real reasoning now that
>> 	we autodetect udiv/sdiv]
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  arch/arm/mm/proc-v7.S | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
> Acked-by: Will Deacon <will.deacon@arm.com>
>

Thanks. Put all three in the patch tracker.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation