All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
@ 2011-09-20  4:50 ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2011-09-20  4:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

We used to overwrite with CONFIG_CMDLINE if we found a chosen
node but failed to get bootargs out of it or they were empty,
unless CONFIG_CMDLINE_FORCE is set.

Instead change that to overwrite if "data" is non empty after
the bootargs check. It allows arch code to have other mechanisms
to retrieve the command line prior to parsing the device-tree.

Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
as it won't work as it-is if the device-tree has no /chosen node

Signed-off-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
CC: devicetree-discuss-w3QoH2/gNlk/bJ5BZ2RsiQ@public.gmane.org
CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---
 drivers/of/fdt.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

v2. Use "data" instead of "cmd_line" so it works on archs like
mips who don't pass cmd_line to that function to start with, also
add a comment explaining the mechanism.

(resent with right list address as well while at it)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 65200af..323b722 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 	if (p != NULL && l > 0)
 		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
 
+	/*
+	 * CONFIG_CMDLINE is meant to be a default in case nothing else
+	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
+	 * is set in which case we override whatever was found earlier.
+	 */
 #ifdef CONFIG_CMDLINE
 #ifndef CONFIG_CMDLINE_FORCE
-	if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
+	if (!data[0])
 #endif
 		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
 #endif /* CONFIG_CMDLINE */
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
@ 2011-09-20  4:50 ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2011-09-20  4:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss

We used to overwrite with CONFIG_CMDLINE if we found a chosen
node but failed to get bootargs out of it or they were empty,
unless CONFIG_CMDLINE_FORCE is set.

Instead change that to overwrite if "data" is non empty after
the bootargs check. It allows arch code to have other mechanisms
to retrieve the command line prior to parsing the device-tree.

Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
as it won't work as it-is if the device-tree has no /chosen node

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: devicetree-discuss@lists-ozlabs.org
CC: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/of/fdt.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

v2. Use "data" instead of "cmd_line" so it works on archs like
mips who don't pass cmd_line to that function to start with, also
add a comment explaining the mechanism.

(resent with right list address as well while at it)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 65200af..323b722 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 	if (p != NULL && l > 0)
 		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
 
+	/*
+	 * CONFIG_CMDLINE is meant to be a default in case nothing else
+	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
+	 * is set in which case we override whatever was found earlier.
+	 */
 #ifdef CONFIG_CMDLINE
 #ifndef CONFIG_CMDLINE_FORCE
-	if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
+	if (!data[0])
 #endif
 		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
 #endif /* CONFIG_CMDLINE */
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
  2011-09-20  4:50 ` Benjamin Herrenschmidt
@ 2011-09-20  4:55   ` Grant Likely
  -1 siblings, 0 replies; 9+ messages in thread
From: Grant Likely @ 2011-09-20  4:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linuxppc-dev

On Tue, Sep 20, 2011 at 02:50:15PM +1000, Benjamin Herrenschmidt wrote:
> We used to overwrite with CONFIG_CMDLINE if we found a chosen
> node but failed to get bootargs out of it or they were empty,
> unless CONFIG_CMDLINE_FORCE is set.
> 
> Instead change that to overwrite if "data" is non empty after
> the bootargs check. It allows arch code to have other mechanisms
> to retrieve the command line prior to parsing the device-tree.
> 
> Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> as it won't work as it-is if the device-tree has no /chosen node
> 
> Signed-off-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> CC: devicetree-discuss-w3QoH2/gNlk/bJ5BZ2RsiQ@public.gmane.org
> CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

Looks okay to me.

Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

