[U-Boot] [PATCH v1 0/2] stm32f7 gpio driver fixes

This series adds: - Fix gpio bank hole management for stm32 F7 and H7 - Fix SPL code size for stm32 F7
Patrice Chotard (2): gpio: stm32f7: Fix gpio bank hole management gpio: stm32f7: Fix SPL code size
drivers/gpio/stm32f7_gpio.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)

In case "gpio-ranges" property is not present in device tree, use default value for gpio_count and gpio_range. This fixes an issue on stm32 F7 and H7 boards where "pinmux status -a" command didn't return any pin status due to the fact that both stm32 F7 and H7 board DT doesn't use the gpio-ranges property.
Fixes: dbf928dd2634a6("gpio: stm32f7: Add gpio bank holes management")
Signed-off-by: Patrice Chotard patrice.chotard@st.com ---
drivers/gpio/stm32f7_gpio.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpio/stm32f7_gpio.c b/drivers/gpio/stm32f7_gpio.c index f160b4e68957..dbaa80cf8cce 100644 --- a/drivers/gpio/stm32f7_gpio.c +++ b/drivers/gpio/stm32f7_gpio.c @@ -171,6 +171,11 @@ static int gpio_stm32_probe(struct udevice *dev) ret = dev_read_phandle_with_args(dev, "gpio-ranges", NULL, 3, i, &args);
+ if (ret == -ENOENT) { + uc_priv->gpio_count = STM32_GPIOS_PER_BANK; + priv->gpio_range = GENMASK(STM32_GPIOS_PER_BANK - 1, 0); + } + while (ret != -ENOENT) { priv->gpio_range |= GENMASK(args.args[2] + args.args[0] - 1, args.args[0]);

On Fri, Jan 04, 2019 at 10:55:05AM +0100, Patrice Chotard wrote:
In case "gpio-ranges" property is not present in device tree, use default value for gpio_count and gpio_range. This fixes an issue on stm32 F7 and H7 boards where "pinmux status -a" command didn't return any pin status due to the fact that both stm32 F7 and H7 board DT doesn't use the gpio-ranges property.
Fixes: dbf928dd2634a6("gpio: stm32f7: Add gpio bank holes management")
Signed-off-by: Patrice Chotard patrice.chotard@st.com
Applied to u-boot/master, thanks!

In order to keep SPL code size below the 32Kb limit, put under CONFIG_SPL_BUILD flag all unused code in SPL. This is needed for stm32f7xx board which are using SPL.
Signed-off-by: Patrice Chotard patrice.chotard@st.com ---
drivers/gpio/stm32f7_gpio.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/stm32f7_gpio.c b/drivers/gpio/stm32f7_gpio.c index dbaa80cf8cce..5c9f2fe64d47 100644 --- a/drivers/gpio/stm32f7_gpio.c +++ b/drivers/gpio/stm32f7_gpio.c @@ -19,6 +19,7 @@ #define MODE_BITS_MASK 3 #define BSRR_BIT(gpio_pin, value) BIT(gpio_pin + (value ? 0 : 16))
+#ifndef CONFIG_SPL_BUILD /* * convert gpio offset to gpio index taking into account gpio holes * into gpio bank @@ -145,23 +146,27 @@ static const struct dm_gpio_ops gpio_stm32_ops = { .set_value = stm32_gpio_set_value, .get_function = stm32_gpio_get_function, }; +#endif
static int gpio_stm32_probe(struct udevice *dev) { - struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); struct stm32_gpio_priv *priv = dev_get_priv(dev); - struct ofnode_phandle_args args; struct clk clk; fdt_addr_t addr; - const char *name; int ret; - int i;
addr = dev_read_addr(dev); if (addr == FDT_ADDR_T_NONE) return -EINVAL;
priv->regs = (struct stm32_gpio_regs *)addr; + +#ifndef CONFIG_SPL_BUILD + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); + struct ofnode_phandle_args args; + const char *name; + int i; + name = dev_read_string(dev, "st,bank-name"); if (!name) return -EINVAL; @@ -189,7 +194,7 @@ static int gpio_stm32_probe(struct udevice *dev) dev_dbg(dev, "addr = 0x%p bank_name = %s gpio_count = %d gpio_range = 0x%x\n", (u32 *)priv->regs, uc_priv->bank_name, uc_priv->gpio_count, priv->gpio_range); - +#endif ret = clk_get_by_index(dev, 0, &clk); if (ret < 0) return ret; @@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };

On Fri, Jan 04, 2019 at 10:55:06AM +0100, Patrice Chotard wrote:
In order to keep SPL code size below the 32Kb limit, put under CONFIG_SPL_BUILD flag all unused code in SPL. This is needed for stm32f7xx board which are using SPL.
Signed-off-by: Patrice Chotard patrice.chotard@st.com
Applied to u-boot/master, thanks!

On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.

Hi Marek
On 3/31/20 4:51 AM, Marek Vasut wrote:
On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.
I suppose it breaks AV96 boot ?
On my side i have checked on v2020-04-rc4 U-boot release, by reverting "gpio: stm32f7: Fix SPL code size"
the stm32f7 SPL code size is below the 32Kb limit.
I will send a patch to revert it.
Thanks for pointing this.
Patrice

On 3/31/20 10:12 AM, Patrice CHOTARD wrote:
Hi Marek
Hi,
On 3/31/20 4:51 AM, Marek Vasut wrote:
On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.
I suppose it breaks AV96 boot ?
No, it does not. I was trying to read GPIO value in SPL and found this.
On my side i have checked on v2020-04-rc4 U-boot release, by reverting "gpio: stm32f7: Fix SPL code size"
the stm32f7 SPL code size is below the 32Kb limit.
I will send a patch to revert it.
OK sure, does that apply to all the STM32 systems ?

On 3/31/20 1:22 PM, Marek Vasut wrote:
On 3/31/20 10:12 AM, Patrice CHOTARD wrote:
Hi Marek
Hi,
On 3/31/20 4:51 AM, Marek Vasut wrote:
On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.
I suppose it breaks AV96 boot ?
No, it does not. I was trying to read GPIO value in SPL and found this.
On my side i have checked on v2020-04-rc4 U-boot release, by reverting "gpio: stm32f7: Fix SPL code size"
the stm32f7 SPL code size is below the 32Kb limit.
I will send a patch to revert it.
OK sure, does that apply to all the STM32 systems ?
Yes, all STM32 based platforms (F4/F7/H7 and MP1) are using this gpio driver.

On 3/31/20 4:10 PM, Patrice CHOTARD wrote:
On 3/31/20 1:22 PM, Marek Vasut wrote:
On 3/31/20 10:12 AM, Patrice CHOTARD wrote:
Hi Marek
Hi,
On 3/31/20 4:51 AM, Marek Vasut wrote:
On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.
I suppose it breaks AV96 boot ?
No, it does not. I was trying to read GPIO value in SPL and found this.
On my side i have checked on v2020-04-rc4 U-boot release, by reverting "gpio: stm32f7: Fix SPL code size"
the stm32f7 SPL code size is below the 32Kb limit.
I will send a patch to revert it.
OK sure, does that apply to all the STM32 systems ?
Yes, all STM32 based platforms (F4/F7/H7 and MP1) are using this gpio driver.
What I wanted to ask about is whether you're sure they don't overflow in SPL. But I suspect CI would tell you by now.

Hi,
On Mon, 30 Mar 2020 at 20:51, Marek Vasut marex@denx.de wrote:
On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.
Right. You are not allowed to have a driver without operations unless the uclass defines none.
Regards, Simon

Hi Simon and Marek,
From: Simon Glass sjg@chromium.org Sent: mardi 31 mars 2020 18:08
Hi,
On Mon, 30 Mar 2020 at 20:51, Marek Vasut marex@denx.de wrote:
On 1/4/19 10:55 AM, Patrice Chotard wrote:
Hi,
@@ -215,7 +220,9 @@ U_BOOT_DRIVER(gpio_stm32) = { .id = UCLASS_GPIO, .of_match = stm32_gpio_ids, .probe = gpio_stm32_probe, +#ifndef CONFIG_SPL_BUILD .ops = &gpio_stm32_ops, +#endif .flags = DM_UC_FLAG_SEQ_ALIAS, .priv_auto_alloc_size = sizeof(struct stm32_gpio_priv), };
The U-Boot DM GPIO uclass code assumes the .ops is always non-NULL. Hence, this patch breaks all GPIO access (actually crashes SPL) on STM32 in SPL.
Right. You are not allowed to have a driver without operations unless the uclass defines none.
Agree,
it was a dirty patch to reduce the SPL size for one MCU board stm32f7... It is no more needed today: all MCU board compile without this patch.
Moreover, this patch can cause issues for stm32mp1 (crashes).
For example on STM32MP157-EV1, when SD card detect is added in device tree, this opt is required (and it should be the case after the next device tree allignment).
A patch to reactivate this opts was in my backlog to prepare this DT update,
I sent it today to solve the issue: "[13/16] gpio: stm32: support gpio ops in SPL" http://patchwork.ozlabs.org/patch/1264836/
regards
Patrick
participants (6)
-
Marek Vasut
-
Patrice CHOTARD
-
Patrice Chotard
-
Patrick DELAUNAY
-
Simon Glass
-
Tom Rini