Both occurences of div64_u64() just have a u8 or u32 divisor. Use
div_u64() instead.
Reported-by : Dan Carpenter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 3e1f1913536b..028514c6996f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2893,7 +2893,7 @@ static int spi_nor_late_init_params(struct spi_nor *nor)
spi_nor_init_default_locking_ops(nor);
if (params->n_banks > 1)
- params->bank_size = div64_u64(params->size, params->n_banks);
+ params->bank_size = div_u64(params->size, params->n_banks);
return 0;
}
@@ -3406,7 +3406,7 @@ static int spi_nor_set_mtd_eraseregions(struct spi_nor *nor)
return -EINVAL;
mtd_region[i].erasesize = erasesize;
- mtd_region[i].numblocks = div64_ul(region[i].size, erasesize);
+ mtd_region[i].numblocks = div_u64(region[i].size, erasesize);
mtd_region[i].offset = region[i].offset;
}
--
2.39.2
Hi,
On Mon, Apr 29 2024, Michael Walle wrote:
> Both occurences of div64_u64() just have a u8 or u32 divisor. Use
> div_u64() instead.
Does this improve performance or is this only for correctness?
Patch LGTM otherwise.
Reviewed-by: Pratyush Yadav <[email protected]>
BTW, I also noticed that there is a do_div() call in spi_nor_write()
that also uses a u64 dividend and u32 divisor. I was wondering why it
uses do_div() and not div_u64() (I am not sure what the difference
between the two is) but I suppose it doesn't matter much since your
spring cleaning series will delete that code anyway.
--
Regards,
Pratyush Yadav
Hi,
On Mon Apr 29, 2024 at 3:27 PM CEST, Pratyush Yadav wrote:
> On Mon, Apr 29 2024, Michael Walle wrote:
>
> > Both occurences of div64_u64() just have a u8 or u32 divisor. Use
> > div_u64() instead.
>
> Does this improve performance or is this only for correctness?
See function doc for div_u64():
* This is the most common 64bit divide and should be used if possible,
* as many 32bit archs can optimize this variant better than a full 64bit
* divide.
> Patch LGTM otherwise.
>
> Reviewed-by: Pratyush Yadav <[email protected]>
>
> BTW, I also noticed that there is a do_div() call in spi_nor_write()
> that also uses a u64 dividend and u32 divisor. I was wondering why it
> uses do_div() and not div_u64() (I am not sure what the difference
> between the two is) but I suppose it doesn't matter much since your
> spring cleaning series will delete that code anyway.
do_div() is a macro and is modifying the dividend in place, whereas
div_u64() will return it. do_div() is using u32 for the divisor
anyway.
-michael
On Mon, Apr 29 2024, Michael Walle wrote:
> Hi,
>
> On Mon Apr 29, 2024 at 3:27 PM CEST, Pratyush Yadav wrote:
>> On Mon, Apr 29 2024, Michael Walle wrote:
>>
>> > Both occurences of div64_u64() just have a u8 or u32 divisor. Use
>> > div_u64() instead.
>>
>> Does this improve performance or is this only for correctness?
>
> See function doc for div_u64():
>
> * This is the most common 64bit divide and should be used if possible,
> * as many 32bit archs can optimize this variant better than a full 64bit
> * divide.
Thanks. I think it would be good to add this to the commit message:
Both occurences of div64_u64() just have a u8 or u32 divisor. Use
div_u64() instead. Many 32 bit architectures can optimize this
variant better than a full 64 bit divide.
No need to resend, I can do this when applying.
>
>> Patch LGTM otherwise.
>>
>> Reviewed-by: Pratyush Yadav <[email protected]>
>>
>> BTW, I also noticed that there is a do_div() call in spi_nor_write()
>> that also uses a u64 dividend and u32 divisor. I was wondering why it
>> uses do_div() and not div_u64() (I am not sure what the difference
>> between the two is) but I suppose it doesn't matter much since your
>> spring cleaning series will delete that code anyway.
>
> do_div() is a macro and is modifying the dividend in place, whereas
> div_u64() will return it. do_div() is using u32 for the divisor
> anyway.
>
> -michael
>
--
Regards,
Pratyush Yadav
On Mon, Apr 29 2024, Michael Walle wrote:
> Both occurences of div64_u64() just have a u8 or u32 divisor. Use
> div_u64() instead.
Applied, thanks! I fixed up the commit message a little by adding the bit
about performance on 32 bit arch, and fixed some typos.
[1/1] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()
commit: https://git.kernel.org/mtd/c/c84b3925c7d6c
--
Regards,
Pratyush Yadav