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

[Yoshitaka Ikeda <ikeda@xxxxxxxxxxxx>]

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@xxxxxxxxxxxx>
>> ---
>> 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
>