| Message ID | 0102016e70227f41-fa11b1eb-51df-4af7-a6c5-70ba8cb73c02-000000@eu-west-1.amazonses.com |
|---|---|
| State | Superseded |
| Delegated to: | Eugen Hristev |
| Headers | show |
| Series | [U-Boot] gpio: at91_gpio: Add bank names | expand |
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 > >
Hi Eugen, On 18/11/2019 08:59, Eugen.Hristev@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
On 19.11.2019 16:17, James Byrne wrote: > > Hi Eugen, > > On 18/11/2019 08:59, Eugen.Hristev@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 >
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; }
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(-)