[PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bit copy

[PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bit copy

[Vinod Koul]

The driver was also using own method to do 32bit copy, turns out
we have a kernel API so use that instead

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/mtd/nand/atmel_nand.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index e321c56..b03e80d 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -265,14 +265,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
 		*t++ = readl_relaxed(s++);
 }
 
-static void memcpy32_toio(void __iomem *trg, const void *src, int size)
+static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
 {
-	int i;
-	u32 __iomem *t = trg;
-	const u32 *s = src;
-
-	for (i = 0; i < (size >> 2); i++)
-		writel_relaxed(*s++, t++);
+	/* __iowrite32_copy use 32bit size values so divide by 4 */
+	__iowrite32_copy(trg, src, size/4);
 }
 
 /*
-- 
1.7.0.4

[PATCH 2/2] mtd: mxc_nand: use __iowrite32_copy for 32 bit copy

[Vinod Koul]

The driver was also using own method to do 32bit copy, turns out
we have a kernel API so use that instead

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/mtd/nand/mxc_nand.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index dba262b..3794cd7 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -280,14 +280,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
 		*t++ = __raw_readl(s++);
 }
 
-static void memcpy32_toio(void __iomem *trg, const void *src, int size)
+static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
 {
-	int i;
-	u32 __iomem *t = trg;
-	const u32 *s = src;
-
-	for (i = 0; i < (size >> 2); i++)
-		__raw_writel(*s++, t++);
+	/* __iowrite32_copy use 32bit size values so divide by 4 */
+	__iowrite32_copy(trg, src, size/4);
 }
 
 static int check_int_v3(struct mxc_nand_host *host)
-- 
1.7.0.4

Re: [PATCH 2/2] mtd: mxc_nand: use __iowrite32_copy for 32 bit copy

[Brian Norris]

On Mon, Oct 20, 2014 at 09:36:13PM +0530, Vinod Koul wrote:
> The driver was also using own method to do 32bit copy, turns out
> we have a kernel API so use that instead
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  drivers/mtd/nand/mxc_nand.c |   10 +++-------
>  1 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index dba262b..3794cd7 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -280,14 +280,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
>  		*t++ = __raw_readl(s++);
>  }
>  
> -static void memcpy32_toio(void __iomem *trg, const void *src, int size)
> +static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
>  {
> -	int i;
> -	u32 __iomem *t = trg;
> -	const u32 *s = src;
> -
> -	for (i = 0; i < (size >> 2); i++)
> -		__raw_writel(*s++, t++);
> +	/* __iowrite32_copy use 32bit size values so divide by 4 */
> +	__iowrite32_copy(trg, src, size/4);

Fixed up the spacing around the division, and pushed to l2-mtd.git.

>  }
>  
>  static int check_int_v3(struct mxc_nand_host *host)

Brian

