If board.h defining a bunch of macros is violating modularity principle, then pretty much all NuttX header files are living in sin
What I proposed is _not_ violating modularity more than stm32f40xxx_memorymap.h defining STM32_GPIOE_BASE El vie, 5 feb 2021 a las 21:05, Gregory Nutt (<spudan...@gmail.com>) escribió: > A good modular design should strive to hide information behind > interfaces and not expose that information globally. Global anything is > the path to spaghetti code: > > Google for "information hiding in modular design" > > * https://john.cs.olemiss.edu/~hcc/csci450/notes/ModularDesign.html > * https://en.wikipedia.org/wiki/Information_hiding > * > https://medium.com/the-software-firehose/information-hiding-encapsulation-and-modularity-of-software-281a16ced > * https://people.cs.clemson.edu/~steve/CW/241/241hints/node16.html > > Matias is 100% correct the very best modular design would hide all > details of the an interface and expose as little as possible globally. > That GPIO interface was designed specifically to support such a modular > design and information hiding. Almost all interfaces work this way and > is one of the greatest strengths of NuttX. > > I would be very strongly opposed to violating that important modularity > principle. > > On 2/5/2021 8:41 PM, Grr wrote: > > Per arch is better but not enough if the goal is to keep header files in > > src/ instead of include/, where every interested party would access them > > easily > > > > El vie, 5 feb 2021 a las 20:21, Matias N. (<mat...@imap.cc>) escribió: > > > >> I'm not entirely following the problem, but it sounds like you could > >> decouple your arch-independent driver logic in an upper-half and then > have > >> a lower-half per-arch, using board-common drivers (ie. > >> boards/stm32/drivers). The lower-half would then be able to use stm32 > >> internal headers. In this scenario, you don't need to replicate things > >> per-board, only per-arch (which I guess is the minimum you could do if > >> you're using stm32 specific pin handling). > >> > >> Best, > >> Matias > >> > >> On Fri, Feb 5, 2021, at 23:13, Grr wrote: > >>>> Is this helpful? > >>>> > >>>> > >> > https://cwiki.apache.org/confluence/display/NUTTX/Including+Files+in+board.h > >>> > >>> Maybe but I think it would obscure things. I explain: > >>> > >>> Per my idea, exporting available GPIOs is done from board.h like this: > >>> > >>> #ifdef CONFIG_GPIO_1 > >>> # if defined(CONFIG_GPIO_1_OUT) > >>> # define GPIO_1_TYPE GPIO_OUTPUT_PIN > >>> # elif defined(CONFIG_GPIO_1_IN) > >>> # define GPIO_1_TYPE GPIO_INPUT_PIN > >>> # else /* CONFIG_GPIO_1_IRQ */ > >>> # define GPIO_1_TYPE GPIO_INTERRUPT_PIN > >>> # endif > >>> # define GPIO_1_PIN 2 > >>> # define GPIO_1_PORT 5 > >>> # define GPIO_1_ADDR STM32_GPIOE_BASE > >>> #endif /* CONFIG_GPIO_1 */ > >>> > >>> That means I should be able to access > >>> arch/arm/src/stm32/hardware/stm32f40xxx_memorymap.h from board.h, which > >> is > >>> no dangerous at all, except that it's not easy > >>> > >>> Why not follow that page advice and do it from the C file? Because > next C > >>> file is a system-wide file in drivers/ that fills system-wide GPIO > struct > >>> with board information. Otherwise, I'd have to make a board function > that > >>> fills system-wide struct and replicate that for every board thus > >> recreating > >>> the problem I want to fix. > >>> > >>> If STM32 header files were in include/, there would be no problem, so > I'm > >>> almost convinced that will be the best solution > >>> > >>> > >>>> On 2/5/2021 7:34 PM, Brennan Ashton wrote: > >>>>> On Fri, Feb 5, 2021, 4:40 PM Grr <gebbe...@gmail.com> wrote: > >>>>> > >>>>>>> You cannot include arch specific headers in board.h like that! > >> That > >>>> will > >>>>>>> break a lot of things. > >>>>>>> > >>>>>> board.h is arch specific > >>>>>> It absolutely is not including arch headers will break things. > >>>>> board.h: > >>>>> > >> > https://www.github.com/apache/incubator-nuttx/tree/master/boards%2Farm%2Fstm32%2Fstm32f4discovery%2Finclude%2Fboard.h > >>>>> And the arch specific board configuration: > >>>>> > >> > https://www.github.com/apache/incubator-nuttx/tree/master/boards%2Farm%2Fstm32%2Fstm32f4discovery%2Fsrc%2Fstm32f4discovery.h > >>>>> > >>>>> > >>>>>>> The expectation is that you pass the interfaces that you need into > >> the > >>>>>>> drivers. If you have a particular "module" which contains multiple > >>>>>> devices > >>>>>>> that you want to support across different boards then just create a > >>>> thin > >>>>>>> driver that wraps everything else inside of it. Bypassing things > >> like > >>>> the > >>>>>>> existing SPI CS or interrupt infrastructure is asking for trouble. > >>>>>>> > >>>>>> Combining my example (I forgot to mention ESP32) with that technique > >>>> means > >>>>>> creating at least 10 thin > >>>>>> > >>>>> You should not need to do that. Here is an example of how GPIO is > >>>> normally > >>>>> attached. Note this be done in per arch common code but it is arch > >>>> specific. > >>>>> > >> > https://www.github.com/apache/incubator-nuttx/tree/master/boards%2Farm%2Fstm32%2Fstm32f4discovery%2Fsrc%2Fstm32_gs2200m.c > >>>>> You could also create a shim for the driver to talk to the generic > >> GPIO > >>>>> interface as we suggested earlier, but I'm not convinced that we > >> would > >>>> gain > >>>>> much with that. > >>>>> > >>>>> > >>>>>>> For example I have a breakout module with a touchscreen, keyboard, > >> and > >>>> SD > >>>>>>> card. This requires the I2C bus, SPI bus, and GPIO for card > >> detection > >>>> and > >>>>>>> chip select. I just define a common name for the IO on the device > >> and > >>>>>> then > >>>>>>> map that in board.h and then add the callbacks for the GPIO > >> interrupt > >>>> and > >>>>>>> the chip select. This in total is about 50 lines of actual board > >> code > >>>>>> and > >>>>>>> that is if you are not already using things like the SPI chip > >> select. > >>>>>> My idea goes like this: > >>>>>> > >>>>>> 1-Board exports available GPIOs (among other things) > >>>>>> 2-Menuconfig offer GPIOs to user for selection. Selected ones become > >>>>>> available to drivers > >>>>>> 3-User configures needed drivers with selected GPIOs, also in > >> Menuconfig > >>>>>> 4-Compiles > >>>>>> 5-If developing, user can test with incorporated char driver that > >>>> allows to > >>>>>> turn on and off selected GPIOs from a system testing utility > >>>>> To do this you would use the ioexpander/GPIO interface that was > >>>> mentioned. > >>>>> It can provide that functionality but you would still have to write > >> lower > >>>>> half shims for the drivers that would consume GPIO this way. > >>>>> > >>>>> > >>>>>>> --Brennan > >>>>>>> > >>>>>>> On Fri, Feb 5, 2021, 3:08 PM Grr <gebbe...@gmail.com> wrote: > >>>>>>> > >>>>>>>>> Yes, we need an additional struct for port number: > >>>>>>>>> > >>>>>>>>> > >> > https://github.com/FishsemiCode/nuttx/blob/song-u1/include/nuttx/lcd/ili9486_lcd.h#L49-L57 > >>>>>>>>> struct lcd_ili9486_config_s > >>>>>>>>> { > >>>>>>>>> uint8_t power_gpio; > >>>>>>>>> uint8_t rst_gpio; > >>>>>>>>> uint8_t spi_cs_num; > >>>>>>>>> uint8_t spi_rs_gpio; > >>>>>>>>> uint32_t spi_freq; > >>>>>>>>> uint8_t spi_nbits; > >>>>>>>>> }; > >>>>>>>>> But, we can reuse ioexpander_dev_s or gpio_dev_s, what's benefit > >> to > >>>>>>> add a > >>>>>>>>> new gpioops_s? > >>>>>>>>> > >>>>>>>> I mean GPIO port, not SPÎ port. I mentioned SPI because GPIOs are > >>>>>> needed > >>>>>>> as > >>>>>>>> chip selects, resets and interrupt inputs for SPI devices > >>>>>>>> > >>>>>>>> But that's only my particular case. Point is to have GPIOs that > >> can be > >>>>>>> used > >>>>>>>> anywhere for anything in a standard way > >>>>>>>> > >>>>>>>> We can add the public interface to > >> arch/archx/include/chipy/chip.h, > >>>>>> like > >>>>>>>>> this: > >>>>>>>>> > >>>>>>>>> > >> > https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/arm/include/song/chip.h#L75-L84 > >>>>>>>> My question is how to reach arch/arm/src/stm32/stm32.h from > >>>>>>>> include/arch/board/board.h and sort the hardwired assumption of > >> the > >>>>>> rest > >>>>>>> of > >>>>>>>> headers being local. It seems to me that only reasonable solution > >> is > >>>>>>> moving > >>>>>>>> all header files to include/arch/ but I may be wrong >
