Re: [PATCH] mtd: spinand: micron: add support for MT29F1G01AAADD

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Uwe,

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote on Tue, 20 Aug
2019 10:28:37 +0200:

> Hello Miquèl,
> 
> On Mon, Aug 19, 2019 at 10:17:18AM +0200, Miquel Raynal wrote:
> > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019
> > 10:22:32 +0200:  
> > > +static int mt29f1g01aaadd_ooblayout_ecc(struct mtd_info *mtd, int section,
> > > +					struct mtd_oob_region *region)
> > > +{
> > > +	if (section > 3)
> > > +		return -ERANGE;
> > > +
> > > +	region->offset = (section * 0x10) + 8;  
> > 
> > Any reason to use hex here?         ^
> > 
> > If not I would prefer decimal numbers.  
> 
> IMHO it is quite common to use hexadecimal also for register offsets,
> not only for register values.
> 
> I checked a few drivers (drivers/mtd/nand/raw/mxc_nand.c,
> drivers/clk/meson/g12a.c, drivers/gpio/gpio-sch.c) and they all use hex
> also for the offsets, so it seems to be at least usual. Also as offsets
> for repeating registers are usually powers of two, hexadecimal constants
> are more suitable IMHO.

Absolutely. But here region->offset is not a register offset at all, it
is an indication for the upper layers of the position of an area within
a bigger buffer (here: where are the ECC bytes in my buffer in RAM). I
don't think hexadecimal numbers have any interest here.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/