diff mbox

PowerPC: Remove hardcoded BAT configuration of IMMR in CPM early debug console

Message ID 20100528151836.5889.10393.stgit@ES-J7S4D2J.amer.consind.ge.com (mailing list archive)
State Superseded
Delegated to: Kumar Gala
Headers show

Commit Message

Martyn Welch May 28, 2010, 3:18 p.m. UTC 
The CPM early debug console hardcodes the BAT to cover the IMMR at
0xf0000000. The IMMR (on the mpc8270 at the very least) can be set to a
number of locations with bootstrap configuration, which are outside the
hardcoded BAT configuration.

This patch determines the correct location at which to configure a BAT
during the boot process from the supplied PPC_EARLY_DEBUG_CPM_ADDR.

Signed-off-by: Martyn Welch <martyn.welch@ge.com>
---

 arch/powerpc/kernel/head_32.S    |    5 +++--
 arch/powerpc/sysdev/cpm_common.c |    4 +++-
 2 files changed, 6 insertions(+), 3 deletions(-)



--
Martyn Welch (Principal Software Engineer)   |   Registered in England and
GE Intelligent Platforms                     |   Wales (3828642) at 100
T +44(0)127322748                            |   Barbirolli Square, Manchester,
E martyn.welch@ge.com                        |   M2 3AB  VAT:GB 927559189

Comments

Scott Wood May 28, 2010, 4:18 p.m. UTC | #1 
On 05/28/2010 10:18 AM, Martyn Welch wrote:
> The CPM early debug console hardcodes the BAT to cover the IMMR at
> 0xf0000000. The IMMR (on the mpc8270 at the very least) can be set to a
> number of locations with bootstrap configuration, which are outside the
> hardcoded BAT configuration.
>
> This patch determines the correct location at which to configure a BAT
> during the boot process from the supplied PPC_EARLY_DEBUG_CPM_ADDR.
>
> Signed-off-by: Martyn Welch<martyn.welch@ge.com>
> ---
>
>   arch/powerpc/kernel/head_32.S    |    5 +++--
>   arch/powerpc/sysdev/cpm_common.c |    4 +++-
>   2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
> index e025e89..861cace 100644
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>   #endif /* CONFIG_BOOTX_TEXT */
>
>   #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
> +#define PPC_EARLY_DEBUG_CPM_ADDR ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>   setup_cpm_bat:
> -	lis	r8, 0xf000
> +	lis	r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>   	ori	r8, r8,	0x002a
>   	mtspr	SPRN_DBAT1L, r8
>
> -	lis	r11, 0xf000
> +	lis	r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>   	ori	r11, r11, (BL_1M << 2) | 2
>   	mtspr	SPRN_DBAT1U, r11

Only the physical address should depend on where IMMR is.  We should use 
fixmap instead of an arbitrary address for the effective address. 
There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so we'll 
have to either grow it, or map only a subset of IMMR.

Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not 
to the beginning of IMMR, so you should mask off the lower 20 bits (the 
offset is probably less than 64K, and the BAT might just ignore the 
extra bits anyway, but why take chances?).

-Scott
Benjamin Herrenschmidt May 29, 2010, 3:37 a.m. UTC | #2 
On Fri, 2010-05-28 at 11:18 -0500, Scott Wood wrote:
> Only the physical address should depend on where IMMR is.  We should
> use 
> fixmap instead of an arbitrary address for the effective address. 
> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
> we'll 
> have to either grow it, or map only a subset of IMMR.
> 
> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor,
> not 
> to the beginning of IMMR, so you should mask off the lower 20 bits
> (the 
> offset is probably less than 64K, and the BAT might just ignore the 
> extra bits anyway, but why take chances?). 

BAT has other advantages such as limiting TLB usage for things that are
used often. I think we might want to revive Grant work on early ioremap
here :-)

Cheers,
Ben.
Martyn Welch June 1, 2010, 12:52 p.m. UTC | #3 
Scott Wood wrote:
> On 05/28/2010 10:18 AM, Martyn Welch wrote:
>> The CPM early debug console hardcodes the BAT to cover the IMMR at
>> 0xf0000000. The IMMR (on the mpc8270 at the very least) can be set to a
>> number of locations with bootstrap configuration, which are outside the
>> hardcoded BAT configuration.
>>
>> This patch determines the correct location at which to configure a BAT
>> during the boot process from the supplied PPC_EARLY_DEBUG_CPM_ADDR.
>>
>> Signed-off-by: Martyn Welch<martyn.welch@ge.com>
>> ---
>>
>>   arch/powerpc/kernel/head_32.S    |    5 +++--
>>   arch/powerpc/sysdev/cpm_common.c |    4 +++-
>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/head_32.S
>> b/arch/powerpc/kernel/head_32.S
>> index e025e89..861cace 100644
>> --- a/arch/powerpc/kernel/head_32.S
>> +++ b/arch/powerpc/kernel/head_32.S
>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>   #endif /* CONFIG_BOOTX_TEXT */
>>
>>   #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>   setup_cpm_bat:
>> -    lis    r8, 0xf000
>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>       ori    r8, r8,    0x002a
>>       mtspr    SPRN_DBAT1L, r8
>>
>> -    lis    r11, 0xf000
>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>       ori    r11, r11, (BL_1M << 2) | 2
>>       mtspr    SPRN_DBAT1U, r11
>
> Only the physical address should depend on where IMMR is.  We should
> use fixmap instead of an arbitrary address for the effective address.
> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
> we'll have to either grow it, or map only a subset of IMMR.
>