Re: [PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bit copy

[Josh Wu]

Hi, Vinod

On 10/21/2014 12:06 AM, Vinod Koul wrote:
> The driver was also using own method to do 32bit copy, turns out
> we have a kernel API so use that instead
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

Thanks for the patch.
Acked-by: Josh Wu <josh.wu@atmel.com>

BTW, is there any similar kernel API that is for the read from io?

Best Regards,
Josh Wu

> ---
>   drivers/mtd/nand/atmel_nand.c |   10 +++-------
>   1 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index e321c56..b03e80d 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -265,14 +265,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
>   		*t++ = readl_relaxed(s++);
>   }
>   
> -static void memcpy32_toio(void __iomem *trg, const void *src, int size)
> +static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
>   {
> -	int i;
> -	u32 __iomem *t = trg;
> -	const u32 *s = src;
> -
> -	for (i = 0; i < (size >> 2); i++)
> -		writel_relaxed(*s++, t++);
> +	/* __iowrite32_copy use 32bit size values so divide by 4 */
> +	__iowrite32_copy(trg, src, size/4);
>   }
>   
>   /*

Re: [PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bitcopy

[Herve Codina]

Hi,

I didn't go deeper in atmel_nand.c code to see other accesses but old
copy use writel_relaxed which is a macro to __raw_writel((__force u32)
cpu_to_le32(v),c)

__iowrite32_copy use directly __raw_writel(*src++, dst++)

So we skip cpu_to_le32. Is it ok for all system using atmel_nand ?

Best regards,
Herve Codina


Le 21/10/2014 12:03, Josh Wu a écrit :
> Hi, Vinod
> 
> On 10/21/2014 12:06 AM, Vinod Koul wrote:
>> The driver was also using own method to do 32bit copy, turns out
>> we have a kernel API so use that instead
>>
>> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> 
> Thanks for the patch.
> Acked-by: Josh Wu <josh.wu@atmel.com>
> 
> BTW, is there any similar kernel API that is for the read from io?
> 
> Best Regards,
> Josh Wu
> 
>> ---
>>   drivers/mtd/nand/atmel_nand.c |   10 +++-------
>>   1 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index e321c56..b03e80d 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -265,14 +265,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
>>   		*t++ = readl_relaxed(s++);
>>   }
>>   
>> -static void memcpy32_toio(void __iomem *trg, const void *src, int size)
>> +static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
>>   {
>> -	int i;
>> -	u32 __iomem *t = trg;
>> -	const u32 *s = src;
>> -
>> -	for (i = 0; i < (size >> 2); i++)
>> -		writel_relaxed(*s++, t++);
>> +	/* __iowrite32_copy use 32bit size values so divide by 4 */
>> +	__iowrite32_copy(trg, src, size/4);
>>   }
>>   
>>   /*
> 

Re: [PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bitcopy

[Vinod Koul]

On Tue, Oct 21, 2014 at 12:20:06PM +0200, Herve Codina wrote:
> Hi,
Please don't top post
> 
> I didn't go deeper in atmel_nand.c code to see other accesses but old
> copy use writel_relaxed which is a macro to __raw_writel((__force u32)
> cpu_to_le32(v),c)
> 
> __iowrite32_copy use directly __raw_writel(*src++, dst++)
> 
> So we skip cpu_to_le32. Is it ok for all system using atmel_nand ?
Also would be a good question if we need barriers as __iowrite32_copy()
doesn't guarantee any ordering.

-- 
~Vinod

> 
> Best regards,
> Herve Codina
> 
> 
> Le 21/10/2014 12:03, Josh Wu a écrit :
> > Hi, Vinod
> > 
> > On 10/21/2014 12:06 AM, Vinod Koul wrote:
> >> The driver was also using own method to do 32bit copy, turns out
> >> we have a kernel API so use that instead
> >>
> >> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > 
> > Thanks for the patch.
> > Acked-by: Josh Wu <josh.wu@atmel.com>
> > 
> > BTW, is there any similar kernel API that is for the read from io?
> > 
> > Best Regards,
> > Josh Wu
> > 
> >> ---
> >>   drivers/mtd/nand/atmel_nand.c |   10 +++-------
> >>   1 files changed, 3 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> >> index e321c56..b03e80d 100644
> >> --- a/drivers/mtd/nand/atmel_nand.c
> >> +++ b/drivers/mtd/nand/atmel_nand.c
> >> @@ -265,14 +265,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
> >>   		*t++ = readl_relaxed(s++);
> >>   }
> >>   
> >> -static void memcpy32_toio(void __iomem *trg, const void *src, int size)
> >> +static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
> >>   {
> >> -	int i;
> >> -	u32 __iomem *t = trg;
> >> -	const u32 *s = src;
> >> -
> >> -	for (i = 0; i < (size >> 2); i++)
> >> -		writel_relaxed(*s++, t++);
> >> +	/* __iowrite32_copy use 32bit size values so divide by 4 */
> >> +	__iowrite32_copy(trg, src, size/4);
> >>   }
> >>   
> >>   /*
> > 

-- 

Re: [PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bitcopy

[Josh Wu]

Hi,

On 10/21/2014 6:35 PM, Vinod Koul wrote:
> On Tue, Oct 21, 2014 at 12:20:06PM +0200, Herve Codina wrote:
>> Hi,
> Please don't top post
>> I didn't go deeper in atmel_nand.c code to see other accesses but old
>> copy use writel_relaxed which is a macro to __raw_writel((__force u32)
>> cpu_to_le32(v),c)
>>
>> __iowrite32_copy use directly __raw_writel(*src++, dst++)
>>
>> So we skip cpu_to_le32. Is it ok for all system using atmel_nand ?
> Also would be a good question if we need barriers as __iowrite32_copy()
> doesn't guarantee any ordering.
>

Just diving the code, I found the atmel-nand code use this function to 
transfer write these buffer to NFC sram.
And the NFC sram is not io space.
Also there should has no issue in barriers as it is in a SRAM.

So I think right way is use memcpy function to replace the 
ioread32/iowrite32. Since we use them for SRAM transfer not IO.
I'll prepare a new patch which do above replace.

Best Regards,
Josh Wu

Re: [PATCH 1/2] mtd: atmel_nand: use __iowrite32_copy for 32 bit copy

[Vinod Koul]

On Tue, Oct 21, 2014 at 06:03:24PM +0800, Josh Wu wrote:
> Hi, Vinod
> 
> On 10/21/2014 12:06 AM, Vinod Koul wrote:
> >The driver was also using own method to do 32bit copy, turns out
> >we have a kernel API so use that instead
> >
> >Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> 
> Thanks for the patch.
> Acked-by: Josh Wu <josh.wu@atmel.com>
> 
> BTW, is there any similar kernel API that is for the read from io?
I have not stumbled out on that yet, if someone know of it pls yell!!
If there seems to be lack of it, I would suggest we add a similar API in
core like __iowrite32_copy()

-- 
~Vinod

> 
> Best Regards,
> Josh Wu
> 
> >---
> >  drivers/mtd/nand/atmel_nand.c |   10 +++-------
> >  1 files changed, 3 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> >index e321c56..b03e80d 100644
> >--- a/drivers/mtd/nand/atmel_nand.c
> >+++ b/drivers/mtd/nand/atmel_nand.c
> >@@ -265,14 +265,10 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
> >  		*t++ = readl_relaxed(s++);
> >  }
> >-static void memcpy32_toio(void __iomem *trg, const void *src, int size)
> >+static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
> >  {
> >-	int i;
> >-	u32 __iomem *t = trg;
> >-	const u32 *s = src;
> >-
> >-	for (i = 0; i < (size >> 2); i++)
> >-		writel_relaxed(*s++, t++);
> >+	/* __iowrite32_copy use 32bit size values so divide by 4 */
> >+	__iowrite32_copy(trg, src, size/4);
> >  }
> >  /*
> 

--