> ---
>  drivers/of/fdt.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> v2. Use "data" instead of "cmd_line" so it works on archs like
> mips who don't pass cmd_line to that function to start with, also
> add a comment explaining the mechanism.
> 
> (resent with right list address as well while at it)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 65200af..323b722 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	if (p != NULL && l > 0)
>  		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
>  
> +	/*
> +	 * CONFIG_CMDLINE is meant to be a default in case nothing else
> +	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> +	 * is set in which case we override whatever was found earlier.
> +	 */
>  #ifdef CONFIG_CMDLINE
>  #ifndef CONFIG_CMDLINE_FORCE
> -	if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> +	if (!data[0])
>  #endif
>  		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>  #endif /* CONFIG_CMDLINE */
> -- 
> 1.7.4.1
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
@ 2011-09-20  4:55   ` Grant Likely
  0 siblings, 0 replies; 9+ messages in thread
From: Grant Likely @ 2011-09-20  4:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: devicetree-discuss, linuxppc-dev

On Tue, Sep 20, 2011 at 02:50:15PM +1000, Benjamin Herrenschmidt wrote:
> We used to overwrite with CONFIG_CMDLINE if we found a chosen
> node but failed to get bootargs out of it or they were empty,
> unless CONFIG_CMDLINE_FORCE is set.
> 
> Instead change that to overwrite if "data" is non empty after
> the bootargs check. It allows arch code to have other mechanisms
> to retrieve the command line prior to parsing the device-tree.
> 
> Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> as it won't work as it-is if the device-tree has no /chosen node
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: devicetree-discuss@lists-ozlabs.org
> CC: Grant Likely <grant.likely@secretlab.ca>

Looks okay to me.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  drivers/of/fdt.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> v2. Use "data" instead of "cmd_line" so it works on archs like
> mips who don't pass cmd_line to that function to start with, also
> add a comment explaining the mechanism.
> 
> (resent with right list address as well while at it)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 65200af..323b722 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	if (p != NULL && l > 0)
>  		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
>  
> +	/*
> +	 * CONFIG_CMDLINE is meant to be a default in case nothing else
> +	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> +	 * is set in which case we override whatever was found earlier.
> +	 */
>  #ifdef CONFIG_CMDLINE
>  #ifndef CONFIG_CMDLINE_FORCE
> -	if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> +	if (!data[0])
>  #endif
>  		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>  #endif /* CONFIG_CMDLINE */
> -- 
> 1.7.4.1
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
  2011-09-20  4:55   ` Grant Likely
@ 2012-01-07  0:48       ` Doug Anderson
  -1 siblings, 0 replies; 9+ messages in thread
From: Doug Anderson @ 2012-01-07  0:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linuxppc-dev

I know this is a long-dead thread, but I was a little curious about
the motivation here.

I'm looking at trying to support CONFIG_CMDLINE_EXTEND (an ARM
Kconfig) in this function and don't know in which cases I should look
at the CONFIG_CMDLINE and in which cases I should use whatever
happened to be in data before the function was called.

Here's the definition in the KConfig:
<http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/Kconfig;h=24626b0419ee97e963e68329a8eb6769360b46ea;hb=HEAD#l1984>

Which case do you have CONFIG_CMDLINE defined but not CMDLINE_FORCE?
In those cases, do you happen to have CONFIG_CMDLINE_EXTEND or
CMDLINE_FROM_BOOTLOADER defined?

Thanks much!

-Doug

---

On Mon, Sep 19, 2011 at 9:55 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>
> On Tue, Sep 20, 2011 at 02:50:15PM +1000, Benjamin Herrenschmidt wrote:
> > We used to overwrite with CONFIG_CMDLINE if we found a chosen
> > node but failed to get bootargs out of it or they were empty,
> > unless CONFIG_CMDLINE_FORCE is set.
> >
> > Instead change that to overwrite if "data" is non empty after
> > the bootargs check. It allows arch code to have other mechanisms
> > to retrieve the command line prior to parsing the device-tree.
> >
> > Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> > as it won't work as it-is if the device-tree has no /chosen node
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> > CC: devicetree-discuss-w3QoH2/gNlk/bJ5BZ2RsiQ@public.gmane.org
> > CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>
> Looks okay to me.
>
> Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>
> > ---
> >  drivers/of/fdt.c |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > v2. Use "data" instead of "cmd_line" so it works on archs like
> > mips who don't pass cmd_line to that function to start with, also
> > add a comment explaining the mechanism.
> >
> > (resent with right list address as well while at it)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 65200af..323b722 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> >       if (p != NULL && l > 0)
> >               strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> >
> > +     /*
> > +      * CONFIG_CMDLINE is meant to be a default in case nothing else
> > +      * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> > +      * is set in which case we override whatever was found earlier.
> > +      */
> >  #ifdef CONFIG_CMDLINE
> >  #ifndef CONFIG_CMDLINE_FORCE
> > -     if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> > +     if (!data[0])
> >  #endif
> >               strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> >  #endif /* CONFIG_CMDLINE */
> > --
> > 1.7.4.1
> >
> >
> >
> >
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
@ 2012-01-07  0:48       ` Doug Anderson
  0 siblings, 0 replies; 9+ messages in thread
From: Doug Anderson @ 2012-01-07  0:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: devicetree-discuss, linuxppc-dev

I know this is a long-dead thread, but I was a little curious about
the motivation here.

