Re: [PATCH 1/3] [MTD] Flex-OneNAND support

From: Andrew Morton
Date: Wed Mar 04 2009 - 11:51:37 EST


On Wed, 04 Mar 2009 13:41:59 +0200 Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:

> Andrew Morton wrote:
> >> ...
> >>
> >> +static loff_t flexonenand_get_addr(struct onenand_chip *this, int block)
> >> +{
> >> + loff_t ofs = 0;
> >> + int die = 0, boundary;
> >> +
> >> + if (ONENAND_IS_DDP(this) && block >= this->density_mask) {
> >> + block -= this->density_mask;
> >> + die = 1;
> >> + ofs = this->diesize[0];
> >> + }
> >> +
> >> + boundary = this->boundary[die];
> >> + ofs += block << (this->erase_shift - 1);
> >> + if (block > (boundary + 1))
> >> + ofs += (block - boundary - 1) << (this->erase_shift - 1);
> >
> > Both `block' and `boundary' have 32-bit types. Are you sure that the
> > left-shift cannot overflow?
>
> Only very recently has MTD supported sizes greater than 32 bits internally
> for any type of flash. The external APIs (ioctls) are still 32-bit based.
>
> For this driver, supporting sizes over 32-bits is a separate issue - and
> may never be needed.

So it doesn't support files >4G? What's the max device size (now and
projected)?

> >> + return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
> >> +}
> >
> > I wonder what the heck EUCLEAN was invented for and whether MTD's
> > extensive use of it is appropriate.
>
> UBI uses it to detect bit-flips so that data can be moved before it
> can no longer be read. So it is pretty much essential for flash
> memories.

That's not the point.

My point is: for what purpose was EUNCLEAN created by whoever created
it and, given that, is its use by MTD appropriate? Because it does
appear that this gets returned all the way to userspace sometimes.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/