[U-Boot] [PATCH 2/5] arch-stm32: Move gpio.h for STM32 SoCs in include/asm/

[Patrice CHOTARD]

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.chotard at st.com wrote:
>>>> From: Patrice Chotard <patrice.chotard at 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.chotard at 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