[U-Boot] [PATCH] gpio: dwapb: Add support for port B

[Marek Vasut]

On 11/03/2016 02:56 PM, Simon Glass wrote:
> Hi,
> 
> On 2 November 2016 at 13:38, Marek Vasut <marex at denx.de
> <mailto:marex at denx.de>> wrote:
>> On 11/02/2016 04:24 PM, Phil Edworthy wrote:
>>> The IP supports two ports, A and B, each providing up to 32 gpios.
>>> The driver already creates a 2nd gpio bank by reading the 2nd node
>>> from DT, so this is quite a simple change to support the 2nd bank.
>>>
>>> Signed-off-by: Phil Edworthy <phil.edworthy at renesas.com
> <mailto:phil.edworthy at renesas.com>>
>>> ---
>>> drivers/gpio/dwapb_gpio.c | 40 +++++++++++++++++++++++++++++++++-------
>>> 1 file changed, 33 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
>>> index 471e18a..dda0b42 100644
>>> --- a/drivers/gpio/dwapb_gpio.c
>>> +++ b/drivers/gpio/dwapb_gpio.c
>>> @@ -21,6 +21,8 @@ DECLARE_GLOBAL_DATA_PTR;
>>>
>>> #define GPIO_SWPORTA_DR 0x00
>>> #define GPIO_SWPORTA_DDR 0x04
>>> +#define GPIO_SWPORTB_DR 0x0C
>>> +#define GPIO_SWPORTB_DDR 0x10
>>> #define GPIO_INTEN 0x30
>>> #define GPIO_INTMASK 0x34
>>> #define GPIO_INTTYPE_LEVEL 0x38
>>> @@ -29,6 +31,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>> #define GPIO_PORTA_DEBOUNCE 0x48
>>> #define GPIO_PORTA_EOI 0x4c
>>> #define GPIO_EXT_PORTA 0x50
>>> +#define GPIO_EXT_PORTB 0x54
>>>
>>> struct gpio_dwapb_platdata {
>>> const char *name;
>>> @@ -41,7 +44,11 @@ static int dwapb_gpio_direction_input(struct
> udevice *dev, unsigned pin)
>>> {
>>> struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
>>>
>>> - clrbits_le32(plat->base + GPIO_SWPORTA_DDR, 1 << pin);
>>> + if (plat->bank == 0)
>>> + clrbits_le32(plat->base + GPIO_SWPORTA_DDR, 1 << pin);
>>> + else
>>> + clrbits_le32(plat->base + GPIO_SWPORTB_DDR, 1 << pin);
>>
>> What about doing plat->base + plat->offset + GPIO_SWPORT_DDR ? Then you
>> don't need this if-else stuff.
> 
> Yes. In fact this seems to be crying out to use a C structure instead.
> 
> struct gpio_port {
> uint32_t dr;
> uint32_t ddr;
> };
> 
> struct gpio_regs {
> struct gpio_port port[2];
> uint32_t reserved[8];
> uint32_t inten;
> ...
> }

Please no c-structure register definitions, this does not scale :(


-- 
Best regards,
Marek Vasut