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@xxxxxxxxxxxxx
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
_______________________________