I'm looking at trying to support CONFIG_CMDLINE_EXTEND (an ARM
Kconfig) in this function and don't know in which cases I should look
at the CONFIG_CMDLINE and in which cases I should use whatever
happened to be in data before the function was called.

Here's the=A0definition in the KConfig:
<http://git.kernel.org/?p=3Dlinux/kernel/git/next/linux-next.git;a=3Dblob;f=
=3Darch/arm/Kconfig;h=3D24626b0419ee97e963e68329a8eb6769360b46ea;hb=3DHEAD#=
l1984>

Which case do you have=A0CONFIG_CMDLINE defined but not CMDLINE_FORCE?
In those cases, do you happen to have=A0CONFIG_CMDLINE_EXTEND or
CMDLINE_FROM_BOOTLOADER defined?

Thanks much!

-Doug

---

On Mon, Sep 19, 2011 at 9:55 PM, Grant Likely <grant.likely@secretlab.ca> w=
rote:
>
> On Tue, Sep 20, 2011 at 02:50:15PM +1000, Benjamin Herrenschmidt wrote:
> > We used to overwrite with CONFIG_CMDLINE if we found a chosen
> > node but failed to get bootargs out of it or they were empty,
> > unless CONFIG_CMDLINE_FORCE is set.
> >
> > Instead change that to overwrite if "data" is non empty after
> > the bootargs check. It allows arch code to have other mechanisms
> > to retrieve the command line prior to parsing the device-tree.
> >
> > Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> > as it won't work as it-is if the device-tree has no /chosen node
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: devicetree-discuss@lists-ozlabs.org
> > CC: Grant Likely <grant.likely@secretlab.ca>
>
> Looks okay to me.
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> > ---
> > =A0drivers/of/fdt.c | =A0 =A07 ++++++-
> > =A01 files changed, 6 insertions(+), 1 deletions(-)
> >
> > v2. Use "data" instead of "cmd_line" so it works on archs like
> > mips who don't pass cmd_line to that function to start with, also
> > add a comment explaining the mechanism.
> >
> > (resent with right list address as well while at it)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 65200af..323b722 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long=
 node, const char *uname,
> > =A0 =A0 =A0 if (p !=3D NULL && l > 0)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 strlcpy(data, p, min((int)l, COMMAND_LINE_S=
IZE));
> >
> > + =A0 =A0 /*
> > + =A0 =A0 =A0* CONFIG_CMDLINE is meant to be a default in case nothing =
else
> > + =A0 =A0 =A0* managed to set the command line, unless CONFIG_CMDLINE_F=
ORCE
> > + =A0 =A0 =A0* is set in which case we override whatever was found earl=
ier.
> > + =A0 =A0 =A0*/
> > =A0#ifdef CONFIG_CMDLINE
> > =A0#ifndef CONFIG_CMDLINE_FORCE
> > - =A0 =A0 if (p =3D=3D NULL || l =3D=3D 0 || (l =3D=3D 1 && (*p) =3D=3D=
 0))
> > + =A0 =A0 if (!data[0])
> > =A0#endif
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_=
SIZE);
> > =A0#endif /* CONFIG_CMDLINE */
> > --
> > 1.7.4.1
> >
> >
> >
> >
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
  2012-01-07  0:48       ` Doug Anderson
@ 2012-01-10  9:10           ` Benjamin Herrenschmidt
  -1 siblings, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2012-01-10  9:10 UTC (permalink / raw)
  To: Doug Anderson; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linuxppc-dev

On Fri, 2012-01-06 at 16:48 -0800, Doug Anderson wrote:
> I know this is a long-dead thread, but I was a little curious about
> the motivation here.

Hi ! Sorry, I planned to reply earlier and then forgot about it...

> I'm looking at trying to support CONFIG_CMDLINE_EXTEND (an ARM
> Kconfig) in this function and don't know in which cases I should look
> at the CONFIG_CMDLINE and in which cases I should use whatever
> happened to be in data before the function was called.

I'll have a look later (gotta run soon) but basically, the reason I did
that logic change is that in some specific circumstances and firmware
version, I end up writing the user-specified command line in the global
prior to actually booting the kernel :-)

So in that case, I really don't want CONFIG_CMDLINE to take over because
there's nothing in the device-tree, the user -did- specify something but
not via the device-tree.

Now, that's a bit of an oddball scenario but I felt that the logic
change was harmless.

