Hi Vikas On 02/08/2018 11:19 PM, Vikas Manocha wrote: > Hi Patrice, > > On 02/08/2018 05:35 AM, Patrice CHOTARD wrote: >> Hi Vikas >> >> On 02/07/2018 08:28 PM, Vikas Manocha wrote: >>> Hi Patrice, >>> >>> On 02/07/2018 07:50 AM, patrice.chot...@st.com wrote: >>>> From: Patrice Chotard <patrice.chot...@st.com> >>>> >>>> Instead to have 3 identical gpio.h for all STM32 SoCs, >>>> migrate them in one file in include/asm. >>> >>> good move to consolidate these headers. >>> One comment below. >>> >>>> >>>> Signed-off-by: Patrice Chotard <patrice.chot...@st.com> >> >> [...] >> >>>> -static inline unsigned stm32_gpio_to_pin(unsigned gpio) >>>> -{ >>>> - return gpio % 16; >>>> -} >>>> +#include <asm/stm32_gpio.h> >>> >>> Hmm.. this header seems like dummy header(in all architectures f4/f7/h7) >>> only to include gpio header here. >>> Also arch/arm/include/asm/ does not seems like good place for soc specific >>> header files. >> >> Agree, but omap have put several omap_xxxx.h files too. > > I see omap files, they might be first ones to use this structure, i am not > sure. But in any case, it does not look clean today. > How about creating asm/arch-stm32 to put common stuff like this gpio header. > > -#include <asm/stm32_gpio.h> > +#include <asm/arch-stm32/stm32_gpio.h> > > It can be done with no modification required in SYS_SOC, the point you > mentioned below. > It does not remove the arch-stm32f7/f7/h7/ gpio headers but avoids > include/asm cluttering with SOC files.
Yes agree, i will send a v2 with this update Thanks Vikas ;-) Patrice > > Cheers, > Vikas > >> >>> >>> how about creating one level like arch/arm/include/asm/arch-stm32/ to >>> include common gpio.h here. It would fix both of above points. >>> The same location can be used to move other commonalities in future. >> >> It's possible to create an additionnal level >> arch/arm/include/asm/arch-stm32/ and put specificities to each SoCs into : >> >> arch/arm/include/asm/arch-stm32/stm32f4 >> arch/arm/include/asm/arch-stm32/stm32f7 >> arch/arm/include/asm/arch-stm32/stm32h7 >> >> If we focus on stm32f7, this implies to modify the content of >> CONFIG_SYS_SOC from "stm32f7" to "stm32/stm32f7" in >> board/st/stm32f746-disco/Kconfig but: >> >> 1) In any case, we can't include directly files located in >> arch/arm/include/asm/arch-stm32 because SYS_SOC is used to build include >> path. >> >> For example in drivers/gpio/gpio-uclass.c, >> #include <asm/gpio.h> is in fact #include <asm/<SYS_SOC>/gpio.h >> >> so equal to #include <asm/arch-stm32/stm32f7/gpio.h >> >> 2) Other effect, now in "soc" environment variable, we will obtain >> "stm32/stm32f7" instead of "stm32f7". This is not a big deal, but we >> must add some code to extract the soc name from "soc" environment variable. >> >> Both solution are not perfect. >> >> Thanks >> >> Patrice >> >> >>> >>> Cheers, >>> Vikas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot