Patchwork avr32: fix build error in atstk1006_defconfig

login
register
mail settings
Submitter Wu, Josh
Date Oct. 16, 2012, 7:19 a.m.
Message ID <1350371967-8914-1-git-send-email-josh.wu@atmel.com>
Download mbox | patch
Permalink /patch/191742/
State New
Headers show

Comments

Wu, Josh - Oct. 16, 2012, 7:19 a.m.
fixed the following compile error when use avr32 atstk1006_defconfig:
  drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
  drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 'writel_relaxed'

which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add Programmable Multibit ECC controller support").
The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is no macro "writel_relaxed" defined. This patch add writex_relaxed macro definitions.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 arch/avr32/include/asm/io.h |    4 ++++
 1 file changed, 4 insertions(+)
Hans-Christian Egtvedt - Oct. 16, 2012, 9:17 a.m.
Around Tue 16 Oct 2012 15:19:27 +0800 or thereabout, Josh Wu wrote:
> fixed the following compile error when use avr32 atstk1006_defconfig:
>   drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
>   drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 'writel_relaxed'
> 
> which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add Programmable Multibit ECC controller support").
> The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is no macro "writel_relaxed" defined. This patch add writex_relaxed macro definitions.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>  arch/avr32/include/asm/io.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> index cf60d0a..fc6483f 100644
> --- a/arch/avr32/include/asm/io.h
> +++ b/arch/avr32/include/asm/io.h
> @@ -165,6 +165,10 @@ BUILDIO_IOPORT(l, u32)
>  #define readw_be			__raw_readw
>  #define readl_be			__raw_readl
>  
> +#define writeb_relaxed			writeb
> +#define writew_relaxed			writew
> +#define writel_relaxed			writel
> +

I'm wondering if they should be something similar to SH arch:

#define writeb_relaxed(v,c) ((void)__raw_writeb((__force u8)ioswabb(v),c))

What is the intention behind the macro? Which restriction is relaxed?

>  #define writeb_be			__raw_writeb
>  #define writew_be			__raw_writew
>  #define writel_be			__raw_writel
Wu, Josh - Oct. 16, 2012, 10:30 a.m.
On 10/16/2012 5:17 PM, Hans-Christian Egtvedt wrote:
> Around Tue 16 Oct 2012 15:19:27 +0800 or thereabout, Josh Wu wrote:
>> fixed the following compile error when use avr32 atstk1006_defconfig:
>>    drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
>>    drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 'writel_relaxed'
>>
>> which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add Programmable Multibit ECC controller support").
>> The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is no macro "writel_relaxed" defined. This patch add writex_relaxed macro definitions.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>   arch/avr32/include/asm/io.h |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
>> index cf60d0a..fc6483f 100644
>> --- a/arch/avr32/include/asm/io.h
>> +++ b/arch/avr32/include/asm/io.h
>> @@ -165,6 +165,10 @@ BUILDIO_IOPORT(l, u32)
>>   #define readw_be			__raw_readw
>>   #define readl_be			__raw_readl
>>   
>> +#define writeb_relaxed			writeb
>> +#define writew_relaxed			writew
>> +#define writel_relaxed			writel
>> +
> I'm wondering if they should be something similar to SH arch:
>
> #define writeb_relaxed(v,c) ((void)__raw_writeb((__force u8)ioswabb(v),c))
>
> What is the intention behind the macro? Which restriction is relaxed?

According to my understanding, the xxx_relaxed() is that I/O function 
without any memory barriers. for Multi-cpu, the execute order are less 
limited.
So the relaxed write function should be more effective than non-relaxed one.

But for single cpu, relaxed function should work same as non-relaxed 
function.
Please correct me if I'm understand in a wrong way.

Best Regards,
Josh Wu

>
>>   #define writeb_be			__raw_writeb
>>   #define writew_be			__raw_writew
>>   #define writel_be			__raw_writel
Andreas Bießmann - Oct. 16, 2012, 11:30 a.m.
On 16.10.2012 11:17, Hans-Christian Egtvedt wrote:
> Around Tue 16 Oct 2012 15:19:27 +0800 or thereabout, Josh Wu wrote:
>> fixed the following compile error when use avr32 atstk1006_defconfig:
>>   drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
>>   drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 'writel_relaxed'
>>
>> which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add Programmable Multibit ECC controller support").
>> The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is no macro "writel_relaxed" defined. This patch add writex_relaxed macro definitions.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>  arch/avr32/include/asm/io.h |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
>> index cf60d0a..fc6483f 100644
>> --- a/arch/avr32/include/asm/io.h
>> +++ b/arch/avr32/include/asm/io.h
>> @@ -165,6 +165,10 @@ BUILDIO_IOPORT(l, u32)
>>  #define readw_be			__raw_readw
>>  #define readl_be			__raw_readl
>>  
>> +#define writeb_relaxed			writeb
>> +#define writew_relaxed			writew
>> +#define writel_relaxed			writel
>> +
> 
> I'm wondering if they should be something similar to SH arch:
> 
> #define writeb_relaxed(v,c) ((void)__raw_writeb((__force u8)ioswabb(v),c))

I think it is OK to just use the writeX here. These are in fact an
inlined function built out of a macro. The inlined function does nearly
what you have written down here and in contrast to the ARM version omits
the memory barrier.
The main difference between __raw_ access and the writeX/readX access
for AVR32 is this little __swizzle_addr_. It takes the configuration of
SMC (8/16/32 bit access) into account which would be omitted when we
switch to __raw_ accessors here. This would be a major change if one
used the writeX/readX accessors before and switched the SMC to something
different than 32 Bit access.

Best regards

Andreas Bießmann
Havard Skinnemoen - Oct. 16, 2012, 6:04 p.m.
On Tue, Oct 16, 2012 at 12:19 AM, Josh Wu <josh.wu@atmel.com> wrote:
> fixed the following compile error when use avr32 atstk1006_defconfig:
>   drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
>   drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 'writel_relaxed'
>
> which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add Programmable Multibit ECC controller support").
> The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is no macro "writel_relaxed" defined. This patch add writex_relaxed macro definitions.
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>

Acked-by: Havard Skinnemoen <havard@skinnemoen.net>

I think it's OK to just forward to write[bwl] -- the relaxed variants
merely provide fewer ordering guarantees than the non-relaxed
variants. The worst that could happen is a small performance hit (and
on AVR32, even that is highly unlikely).

Havard
Wu, Josh - April 1, 2013, 6:12 a.m.
Sorry, please drop this email. I will send out another one to include 
akpm@linux-foundation.org

Best Regards,
Josh Wu

On 4/1/2013 1:58 PM, Josh Wu wrote:
> fixed the following compile error when use avr32 atstk1006_defconfig:
>    drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
>    drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 'writel_relaxed'
>
> which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add Programmable Multibit ECC controller support").
> The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is no macro "writel_relaxed" defined. This patch add writex_relaxed macro definitions.
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> Acked-by: Havard Skinnemoen <havard@skinnemoen.net>
> Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>
>
> ---
> arch/avr32/include/asm/io.h |    4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
> index cf60d0a..fc6483f 100644
> --- a/arch/avr32/include/asm/io.h
> +++ b/arch/avr32/include/asm/io.h
> @@ -165,6 +165,10 @@ BUILDIO_IOPORT(l, u32)
>   #define readw_be			__raw_readw
>   #define readl_be			__raw_readl
>   
> +#define writeb_relaxed			writeb
> +#define writew_relaxed			writew
> +#define writel_relaxed			writel
> +
>   #define writeb_be			__raw_writeb
>   #define writew_be			__raw_writew
>   #define writel_be			__raw_writel

Patch

diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index cf60d0a..fc6483f 100644
--- a/arch/avr32/include/asm/io.h
+++ b/arch/avr32/include/asm/io.h
@@ -165,6 +165,10 @@  BUILDIO_IOPORT(l, u32)
 #define readw_be			__raw_readw
 #define readl_be			__raw_readl
 
+#define writeb_relaxed			writeb
+#define writew_relaxed			writew
+#define writel_relaxed			writel
+
 #define writeb_be			__raw_writeb
 #define writew_be			__raw_writew
 #define writel_be			__raw_writel