Re: [PATCH] eXcite nand flash driver

[Thomas Koeller <thomas.koeller@baslerweb.com>]

On Thursday 08 February 2007 09:20, Ulrich Eckhardt wrote:
> Just some cosmetic notes ...
>
> On Thursday 08 February 2007 01:57, Thomas Koeller wrote:
> > @@ -216,10 +216,26 @@ config MTD_NAND_DISKONCHIP_BBTWRITE
> >  	  Even if you leave this disabled, you can enable BBT writes at module
> >  	  load time (assuming you build diskonchip as a module) with the module
> >  	  parameter "inftl_bbt_write=1".
> > -
> > +
>
> This just inserts whitespace.

No, it actually _removes_ whitspace.

>
> >         tristate "NAND support for OLPC CAFÉ chip"
>
> The accent on the E might cause problems, but I'm not sure.

This has been in there before.

>
> > +#define io_readb(__a__)		__raw_readb((__a__))
> > +#define io_writeb(__v__, __a__)	__raw_writeb((__v__), (__a__))
>
> I would have used an inline function. Also, aren't there functions to use
> IOs already? What's the reason here?
>
> > +/* prefix for debug output */
> > +static const char module_id[] = "excite_nandflash";
>
> I'd have used
>
>   static const char* module_id = "excite_nandflash";
>
> except if you need the sizeof it to capture the length.

Then I'd have used
	static const char * const module_id = "excite_nandflash";

but what is the advantage of doing so?

>
> > +static inline const struct resource *excite_nand_get_resource
> > +    (struct platform_device *d, unsigned long flags, const char
> > *basename){ +	const char fmt[] = "%s_%u";
>
> Here, too, is no need for a local buffer, just use a pointer.
>
> > +/* excite_nand_probe
> > + *
> > + * called by device layer when it finds a device matching
> > + * one our driver can handled. [...]
>
> "... when it finds a device our driver can handle."

Right.

>
> Otherwise I'd say it looks very clean, better than quite some other stuff I
> have seen.
>
> Uli
>

Thanks for your comments!


_______________________________

Thomas Köller, Software Developer

Basler Vision Technologies
An der Strusbek 60-62
22926 Ahrensburg
Germany

Tel. +49 (0) 4102 - 463 390
Fax +49 (0) 4102 - 463 390

mailto:thomas.koeller@baslerweb.com
http://www.baslerweb.com

Vorstand: Dr.-Ing. Dietmar Ley (Vorsitzender) · John P. Jennings · Peter Krumhoff · Aufsichtsratsvorsitzender: Norbert Basler
Basler AG · Amtsgericht Ahrensburg HRB 4090 · Ust-IdNr.: DE 135 098 121 · Steuer-Nr.: 30 292 04497

_______________________________