| Message ID | 1397242806-7575-12-git-send-email-computersforpeace@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Fri, Apr 11, 2014 at 12:00:06PM -0700, Brian Norris wrote: > This corrects some sparse warnings: > > drivers/mtd/spi-nor/fsl-quadspi.c:281:31: warning: incorrect type in initializer (different address spaces) [sparse] > drivers/mtd/spi-nor/fsl-quadspi.c:281:31: expected void *[noderef] <asn:2>base [sparse] > drivers/mtd/spi-nor/fsl-quadspi.c:281:31: got void [noderef] <asn:2>*iobase [sparse] > (etc.) > > and > > drivers/mtd/spi-nor/fsl-quadspi.c:733:53: warning: incorrect type in argument 2 (different address spaces) > drivers/mtd/spi-nor/fsl-quadspi.c:733:53: expected void const *<noident> > drivers/mtd/spi-nor/fsl-quadspi.c:733:53: got void [noderef] <asn:2>* > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > new in v3 > > drivers/mtd/spi-nor/fsl-quadspi.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c > index b41bbbc531ff..e85046e734e4 100644 > --- a/drivers/mtd/spi-nor/fsl-quadspi.c > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c > @@ -278,7 +278,7 @@ static irqreturn_t fsl_qspi_irq_handler(int irq, void *dev_id) > > static void fsl_qspi_init_lut(struct fsl_qspi *q) > { > - void *__iomem base = q->iobase; > + void __iomem *base = q->iobase; > int rxfifo = q->devtype_data->rxfifo; > u32 lut_base; > u8 cmd, addrlen, dummy; > @@ -422,7 +422,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) > static int > fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len) > { > - void *__iomem base = q->iobase; > + void __iomem *base = q->iobase; thanks for pointing this. > int seqid; > u32 reg, reg2; > int err; > @@ -730,7 +730,7 @@ static int fsl_qspi_read(struct spi_nor *nor, loff_t from, > return ret; > > /* Read out the data directly from the AHB buffer.*/ > - memcpy(buf, q->ahb_base + q->chip_base_addr + from, len); > + memcpy_fromio(buf, q->ahb_base + q->chip_base_addr + from, len); ARM does some optimazation to the memcpy, please see arch/arm/lib/memcpy.S. But the memcpy_fromio does not do optimazation, please see _memcpy_fromio. Someone ever sent a patch to fix the issue, but Russell did not merge it. please see: http://lists.infradead.org/pipermail/linux-arm-kernel/2009-November/003860.html So we'd better do not change to use memcpy_fromio. BTW: i am okay with other patches about the spi-nor. thanks Huang Shijie
On Sat, Apr 12, 2014 at 12:57:03PM +0800, Huang Shijie wrote: > On Fri, Apr 11, 2014 at 12:00:06PM -0700, Brian Norris wrote: > > @@ -730,7 +730,7 @@ static int fsl_qspi_read(struct spi_nor *nor, loff_t from, > > return ret; > > > > /* Read out the data directly from the AHB buffer.*/ > > - memcpy(buf, q->ahb_base + q->chip_base_addr + from, len); > > + memcpy_fromio(buf, q->ahb_base + q->chip_base_addr + from, len); > ARM does some optimazation to the memcpy, please see arch/arm/lib/memcpy.S. > > But the memcpy_fromio does not do optimazation, please see _memcpy_fromio. Ah, yes. I noticed this actually, but didn't think much about it. > Someone ever sent a patch to fix the issue, but Russell did not merge it. That's unfortunate. > please see: > http://lists.infradead.org/pipermail/linux-arm-kernel/2009-November/003860.html > > So we'd better do not change to use memcpy_fromio. How about the last recommendation of that thread, readsl()? http://lists.infradead.org/pipermail/linux-arm-kernel/2009-November/003984.html It has your optimization, and it has the type-checking we'd like. > BTW: i am okay with other patches about the spi-nor. Thanks. I'll probably merge the first 10 soon and see about respinning this one eventually. Brian
On Fri, Apr 11, 2014 at 11:13:33PM -0700, Brian Norris wrote: > On Sat, Apr 12, 2014 at 12:57:03PM +0800, Huang Shijie wrote: > > On Fri, Apr 11, 2014 at 12:00:06PM -0700, Brian Norris wrote: > > > @@ -730,7 +730,7 @@ static int fsl_qspi_read(struct spi_nor *nor, loff_t from, > > > return ret; > > > > > > /* Read out the data directly from the AHB buffer.*/ > > > - memcpy(buf, q->ahb_base + q->chip_base_addr + from, len); > > > + memcpy_fromio(buf, q->ahb_base + q->chip_base_addr + from, len); > > ARM does some optimazation to the memcpy, please see arch/arm/lib/memcpy.S. > > > > But the memcpy_fromio does not do optimazation, please see _memcpy_fromio. > > Ah, yes. I noticed this actually, but didn't think much about it. > > > Someone ever sent a patch to fix the issue, but Russell did not merge it. > > That's unfortunate. > > > please see: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2009-November/003860.html > > > > So we'd better do not change to use memcpy_fromio. > > How about the last recommendation of that thread, readsl()? > [1] firstly, I think it is not an IO read actually. We read the data from the AHB memory address in fact, a miss will make the controller to trigger a QUAD read to the NOR flash. [2] secondly, the memcpy can work by the unit of 8 bytes (or more?), while the readsl can work by the unit of 4 bytes at most. So use memcpy here is better then using the readsl. > http://lists.infradead.org/pipermail/linux-arm-kernel/2009-November/003984.html > > It has your optimization, and it has the type-checking we'd like. > > > BTW: i am okay with other patches about the spi-nor. > > Thanks. I'll probably merge the first 10 soon and see about respinning > this one eventually. Please merge the first 10 patches as soon as possible. :) thanks Huang Shijie
diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c index b41bbbc531ff..e85046e734e4 100644 --- a/drivers/mtd/spi-nor/fsl-quadspi.c +++ b/drivers/mtd/spi-nor/fsl-quadspi.c @@ -278,7 +278,7 @@ static irqreturn_t fsl_qspi_irq_handler(int irq, void *dev_id) static void fsl_qspi_init_lut(struct fsl_qspi *q) { - void *__iomem base = q->iobase; + void __iomem *base = q->iobase; int rxfifo = q->devtype_data->rxfifo; u32 lut_base; u8 cmd, addrlen, dummy; @@ -422,7 +422,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) static int fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len) { - void *__iomem base = q->iobase; + void __iomem *base = q->iobase; int seqid; u32 reg, reg2; int err; @@ -730,7 +730,7 @@ static int fsl_qspi_read(struct spi_nor *nor, loff_t from, return ret; /* Read out the data directly from the AHB buffer.*/ - memcpy(buf, q->ahb_base + q->chip_base_addr + from, len); + memcpy_fromio(buf, q->ahb_base + q->chip_base_addr + from, len); *retlen += len; return 0;
This corrects some sparse warnings: drivers/mtd/spi-nor/fsl-quadspi.c:281:31: warning: incorrect type in initializer (different address spaces) [sparse] drivers/mtd/spi-nor/fsl-quadspi.c:281:31: expected void *[noderef] <asn:2>base [sparse] drivers/mtd/spi-nor/fsl-quadspi.c:281:31: got void [noderef] <asn:2>*iobase [sparse] (etc.) and drivers/mtd/spi-nor/fsl-quadspi.c:733:53: warning: incorrect type in argument 2 (different address spaces) drivers/mtd/spi-nor/fsl-quadspi.c:733:53: expected void const *<noident> drivers/mtd/spi-nor/fsl-quadspi.c:733:53: got void [noderef] <asn:2>* Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- new in v3 drivers/mtd/spi-nor/fsl-quadspi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)