[U-Boot] [PATCH] gpio: at91_gpio: Add bank names

[U-Boot] [PATCH] gpio: at91_gpio: Add bank names

[James Byrne]

Make the at91_gpio driver set sensible GPIO bank names in the platform
data. This makes the 'gpio status' command a lot more useful.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>

---

 drivers/gpio/at91_gpio.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
index 965becf77a..94b2b63a04 100644
--- a/drivers/gpio/at91_gpio.c
+++ b/drivers/gpio/at91_gpio.c
@@ -19,6 +19,28 @@
 
 #define GPIO_PER_BANK	32
 
+static const char *at91_get_bank_name(uint32_t base_addr)
+{
+	switch (base_addr) {
+	case ATMEL_BASE_PIOA:
+		return "PIOA";
+	case ATMEL_BASE_PIOB:
+		return "PIOB";
+	case ATMEL_BASE_PIOC:
+		return "PIOC";
+#if (ATMEL_PIO_PORTS > 3)
+	case ATMEL_BASE_PIOD:
+		return "PIOD";
+#if (ATMEL_PIO_PORTS > 4)
+	case ATMEL_BASE_PIOE:
+		return "PIOE";
+#endif
+#endif
+	}
+
+	return "";
+}
+
 static struct at91_port *at91_pio_get_port(unsigned port)
 {
 	switch (port) {
@@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
 
 	clk_free(&clk);
 
-	uc_priv->bank_name = plat->bank_name;
-	uc_priv->gpio_count = GPIO_PER_BANK;
-
 #if CONFIG_IS_ENABLED(OF_CONTROL)
 	plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
 #endif
+	plat->bank_name = at91_get_bank_name(plat->base_addr);
 	port->regs = (struct at91_port *)plat->base_addr;
 
+	uc_priv->bank_name = plat->bank_name;
+	uc_priv->gpio_count = GPIO_PER_BANK;
+
 	return 0;
 }
 
-- 
2.24.0

[U-Boot] [PATCH] gpio: at91_gpio: Add bank names

[Eugen.Hristev at microchip.com]

On 15.11.2019 19:35, James Byrne wrote:

> Make the at91_gpio driver set sensible GPIO bank names in the platform
> data. This makes the 'gpio status' command a lot more useful.
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> 
> ---
> 
>   drivers/gpio/at91_gpio.c | 29 ++++++++++++++++++++++++++---
>   1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
> index 965becf77a..94b2b63a04 100644
> --- a/drivers/gpio/at91_gpio.c
> +++ b/drivers/gpio/at91_gpio.c
> @@ -19,6 +19,28 @@
> 
>   #define GPIO_PER_BANK  32
> 
> +static const char *at91_get_bank_name(uint32_t base_addr)
> +{
> +       switch (base_addr) {
> +       case ATMEL_BASE_PIOA:
> +               return "PIOA";
> +       case ATMEL_BASE_PIOB:
> +               return "PIOB";
> +       case ATMEL_BASE_PIOC:
> +               return "PIOC";
> +#if (ATMEL_PIO_PORTS > 3)
> +       case ATMEL_BASE_PIOD:
> +               return "PIOD";
> +#if (ATMEL_PIO_PORTS > 4)
> +       case ATMEL_BASE_PIOE:
> +               return "PIOE";
> +#endif
> +#endif
> +       }
> +
> +       return "";
> +}
> +
>   static struct at91_port *at91_pio_get_port(unsigned port)
>   {
>          switch (port) {
> @@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
> 
>          clk_free(&clk);
> 
> -       uc_priv->bank_name = plat->bank_name;
> -       uc_priv->gpio_count = GPIO_PER_BANK;
> -
>   #if CONFIG_IS_ENABLED(OF_CONTROL)
>          plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
>   #endif
> +       plat->bank_name = at91_get_bank_name(plat->base_addr);

Hello James,

Here you are rewriting the plat->bank_name... What was the old name that 
comes from the platform struct ?

Thanks for the patch,

Eugen


>          port->regs = (struct at91_port *)plat->base_addr;
> 
> +       uc_priv->bank_name = plat->bank_name;
> +       uc_priv->gpio_count = GPIO_PER_BANK;
> +
>          return 0;
>   }
> 
> --
> 2.24.0
> 
> 

[U-Boot] [PATCH] gpio: at91_gpio: Add bank names

[James Byrne]

Hi Eugen,

On 18/11/2019 08:59, Eugen.Hristev at microchip.com wrote:
>> @@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
>>
>>           clk_free(&clk);
>>
>> -       uc_priv->bank_name = plat->bank_name;
>> -       uc_priv->gpio_count = GPIO_PER_BANK;
>> -
>>    #if CONFIG_IS_ENABLED(OF_CONTROL)
>>           plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
>>    #endif
>> +       plat->bank_name = at91_get_bank_name(plat->base_addr);
> 
> Hello James,
> 
> Here you are rewriting the plat->bank_name... What was the old name that
> comes from the platform struct ?

It was unset (null).

Regards,

James

[U-Boot] [PATCH] gpio: at91_gpio: Add bank names

[Eugen.Hristev at microchip.com]

On 19.11.2019 16:17, James Byrne wrote:
> 
> Hi Eugen,
> 
> On 18/11/2019 08:59, Eugen.Hristev at microchip.com wrote:
>>> @@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
>>>
>>>           clk_free(&clk);
>>>
>>> -       uc_priv->bank_name = plat->bank_name;
>>> -       uc_priv->gpio_count = GPIO_PER_BANK;
>>> -
>>>    #if CONFIG_IS_ENABLED(OF_CONTROL)
>>>           plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
>>>    #endif
>>> +       plat->bank_name = at91_get_bank_name(plat->base_addr);
>>
>> Hello James,
>>
>> Here you are rewriting the plat->bank_name... What was the old name that
>> comes from the platform struct ?
> 
> It was unset (null).

Ok. Can you make the default name go to something like 'undefined' 
instead of an empty string ?  Maybe it would be more meaningful, what do 
you think ? (and easier to spot in the CLI )

Thanks
> 
> Regards,
> 
> James
>