| Message ID | 20191211193954.747745-3-jean.pihet@newoldbits.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: spi-ti-qspi: Support large NOR SPI flash | expand |
On Wed, Dec 11, 2019 at 08:39:53PM +0100, Jean Pihet wrote: > + if (op->addr.val + op->data.nbytes > qspi->mmap_size) { > + max_len = qspi->mmap_size - op->addr.val; > + op->data.nbytes = min(op->data.nbytes, max_len); > + } This introduces a massive warning splat for me (just one warning but it's very verbose): CC drivers/spi/spi-ti-qspi.o In file included from drivers/spi/spi-ti-qspi.c:9: drivers/spi/spi-ti-qspi.c: In function ‘ti_qspi_adjust_op_size’: ./include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ^~ ./include/linux/kernel.h:858:4: note: in expansion of macro ‘__typecheck’ (__typecheck(x, y) && __no_side_effects(x, y)) ^~~~~~~~~~~ ./include/linux/kernel.h:868:24: note: in expansion of macro ‘__safe_cmp’ __builtin_choose_expr(__safe_cmp(x, y), \ ^~~~~~~~~~ ./include/linux/kernel.h:877:19: note: in expansion of macro ‘__careful_cmp’ #define min(x, y) __careful_cmp(x, y, <) ^~~~~~~~~~~~~ drivers/spi/spi-ti-qspi.c:535:23: note: in expansion of macro ‘min’ op->data.nbytes = min(op->data.nbytes, max_len); ^~~ ./include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ^~ ./include/linux/kernel.h:858:4: note: in expansion of macro ‘__typecheck’ (__typecheck(x, y) && __no_side_effects(x, y)) ^~~~~~~~~~~ ./include/linux/kernel.h:868:24: note: in expansion of macro ‘__safe_cmp’ __builtin_choose_expr(__safe_cmp(x, y), \ ^~~~~~~~~~ ./include/linux/kernel.h:877:19: note: in expansion of macro ‘__careful_cmp’ #define min(x, y) __careful_cmp(x, y, <) ^~~~~~~~~~~~~ drivers/spi/spi-ti-qspi.c:545:22: note: in expansion of macro ‘min’ op->data.nbytes = min(op->data.nbytes, max_len); ^~~ Using compilers from Debian stable.
On Fri, Dec 20, 2019 at 2:08 PM Mark Brown <broonie@kernel.org> wrote: > > On Wed, Dec 11, 2019 at 08:39:53PM +0100, Jean Pihet wrote: > > > + if (op->addr.val + op->data.nbytes > qspi->mmap_size) { > > + max_len = qspi->mmap_size - op->addr.val; > > + op->data.nbytes = min(op->data.nbytes, max_len); > > + } > > This introduces a massive warning splat for me (just one warning but > it's very verbose): I do not have it here on a buildroot build. Let me check it with more verbose compile flags. Thanks for reviewing! Regards, Jean > > CC drivers/spi/spi-ti-qspi.o > In file included from drivers/spi/spi-ti-qspi.c:9: > drivers/spi/spi-ti-qspi.c: In function ‘ti_qspi_adjust_op_size’: > ./include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > ^~ > ./include/linux/kernel.h:858:4: note: in expansion of macro ‘__typecheck’ > (__typecheck(x, y) && __no_side_effects(x, y)) > ^~~~~~~~~~~ > ./include/linux/kernel.h:868:24: note: in expansion of macro ‘__safe_cmp’ > __builtin_choose_expr(__safe_cmp(x, y), \ > ^~~~~~~~~~ > ./include/linux/kernel.h:877:19: note: in expansion of macro ‘__careful_cmp’ > #define min(x, y) __careful_cmp(x, y, <) > ^~~~~~~~~~~~~ > drivers/spi/spi-ti-qspi.c:535:23: note: in expansion of macro ‘min’ > op->data.nbytes = min(op->data.nbytes, max_len); > ^~~ > ./include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > ^~ > ./include/linux/kernel.h:858:4: note: in expansion of macro ‘__typecheck’ > (__typecheck(x, y) && __no_side_effects(x, y)) > ^~~~~~~~~~~ > ./include/linux/kernel.h:868:24: note: in expansion of macro ‘__safe_cmp’ > __builtin_choose_expr(__safe_cmp(x, y), \ > ^~~~~~~~~~ > ./include/linux/kernel.h:877:19: note: in expansion of macro ‘__careful_cmp’ > #define min(x, y) __careful_cmp(x, y, <) > ^~~~~~~~~~~~~ > drivers/spi/spi-ti-qspi.c:545:22: note: in expansion of macro ‘min’ > op->data.nbytes = min(op->data.nbytes, max_len); > ^~~ > > Using compilers from Debian stable.
Hi Mark, On Fri, Dec 20, 2019 at 2:08 PM Mark Brown <broonie@kernel.org> wrote: > > On Wed, Dec 11, 2019 at 08:39:53PM +0100, Jean Pihet wrote: > > > + if (op->addr.val + op->data.nbytes > qspi->mmap_size) { > > + max_len = qspi->mmap_size - op->addr.val; > > + op->data.nbytes = min(op->data.nbytes, max_len); > > + } > > This introduces a massive warning splat for me (just one warning but > it's very verbose): > > CC drivers/spi/spi-ti-qspi.o > In file included from drivers/spi/spi-ti-qspi.c:9: > drivers/spi/spi-ti-qspi.c: In function ‘ti_qspi_adjust_op_size’: > ./include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > ^~ > ./include/linux/kernel.h:858:4: note: in expansion of macro ‘__typecheck’ > (__typecheck(x, y) && __no_side_effects(x, y)) > ^~~~~~~~~~~ > ./include/linux/kernel.h:868:24: note: in expansion of macro ‘__safe_cmp’ > __builtin_choose_expr(__safe_cmp(x, y), \ > ^~~~~~~~~~ > ./include/linux/kernel.h:877:19: note: in expansion of macro ‘__careful_cmp’ > #define min(x, y) __careful_cmp(x, y, <) > ^~~~~~~~~~~~~ > drivers/spi/spi-ti-qspi.c:535:23: note: in expansion of macro ‘min’ > op->data.nbytes = min(op->data.nbytes, max_len); > ^~~ > ./include/linux/kernel.h:844:29: warning: comparison of distinct pointer types lacks a cast > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > ^~ > ./include/linux/kernel.h:858:4: note: in expansion of macro ‘__typecheck’ > (__typecheck(x, y) && __no_side_effects(x, y)) > ^~~~~~~~~~~ > ./include/linux/kernel.h:868:24: note: in expansion of macro ‘__safe_cmp’ > __builtin_choose_expr(__safe_cmp(x, y), \ > ^~~~~~~~~~ > ./include/linux/kernel.h:877:19: note: in expansion of macro ‘__careful_cmp’ > #define min(x, y) __careful_cmp(x, y, <) > ^~~~~~~~~~~~~ > drivers/spi/spi-ti-qspi.c:545:22: note: in expansion of macro ‘min’ > op->data.nbytes = min(op->data.nbytes, max_len); > ^~~ > > Using compilers from Debian stable. A new v3 release has been submitted, with the warnings fixed with CONFIG_COMPILE_TEST enabled. Thanks, Jean
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c index a18835128ad0..aee4709d105e 100644 --- a/drivers/spi/spi-ti-qspi.c +++ b/drivers/spi/spi-ti-qspi.c @@ -522,6 +522,33 @@ static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8 opcode, QSPI_SPI_SETUP_REG(spi->chip_select)); } +static int ti_qspi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op) +{ + struct ti_qspi *qspi = spi_controller_get_devdata(mem->spi->master); + size_t max_len; + + if (op->data.dir == SPI_MEM_DATA_IN) { + if (op->addr.val < qspi->mmap_size) { + /* Limit MMIO to the mmaped region */ + if (op->addr.val + op->data.nbytes > qspi->mmap_size) { + max_len = qspi->mmap_size - op->addr.val; + op->data.nbytes = min(op->data.nbytes, max_len); + } + } else { + /* + * Use fallback mode (SW generated transfers) above the + * mmaped region. + * Adjust size to comply with the QSPI max frame length. + */ + max_len = QSPI_FRAME; + max_len -= 1 + op->addr.nbytes + op->dummy.nbytes; + op->data.nbytes = min(op->data.nbytes, max_len); + } + } + + return 0; +} + static int ti_qspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op) { @@ -572,6 +599,7 @@ static int ti_qspi_exec_mem_op(struct spi_mem *mem, static const struct spi_controller_mem_ops ti_qspi_mem_ops = { .exec_op = ti_qspi_exec_mem_op, + .adjust_op_size = ti_qspi_adjust_op_size, }; static int ti_qspi_start_transfer_one(struct spi_master *master,
The TI QSPI IP has limitations: - the MMIO region is 64MB in size - in non-MMIO mode, the transfer can handle 4096 words max. Add support for bigger devices. Use MMIO and DMA transfers below the 64MB boundary, use software generated transfers above. Signed-off-by: Jean Pihet <jean.pihet@newoldbits.com> Cc: Ryan Barnett <ryan.barnett@rockwellcollins.com> Cc: Conrad Ratschan <conrad.ratschan@rockwellcollins.com> Cc: Arnout Vandecappelle <arnout.vandecappelle@essensium.com> --- drivers/spi/spi-ti-qspi.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)