* [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
@ 2021-03-04 4:47 Daniel Walker
2021-03-04 7:09 ` Christophe Leroy
2021-03-04 14:32 ` Rob Herring
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Walker @ 2021-03-04 4:47 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, Rob Herring,
Frank Rowand
Cc: xe-linux-external, Ruslan Ruslichenko, devicetree, linux-kernel
It looks like there's some seepage of cmdline stuff into
the generic device tree code. This conflicts with the
generic cmdline implementation so I remove it in the case
when that's enabled.
Cc: xe-linux-external@cisco.com
Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
Signed-off-by: Daniel Walker <danielwa@cisco.com>
---
drivers/of/fdt.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index feb0f2d67fc5..cfe4f8d3c9f5 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -25,6 +25,7 @@
#include <linux/serial_core.h>
#include <linux/sysfs.h>
#include <linux/random.h>
+#include <linux/cmdline.h>
#include <asm/setup.h> /* for COMMAND_LINE_SIZE */
#include <asm/page.h>
@@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
early_init_dt_check_for_initrd(node);
+#ifdef CONFIG_GENERIC_CMDLINE
/* Retrieve command line */
p = of_get_flat_dt_prop(node, "bootargs", &l);
+
+ /*
+ * The builtin command line will be added here, or it can override
+ * with the DT bootargs.
+ */
+ cmdline_add_builtin(data,
+ ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
+ COMMAND_LINE_SIZE);
+#else
if (p != NULL && l > 0)
strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
@@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
#endif
#endif /* CONFIG_CMDLINE */
+#endif /* CONFIG_GENERIC_CMDLINE */
pr_debug("Command line is: %s\n", (char *)data);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
2021-03-04 4:47 [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section Daniel Walker
@ 2021-03-04 7:09 ` Christophe Leroy
2021-03-04 21:24 ` Daniel Walker
2021-03-04 14:32 ` Rob Herring
1 sibling, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2021-03-04 7:09 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, Rob Herring,
Frank Rowand
Cc: xe-linux-external, Ruslan Ruslichenko, devicetree, linux-kernel
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
> It looks like there's some seepage of cmdline stuff into
> the generic device tree code. This conflicts with the
> generic cmdline implementation so I remove it in the case
> when that's enabled.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> drivers/of/fdt.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index feb0f2d67fc5..cfe4f8d3c9f5 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -25,6 +25,7 @@
> #include <linux/serial_core.h>
> #include <linux/sysfs.h>
> #include <linux/random.h>
> +#include <linux/cmdline.h>
>
> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> #include <asm/page.h>
> @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
> early_init_dt_check_for_initrd(node);
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> /* Retrieve command line */
> p = of_get_flat_dt_prop(node, "bootargs", &l);
> +
> + /*
> + * The builtin command line will be added here, or it can override
> + * with the DT bootargs.
> + */
> + cmdline_add_builtin(data,
> + ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
Can we do more simple ? If p is NULL, p is already NULL, so (l > 0 ? p : NULL) should be enough.
> + COMMAND_LINE_SIZE);
> +#else
What does p contains now that "p = of_get_flat_dt_prop(node, "bootargs", &l);" is only called when
CONFIG_GENERIC_CMDLINE is defined ?
> if (p != NULL && l > 0)
> strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
>
> @@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> #endif
> #endif /* CONFIG_CMDLINE */
> +#endif /* CONFIG_GENERIC_CMDLINE */
>
> pr_debug("Command line is: %s\n", (char *)data);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
2021-03-04 7:09 ` Christophe Leroy
@ 2021-03-04 21:24 ` Daniel Walker
2021-03-04 22:29 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Walker @ 2021-03-04 21:24 UTC (permalink / raw)
To: Christophe Leroy
Cc: Will Deacon, ob Herring, Daniel Gimpelevich, Andrew Morton, x86,
linux-mips, linuxppc-dev, Rob Herring, Frank Rowand,
xe-linux-external, Ruslan Ruslichenko, devicetree, linux-kernel
On Thu, Mar 04, 2021 at 08:09:52AM +0100, Christophe Leroy wrote:
>
>
> Le 04/03/2021 à 05:47, Daniel Walker a écrit :
> > It looks like there's some seepage of cmdline stuff into
> > the generic device tree code. This conflicts with the
> > generic cmdline implementation so I remove it in the case
> > when that's enabled.
> >
> > Cc: xe-linux-external@cisco.com
> > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > ---
> > drivers/of/fdt.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index feb0f2d67fc5..cfe4f8d3c9f5 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -25,6 +25,7 @@
> > #include <linux/serial_core.h>
> > #include <linux/sysfs.h>
> > #include <linux/random.h>
> > +#include <linux/cmdline.h>
> > #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> > #include <asm/page.h>
> > @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > early_init_dt_check_for_initrd(node);
> > +#ifdef CONFIG_GENERIC_CMDLINE
> > /* Retrieve command line */
> > p = of_get_flat_dt_prop(node, "bootargs", &l);
> > +
> > + /*
> > + * The builtin command line will be added here, or it can override
> > + * with the DT bootargs.
> > + */
> > + cmdline_add_builtin(data,
> > + ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
>
> Can we do more simple ? If p is NULL, p is already NULL, so (l > 0 ? p : NULL) should be enough.
I believe Rob gave me this line. Maybe he can comment on it.
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
2021-03-04 21:24 ` Daniel Walker
@ 2021-03-04 22:29 ` Rob Herring
0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2021-03-04 22:29 UTC (permalink / raw)
To: Daniel Walker
Cc: Christophe Leroy, Will Deacon, Daniel Gimpelevich, Andrew Morton,
X86 ML, open list:MIPS, linuxppc-dev, Frank Rowand,
xe-linux-external, Ruslan Ruslichenko, devicetree,
linux-kernel@vger.kernel.org
On Thu, Mar 4, 2021 at 3:24 PM Daniel Walker <danielwa@cisco.com> wrote:
>
> On Thu, Mar 04, 2021 at 08:09:52AM +0100, Christophe Leroy wrote:
> >
> >
> > Le 04/03/2021 à 05:47, Daniel Walker a écrit :
> > > It looks like there's some seepage of cmdline stuff into
> > > the generic device tree code. This conflicts with the
> > > generic cmdline implementation so I remove it in the case
> > > when that's enabled.
> > >
> > > Cc: xe-linux-external@cisco.com
> > > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> > > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > > ---
> > > drivers/of/fdt.c | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > > index feb0f2d67fc5..cfe4f8d3c9f5 100644
> > > --- a/drivers/of/fdt.c
> > > +++ b/drivers/of/fdt.c
> > > @@ -25,6 +25,7 @@
> > > #include <linux/serial_core.h>
> > > #include <linux/sysfs.h>
> > > #include <linux/random.h>
> > > +#include <linux/cmdline.h>
> > > #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> > > #include <asm/page.h>
> > > @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > > early_init_dt_check_for_initrd(node);
> > > +#ifdef CONFIG_GENERIC_CMDLINE
> > > /* Retrieve command line */
> > > p = of_get_flat_dt_prop(node, "bootargs", &l);
> > > +
> > > + /*
> > > + * The builtin command line will be added here, or it can override
> > > + * with the DT bootargs.
> > > + */
> > > + cmdline_add_builtin(data,
> > > + ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
> >
> > Can we do more simple ? If p is NULL, p is already NULL, so (l > 0 ? p : NULL) should be enough.
>
>
> I believe Rob gave me this line. Maybe he can comment on it.
It's an obvious improvement and LGTM.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
2021-03-04 4:47 [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section Daniel Walker
2021-03-04 7:09 ` Christophe Leroy
@ 2021-03-04 14:32 ` Rob Herring
2021-03-04 20:48 ` Daniel Walker
1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2021-03-04 14:32 UTC (permalink / raw)
To: Daniel Walker
Cc: Will Deacon, Christophe Leroy, Daniel Gimpelevich, Andrew Morton,
X86 ML, open list:MIPS, linuxppc-dev, Frank Rowand,
xe-linux-external, Ruslan Ruslichenko, devicetree,
linux-kernel@vger.kernel.org
On Wed, Mar 3, 2021 at 10:48 PM Daniel Walker <danielwa@cisco.com> wrote:
>
> It looks like there's some seepage of cmdline stuff into
> the generic device tree code. This conflicts with the
> generic cmdline implementation so I remove it in the case
> when that's enabled.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> drivers/of/fdt.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index feb0f2d67fc5..cfe4f8d3c9f5 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -25,6 +25,7 @@
> #include <linux/serial_core.h>
> #include <linux/sysfs.h>
> #include <linux/random.h>
> +#include <linux/cmdline.h>
>
> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> #include <asm/page.h>
> @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
> early_init_dt_check_for_initrd(node);
>
> +#ifdef CONFIG_GENERIC_CMDLINE
What I like about Christophe's version is it removes the old DT
implementation. Who's going to convert the rest of the DT based
arches? That's arm, arm64, hexagon, microblaze, nios2, openrisc,
riscv, sh, and xtensa. Either separate the common code from the config
like Christophe's version or these all need converting. Though it's
fine to hash out patch 1 with a couple of arches first.
> /* Retrieve command line */
> p = of_get_flat_dt_prop(node, "bootargs", &l);
This needs to be outside the ifdef.
> +
> + /*
> + * The builtin command line will be added here, or it can override
> + * with the DT bootargs.
> + */
> + cmdline_add_builtin(data,
> + ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
> + COMMAND_LINE_SIZE);
> +#else
> if (p != NULL && l > 0)
> strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
>
> @@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> #endif
> #endif /* CONFIG_CMDLINE */
> +#endif /* CONFIG_GENERIC_CMDLINE */
>
> pr_debug("Command line is: %s\n", (char *)data);
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
2021-03-04 14:32 ` Rob Herring
@ 2021-03-04 20:48 ` Daniel Walker
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Walker @ 2021-03-04 20:48 UTC (permalink / raw)
To: Rob Herring
Cc: Will Deacon, Christophe Leroy, Daniel Gimpelevich, Andrew Morton,
X86 ML, open list:MIPS, linuxppc-dev, Frank Rowand,
xe-linux-external, Ruslan Ruslichenko, devicetree,
linux-kernel@vger.kernel.org
On Thu, Mar 04, 2021 at 08:32:37AM -0600, Rob Herring wrote:
> On Wed, Mar 3, 2021 at 10:48 PM Daniel Walker <danielwa@cisco.com> wrote:
> >
> > It looks like there's some seepage of cmdline stuff into
> > the generic device tree code. This conflicts with the
> > generic cmdline implementation so I remove it in the case
> > when that's enabled.
> >
> > Cc: xe-linux-external@cisco.com
> > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > ---
> > drivers/of/fdt.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index feb0f2d67fc5..cfe4f8d3c9f5 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -25,6 +25,7 @@
> > #include <linux/serial_core.h>
> > #include <linux/sysfs.h>
> > #include <linux/random.h>
> > +#include <linux/cmdline.h>
> >
> > #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> > #include <asm/page.h>
> > @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> >
> > early_init_dt_check_for_initrd(node);
> >
> > +#ifdef CONFIG_GENERIC_CMDLINE
>
> What I like about Christophe's version is it removes the old DT
> implementation. Who's going to convert the rest of the DT based
> arches? That's arm, arm64, hexagon, microblaze, nios2, openrisc,
> riscv, sh, and xtensa. Either separate the common code from the config
> like Christophe's version or these all need converting. Though it's
> fine to hash out patch 1 with a couple of arches first.
I'm limited in what I can test, so I can't know for sure that I have something
which works on those architectures. Even powerpc 64 is part of this series but
I can't really test it at this time. Also Cisco's needs out strip the
implementation of extend or override.
I have un-tested conversions for arm32, arm64, c6x, microblaze, nios2, and
openrisc. These could go into -next and we can see who complains. The
implementation on these architectures isn't all uniform.
> > /* Retrieve command line */
> > p = of_get_flat_dt_prop(node, "bootargs", &l);
>
> This needs to be outside the ifdef.
Ok ..
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-04 22:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-04 4:47 [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section Daniel Walker
2021-03-04 7:09 ` Christophe Leroy
2021-03-04 21:24 ` Daniel Walker
2021-03-04 22:29 ` Rob Herring
2021-03-04 14:32 ` Rob Herring
2021-03-04 20:48 ` Daniel Walker
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).