[nuttx] Too many includes in drivers/lcd/lcd_framebuffer.c

[Sebastien Lorquet [e-mail] [nuttx]]

Greg,

drivers/lcd/lcd_framebuffer.c

incorrectly includes this board file, which breaks the build here:

(line 54) #include <arch/board/board.h>

Removing this line fixes the build and doesnt break anything.

Sebastien

[[e-mail] [nuttx]]

This has been discussed a few times before. No, the inclusion is correct.


What is wrong is that your board.h header file includes STM32-internal header files that may not be included in the context. The fix: You must remove all inclusion of STM32 header files from the board.h header file. Those are illegal.


Greg

[[e-mail] [nuttx]]

Another interesting framebuffer configuration: ./lpcxpresso-lpc54628/lvgl

The fonts are too big in that example to look good, but consider:

https://www.linkedin.com/groups/12002792/12002792-6342318328538763266
https://acassis.wordpress.com/2017/12/01/nuttx-now-supports-lvgl/

Lvgl (LittlevGl) is a framebuffer based GUI.

Greg

[[e-mail] [nuttx]]

I added a bit to the porting guide to make the layering concepts a little clearer: http://nuttx.org/Documentation/NuttxPortingGuide.html#naming under "Scope of Inclusions."

[Sebastien Lorquet [e-mail] [nuttx]]

It does not come from nowhere, I copied these files from the stm32f429
discovery config. Guess what, there are stm32 includes in there.

https://bitbucket.org/nuttx/nuttx/src/75ae584922d1cc932b12fd590bdfa4a3dbf6a849/configs/stm32f429i-disco/include/board.h?at=master&fileviewer=file-view-default

I tried removing these includes from the include/board.h file, and OK,
"strangely" it still works. This means that this file is relying on the
order of include files from the stm32 drivers. stm32 includes have to
happen before this file is included from these sources.

Sebastien

Post by ***@yahoo.com [nuttx]
 
I added a bit to the porting guide to make the layering concepts a
http://nuttx.org/Documentation/NuttxPortingGuide.html#naming under
"Scope of Inclusions."

[[e-mail] [nuttx]]

It does not come from nowhere, I copied these files from the stm32f429 discovery config. Guess what, there are stm32 includes in there.

Yes, many people put those inclusions in the board.h header file and it works fine until the header file is included in a common driver. The only solution is to remove the inclusions (or create two different board.h header files for inclusion at different levels in the architecture.)

I tried removing these includes from the include/board.h file, and OK, "strangely" it still works. This means that this file is relying on the order of include files from the stm32 drivers. stm32 includes have to happen before this file is included from these sources.

Yes, that is the natural consequence of a single board.h header file that is included at multiple layers in the architecture. That basic rules are:

- board.h may not include any microcontroller specific header files (like stm32_gpio.h).

- MCU source files that include board.h must provide all of the header file dependencies (which, as you already noted it usually... but not always... does).

- board.h should be the last header file that is included by the microntroller-specific source file

Greg


Sebastien

On 14/02/2018 20:30, ***@yahoo.com mailto:***@yahoo.com [nuttx] wrote:

I added a bit to the porting guide to make the layering concepts a little clearer: http://nuttx.org/Documentation/NuttxPortingGuide.html#naming http://nuttx.org/Documentation/NuttxPortingGuide.html#naming under "Scope of Inclusions."

[Sebastien Lorquet [e-mail] [nuttx]]

Okay, but this file contains only basic GPIO definitions, none of which is
useful for the lcd_framebuffer source file... so I dont understand why this
common driver would require the inclusion board specific definitions in the
first place!

Sebastien

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
 

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
It does not come from nowhere, I copied these files from the stm32f429

discovery config. Guess what, there are stm32 includes in there.
Yes, many people put those inclusions in the board.h header file and it works
fine until the header file is included in a common driver. The only solution
is to remove the inclusions (or create two different board.h header files for
inclusion at different levels in the architecture.)

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
I tried removing these includes from the include/board.h file, and OK,

"strangely" it still works. This means that this file is relying on the order
of include files from the stm32 drivers. stm32 includes have to happen before
this file is included from these sources.
Yes, that is the natural consequence of a single board.h header file that is
- board.h may not include any microcontroller specific header files (like stm32_gpio.h).
- MCU source files that include board.h must provide all of the header file
dependencies (which, as you already noted it usually... but not always... does).
- board.h should be the last header file that is included by the
microntroller-specific source file
Greg
Sebastien
I added a bit to the porting guide to make the layering concepts a little
clearer: http://nuttx.org/Documentation/NuttxPortingGuide.html#naming
http://nuttx.org/Documentation/NuttxPortingGuide.html#naming under "Scope of
Inclusions."

[Sebastien Lorquet [e-mail] [nuttx]]

oh I got it: is that because CONFIG_LCD_EXTERNINIT calls some board code?

That is surprising, as far as I can tell, all other board code is called from
CPU specific init code, not from generic drivers.

But that is the correct way, I see that it makes sense. You probably dont want
framebuffer initialization to take place in the cpu boot sequence if the user
does not have a framebuffer. And there can be multiple framebuffers!

Okay!

Sebastien

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
 
Okay, but this file contains only basic GPIO definitions, none of which is
useful for the lcd_framebuffer source file... so I dont understand why this
common driver would require the inclusion board specific definitions in the
first place!
Sebastien

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
 

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
It does not come from nowhere, I copied these files from the stm32f429

