[PATCH v3] spi: spi-cadence-quadspi: Fix division by zero warning

[PATCH v3] spi: spi-cadence-quadspi: Fix division by zero warning

[Yoshitaka Ikeda]

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(-)

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;
 }
-- 
2.32.0

Re: [PATCH v3] spi: spi-cadence-quadspi: Fix division by zero warning

[Pratyush Yadav]

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

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH v3] spi: spi-cadence-quadspi: Fix division by zero warning

[Mark Brown]

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

Re: [PATCH v3] spi: spi-cadence-quadspi: Fix division by zero warning

[Yoshitaka Ikeda]

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
> 

Re: [PATCH v3] spi: spi-cadence-quadspi: Fix division by zero warning

[Pratyush Yadav]

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

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.