Re: [U-Boot] [PATCH 2/5] arch-stm32: Move gpio.h for STM32 SoCs in include/asm/
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
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
>>
>> [...]
>>
-static inline unsigned stm32_gpio_to_pin(unsigned gpio)
-{
- return gpio % 16;
-}
+#include
>>>
>>> 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_.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
> +#include
>
> 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 is in fact #include /gpio.h
>>
>> so equal to #include >
>> 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
Re: [U-Boot] [PATCH 2/5] arch-stm32: Move gpio.h for STM32 SoCs in include/asm/
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
>>>
>>> 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
>
> [...]
>
>>> -static inline unsigned stm32_gpio_to_pin(unsigned gpio)
>>> -{
>>> - return gpio % 16;
>>> -}
>>> +#include
>>
>> 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_.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
+#include
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.
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 is in fact #include /gpio.h
>
> so equal to #include
> 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
Re: [U-Boot] [PATCH 2/5] arch-stm32: Move gpio.h for STM32 SoCs in include/asm/
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
>>
>> 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
[...]
>> -static inline unsigned stm32_gpio_to_pin(unsigned gpio)
>> -{
>> -return gpio % 16;
>> -}
>> +#include
>
> 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_.h files too.
>
> 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 is in fact #include /gpio.h
so equal to #include
> Cheers,
> Vikas
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/5] arch-stm32: Move gpio.h for STM32 SoCs in include/asm/
Hi Patrice,
On 02/07/2018 07:50 AM, patrice.chot...@st.com wrote:
> From: Patrice Chotard
>
> 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
> ---
> arch/arm/include/asm/arch-stm32f4/gpio.h | 146
> +--
> arch/arm/include/asm/arch-stm32f7/gpio.h | 116 +---
> arch/arm/include/asm/arch-stm32h7/gpio.h | 115 +---
> arch/arm/include/asm/stm32_gpio.h| 115
> 4 files changed, 118 insertions(+), 374 deletions(-)
> create mode 100644 arch/arm/include/asm/stm32_gpio.h
>
> diff --git a/arch/arm/include/asm/arch-stm32f4/gpio.h
> b/arch/arm/include/asm/arch-stm32f4/gpio.h
> index 6173fa13..bb4427e6f4ac 100644
> --- a/arch/arm/include/asm/arch-stm32f4/gpio.h
> +++ b/arch/arm/include/asm/arch-stm32f4/gpio.h
> @@ -11,150 +11,6 @@
> #ifndef _STM32_GPIO_H_
> #define _STM32_GPIO_H_
>
> -#if (CONFIG_STM32_USART == 1)
> -#define STM32_GPIO_PORT_X STM32_GPIO_PORT_A
> -#define STM32_GPIO_PIN_TX STM32_GPIO_PIN_9
> -#define STM32_GPIO_PIN_RX STM32_GPIO_PIN_10
> -#define STM32_GPIO_USARTSTM32_GPIO_AF7
> -
> -#elif (CONFIG_STM32_USART == 2)
> -#define STM32_GPIO_PORT_X STM32_GPIO_PORT_D
> -#define STM32_GPIO_PIN_TX STM32_GPIO_PIN_5
> -#define STM32_GPIO_PIN_RX STM32_GPIO_PIN_6
> -#define STM32_GPIO_USARTSTM32_GPIO_AF7
> -
> -#elif (CONFIG_STM32_USART == 3)
> -#define STM32_GPIO_PORT_X STM32_GPIO_PORT_C
> -#define STM32_GPIO_PIN_TX STM32_GPIO_PIN_10
> -#define STM32_GPIO_PIN_RX STM32_GPIO_PIN_11
> -#define STM32_GPIO_USARTSTM32_GPIO_AF7
> -
> -#elif (CONFIG_STM32_USART == 6)
> -#define STM32_GPIO_PORT_X STM32_GPIO_PORT_G
> -#define STM32_GPIO_PIN_TX STM32_GPIO_PIN_14
> -#define STM32_GPIO_PIN_RX STM32_GPIO_PIN_9
> -#define STM32_GPIO_USARTSTM32_GPIO_AF8
> -
> -#else
> -#define STM32_GPIO_PORT_X STM32_GPIO_PORT_A
> -#define STM32_GPIO_PIN_TX STM32_GPIO_PIN_9
> -#define STM32_GPIO_PIN_RX STM32_GPIO_PIN_10
> -#define STM32_GPIO_USARTSTM32_GPIO_AF7
> -
> -#endif
> -
> -enum stm32_gpio_port {
> - STM32_GPIO_PORT_A = 0,
> - STM32_GPIO_PORT_B,
> - STM32_GPIO_PORT_C,
> - STM32_GPIO_PORT_D,
> - STM32_GPIO_PORT_E,
> - STM32_GPIO_PORT_F,
> - STM32_GPIO_PORT_G,
> - STM32_GPIO_PORT_H,
> - STM32_GPIO_PORT_I
> -};
> -
> -enum stm32_gpio_pin {
> - STM32_GPIO_PIN_0 = 0,
> - STM32_GPIO_PIN_1,
> - STM32_GPIO_PIN_2,
> - STM32_GPIO_PIN_3,
> - STM32_GPIO_PIN_4,
> - STM32_GPIO_PIN_5,
> - STM32_GPIO_PIN_6,
> - STM32_GPIO_PIN_7,
> - STM32_GPIO_PIN_8,
> - STM32_GPIO_PIN_9,
> - STM32_GPIO_PIN_10,
> - STM32_GPIO_PIN_11,
> - STM32_GPIO_PIN_12,
> - STM32_GPIO_PIN_13,
> - STM32_GPIO_PIN_14,
> - STM32_GPIO_PIN_15
> -};
> -
> -enum stm32_gpio_mode {
> - STM32_GPIO_MODE_IN = 0,
> - STM32_GPIO_MODE_OUT,
> - STM32_GPIO_MODE_AF,
> - STM32_GPIO_MODE_AN
> -};
> -
> -enum stm32_gpio_otype {
> - STM32_GPIO_OTYPE_PP = 0,
> - STM32_GPIO_OTYPE_OD
> -};
> -
> -enum stm32_gpio_speed {
> - STM32_GPIO_SPEED_2M = 0,
> - STM32_GPIO_SPEED_25M,
> - STM32_GPIO_SPEED_50M,
> - STM32_GPIO_SPEED_100M
> -};
> -
> -enum stm32_gpio_pupd {
> - STM32_GPIO_PUPD_NO = 0,
> - STM32_GPIO_PUPD_UP,
> - STM32_GPIO_PUPD_DOWN
> -};
> -
> -enum stm32_gpio_af {
> - STM32_GPIO_AF0 = 0,
> - STM32_GPIO_AF1,
> - STM32_GPIO_AF2,
> - STM32_GPIO_AF3,
> - STM32_GPIO_AF4,
> - STM32_GPIO_AF5,
> - STM32_GPIO_AF6,
> - STM32_GPIO_AF7,
> - STM32_GPIO_AF8,
> - STM32_GPIO_AF9,
> - STM32_GPIO_AF10,
> - STM32_GPIO_AF11,
> - STM32_GPIO_AF12,
> - STM32_GPIO_AF13,
> - STM32_GPIO_AF14,
> - STM32_GPIO_AF15
> -};
> -
> -struct stm32_gpio_dsc {
> - enum stm32_gpio_portport;
> - enum stm32_gpio_pin pin;
> -};
> -
> -struct stm32_gpio_ctl {
> - enum stm32_gpio_modemode;
> - enum stm32_gpio_otype otype;
> - enum stm32_gpio_speed speed;
> - enum stm32_gpio_pupdpupd;
> - enum stm32_gpio_af af;
> -};
> -
> -struct stm32_gpio_regs {
> - u32 moder; /* GPIO port mode */
> - u32 otyper; /* GPIO port output type */
> - u32 ospeedr;/* GPIO port output speed */
> - u32 pupdr; /* GPIO port pull-up/pull-down */
> - u32 idr;/* GPIO port input data */
> - u32 odr;/* GPIO port output data */
> - u32 bsrr; /* GPIO port bit set/reset */
> - u32 lckr; /* GPIO port configuration lock */
> - u32 afr[2]; /* GPIO alternate function */
> -};
> -
> -struct stm32_gpio_priv {
> - struct stm32_gpio_regs *regs;
> -};
> -
> -static inline unsigned stm32_gpio_to_port(unsigned gpio)
> -{
> - return gpio / 16;
> -}
> -
> -static inline unsigned stm32_gpio_to_pin(unsigned gpio)
> -{
> - return
