[PATCH] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()

[PATCH] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()

[Michael Walle]

Both occurences of div64_u64() just have a u8 or u32 divisor. Use
div_u64() instead.

Reported-by : Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/r/9ba7f4e6-2b8b-44a3-9cac-9ed6e50f1700@moroto.mountain/
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 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

Re: [PATCH] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()

[Pratyush Yadav]

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 <pratyush@kernel.org>

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

Re: [PATCH] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

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 <pratyush@kernel.org>
>
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()

[Pratyush Yadav]

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 <pratyush@kernel.org>
>>
>> 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

Re: [PATCH] mtd: spi-nor: replace unnecessary div64_u64() with div_u64()

[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