diff mbox

Make phram work with kernel cmdline args when built-in

Message ID CA+Xn3W18bwZWzdjmRzEDUCqe3bVHnmW-7bJ_wvaoXpJxO1KmjQ@mail.gmail.com
State New, archived
Headers show

Commit Message

Fache, Herve Oct. 13, 2011, 1:30 p.m. UTC 
Hi again Jörn,

Please find attached a revised version of the patch.

On Wed, Oct 12, 2011 at 18:48, Jörn Engel <joern@logfs.org> wrote:
> Looks good in principle.

Thanks :-)

> What I slightly dislike about this patch is the tiny differences
> between Ville Herva's and this one.  In block2mtd those same lines are
> added in a different place.  While I don't care much where to put
> them, I see some value in having duplicated code stand out as
> duplicated code and not have subtle variations all over the place.  So
> my preference would be to either change this patch or alternatively
> move the code in block2mtd as well.

Thanks for your politeness. I have now moved things in the same place
as the original patch for consistency.

> Same here.  phram_setup_late may be a better name, but it is
> inconsistent with block2mtd_setup2.

Indeed, I have now renamed the function phram_setup2 in my patch.

> So while I have no preference for either of the two variants, I would
> like to keep the two drivers as similar as possible.  Apart from that
> minor thing, nice work!

I have updated my patch rather than block2mtd, so we minimize the changes.

Thanks a lot for your help.
Hervé

--
Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve
Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920

Comments

Artem Bityutskiy Oct. 14, 2011, 1:03 p.m. UTC | #1 
On Thu, 2011-10-13 at 15:30 +0200, Fache, Herve wrote:
> Hi again Jörn,
> 
> Please find attached a revised version of the patch.

Please, send patches inline. Also, try to make sure you do not
have many of these #ifdef MODULE sections.
Fache, Herve Oct. 14, 2011, 1:44 p.m. UTC | #2 
Hi Artem,

2011/10/14 Artem Bityutskiy <dedekind1@gmail.com>
> Please, send patches inline. Also, try to make sure you do not
> have many of these #ifdef MODULE sections.

I am sorry but AIUI my company's e-mail system does not seem to allow
me to set patches the proper way. Is there any chance that you can
accept it inline for this time? Or can I have an account on
infradead.org, in which case I'll just push a git tree?

Also, I have tried to make my patch look like the other patch to
block2mtd, as asked by Jörn, which explains the number of #ifdef
MODULE sections...

Regards,
Hervé
--
Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve
Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920
Jörn Engel Oct. 14, 2011, 2:38 p.m. UTC | #3 
On Fri, 14 October 2011 15:42:09 +0200, Fache, Herve wrote:
> 2011/10/14 Artem Bityutskiy <dedekind1@gmail.com>
> 
> > Please, send patches inline. Also, try to make sure you do not
> > have many of these #ifdef MODULE sections.
> 
> Also, I have tried to make my patch look like the other patch to block2mtd,
> as asked by Jörn, which explains the number of #ifdef MODULE sections...

If there were a way to get rid of the #ifdefs, I would certainly be in
favor of that.  While I am happy with the functional improvements, I
would be even happier if the code looked less scarred.

I wonder how everyone else does it.  Those two driver surely can't be
the only ones that depend on module parameters for initialization.

Jörn
Fache, Herve Oct. 14, 2011, 2:48 p.m. UTC | #4 
On Fri, Oct 14, 2011 at 16:38, Jörn Engel <joern@logfs.org> wrote:
> If there were a way to get rid of the #ifdefs, I would certainly be in
> favor of that.  While I am happy with the functional improvements, I
> would be even happier if the code looked less scarred.
>
> I wonder how everyone else does it.  Those two driver surely can't be
> the only ones that depend on module parameters for initialization.

Looks like it's just a matter of properly using 'phram_init_called'.
Shall I submit a new version of the patch that does that?

Hervé
Jörn Engel Oct. 14, 2011, 3:18 p.m. UTC | #5 
On Fri, 14 October 2011 16:48:45 +0200, Fache, Herve wrote:
> 
> Looks like it's just a matter of properly using 'phram_init_called'.
> Shall I submit a new version of the patch that does that?

Since my brain cannot quite translate that into code yet: yes please.

Jörn
Artem Bityutskiy Oct. 16, 2011, 11:55 a.m. UTC | #6 
On Fri, 2011-10-14 at 15:44 +0200, Fache, Herve wrote:
> Hi Artem,
> 
> 2011/10/14 Artem Bityutskiy <dedekind1@gmail.com>
> > Please, send patches inline. Also, try to make sure you do not
> > have many of these #ifdef MODULE sections.
> 
> I am sorry but AIUI my company's e-mail system does not seem to allow
> me to set patches the proper way. Is there any chance that you can
> accept it inline for this time? Or can I have an account on
> infradead.org, in which case I'll just push a git tree?

Not sure about infradead.org, you should ask David Woodhouse, not me. 
Just create gmail.com account and use ssh tunneling via your home
machine or some other host, or use your corporate proxy if it allows
smtp/imap traffic. Note, infradead.org account is not easier to used
than gmail.com.
diff mbox

Patch

From a42930c635d00afa9dab80476eae964213694455 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herv=C3=A9=20Fache?= <h-fache@ti.com>
Date: Tue, 4 Oct 2011 18:35:46 +0200
Subject: [PATCH] phram: make kernel boot command line arguments work
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch is based on Ville Herva's similar patch to block2mtd.

Trying to pass kernel command line arguments at boot-time did not work, as
phram_setup() was called so early that kmalloc() was not functional.

This patch only saves the option string at the early boot stage, and parses
it later when init_phram() is called.

With this patch, I can boot with a statically-compiled phram, and mount a
ext2 root fs from physical RAM, without the need for a initrd.

Signed-off-by: Hervé Fache <h-fache@ti.com>
---
 drivers/mtd/devices/phram.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 23423bd..93c7ebf 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -233,7 +233,12 @@  static inline void kill_final_newline(char *str)
 	return 1;		\
 } while (0)
 
-static int phram_setup(const char *val, struct kernel_param *kp)
+#ifndef MODULE
+static int phram_init_called;
+static __initdata char phram_paramline[64+12+12];
+#endif
+
+static int phram_setup2(const char *val)
 {
 	char buf[64+12+12], *str = buf;
 	char *token[3];
@@ -282,13 +287,47 @@  static int phram_setup(const char *val, struct kernel_param *kp)
 	return ret;
 }
 
+static int phram_setup(const char *val, struct kernel_param *kp)
+{
+#ifdef MODULE
+	return phram_setup2(val);
+#else
+	/* 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 directly. */
+
+	if (phram_init_called)
+		return phram_setup2(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_setup() is
+	   called so early that it is not possible to resolve
+	   the device (kmalloc() fails). Defer that work to
+	   phram_setup2(), called by init_phram(). */
+
+	strlcpy(phram_paramline, val, sizeof(phram_paramline));
+
+	return 0;
+#endif
+}
+
 module_param_call(phram, phram_setup, NULL, NULL, 000);
 MODULE_PARM_DESC(phram, "Memory region to map. \"phram=<name>,<start>,<length>\"");
 
 
 static int __init init_phram(void)
 {
-	return 0;
+	int ret = 0;
+
+#ifndef MODULE
+	if (strlen(phram_paramline))
+		ret = phram_setup2(phram_paramline);
+	phram_init_called = 1;
+#endif
+
+	return ret;
 }
 
 static void __exit cleanup_phram(void)
-- 
1.7.0.4