Re: [2.6 patch] make drivers/mtd/cmdlinepart.c:mtdpart_setup() static

["Juha Yrjölä" <juha.yrjola@solidboot.com>]

David Woodhouse wrote:

>>>> --- linux-2.6.17-mm2-full/drivers/mtd/cmdlinepart.c.old 2006-06-26 23:18:39.000000000 +0200
>>>> +++ linux-2.6.17-mm2-full/drivers/mtd/cmdlinepart.c     2006-06-26 23:18:51.000000000 +0200
>>>> @@ -346,7 +346,7 @@
>>>>   *
>>>>   * This function needs to be visible for bootloaders.
>>>>   */
>>>> -int mtdpart_setup(char *s)
>>>> +static int mtdpart_setup(char *s) 
>>> Patch lacks sufficient explanation. Explain the relevance of the comment
>>> immediately above the function declaration, in the context of your
>>> patch. Explain your decision to change the behaviour, but not change the
>>> comment itself.
>> My explanation regarding the relevance of the comment is that it seems 
>> to be nonsense.
>>
>> Do I miss something, or why and how should a bootloader access 
>> in-kernel functions? 
> 
> I'm not entirely sure, but allegedly it does -- Juha, can you elaborate?

Our bootloader doesn't access in-kernel functions, for obvious reasons. 
  The comment in cmdlinepart.c is unfortunately inaccurate.  The 
bootloader does need a mechanism for passing the partition table to the 
kernel, though.  Our partition table is generated on-the-fly by the 
bootloader to guarantee that each partition gets a certain number of 
non-bad NAND blocks.

We used to do this by passing the partition table in a string compatible 
with cmdlinepart syntax.  The kernel NAND driver then just passed the 
string received from the bootloader to mtdpart_setup().

Nowadays there is a better way of doing this, so as far as we are 
concerned, mtdpart_setup() can be made static again.  We'll migrate our 
MTD drivers to use the platform_data mechanism instead.

Cheers,
Juha