michallenc commented on PR #19004: URL: https://github.com/apache/nuttx/pull/19004#issuecomment-4599658208
> > So far I am sceptical towards this. One of the greatest NuttX design characteristics is the simplicity of source code and its arch section. Having a directory with common drivers with various compile time or runtime conditions deciding what code should be run breaks this simplicity. Yes, the current approach can bring some duplicity, but I still think it's a better approach. I would cite NuttX principles here: Sometimes is better to duplicate some logic than to introduce coupling. > > @michallenc Some code duplication makes sense. Code duplication across ~20 families and all posbile peripherals is pointless and difficult to maintain, especially when IP core implementations are shared between families. The problem here is that ST doesn't provide this information in an easy and readable way. Therefore, we can use ChibiOS whose author is or was an ST employee: https://github.com/ChibiOS/ChibiOS/tree/master/os/hal/ports/STM32/LLD > > > We need to really consider this, because the architectural design is what makes NuttX accessible. It's super easy to explain someone the driver's organization and the relation between boards and arch section. You just call stm32_adc_initialize from your BSP and go to arch/arm/stm32l4/stm32_adc.c if you want to know what the function does. No need to figure out what file in common section is for my chip and what code actually applies because of various ifdef guards. > > That's why documentation was created that directly indicates which IP core is used by which family. Regarding the STM32L4 - this port was created based on the principle that "code duplication is not a bad thing." Currently, this code has drifted from the rest of the ports to the point that it's the worst to generalize, and it uses essentially the same peripherals as other STM32 M4s. > > If we're duplicating code, we should duplicate it for all families, not as currently, where some are done this way, some are done that way (`stm32`/`stm32f0l0g0` vs `stm32l4`/`stm32f7`/`stm32h7`/....). Duplicating it for all families means we need to duplicate the code for the old `arch/stm32`: f1, f2, f3, f4, g4, l1 and for the old `arch/stm32f0l0g0`: f0, l0, g0, c0. **10 new architectures to maintain.** From the common stm32 arch drivers way, or ten new architectures and code duplication way, I think the common arch is the only reasonable way. I agree the support for STM32 is quite messy and STM doesn't make it easy with their naming convention. We don't need duplicate implementations for f1, f2, f3 and so on though. These can be in one common folder, if the chips are the same (or with really little changes). We also have imxrt1060 and imxrt1170 in common `imxrt` folder or same70 and samv71 in `samv7`. These chips use one IP core, the differences are mostly in having different peripherals available. But with this change, we would not only mix different IP core implementations in one directory but also different Cortex versions if I understand it correctly. And this is something I think we should duplicate. From my point of view the unification abandons the way we wrote the code for many years and I am afraid we open a Pandora box leading to more messy and unclear code. It also forces us to assign IP core versions to chips - yes, we can automate it by relying on 3rd party project which may end in a year or two or on LLM, which is even worse, because we can't trust that thing. So we have to do it manually by comparing registers and hoping we got it right. But that's a minor issue we can handle. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