discovery config. Guess what, there are stm32 includes in there.
Yes, many people put those inclusions in the board.h header file and it works
fine until the header file is included in a common driver. The only solution
is to remove the inclusions (or create two different board.h header files for
inclusion at different levels in the architecture.)

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
I tried removing these includes from the include/board.h file, and OK,

"strangely" it still works. This means that this file is relying on the order
of include files from the stm32 drivers. stm32 includes have to happen before
this file is included from these sources.
Yes, that is the natural consequence of a single board.h header file that is
- board.h may not include any microcontroller specific header files (like stm32_gpio.h).
- MCU source files that include board.h must provide all of the header file
dependencies (which, as you already noted it usually... but not always... does).
- board.h should be the last header file that is included by the
microntroller-specific source file
Greg
Sebastien
I added a bit to the porting guide to make the layering concepts a little
clearer: http://nuttx.org/Documentation/NuttxPortingGuide.html#naming
http://nuttx.org/Documentation/NuttxPortingGuide.html#naming under "Scope of
Inclusions."

[Sebastien Lorquet [e-mail] [nuttx]]

Final thoughts:

Did we really need a board lcd initialization routine called back from the fb
driver? The user can probably initialize its own LCD during the
<arch>_boardinitialize() routine...

Note that I'm not asking for any change. Just discussing options. Everything
works cleanly as of now.

Sebastien

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
 
oh I got it: is that because CONFIG_LCD_EXTERNINIT calls some board code?
That is surprising, as far as I can tell, all other board code is called from
CPU specific init code, not from generic drivers.
But that is the correct way, I see that it makes sense. You probably dont want
framebuffer initialization to take place in the cpu boot sequence if the user
does not have a framebuffer. And there can be multiple framebuffers!
Okay!
Sebastien

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
 
Okay, but this file contains only basic GPIO definitions, none of which is
useful for the lcd_framebuffer source file... so I dont understand why this
common driver would require the inclusion board specific definitions in the
first place!
Sebastien

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
 

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
It does not come from nowhere, I copied these files from the stm32f429

discovery config. Guess what, there are stm32 includes in there.
Yes, many people put those inclusions in the board.h header file and it
works fine until the header file is included in a common driver. The only
solution is to remove the inclusions (or create two different board.h header
files for inclusion at different levels in the architecture.)

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
I tried removing these includes from the include/board.h file, and OK,

"strangely" it still works. This means that this file is relying on the
order of include files from the stm32 drivers. stm32 includes have to happen
before this file is included from these sources.
Yes, that is the natural consequence of a single board.h header file that is
- board.h may not include any microcontroller specific header files (like
stm32_gpio.h).
- MCU source files that include board.h must provide all of the header file
dependencies (which, as you already noted it usually... but not always... does).
- board.h should be the last header file that is included by the
microntroller-specific source file
Greg
Sebastien
I added a bit to the porting guide to make the layering concepts a little
clearer: http://nuttx.org/Documentation/NuttxPortingGuide.html#naming
http://nuttx.org/Documentation/NuttxPortingGuide.html#naming under "Scope of
Inclusions."

[[e-mail] [nuttx]]

Did we really need a board lcd initialization routine called back from the fb driver? The user can probably initialize its own LCD during the <arch>_boardinitialize() routine...

Yes, I think so. Certainly in the current design of the graphics system that is necessary.


It is not just a simply matter of calling a initialization function. It must call several initialization routines (the first provides a structure references that includes pointers to the next level functions). That must be done by the graphics logic itself because it needs all of the intermediate products of the initialization.


So initialization cannot be performed by a disinterested third party that discards all of the returned values from the initialization.


Of course that could be designed differently, I imagine. But the current design is correct and near optimal in this regard.


Greg

[[e-mail] [nuttx]]

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
oh I got it: is that because CONFIG_LCD_EXTERNINIT calls some board code?

No, those are all prototyped in include/nuttx/board.h. The board-specific board.h is not needed by lcd_framebuffer.c as far as I can tell.

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
That is surprising, as far as I can tell, all other board code is called from CPU specific init code, not from generic drivers.

board-level functions are called throughout the sched/ and drivers/ files. That is pretty common. The important thing is that the interfaces are well defined and common across all boards (as are the common interfaces defined in include/nuttx/board.h).


In the same way that code calls directly into MCU-specific code, but only via the common interfaces defined in include/nuttx/arch.h


Does that violate strictly layered architecture? Maybe, but I am never disturbed if the in interfaces are standardized and well defined.

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
... And there can be multiple framebuffers!

And video devices with video layers like the F429 need a framebuffer for each layer.


Greg

[Gregory Nutt [e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
board-level functions are called throughout the sched/ and drivers/
files.  That is pretty common. The important thing is that the
interfaces are well defined and common across all boards (as are the
common interfaces defined in include/nuttx/board.h).

That is probably an overstatement on my part.  I see only these references:

drivers/analog/ad5410.c - Probably unnecessary
drivers/input/button_lower.c - Button definitions
drivers/leds/userled_lower.c - User LED definitions
drivers/serial/uart_16550.c - Might be unnecessary
drivers/wireless/spirit/drivers/spirit_netdev.c - Spirit configuration

[[e-mail] [nuttx]]

Okay, but this file contains only basic GPIO definitions, none of which is useful for the lcd_framebuffer source file... so I dont understand why this common driver would require the inclusion board specific definitions in the first place!

Now that is a different subject.

Subject #1: board.h needs to be include-able from any source file in the OS (and even by applications).
Subject #2: No, I don't see any reason why lcd_framebuffer.c needs to include board.h

Greg