| Message ID | ed989af6-da88-4e0b-9ed8-126db6cad2e4@nskint.co.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | 55cef88bbf12f3bfbe5c2379a8868a034707e755 |
| Headers | show |
| Series | [v3] spi: spi-cadence-quadspi: Fix division by zero warning | expand |
On 15/07/21 04:21PM, Yoshitaka Ikeda wrote: > Fix below division by zero warning: > - Added an if statement because buswidth can be zero, resulting in division by zero. > - The modified code was based on another driver (atmel-quadspi). > > [ 0.795337] Division by zero in kernel. > : > [ 0.834051] [<807fd40c>] (__div0) from [<804e1acc>] (Ldiv0+0x8/0x10) > [ 0.839097] [<805f0710>] (cqspi_exec_mem_op) from [<805edb4c>] (spi_mem_exec_op+0x3b0/0x3f8) Please add a fixes tag. Fixes: 7512eaf54190 ("spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1") > > Signed-off-by: Yoshitaka Ikeda <ikeda@nskint.co.jp> > --- > v2: > - Fix commit message. > v3: > - repost. > > drivers/spi/spi-cadence-quadspi.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c > index 7a00346ff9b9..13d1f0ce618e 100644 > --- a/drivers/spi/spi-cadence-quadspi.c > +++ b/drivers/spi/spi-cadence-quadspi.c > @@ -307,11 +307,13 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata) > > static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op, bool dtr) > { > - unsigned int dummy_clk; > + unsigned int dummy_clk = 0; I think you can just do: if (!op->dummy.nbytes) return 0; and leave the rest same. I don't think we should have to check for buswidth here, even though it is the one causing divide-by-zero. Any op with positive dummy nbytes but with 0 buswidth is invalid. That should be rejected by the SPI MEM core or the supports_op(), so it should never even get here. > > - dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth); > - if (dtr) > - dummy_clk /= 2; > + if (op->dummy.buswidth && op->dummy.nbytes) { > + dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth); > + if (dtr) > + dummy_clk /= 2; > + } > > return dummy_clk; > } > -- > 2.32.0
On Thu, 15 Jul 2021 16:21:32 +0000, Yoshitaka Ikeda wrote: > Fix below division by zero warning: > - Added an if statement because buswidth can be zero, resulting in division by zero. > - The modified code was based on another driver (atmel-quadspi). > > [ 0.795337] Division by zero in kernel. > : > [ 0.834051] [<807fd40c>] (__div0) from [<804e1acc>] (Ldiv0+0x8/0x10) > [ 0.839097] [<805f0710>] (cqspi_exec_mem_op) from [<805edb4c>] (spi_mem_exec_op+0x3b0/0x3f8) Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: spi-cadence-quadspi: Fix division by zero warning commit: 55cef88bbf12f3bfbe5c2379a8868a034707e755 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
On 2021/07/16 3:52, Pratyush Yadav wrote: > On 15/07/21 04:21PM, Yoshitaka Ikeda wrote: >> Fix below division by zero warning: >> - Added an if statement because buswidth can be zero, resulting in division by zero. >> - The modified code was based on another driver (atmel-quadspi). >> >> [ 0.795337] Division by zero in kernel. >> : >> [ 0.834051] [<807fd40c>] (__div0) from [<804e1acc>] (Ldiv0+0x8/0x10) >> [ 0.839097] [<805f0710>] (cqspi_exec_mem_op) from [<805edb4c>] (spi_mem_exec_op+0x3b0/0x3f8) > > Please add a fixes tag. > > Fixes: 7512eaf54190 ("spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1") > >> >> Signed-off-by: Yoshitaka Ikeda <ikeda@nskint.co.jp> >> --- >> v2: >> - Fix commit message. >> v3: >> - repost. >> >> drivers/spi/spi-cadence-quadspi.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c >> index 7a00346ff9b9..13d1f0ce618e 100644 >> --- a/drivers/spi/spi-cadence-quadspi.c >> +++ b/drivers/spi/spi-cadence-quadspi.c >> @@ -307,11 +307,13 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata) >> >> static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op, bool dtr) >> { >> - unsigned int dummy_clk; >> + unsigned int dummy_clk = 0; > > I think you can just do: > > if (!op->dummy.nbytes) > return 0; > > and leave the rest same. > > I don't think we should have to check for buswidth here, even though it > is the one causing divide-by-zero. Any op with positive dummy nbytes but > with 0 buswidth is invalid. That should be rejected by the SPI MEM core > or the supports_op(), so it should never even get here. Thanks for the detailed explanation. I understand now. I will fix it as you suggested. >> >> - dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth); >> - if (dtr) >> - dummy_clk /= 2; >> + if (op->dummy.buswidth && op->dummy.nbytes) { >> + dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth); >> + if (dtr) >> + dummy_clk /= 2; >> + } >> >> return dummy_clk; >> } >> -- >> 2.32.0 >
Hi Mark, On 15/07/21 08:53PM, Mark Brown wrote: > On Thu, 15 Jul 2021 16:21:32 +0000, Yoshitaka Ikeda wrote: > > Fix below division by zero warning: > > - Added an if statement because buswidth can be zero, resulting in division by zero. > > - The modified code was based on another driver (atmel-quadspi). > > > > [ 0.795337] Division by zero in kernel. > > : > > [ 0.834051] [<807fd40c>] (__div0) from [<804e1acc>] (Ldiv0+0x8/0x10) > > [ 0.839097] [<805f0710>] (cqspi_exec_mem_op) from [<805edb4c>] (spi_mem_exec_op+0x3b0/0x3f8) > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next > > Thanks! > > [1/1] spi: spi-cadence-quadspi: Fix division by zero warning > commit: 55cef88bbf12f3bfbe5c2379a8868a034707e755 FYI, I commented on the patch yesterday and Yoshitaka re-rolled it [0]. Your call whether you want to replace this patch with v4. In either case, I think this patch should be backported to stable so it needs a Fixes tag (which v4 has). [0] https://lore.kernel.org/linux-spi/958bb93b-db83-b685-5fa1-8e5dc40a5975@nskint.co.jp/ > > 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-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c index 7a00346ff9b9..13d1f0ce618e 100644 --- a/drivers/spi/spi-cadence-quadspi.c +++ b/drivers/spi/spi-cadence-quadspi.c @@ -307,11 +307,13 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata) static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op, bool dtr) { - unsigned int dummy_clk; + unsigned int dummy_clk = 0; - dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth); - if (dtr) - dummy_clk /= 2; + if (op->dummy.buswidth && op->dummy.nbytes) { + dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth); + if (dtr) + dummy_clk /= 2; + } return dummy_clk; }
Fix below division by zero warning: - Added an if statement because buswidth can be zero, resulting in division by zero. - The modified code was based on another driver (atmel-quadspi). [ 0.795337] Division by zero in kernel. : [ 0.834051] [<807fd40c>] (__div0) from [<804e1acc>] (Ldiv0+0x8/0x10) [ 0.839097] [<805f0710>] (cqspi_exec_mem_op) from [<805edb4c>] (spi_mem_exec_op+0x3b0/0x3f8) Signed-off-by: Yoshitaka Ikeda <ikeda@nskint.co.jp> --- v2: - Fix commit message. v3: - repost. drivers/spi/spi-cadence-quadspi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)