For those interested in gory details, it's when doing the OPAL takeover
on some machines with the version 1 of OPAL firmware. We basically boot
under pHyp normally (pSeries hypervisor) and do a magic hcall which
relocates the kernel to contiguous physical memory and re-starts it with
a flat device-tree.

The takeover mechanism didn't provide me with a way for passing a
command line in that fdt. So I had to do it from prom_init (still
running under pHyp context), change the kernel global before it gets
relocated.

Cheers,
Ben.


> Here's the definition in the KConfig:
> <http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/Kconfig;h=24626b0419ee97e963e68329a8eb6769360b46ea;hb=HEAD#l1984>
> 
> Which case do you have CONFIG_CMDLINE defined but not CMDLINE_FORCE?
> In those cases, do you happen to have CONFIG_CMDLINE_EXTEND or
> CMDLINE_FROM_BOOTLOADER defined?
> 
> Thanks much!
> 
> -Doug
> 
> ---
> 
> On Mon, Sep 19, 2011 at 9:55 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> >
> > On Tue, Sep 20, 2011 at 02:50:15PM +1000, Benjamin Herrenschmidt wrote:
> > > We used to overwrite with CONFIG_CMDLINE if we found a chosen
> > > node but failed to get bootargs out of it or they were empty,
> > > unless CONFIG_CMDLINE_FORCE is set.
> > >
> > > Instead change that to overwrite if "data" is non empty after
> > > the bootargs check. It allows arch code to have other mechanisms
> > > to retrieve the command line prior to parsing the device-tree.
> > >
> > > Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> > > as it won't work as it-is if the device-tree has no /chosen node
> > >
> > > Signed-off-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> > > CC: devicetree-discuss-w3QoH2/gNlk/bJ5BZ2RsiQ@public.gmane.org
> > > CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> >
> > Looks okay to me.
> >
> > Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> >
> > > ---
> > >  drivers/of/fdt.c |    7 ++++++-
> > >  1 files changed, 6 insertions(+), 1 deletions(-)
> > >
> > > v2. Use "data" instead of "cmd_line" so it works on archs like
> > > mips who don't pass cmd_line to that function to start with, also
> > > add a comment explaining the mechanism.
> > >
> > > (resent with right list address as well while at it)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index 65200af..323b722 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > >       if (p != NULL && l > 0)
> > >               strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> > >
> > > +     /*
> > > +      * CONFIG_CMDLINE is meant to be a default in case nothing else
> > > +      * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> > > +      * is set in which case we override whatever was found earlier.
> > > +      */
> > >  #ifdef CONFIG_CMDLINE
> > >  #ifndef CONFIG_CMDLINE_FORCE
> > > -     if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> > > +     if (!data[0])
> > >  #endif
> > >               strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > >  #endif /* CONFIG_CMDLINE */
> > > --
> > > 1.7.4.1
> > >
> > >
> > >
> > >
> > _______________________________________________
> > devicetree-discuss mailing list
> > devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
@ 2012-01-10  9:10           ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2012-01-10  9:10 UTC (permalink / raw)
  To: Doug Anderson; +Cc: devicetree-discuss, linuxppc-dev

On Fri, 2012-01-06 at 16:48 -0800, Doug Anderson wrote:
> I know this is a long-dead thread, but I was a little curious about
> the motivation here.

Hi ! Sorry, I planned to reply earlier and then forgot about it...

> I'm looking at trying to support CONFIG_CMDLINE_EXTEND (an ARM
> Kconfig) in this function and don't know in which cases I should look
> at the CONFIG_CMDLINE and in which cases I should use whatever
> happened to be in data before the function was called.

I'll have a look later (gotta run soon) but basically, the reason I did
that logic change is that in some specific circumstances and firmware
version, I end up writing the user-specified command line in the global
prior to actually booting the kernel :-)

So in that case, I really don't want CONFIG_CMDLINE to take over because
there's nothing in the device-tree, the user -did- specify something but
not via the device-tree.

Now, that's a bit of an oddball scenario but I felt that the logic
change was harmless.

For those interested in gory details, it's when doing the OPAL takeover
on some machines with the version 1 of OPAL firmware. We basically boot
under pHyp normally (pSeries hypervisor) and do a magic hcall which
relocates the kernel to contiguous physical memory and re-starts it with
a flat device-tree.

The takeover mechanism didn't provide me with a way for passing a
command line in that fdt. So I had to do it from prom_init (still
running under pHyp context), change the kernel global before it gets
relocated.

Cheers,
Ben.


