| Message ID | 1413821173-18347-1-git-send-email-vinod.koul@intel.com |
|---|---|
| State | Rejected |
| Headers | show |
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); > } > > /*
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); >> } >> >> /* >
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()
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.
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
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); } /*
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(-)