| Message ID | 20200922114905.2942859-1-gch981213@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4cafaddedb5fbef9531202ee547784409fd0de33 |
| Headers | show |
| Series | [v2] spi: spi-mtk-nor: fix timeout calculation overflow | expand |
On Tue, Sep 22, 2020 at 07:49:02PM +0800, Chuanhong Guo wrote: > if ((op->data.dir == SPI_MEM_DATA_IN) && > mtk_nor_match_read(op)) { > + // limit size to prevent timeout calculation overflow > + if (op->data.nbytes > 0x400000) > + op->data.nbytes = 0x400000; If there's a limit on transfer sizes there should also be a max_transfer_size or max_message_size set (which we should pay attention to in the core for flash stuff but IIRC we didn't do that yet).
Hi! On Tue, Sep 22, 2020 at 8:02 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Sep 22, 2020 at 07:49:02PM +0800, Chuanhong Guo wrote: > > > if ((op->data.dir == SPI_MEM_DATA_IN) && > > mtk_nor_match_read(op)) { > > + // limit size to prevent timeout calculation overflow > > + if (op->data.nbytes > 0x400000) > > + op->data.nbytes = 0x400000; > > If there's a limit on transfer sizes there should also be a > max_transfer_size or max_message_size set (which we should pay attention > to in the core for flash stuff but IIRC we didn't do that yet). There's already a 6-byte max_message_size limit on this controller. spi-mem dma read is the only operation which allows such a long transfer.
On Tue, Sep 22, 2020 at 8:02 PM Mark Brown <broonie@kernel.org> wrote: > (which we should pay attention to in the core for flash > stuff but IIRC we didn't do that yet). BTW we do have that taken care. spi_mem_adjust_op_size will adjust the transfer size according to max_transfer/message_size if no custom adjust_op_size hook is defined in the driver. If a custom adjust_op_size is defined, the driver adjusts the transfer size for it's exec_op hook. The size limit between exec_op and transfer_one_message can be different. (this spi-mtk-nor is an example of that.)
On Tue, 22 Sep 2020 19:49:02 +0800, Chuanhong Guo wrote: > CLK_TO_US macro is used to calculate potential transfer time for various > timeout handling. However it overflows on transfer bigger than 512 bytes > because it first did (len * 8 * 1000000). > This controller typically operates at 45MHz. This patch did 2 things: > 1. calculate clock / 1000000 first > 2. add a 4M transfer size cap so that the final timeout in DMA reading > doesn't overflow Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: spi-mtk-nor: fix timeout calculation overflow commit: 4cafaddedb5fbef9531202ee547784409fd0de33 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c index 6e6ca2b8e6c82..62f5ff2779884 100644 --- a/drivers/spi/spi-mtk-nor.c +++ b/drivers/spi/spi-mtk-nor.c @@ -89,7 +89,7 @@ // Buffered page program can do one 128-byte transfer #define MTK_NOR_PP_SIZE 128 -#define CLK_TO_US(sp, clkcnt) ((clkcnt) * 1000000 / sp->spi_freq) +#define CLK_TO_US(sp, clkcnt) DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000) struct mtk_nor { struct spi_controller *ctlr; @@ -177,6 +177,10 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op) if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) { if ((op->data.dir == SPI_MEM_DATA_IN) && mtk_nor_match_read(op)) { + // limit size to prevent timeout calculation overflow + if (op->data.nbytes > 0x400000) + op->data.nbytes = 0x400000; + if ((op->addr.val & MTK_NOR_DMA_ALIGN_MASK) || (op->data.nbytes < MTK_NOR_DMA_ALIGN)) op->data.nbytes = 1;
CLK_TO_US macro is used to calculate potential transfer time for various timeout handling. However it overflows on transfer bigger than 512 bytes because it first did (len * 8 * 1000000). This controller typically operates at 45MHz. This patch did 2 things: 1. calculate clock / 1000000 first 2. add a 4M transfer size cap so that the final timeout in DMA reading doesn't overflow Fixes: 881d1ee9fe81f ("spi: add support for mediatek spi-nor controller") Cc: <stable@vger.kernel.org> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> --- Change since v1: fix transfer size cap to 4M drivers/spi/spi-mtk-nor.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)