> Here's the definition in the KConfig:
> <http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/Kconfig;h=24626b0419ee97e963e68329a8eb6769360b46ea;hb=HEAD#l1984>
> 
> Which case do you have CONFIG_CMDLINE defined but not CMDLINE_FORCE?
> In those cases, do you happen to have CONFIG_CMDLINE_EXTEND or
> CMDLINE_FROM_BOOTLOADER defined?
> 
> Thanks much!
> 
> -Doug
> 
> ---
> 
> On Mon, Sep 19, 2011 at 9:55 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >
> > On Tue, Sep 20, 2011 at 02:50:15PM +1000, Benjamin Herrenschmidt wrote:
> > > We used to overwrite with CONFIG_CMDLINE if we found a chosen
> > > node but failed to get bootargs out of it or they were empty,
> > > unless CONFIG_CMDLINE_FORCE is set.
> > >
> > > Instead change that to overwrite if "data" is non empty after
> > > the bootargs check. It allows arch code to have other mechanisms
> > > to retrieve the command line prior to parsing the device-tree.
> > >
> > > Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> > > as it won't work as it-is if the device-tree has no /chosen node
> > >
> > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > CC: devicetree-discuss@lists-ozlabs.org
> > > CC: Grant Likely <grant.likely@secretlab.ca>
> >
> > Looks okay to me.
> >
> > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> >
> > > ---
> > >  drivers/of/fdt.c |    7 ++++++-
> > >  1 files changed, 6 insertions(+), 1 deletions(-)
> > >
> > > v2. Use "data" instead of "cmd_line" so it works on archs like
> > > mips who don't pass cmd_line to that function to start with, also
> > > add a comment explaining the mechanism.
> > >
> > > (resent with right list address as well while at it)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index 65200af..323b722 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > >       if (p != NULL && l > 0)
> > >               strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> > >
> > > +     /*
> > > +      * CONFIG_CMDLINE is meant to be a default in case nothing else
> > > +      * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> > > +      * is set in which case we override whatever was found earlier.
> > > +      */
> > >  #ifdef CONFIG_CMDLINE
> > >  #ifndef CONFIG_CMDLINE_FORCE
> > > -     if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> > > +     if (!data[0])
> > >  #endif
> > >               strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > >  #endif /* CONFIG_CMDLINE */
> > > --
> > > 1.7.4.1
> > >
> > >
> > >
> > >
> > _______________________________________________
> > devicetree-discuss mailing list
> > devicetree-discuss@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE
@ 2011-09-20  4:48 Benjamin Herrenschmidt
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2011-09-20  4:48 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss

We used to overwrite with CONFIG_CMDLINE if we found a chosen
node but failed to get bootargs out of it or they were empty,
unless CONFIG_CMDLINE_FORCE is set.

Instead change that to overwrite if "data" is non empty after
the bootargs check. It allows arch code to have other mechanisms
to retrieve the command line prior to parsing the device-tree.

Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
as it won't work as it-is if the device-tree has no /chosen node

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: devicetree-discuss@lists-ozlabs.org
CC: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/of/fdt.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

v2. Use "data" instead of "cmd_line" so it works on archs like
mips who don't pass cmd_line to that function to start with, also
add a comment explaining the mechanism.

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 65200af..323b722 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -681,9 +681,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 	if (p != NULL && l > 0)
 		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
 
+	/*
+	 * CONFIG_CMDLINE is meant to be a default in case nothing else
+	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
+	 * is set in which case we override whatever was found earlier.
+	 */
 #ifdef CONFIG_CMDLINE
 #ifndef CONFIG_CMDLINE_FORCE
-	if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
+	if (!data[0])
 #endif
 		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
 #endif /* CONFIG_CMDLINE */
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-01-10  9:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-20  4:50 [PATCH v2] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE Benjamin Herrenschmidt
2011-09-20  4:50 ` Benjamin Herrenschmidt
2011-09-20  4:55 ` Grant Likely
2011-09-20  4:55   ` Grant Likely
     [not found]   ` <20110920045510.GA31886-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2012-01-07  0:48     ` Doug Anderson
2012-01-07  0:48       ` Doug Anderson
     [not found]       ` <CAD=FV=UVo-vCe-s9tdXq_Od3jJnJ58Nx=4f-S3XfWBfDhxoXMg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-01-10  9:10         ` Benjamin Herrenschmidt
2012-01-10  9:10           ` Benjamin Herrenschmidt
  -- strict thread matches above, loose matches on Subject: below --
2011-09-20  4:48 Benjamin Herrenschmidt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.