| Message ID | 20240209143520.2724221-1-trini@konsulko.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | doc/develop/codingstyle.rst: Clarify include section | expand |
Hi Tom, On Fri, Feb 9, 2024 at 3:52 PM Tom Rini <trini@konsulko.com> wrote: > > Rework the section about includes slightly. We should not be using > common.h anywhere, so remove that from examples and ask people to send > patches removing it when found. Doing this also means we need to reword > other parts of this section. Be clearer about using alphabetical > ordering. > > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > doc/develop/codingstyle.rst | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst > index b25bfbd271f0..cdf5aedfcbb0 100644 > --- a/doc/develop/codingstyle.rst > +++ b/doc/develop/codingstyle.rst > @@ -108,30 +108,29 @@ expected size, or that particular members appear at the right offset. > Include files > ------------- > > -You should follow this ordering in U-Boot. The common.h header (which is going > -away at some point) should always be first, followed by other headers in order, > -then headers with directories, then local files: > +You should follow this ordering in U-Boot. In all cases, they should be listed > +in alphabetical order. First comes headers which are located directly in our > +top-level include diretory. This excludes the common.h header file which is to > +be removed. Second are headers within subdirectories, Finally directory-local > +includes should be listed. See this example: > > .. code-block:: C > > - #include <common.h> > #include <bootstage.h> > #include <dm.h> > #include <others.h> > #include <asm/...> > - #include <arm/arch/...> > + #include <asm/arch/...> > #include <dm/device_compat.h> > #include <linux/...> > #include "local.h" > > -Within that order, sort your includes. > - > -It is important to include common.h first since it provides basic features used > -by most files, e.g. CONFIG options. > - > For files that need to be compiled for the host (e.g. tools), you need to use > -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of > -internal U-Boot things. See common/image.c for an example. > +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. See > +common/image.c for an example. > + > +If you encounter ccode which still uses <common.h> a patch to remove that and nitpick: s/ccode/code/g > +replace it with any required include files directly is much appreciated. > > If your file uses driver model, include <dm.h> in the C file. Do not include > dm.h in a header file. Try to use forward declarations (e.g. ``struct > -- > 2.34.1 > Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
On Sun, Feb 11, 2024 at 06:53:47PM +0100, Igor Opaniuk wrote: > Hi Tom, > > On Fri, Feb 9, 2024 at 3:52 PM Tom Rini <trini@konsulko.com> wrote: > > > > Rework the section about includes slightly. We should not be using > > common.h anywhere, so remove that from examples and ask people to send > > patches removing it when found. Doing this also means we need to reword > > other parts of this section. Be clearer about using alphabetical > > ordering. > > > > Signed-off-by: Tom Rini <trini@konsulko.com> > > --- > > doc/develop/codingstyle.rst | 23 +++++++++++------------ > > 1 file changed, 11 insertions(+), 12 deletions(-) > > > > diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst > > index b25bfbd271f0..cdf5aedfcbb0 100644 > > --- a/doc/develop/codingstyle.rst > > +++ b/doc/develop/codingstyle.rst > > @@ -108,30 +108,29 @@ expected size, or that particular members appear at the right offset. > > Include files > > ------------- > > > > -You should follow this ordering in U-Boot. The common.h header (which is going > > -away at some point) should always be first, followed by other headers in order, > > -then headers with directories, then local files: > > +You should follow this ordering in U-Boot. In all cases, they should be listed > > +in alphabetical order. First comes headers which are located directly in our > > +top-level include diretory. This excludes the common.h header file which is to > > +be removed. Second are headers within subdirectories, Finally directory-local > > +includes should be listed. See this example: > > > > .. code-block:: C > > > > - #include <common.h> > > #include <bootstage.h> > > #include <dm.h> > > #include <others.h> > > #include <asm/...> > > - #include <arm/arch/...> > > + #include <asm/arch/...> > > #include <dm/device_compat.h> > > #include <linux/...> > > #include "local.h" > > > > -Within that order, sort your includes. > > - > > -It is important to include common.h first since it provides basic features used > > -by most files, e.g. CONFIG options. > > - > > For files that need to be compiled for the host (e.g. tools), you need to use > > -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of > > -internal U-Boot things. See common/image.c for an example. > > +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. See > > +common/image.c for an example. > > + > > +If you encounter ccode which still uses <common.h> a patch to remove that and > nitpick: s/ccode/code/g Whoops, Heinrich can you please fix when applying?
On 12.02.24 17:19, Tom Rini wrote: > On Sun, Feb 11, 2024 at 06:53:47PM +0100, Igor Opaniuk wrote: >> Hi Tom, >> >> On Fri, Feb 9, 2024 at 3:52 PM Tom Rini <trini@konsulko.com> wrote: >>> >>> Rework the section about includes slightly. We should not be using >>> common.h anywhere, so remove that from examples and ask people to send >>> patches removing it when found. Doing this also means we need to reword >>> other parts of this section. Be clearer about using alphabetical >>> ordering. >>> >>> Signed-off-by: Tom Rini <trini@konsulko.com> >>> --- >>> doc/develop/codingstyle.rst | 23 +++++++++++------------ >>> 1 file changed, 11 insertions(+), 12 deletions(-) >>> >>> diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst >>> index b25bfbd271f0..cdf5aedfcbb0 100644 >>> --- a/doc/develop/codingstyle.rst >>> +++ b/doc/develop/codingstyle.rst >>> @@ -108,30 +108,29 @@ expected size, or that particular members appear at the right offset. >>> Include files >>> ------------- >>> >>> -You should follow this ordering in U-Boot. The common.h header (which is going >>> -away at some point) should always be first, followed by other headers in order, >>> -then headers with directories, then local files: >>> +You should follow this ordering in U-Boot. In all cases, they should be listed >>> +in alphabetical order. First comes headers which are located directly in our >>> +top-level include diretory. This excludes the common.h header file which is to >>> +be removed. Second are headers within subdirectories, Finally directory-local >>> +includes should be listed. See this example: >>> >>> .. code-block:: C >>> >>> - #include <common.h> >>> #include <bootstage.h> >>> #include <dm.h> >>> #include <others.h> >>> #include <asm/...> >>> - #include <arm/arch/...> >>> + #include <asm/arch/...> >>> #include <dm/device_compat.h> >>> #include <linux/...> >>> #include "local.h" >>> >>> -Within that order, sort your includes. >>> - >>> -It is important to include common.h first since it provides basic features used >>> -by most files, e.g. CONFIG options. >>> - >>> For files that need to be compiled for the host (e.g. tools), you need to use >>> -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of >>> -internal U-Boot things. See common/image.c for an example. >>> +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. See >>> +common/image.c for an example. >>> + >>> +If you encounter ccode which still uses <common.h> a patch to remove that and >> nitpick: s/ccode/code/g > > Whoops, Heinrich can you please fix when applying? > Hello Tom, I am missing here some text about the includes that are automatically included, e.g. linux/kconfig.h include/generated/autoconf.h Maybe you could describe this in a future patch. We still have 4107 files including common.h. Should we have a Migration warning in Makefile? Best regards Heinrich
On Mon, Feb 12, 2024 at 05:52:03PM +0100, Heinrich Schuchardt wrote: > On 12.02.24 17:19, Tom Rini wrote: > > On Sun, Feb 11, 2024 at 06:53:47PM +0100, Igor Opaniuk wrote: > > > Hi Tom, > > > > > > On Fri, Feb 9, 2024 at 3:52 PM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > Rework the section about includes slightly. We should not be using > > > > common.h anywhere, so remove that from examples and ask people to send > > > > patches removing it when found. Doing this also means we need to reword > > > > other parts of this section. Be clearer about using alphabetical > > > > ordering. > > > > > > > > Signed-off-by: Tom Rini <trini@konsulko.com> > > > > --- > > > > doc/develop/codingstyle.rst | 23 +++++++++++------------ > > > > 1 file changed, 11 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst > > > > index b25bfbd271f0..cdf5aedfcbb0 100644 > > > > --- a/doc/develop/codingstyle.rst > > > > +++ b/doc/develop/codingstyle.rst > > > > @@ -108,30 +108,29 @@ expected size, or that particular members appear at the right offset. > > > > Include files > > > > ------------- > > > > > > > > -You should follow this ordering in U-Boot. The common.h header (which is going > > > > -away at some point) should always be first, followed by other headers in order, > > > > -then headers with directories, then local files: > > > > +You should follow this ordering in U-Boot. In all cases, they should be listed > > > > +in alphabetical order. First comes headers which are located directly in our > > > > +top-level include diretory. This excludes the common.h header file which is to > > > > +be removed. Second are headers within subdirectories, Finally directory-local > > > > +includes should be listed. See this example: > > > > > > > > .. code-block:: C > > > > > > > > - #include <common.h> > > > > #include <bootstage.h> > > > > #include <dm.h> > > > > #include <others.h> > > > > #include <asm/...> > > > > - #include <arm/arch/...> > > > > + #include <asm/arch/...> > > > > #include <dm/device_compat.h> > > > > #include <linux/...> > > > > #include "local.h" > > > > > > > > -Within that order, sort your includes. > > > > - > > > > -It is important to include common.h first since it provides basic features used > > > > -by most files, e.g. CONFIG options. > > > > - > > > > For files that need to be compiled for the host (e.g. tools), you need to use > > > > -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of > > > > -internal U-Boot things. See common/image.c for an example. > > > > +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. See > > > > +common/image.c for an example. > > > > + > > > > +If you encounter ccode which still uses <common.h> a patch to remove that and > > > nitpick: s/ccode/code/g > > > > Whoops, Heinrich can you please fix when applying? > > > > Hello Tom, > > I am missing here some text about the includes that are automatically > included, e.g. > > linux/kconfig.h > include/generated/autoconf.h > > Maybe you could describe this in a future patch. Yeah, that would be good as there's in turn a few places we do have to manually include those files. > We still have 4107 files including common.h. Should we have a Migration > warning in Makefile? checkpatch.pl already marks this as an error for new patches. In some other threads I've talked with Simon about not wanting to script the removal as there's some style to it (in short, the linux kernel doesn't enforce direct #include of everything needed in every C file, so we aren't either, so it's a matter of pulling common.h out and fixing fallout rather than adding say errno.h everywhere first).
diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst index b25bfbd271f0..cdf5aedfcbb0 100644 --- a/doc/develop/codingstyle.rst +++ b/doc/develop/codingstyle.rst @@ -108,30 +108,29 @@ expected size, or that particular members appear at the right offset. Include files ------------- -You should follow this ordering in U-Boot. The common.h header (which is going -away at some point) should always be first, followed by other headers in order, -then headers with directories, then local files: +You should follow this ordering in U-Boot. In all cases, they should be listed +in alphabetical order. First comes headers which are located directly in our +top-level include diretory. This excludes the common.h header file which is to +be removed. Second are headers within subdirectories, Finally directory-local +includes should be listed. See this example: .. code-block:: C - #include <common.h> #include <bootstage.h> #include <dm.h> #include <others.h> #include <asm/...> - #include <arm/arch/...> + #include <asm/arch/...> #include <dm/device_compat.h> #include <linux/...> #include "local.h" -Within that order, sort your includes. - -It is important to include common.h first since it provides basic features used -by most files, e.g. CONFIG options. - For files that need to be compiled for the host (e.g. tools), you need to use -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of -internal U-Boot things. See common/image.c for an example. +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. See +common/image.c for an example. + +If you encounter ccode which still uses <common.h> a patch to remove that and +replace it with any required include files directly is much appreciated. If your file uses driver model, include <dm.h> in the C file. Do not include dm.h in a header file. Try to use forward declarations (e.g. ``struct
Rework the section about includes slightly. We should not be using common.h anywhere, so remove that from examples and ask people to send patches removing it when found. Doing this also means we need to reword other parts of this section. Be clearer about using alphabetical ordering. Signed-off-by: Tom Rini <trini@konsulko.com> --- doc/develop/codingstyle.rst | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)