Re: [RFC] [PATCH] [MTD] Update internal API to support 64-bit device size

[Artem Bityutskiy <dedekind@infradead.org>]

There are 2 compilation warnings:

fs/jffs2/erase.c: In function ‘jffs2_erase_failed’:                                                                                                                      
fs/jffs2/erase.c:178: warning: comparison is always true due to limited range of data type                                                                               
fs/jffs2/erase.c: In function ‘jffs2_erase_callback’:                                                                                                                    
fs/jffs2/erase.c:212: warning: format ‘%08x’ expects type ‘unsigned int’, but argument 2 has type ‘u_int64_t’                                                            

And the first one points to a real problem.

> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index eae26bb..139aeb7 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -15,6 +15,8 @@
>  #include <linux/mtd/compatmac.h>
>  #include <mtd/mtd-abi.h>
>  
> +#include <asm/div64.h>
> +
>  #define MTD_CHAR_MAJOR 90
>  #define MTD_BLOCK_MAJOR 31
>  #define MAX_MTD_DEVICES 32
> @@ -25,16 +27,16 @@
>  #define MTD_ERASE_DONE          0x08
>  #define MTD_ERASE_FAILED        0x10
>  
> -#define MTD_FAIL_ADDR_UNKNOWN 0xffffffff
> +#define MTD_FAIL_ADDR_UNKNOWN -1LL

OK, but look at the 'jffs2_erase_failed()' function:

--------------------------------------------------------------
static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb, uint32_t bad_offset)
{
        /* For NAND, if the failure did not occur at the device level for a
           specific physical page, don't bother updating the bad block table. */
        if (jffs2_cleanmarker_oob(c) && (bad_offset != MTD_FAIL_ADDR_UNKNOWN)) {
                /* We had a device-level failure to erase.  Let's see if we've
                   failed too many times. */
                if (!jffs2_write_nand_badblock(c, jeb, bad_offset)) {
                        /* We'd like to give this block another try. */
... etc ...
--------------------------------------------------------------

bad_offs is uint32_t, and bad_offset != -1LL is always true, which is
a problem.

> +static inline u_int32_t mtd_div_by_ws(u_int64_t sz, struct mtd_info
> *mtd)
> +{
> +	if (mtd->writesize_shift)
> +		return sz >> mtd->writesize_shift;
> +	do_div(sz, mtd->writesize);
> +	return sz;
> +}

I would introduce mtd->writesize_mask instead of calculating it, because
it is presumably more optimal.

> +
> +static inline u_int32_t mtd_mod_by_ws(u_int64_t sz, struct mtd_info
> *mtd)
> +{
> +	if (mtd->writesize_shift)
> +		return sz & ((1 << mtd->writesize_shift) - 1);
> +	return do_div(sz, mtd->writesize);
> +}

Ditto.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)