* [PATCH v2] mtd: phram: Repair multiple instances support
@ 2013-10-14 16:52 Alexander Sverdlin
2013-11-07 7:31 ` Brian Norris
0 siblings, 1 reply; 4+ messages in thread
From: Alexander Sverdlin @ 2013-10-14 16:52 UTC (permalink / raw)
To: linux-mtd, Joern Engel, David Woodhouse, Brian Norris,
Alexander Sverdlin
mtd: phram: Repair multiple instances support
Commit b2a2a84d35e0f42ad26e326ec4258f6a8b8eecbe (mtd: phram: dot not crash when
built-in and passing boot param) claims to be "based on Ville Herva's similar
patch to block2mtd" (c4e7fb313771ac03dfdca26d30e8b721731c562b), but it has
missed the crucial point of the original path: all these "if(n)def MODULE".
It has broken the possibility to create several phram instances when phram is
compiled as module. The possibility to add instances via /sys writes to
/sys/module/phram/parameters/phram was also broken with mentioned patch.
Proposed patch takes the idea of original block2mtd patch to its full extent.
Assumtion "This function is always called before 'init_phram()'" was also
incorrect, so removed the comment. This patch effectively reverts also
b11ec57fc6e6d4882ef01a0c09a1dde58f50492e (mtd: phram: fix section mismatch for
phram_setup).
v2 changes: Fixed error handling in init_phram().
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
---
--- linux.orig/drivers/mtd/devices/phram.c
+++ linux/drivers/mtd/devices/phram.c
@@ -205,6 +205,8 @@ static inline void kill_final_newline(ch
return 1; \
} while (0)
+#ifndef MODULE
+static int phram_init_called = 0;
/*
* This shall contain the module parameter if any. It is of the form:
* - phram=<device>,<address>,<size> for module case
@@ -213,9 +215,10 @@ static inline void kill_final_newline(ch
* size.
* Example: phram.phram=rootfs,0xa0000000,512Mi
*/
-static __initdata char phram_paramline[64 + 20 + 20];
+static char phram_paramline[64 + 20 + 20];
+#endif
-static int __init phram_setup(const char *val)
+static int phram_setup(const char *val)
{
char buf[64 + 20 + 20], *str = buf;
char *token[3];
@@ -264,17 +267,36 @@ static int __init phram_setup(const char
return ret;
}
-static int __init phram_param_call(const char *val, struct kernel_param *kp)
+static int phram_param_call(const char *val, struct kernel_param *kp)
{
+#ifdef MODULE
+ return phram_setup(val);
+#else
/*
- * This function is always called before 'init_phram()', whether
- * built-in or module.
+ * If more parameters are later passed in via
+ * /sys/module/phram/parameters/phram
+ * and init_phram() has already been called,
+ * we can parse the argument now.
*/
+
+ if (phram_init_called)
+ return phram_setup(val);
+
+ /*
+ * During early boot stage, we only save the parameters
+ * here. We must parse them later: if the param passed
+ * from kernel boot command line, phram_param_call() is
+ * called so early that it is not possible to resolve
+ * the device (even kmalloc() fails). Defer that work to
+ * phram_setup().
+ */
+
if (strlen(val) >= sizeof(phram_paramline))
return -ENOSPC;
strcpy(phram_paramline, val);
return 0;
+#endif
}
module_param_call(phram, phram_param_call, NULL, NULL, 000);
@@ -283,10 +305,15 @@ MODULE_PARM_DESC(phram, "Memory region t
static int __init init_phram(void)
{
+ int ret = 0;
+
+#ifndef MODULE
if (phram_paramline[0])
- return phram_setup(phram_paramline);
+ ret = phram_setup(phram_paramline);
+ phram_init_called = 1;
+#endif
- return 0;
+ return ret;
}
static void __exit cleanup_phram(void)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] mtd: phram: Repair multiple instances support
2013-10-14 16:52 [PATCH v2] mtd: phram: Repair multiple instances support Alexander Sverdlin
@ 2013-11-07 7:31 ` Brian Norris
2013-12-17 7:52 ` Alexander Sverdlin
0 siblings, 1 reply; 4+ messages in thread
From: Brian Norris @ 2013-11-07 7:31 UTC (permalink / raw)
To: Alexander Sverdlin
Cc: Joern Engel, linux-mtd, David Woodhouse, Hervé Fache
Hi Alexander,
On Mon, Oct 14, 2013 at 06:52:23PM +0200, Alexander Sverdlin wrote:
> mtd: phram: Repair multiple instances support
>
> Commit b2a2a84d35e0f42ad26e326ec4258f6a8b8eecbe (mtd: phram: dot not crash when
> built-in and passing boot param) claims to be "based on Ville Herva's similar
> patch to block2mtd" (c4e7fb313771ac03dfdca26d30e8b721731c562b), but it has
> missed the crucial point of the original path: all these "if(n)def MODULE".
> It has broken the possibility to create several phram instances when phram is
> compiled as module. The possibility to add instances via /sys writes to
> /sys/module/phram/parameters/phram was also broken with mentioned patch.
> Proposed patch takes the idea of original block2mtd patch to its full extent.
> Assumtion "This function is always called before 'init_phram()'" was also
> incorrect, so removed the comment. This patch effectively reverts also
> b11ec57fc6e6d4882ef01a0c09a1dde58f50492e (mtd: phram: fix section mismatch for
> phram_setup).
>
> v2 changes: Fixed error handling in init_phram().
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
Can we get any testers? Perhaps Hervé can confirm this?
> ---
> --- linux.orig/drivers/mtd/devices/phram.c
> +++ linux/drivers/mtd/devices/phram.c
> @@ -205,6 +205,8 @@ static inline void kill_final_newline(ch
> return 1; \
> } while (0)
>
> +#ifndef MODULE
> +static int phram_init_called = 0;
> /*
> * This shall contain the module parameter if any. It is of the form:
> * - phram=<device>,<address>,<size> for module case
> @@ -213,9 +215,10 @@ static inline void kill_final_newline(ch
> * size.
> * Example: phram.phram=rootfs,0xa0000000,512Mi
> */
> -static __initdata char phram_paramline[64 + 20 + 20];
> +static char phram_paramline[64 + 20 + 20];
> +#endif
>
> -static int __init phram_setup(const char *val)
> +static int phram_setup(const char *val)
> {
> char buf[64 + 20 + 20], *str = buf;
> char *token[3];
> @@ -264,17 +267,36 @@ static int __init phram_setup(const char
> return ret;
> }
>
> -static int __init phram_param_call(const char *val, struct kernel_param *kp)
> +static int phram_param_call(const char *val, struct kernel_param *kp)
> {
> +#ifdef MODULE
> + return phram_setup(val);
> +#else
> /*
> - * This function is always called before 'init_phram()', whether
> - * built-in or module.
> + * If more parameters are later passed in via
> + * /sys/module/phram/parameters/phram
> + * and init_phram() has already been called,
> + * we can parse the argument now.
> */
> +
> + if (phram_init_called)
> + return phram_setup(val);
> +
> + /*
> + * During early boot stage, we only save the parameters
> + * here. We must parse them later: if the param passed
> + * from kernel boot command line, phram_param_call() is
> + * called so early that it is not possible to resolve
> + * the device (even kmalloc() fails). Defer that work to
> + * phram_setup().
> + */
> +
> if (strlen(val) >= sizeof(phram_paramline))
> return -ENOSPC;
> strcpy(phram_paramline, val);
>
> return 0;
> +#endif
> }
>
> module_param_call(phram, phram_param_call, NULL, NULL, 000);
> @@ -283,10 +305,15 @@ MODULE_PARM_DESC(phram, "Memory region t
>
> static int __init init_phram(void)
> {
> + int ret = 0;
> +
> +#ifndef MODULE
> if (phram_paramline[0])
> - return phram_setup(phram_paramline);
> + ret = phram_setup(phram_paramline);
> + phram_init_called = 1;
> +#endif
>
> - return 0;
> + return ret;
> }
>
> static void __exit cleanup_phram(void)
Brian
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] mtd: phram: Repair multiple instances support
2013-11-07 7:31 ` Brian Norris
@ 2013-12-17 7:52 ` Alexander Sverdlin
2014-01-28 23:45 ` Brian Norris
0 siblings, 1 reply; 4+ messages in thread
From: Alexander Sverdlin @ 2013-12-17 7:52 UTC (permalink / raw)
To: ext Brian Norris
Cc: Joern Engel, linux-mtd, David Woodhouse, Hervé Fache
Hello Brian,
On 07/11/13 08:31, ext Brian Norris wrote:
> On Mon, Oct 14, 2013 at 06:52:23PM +0200, Alexander Sverdlin wrote:
>> mtd: phram: Repair multiple instances support
>>
>> Commit b2a2a84d35e0f42ad26e326ec4258f6a8b8eecbe (mtd: phram: dot not crash when
>> built-in and passing boot param) claims to be "based on Ville Herva's similar
>> patch to block2mtd" (c4e7fb313771ac03dfdca26d30e8b721731c562b), but it has
>> missed the crucial point of the original path: all these "if(n)def MODULE".
>> It has broken the possibility to create several phram instances when phram is
>> compiled as module. The possibility to add instances via /sys writes to
>> /sys/module/phram/parameters/phram was also broken with mentioned patch.
>> Proposed patch takes the idea of original block2mtd patch to its full extent.
>> Assumtion "This function is always called before 'init_phram()'" was also
>> incorrect, so removed the comment. This patch effectively reverts also
>> b11ec57fc6e6d4882ef01a0c09a1dde58f50492e (mtd: phram: fix section mismatch for
>> phram_setup).
>>
>> v2 changes: Fixed error handling in init_phram().
>>
>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
>
> Can we get any testers? Perhaps Hervé can confirm this?
Have addressed Ville Herva, Ryosuke Saito, Hervé Fache on 07.11.2013, no answer
until now. Seems nobody interested in it any more. Any chances we still get this in?
>> ---
>> --- linux.orig/drivers/mtd/devices/phram.c
>> +++ linux/drivers/mtd/devices/phram.c
>> @@ -205,6 +205,8 @@ static inline void kill_final_newline(ch
>> return 1; \
>> } while (0)
>>
>> +#ifndef MODULE
>> +static int phram_init_called = 0;
>> /*
>> * This shall contain the module parameter if any. It is of the form:
>> * - phram=<device>,<address>,<size> for module case
>> @@ -213,9 +215,10 @@ static inline void kill_final_newline(ch
>> * size.
>> * Example: phram.phram=rootfs,0xa0000000,512Mi
>> */
>> -static __initdata char phram_paramline[64 + 20 + 20];
>> +static char phram_paramline[64 + 20 + 20];
>> +#endif
>>
>> -static int __init phram_setup(const char *val)
>> +static int phram_setup(const char *val)
>> {
>> char buf[64 + 20 + 20], *str = buf;
>> char *token[3];
>> @@ -264,17 +267,36 @@ static int __init phram_setup(const char
>> return ret;
>> }
>>
>> -static int __init phram_param_call(const char *val, struct kernel_param *kp)
>> +static int phram_param_call(const char *val, struct kernel_param *kp)
>> {
>> +#ifdef MODULE
>> + return phram_setup(val);
>> +#else
>> /*
>> - * This function is always called before 'init_phram()', whether
>> - * built-in or module.
>> + * If more parameters are later passed in via
>> + * /sys/module/phram/parameters/phram
>> + * and init_phram() has already been called,
>> + * we can parse the argument now.
>> */
>> +
>> + if (phram_init_called)
>> + return phram_setup(val);
>> +
>> + /*
>> + * During early boot stage, we only save the parameters
>> + * here. We must parse them later: if the param passed
>> + * from kernel boot command line, phram_param_call() is
>> + * called so early that it is not possible to resolve
>> + * the device (even kmalloc() fails). Defer that work to
>> + * phram_setup().
>> + */
>> +
>> if (strlen(val) >= sizeof(phram_paramline))
>> return -ENOSPC;
>> strcpy(phram_paramline, val);
>>
>> return 0;
>> +#endif
>> }
>>
>> module_param_call(phram, phram_param_call, NULL, NULL, 000);
>> @@ -283,10 +305,15 @@ MODULE_PARM_DESC(phram, "Memory region t
>>
>> static int __init init_phram(void)
>> {
>> + int ret = 0;
>> +
>> +#ifndef MODULE
>> if (phram_paramline[0])
>> - return phram_setup(phram_paramline);
>> + ret = phram_setup(phram_paramline);
>> + phram_init_called = 1;
>> +#endif
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static void __exit cleanup_phram(void)
>
> Brian
>
>
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] mtd: phram: Repair multiple instances support
2013-12-17 7:52 ` Alexander Sverdlin
@ 2014-01-28 23:45 ` Brian Norris
0 siblings, 0 replies; 4+ messages in thread
From: Brian Norris @ 2014-01-28 23:45 UTC (permalink / raw)
To: Alexander Sverdlin
Cc: Joern Engel, linux-mtd, David Woodhouse, Hervé Fache
On Tue, Dec 17, 2013 at 08:52:49AM +0100, Alexander Sverdlin wrote:
> On 07/11/13 08:31, ext Brian Norris wrote:
> > On Mon, Oct 14, 2013 at 06:52:23PM +0200, Alexander Sverdlin wrote:
> >> mtd: phram: Repair multiple instances support
> >>
> >> Commit b2a2a84d35e0f42ad26e326ec4258f6a8b8eecbe (mtd: phram: dot not crash when
> >> built-in and passing boot param) claims to be "based on Ville Herva's similar
> >> patch to block2mtd" (c4e7fb313771ac03dfdca26d30e8b721731c562b), but it has
> >> missed the crucial point of the original path: all these "if(n)def MODULE".
> >> It has broken the possibility to create several phram instances when phram is
> >> compiled as module. The possibility to add instances via /sys writes to
> >> /sys/module/phram/parameters/phram was also broken with mentioned patch.
> >> Proposed patch takes the idea of original block2mtd patch to its full extent.
> >> Assumtion "This function is always called before 'init_phram()'" was also
> >> incorrect, so removed the comment. This patch effectively reverts also
> >> b11ec57fc6e6d4882ef01a0c09a1dde58f50492e (mtd: phram: fix section mismatch for
> >> phram_setup).
> >>
> >> v2 changes: Fixed error handling in init_phram().
> >>
> >> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
> >
> > Can we get any testers? Perhaps Hervé can confirm this?
>
> Have addressed Ville Herva, Ryosuke Saito, Hervé Fache on 07.11.2013, no answer
> until now. Seems nobody interested in it any more. Any chances we still get this in?
Yeah, looks ok. Pushed to l2-mtd.git/next, for 3.15.
Brian
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-28 23:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-14 16:52 [PATCH v2] mtd: phram: Repair multiple instances support Alexander Sverdlin
2013-11-07 7:31 ` Brian Norris
2013-12-17 7:52 ` Alexander Sverdlin
2014-01-28 23:45 ` Brian Norris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).