I think that's a more fundamental change to CPM early debug than I can
handle right now.

> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not
> to the beginning of IMMR, so you should mask off the lower 20 bits
> (the offset is probably less than 64K, and the BAT might just ignore
> the extra bits anyway, but why take chances?).
>

I assume that an extra instruction "andi   r8, r8, 0xfff0" after each
"lis" instruction  would be what you are looking for?

Martyn

> -Scott
Martyn Welch June 1, 2010, 1:43 p.m. UTC | #4 
Martyn Welch wrote:
> Scott Wood wrote:
>   
>> On 05/28/2010 10:18 AM, Martyn Welch wrote:
>>     
>>> The CPM early debug console hardcodes the BAT to cover the IMMR at
>>> 0xf0000000. The IMMR (on the mpc8270 at the very least) can be set to a
>>> number of locations with bootstrap configuration, which are outside the
>>> hardcoded BAT configuration.
>>>
>>> This patch determines the correct location at which to configure a BAT
>>> during the boot process from the supplied PPC_EARLY_DEBUG_CPM_ADDR.
>>>
>>> Signed-off-by: Martyn Welch<martyn.welch@ge.com>
>>> ---
>>>
>>>   arch/powerpc/kernel/head_32.S    |    5 +++--
>>>   arch/powerpc/sysdev/cpm_common.c |    4 +++-
>>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/head_32.S
>>> b/arch/powerpc/kernel/head_32.S
>>> index e025e89..861cace 100644
>>> --- a/arch/powerpc/kernel/head_32.S
>>> +++ b/arch/powerpc/kernel/head_32.S
>>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>>   #endif /* CONFIG_BOOTX_TEXT */
>>>
>>>   #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>>   setup_cpm_bat:
>>> -    lis    r8, 0xf000
>>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>       ori    r8, r8,    0x002a
>>>       mtspr    SPRN_DBAT1L, r8
>>>
>>> -    lis    r11, 0xf000
>>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>       ori    r11, r11, (BL_1M << 2) | 2
>>>       mtspr    SPRN_DBAT1U, r11
>>>       
>> Only the physical address should depend on where IMMR is.  We should
>> use fixmap instead of an arbitrary address for the effective address.
>> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
>> we'll have to either grow it, or map only a subset of IMMR.
>>
>>     
>
> I think that's a more fundamental change to CPM early debug than I can
> handle right now.
>
>   
>> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not
>> to the beginning of IMMR, so you should mask off the lower 20 bits
>> (the offset is probably less than 64K, and the BAT might just ignore
>> the extra bits anyway, but why take chances?).
>>
>>     
>
> I assume that an extra instruction "andi   r8, r8, 0xfff0" after each
> "lis" instruction  would be what you are looking for?
>
>   
"andis" even.

Martyn

> Martyn
>
>   
>> -Scott
>>     
>
>
>
Scott Wood June 1, 2010, 4:06 p.m. UTC | #5 
On 06/01/2010 08:43 AM, Martyn Welch wrote:
>>>> diff --git a/arch/powerpc/kernel/head_32.S
>>>> b/arch/powerpc/kernel/head_32.S
>>>> index e025e89..861cace 100644
>>>> --- a/arch/powerpc/kernel/head_32.S
>>>> +++ b/arch/powerpc/kernel/head_32.S
>>>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>>>    #endif /* CONFIG_BOOTX_TEXT */
>>>>
>>>>    #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>>>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>>>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>>>    setup_cpm_bat:
>>>> -    lis    r8, 0xf000
>>>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>        ori    r8, r8,    0x002a
>>>>        mtspr    SPRN_DBAT1L, r8
>>>>
>>>> -    lis    r11, 0xf000
>>>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>        ori    r11, r11, (BL_1M<<  2) | 2
>>>>        mtspr    SPRN_DBAT1U, r11
>>>>
>>> Only the physical address should depend on where IMMR is.  We should
>>> use fixmap instead of an arbitrary address for the effective address.
>>> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
>>> we'll have to either grow it, or map only a subset of IMMR.
>>>
>>>
>>
>> I think that's a more fundamental change to CPM early debug than I can
>> handle right now.

Is IMMRBASE on your board at some address that has a low likelihood of 
conflicting when treated as a kernel effective address?

>>> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not
>>> to the beginning of IMMR, so you should mask off the lower 20 bits
>>> (the offset is probably less than 64K, and the BAT might just ignore
>>> the extra bits anyway, but why take chances?).
>>>
>>>
>>
>> I assume that an extra instruction "andi   r8, r8, 0xfff0" after each
>> "lis" instruction  would be what you are looking for?
>>
>>
> "andis" even.

"lis r8, (PPC_EARLY_DEBUG_CPM_ADDR & 0xfff00000)@h" should work too.

