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

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

[Patrice Chotard]

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(-)

-- 
1.9.1

[U-Boot] [PATCH v1 1/2] gpio: stm32f7: Fix gpio bank hole management

[Patrice Chotard]

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]);
-- 
1.9.1

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Patrice Chotard]

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),
 };
-- 
1.9.1

[U-Boot] [U-Boot, v1, 1/2] gpio: stm32f7: Fix gpio bank hole management

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190109/b92877f3/attachment.sig>

[U-Boot] [U-Boot,v1,2/2] gpio: stm32f7: Fix SPL code size

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190109/6c011510/attachment.sig>

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Marek Vasut]

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.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Patrice CHOTARD]

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

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Marek Vasut]

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 ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Patrice CHOTARD]

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.
>

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Marek Vasut]

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.

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Simon Glass]

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

[U-Boot] [PATCH v1 2/2] gpio: stm32f7: Fix SPL code size

[Patrick DELAUNAY]

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