[PATCH 2/3] configs: keystone2: Do not include hardware.h

Simon Glass sjg at chromium.org
Tue Nov 21 03:16:03 CET 2023


Hi Tom,

On Mon, 20 Nov 2023 at 08:26, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Nov 20, 2023 at 08:49:32AM -0600, Andrew Davis wrote:
> > On 11/17/23 4:40 PM, Tom Rini wrote:
> > > On Fri, Nov 17, 2023 at 04:38:28PM -0600, Andrew Davis wrote:
> > >
> > > > This is a hacky way to have this file included in all source files that
> > > > include common.h, instead just include from the files that need it.
> > > >
> > > > Signed-off-by: Andrew Davis <afd at ti.com>
> > > > ---
> > > >   drivers/memory/ti-aemif.c            | 1 +
> > > >   drivers/soc/ti/keystone_serdes.c     | 1 +
> > > >   include/configs/ti_armv7_keystone2.h | 1 -
> > > >   3 files changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> > > > index c4bc88c1510..41325eb0f94 100644
> > > > --- a/drivers/memory/ti-aemif.c
> > > > +++ b/drivers/memory/ti-aemif.c
> > > > @@ -7,6 +7,7 @@
> > > >    */
> > > >   #include <common.h>
> > > > +#include <asm/arch/hardware.h>
> > > >   #include <asm/ti-common/ti-aemif.h>
> > > >   #define AEMIF_WAITCYCLE_CONFIG          (KS2_AEMIF_CNTRL_BASE + 0x4)
> > > > diff --git a/drivers/soc/ti/keystone_serdes.c b/drivers/soc/ti/keystone_serdes.c
> > > > index 2ece1a8f647..0e1bf8ff39d 100644
> > > > --- a/drivers/soc/ti/keystone_serdes.c
> > > > +++ b/drivers/soc/ti/keystone_serdes.c
> > > > @@ -8,6 +8,7 @@
> > > >   #include <errno.h>
> > > >   #include <common.h>
> > > > +#include <asm/io.h>
> > > >   #include <asm/ti-common/keystone_serdes.h>
> > > >   #include <linux/bitops.h>
> > >
> > > Is there going to be a follow-up to remove common.h from these files
> > > then? Thanks.
> >
> > Yes I can take a look at that next.
> >
> > Wasn't there some automation around this? Dropping headers from common.h
> > one at a time until it disappears IIRC.
>
> Simon has posted a python tool he wrote that partially did this, but I
> wasn't quite happy with the results at the time. It might work out
> better now, or on top of the series I posted to remove it from include/

My tool inserts #includes based on the symbols referenced in a file.
For example, reference to strcpy() or memcmp() would result in the
script adding '#include <linux/string.h>' to the top of the file.

I still believe this is a reasonable approach. My original series to
handle this was posted at [1] over 3 years ago. It foundered in part
due to the number of files it touched, but I still believe the
principle of 'include what you use' makes sense in U-Boot.

Other problems are that in a few cases it results in some ugly code,
or misordering of includes

On the plus side, it gets us past this problem. We shouldn't make
perfect an enemy of good and I am disappointed that we are still
trying to get this over the line.

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=193754&state=*


More information about the U-Boot mailing list