-Scott
Martyn Welch June 2, 2010, 8:06 a.m. UTC | #6 
Scott Wood wrote:
> On 06/01/2010 08:43 AM, Martyn Welch wrote:
>>>>> diff --git a/arch/powerpc/kernel/head_32.S
>>>>> b/arch/powerpc/kernel/head_32.S
>>>>> index e025e89..861cace 100644
>>>>> --- a/arch/powerpc/kernel/head_32.S
>>>>> +++ b/arch/powerpc/kernel/head_32.S
>>>>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>>>>    #endif /* CONFIG_BOOTX_TEXT */
>>>>>
>>>>>    #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>>>>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>>>>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>>>>    setup_cpm_bat:
>>>>> -    lis    r8, 0xf000
>>>>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>>        ori    r8, r8,    0x002a
>>>>>        mtspr    SPRN_DBAT1L, r8
>>>>>
>>>>> -    lis    r11, 0xf000
>>>>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>>        ori    r11, r11, (BL_1M<<  2) | 2
>>>>>        mtspr    SPRN_DBAT1U, r11
>>>>>
>>>> Only the physical address should depend on where IMMR is.  We should
>>>> use fixmap instead of an arbitrary address for the effective address.
>>>> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
>>>> we'll have to either grow it, or map only a subset of IMMR.
>>>>
>>>>
>>>
>>> I think that's a more fundamental change to CPM early debug than I can
>>> handle right now.
>
> Is IMMRBASE on your board at some address that has a low likelihood of
> conflicting when treated as a kernel effective address?

It's at 0x0f000000, is seems ok, but then I'm not sure I fully
understand kernel effective addresses.

Martyn
Scott Wood June 2, 2010, 4:28 p.m. UTC | #7 
On 06/02/2010 03:06 AM, Martyn Welch wrote:
>>>> I think that's a more fundamental change to CPM early debug than I can
>>>> handle right now.
>>
>> Is IMMRBASE on your board at some address that has a low likelihood of
>> conflicting when treated as a kernel effective address?
>
> It's at 0x0f000000, is seems ok, but then I'm not sure I fully
> understand kernel effective addresses.

That overlaps userspace -- is the BAT cleared before userspace starts?

If you don't want to do the fixmap stuff, might want to at least just 
leave it at the current arbitrary effective address, which hasn't seemed 
to cause much trouble so far.

But fixmap is the right way to do it.

-Scott
Martyn Welch June 3, 2010, 9:12 a.m. UTC | #8 
Scott Wood wrote:
> On 06/02/2010 03:06 AM, Martyn Welch wrote:
>>>>> I think that's a more fundamental change to CPM early debug than I
>>>>> can
>>>>> handle right now.
>>>
>>> Is IMMRBASE on your board at some address that has a low likelihood of
>>> conflicting when treated as a kernel effective address?
>>
>> It's at 0x0f000000, is seems ok, but then I'm not sure I fully
>> understand kernel effective addresses.
>
> That overlaps userspace -- is the BAT cleared before userspace starts?
>

To be honest, once I'd got the device booting past the early debug
stage, I rebuilt the kernel without udbg in it...

> If you don't want to do the fixmap stuff, might want to at least just
> leave it at the current arbitrary effective address, which hasn't
> seemed to cause much trouble so far.
>

Given that I've now switched udbg off in the kernel config, I really
can't substantiate spending much more time on this. This patch was
mainly to help others that maybe struggling to bring up Linux on a
device with CPM serial.

I'll try and get a revised patch out soon which keeps the current
arbitrary effective address.

Martyn

> But fixmap is the right way to do it.
>
> -Scott
diff mbox

Patch

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index e025e89..861cace 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -1194,12 +1194,13 @@  setup_disp_bat:
 #endif /* CONFIG_BOOTX_TEXT */
 
 #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
+#define PPC_EARLY_DEBUG_CPM_ADDR ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
 setup_cpm_bat:
-	lis	r8, 0xf000
+	lis	r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
 	ori	r8, r8,	0x002a
 	mtspr	SPRN_DBAT1L, r8
 
-	lis	r11, 0xf000
+	lis	r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
 	ori	r11, r11, (BL_1M << 2) | 2
 	mtspr	SPRN_DBAT1U, r11
 
diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
index 88b9812..984614f 100644
--- a/arch/powerpc/sysdev/cpm_common.c
+++ b/arch/powerpc/sysdev/cpm_common.c
@@ -57,7 +57,9 @@  void __init udbg_init_cpm(void)
 {
 	if (cpm_udbg_txdesc) {
 #ifdef CONFIG_CPM2
-		setbat(1, 0xf0000000, 0xf0000000, 1024*1024, PAGE_KERNEL_NCG);
+#define EARLY_DEBUG_CPM_BAT (CONFIG_PPC_EARLY_DEBUG_CPM_ADDR&0xfff00000)
+		setbat(1, EARLY_DEBUG_CPM_BAT, EARLY_DEBUG_CPM_BAT, 1024*1024,
+			PAGE_KERNEL_NCG);
 #endif
 		udbg_putc = udbg_putc_cpm;
 	}