Subject: Re: [RFC PATCH 10/19] powerpc: gamecube/wii: early debugging using usbgecko

From: Albert Herranz <[email-masked]>
Date: Mon, 23 Nov 2009 20:10:55 +0100

Arnd Bergmann wrote:
> On Sunday 22 November 2009, Albert Herranz wrote:
>> +#ifdef CONFIG_PPC_EARLY_DEBUG_USBGECKO
>> +setup_usbgecko_bat:
>> +	/* prepare a BAT for early io */
>> +	lis	r8, 0x0c00
>> +	ori	r8, r8, 0x002a	/* uncached, guarded ,rw */
>> +	lis	r11, 0xcc00
>> +	ori	r11, r11, 0x3	/* 128K */
>> +#ifdef CONFIG_WII
>> +	oris	r8, r8, 0x0100
>> +	oris	r11, r11, 0x0100
>> +#endif
>> +	mtspr	SPRN_DBAT1L, r8
>> +	mtspr	SPRN_DBAT1U, r11
>> +	sync
>> +	isync
>> +	blr
>> +#endif
> 
> This will probably break other platforms if CONFIG_PPC_EARLY_DEBUG_USBGECKO
> is set. In general, we try hard to make it possible to build generic
> kernels for multiple systems, so it would be better to also add a runtime
> check here.
> 

Ok, I see the point.
But, what makes CONFIG_PPC_EARLY_DEBUG_USBGECKO case different from CONFIG_PPC_EARLY_DEBUG_CPM case here?

>> --- a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.h
>> +++ b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.h
>> @@ -27,4 +27,10 @@ static inline void __init ug_udbg_init(void)
>>  
>>  #endif /* CONFIG_USBGECKO_UDBG */
>>  
>> +#ifdef CONFIG_PPC_EARLY_DEBUG_USBGECKO
>> +
>> +void __init udbg_init_usbgecko(void);
>> +
>> +#endif /* CONFIG_PPC_EARLY_DEBUG_USBGECKO */
>> +
> 
> No need to enclose a declaration in #ifdef, better leave it there
> unconditionally, unless you have an alternative version, like
> 
> #ifdef CONFIG_PPC_EARLY_DEBUG_USBGECKO
> void __init udbg_init_usbgecko(void);
> #else /* !CONFIG_PPC_EARLY_DEBUG_USBGECKO */
> static inline void udbg_init_usbgecko(void)
> {
> }
> #endif /* CONFIG_PPC_EARLY_DEBUG_USBGECKO */
> 
> That style is now more common than having additional #ifdefs
> in the code using the function.

I'll fix that too. Thanks.

Cheers,
Albert