Re: [PATCH 2/8] CMDLINE: drivers: of: ifdef out cmdline section

From: Daniel Walker <danielwa@cisco.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: ob Herring <robh@kernel.org>,
	Ruslan Ruslichenko <rruslich@cisco.com>,
	Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,
	Frank Rowand <frowand.list@gmail.com>,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	x86@kernel.org, linux-mips@vger.kernel.org,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	xe-linux-external@cisco.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH 2/8] CMDLINE: drivers: of: ifdef out cmdline section
Date: Tue, 6 Apr 2021 09:35:49 -0700	[thread overview]
Message-ID: <20210406163549.GS2469518@zorba> (raw)
In-Reply-To: <6d50809a-eb6b-b8bb-bb8b-88f66c52c0fa@csgroup.eu>

On Fri, Apr 02, 2021 at 07:32:08PM +0200, Christophe Leroy wrote:
> 
> 
> Le 30/03/2021 à 19:56, 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 | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index dcc1dd96911a..d8805cd9717a 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>
> > @@ -1050,6 +1051,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> >   	/* Retrieve command line */
> >   	p = of_get_flat_dt_prop(node, "bootargs", &l);
> > +
> > +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_GENERIC_CMDLINE_OF)
> > +	/*
> > +	 * The builtin command line will be added here, or it can override
> > +	 * with the DT bootargs.
> > +	 */
> > +	cmdline_add_builtin(data,
> > +			    (l > 0 ? p : NULL), /* This is sanity checking */
> > +			    COMMAND_LINE_SIZE);
> > +#elif defined(CONFIG_GENERIC_CMDLINE)
> > +	strlcpy(data, p, COMMAND_LINE_SIZE);
> > +#else
> 
> Ugly.
> 
> Linux codying style recommend to limit the use of #ifdefs to headers as much as possible.
> 
> Why do we need so many alternatives ? Allthough they are temporary, can we
> order the changes in another way to reduce that ?

I think this whole section can be removed down even all the CMDLINE ifdef's ..
The only architecture which needs this is powerpc because it calls this function
three times.

If powerpc were made to call this only once , and then call the generic handling
for the command line then this whole section would get removed.

Daniel