[PATCH V4] mtd: Add DiskOnChip G3 support
Robert Jarzmik
robert.jarzmik at free.fr
Tue Sep 27 13:51:18 EDT 2011
Arnd Bergmann <arnd at arndb.de> writes:
> On Thursday 22 September 2011, Robert Jarzmik wrote:
>> >> +#define doc_flashSequence(seq) \
>> >> +do { \
>> >> + doc_dbg("doc_flashSequence: %02x " #seq "\n", DoC_Seq_##seq); \
>> >> + doc_writeb(DoC_Seq_##seq, DoC_FlashSequence); \
>> >> +} while (0)
>> >> +
>> ...zip...
>> >
>> > Could you please turn these macros into 'static inline' function - this
>> > is one of the modern patterns of kernel programming - we try to use
>> > functions for better type checking.
>>
>> No sorry, that I cannot. If you look closely, the ##seq is not something you can
>> convert with an inline function, neither the #seq.
>
> Better not obfuscate the code like that then ;-)
>
> Really, passing the entire register name into an inline function is much
> preferred over string concatenation, because it lets you grep for where
> certain definitions are used.
Right. But how do handle the "#seq" then ?
The whole point here is to write humanly readable debug messages. A message of
the like "doc_flashsequence: 48 SET_PLANE1" is much better than
"doc_flashsequence: 48", don't you think ?
If we consider to loose that debugging messages, it's way easier to drop the "do
.. while(0)" sequences, and just keep the register write.
Now if you're convinced removing the "SET_PLANE1" debugging message is less
important that ability to grep the full register name, why not, but from a
maintenance point of view I would prefer letting the "macroness" in.
> You can also convert them to ALL_CAPS
> identifiers instead of cAmeLCAsE.
Point taken for V6.
>
> Finally, don't use the __raw_readb() style functions but instead use
> the regular readb() style, which is the correct one to use in device
> drivers.
Point taken for V6.
Cheers.
--
Robert
More information about the linux-mtd
mailing list