Re: [U-Boot] [PATCH v2 1/2] gpio: Add GPIO driver framework for Marvell SoCs

----- "Simon Guinot" simon@sequanux.org wrote:
AFAIK, Orion and Kirkwood SoCs don't provide bitwise set/clear for GPIO output/direction registers. Instead, a register must be read first to leave other bits unchanged (see __set_direction in kw_gpio.c).
Is it possible to handle Armada SoCs GPIOs in a same way ? maybe using the pin registers (gpxx in the Armada struct gpio_reg array) ?
If not, this code is not Marvell generic but rather specific for Armada SoCs and then maybe armada_gpio is a better name...
Regards,
Simon
Hi Simon,
Yes its possible to implement code that way, Armada SoC does have GPIO registers for set/clear. what about register naming?? I think they are different for Kirkwood and Orion.
One more thing which can be done to make this code generic is to have some macros which can be defined by individual arch for specific registers which are going to be in use e.g.
#define GPIO_PIN_LEVEL_REG #define GPIO_DIR_REG #define GPIO_PIN_SET_REG #define GPIO_PIN_CLR_REG
so anyone can have their own version of these registers in gpio.h of their arch. The only thing which can complicate this is banking of registers, no. of banks etc.
Please provide comments on this, so we can have a better code.
Thanks & Regards, Ajay Bhargav

Hi Ajay,
On Wed, Aug 03, 2011 at 10:10:00AM +0530, Ajay Bhargav wrote:
----- "Simon Guinot" simon@sequanux.org wrote:
AFAIK, Orion and Kirkwood SoCs don't provide bitwise set/clear for GPIO output/direction registers. Instead, a register must be read first to leave other bits unchanged (see __set_direction in kw_gpio.c).
Is it possible to handle Armada SoCs GPIOs in a same way ? maybe using the pin registers (gpxx in the Armada struct gpio_reg array) ?
If not, this code is not Marvell generic but rather specific for Armada SoCs and then maybe armada_gpio is a better name...
Regards,
Simon
Hi Simon,
Yes its possible to implement code that way, Armada SoC does have GPIO registers for set/clear. what about register naming?? I think they are different for Kirkwood and Orion.
I think that the register names could be OK. But here is a most important problem: On Orion/Kirkwood SoCs, a single GPIO output register is available (no set/clear variants as for Armada). I missed that point at my first look. It is quite problematic because only two registers are shared between the different Marvell SoCs: level and direction. In fact, this registers are probably relevant on every machines providing GPIOs...
Maybe that having two common registers is not enough to add Orion/Kirkwood support to the mvgpio driver ?
One more thing which can be done to make this code generic is to have some macros which can be defined by individual arch for specific registers which are going to be in use e.g.
#define GPIO_PIN_LEVEL_REG #define GPIO_DIR_REG #define GPIO_PIN_SET_REG #define GPIO_PIN_CLR_REG
Yes, but how to handle both a single GPI0 output register and some GPIO {set,clear} output registers (in a nice way) ?
Regards,
Simon

Hi Simon,
On 04/08/2011 02:04, Simon Guinot wrote:
Hi Ajay,
On Wed, Aug 03, 2011 at 10:10:00AM +0530, Ajay Bhargav wrote:
----- "Simon Guinot"simon@sequanux.org wrote:
AFAIK, Orion and Kirkwood SoCs don't provide bitwise set/clear for GPIO output/direction registers. Instead, a register must be read first to leave other bits unchanged (see __set_direction in kw_gpio.c).
Is it possible to handle Armada SoCs GPIOs in a same way ? maybe using the pin registers (gpxx in the Armada struct gpio_reg array) ?
If not, this code is not Marvell generic but rather specific for Armada SoCs and then maybe armada_gpio is a better name...
Regards,
Simon
Hi Simon,
Yes its possible to implement code that way, Armada SoC does have GPIO registers for set/clear. what about register naming?? I think they are different for Kirkwood and Orion.
I think that the register names could be OK. But here is a most important problem: On Orion/Kirkwood SoCs, a single GPIO output register is available (no set/clear variants as for Armada). I missed that point at my first look. It is quite problematic because only two registers are shared between the different Marvell SoCs: level and direction. In fact, this registers are probably relevant on every machines providing GPIOs...
Maybe that having two common registers is not enough to add Orion/Kirkwood support to the mvgpio driver ?
One more thing which can be done to make this code generic is to have some macros which can be defined by individual arch for specific registers which are going to be in use e.g.
#define GPIO_PIN_LEVEL_REG #define GPIO_DIR_REG #define GPIO_PIN_SET_REG #define GPIO_PIN_CLR_REG
Yes, but how to handle both a single GPI0 output register and some GPIO {set,clear} output registers (in a nice way) ?
Two distinct gipo drivers for the two marvell variants?
Or a single driver with a single API but implemented differently according to the presence / absence of a variant flag, e.g. CONFIG_MVGIPO_HAS_SET_AND_CLR_REGS?
Regards,
Simon
Amicalement,

On Thu, Aug 4, 2011 at 4:51 PM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Simon,
On 04/08/2011 02:04, Simon Guinot wrote:
Hi Ajay,
On Wed, Aug 03, 2011 at 10:10:00AM +0530, Ajay Bhargav wrote:
----- "Simon Guinot"simon@sequanux.org wrote:
AFAIK, Orion and Kirkwood SoCs don't provide bitwise set/clear for GPIO output/direction registers. Instead, a register must be read first to leave other bits unchanged (see __set_direction in kw_gpio.c).
Is it possible to handle Armada SoCs GPIOs in a same way ? maybe using the pin registers (gpxx in the Armada struct gpio_reg array) ?
If not, this code is not Marvell generic but rather specific for Armada SoCs and then maybe armada_gpio is a better name...
Regards,
Simon
Hi Simon,
Yes its possible to implement code that way, Armada SoC does have GPIO registers for set/clear. what about register naming?? I think they are different for Kirkwood and Orion.
I think that the register names could be OK. But here is a most important problem: On Orion/Kirkwood SoCs, a single GPIO output register is available (no set/clear variants as for Armada). I missed that point at my first look. It is quite problematic because only two registers are shared between the different Marvell SoCs: level and direction. In fact, this registers are probably relevant on every machines providing GPIOs...
Maybe that having two common registers is not enough to add Orion/Kirkwood support to the mvgpio driver ?
>
One more thing which can be done to make this code generic is to have some macros which can be defined by individual arch for specific registers which are going to be in use e.g.
#define GPIO_PIN_LEVEL_REG #define GPIO_DIR_REG #define GPIO_PIN_SET_REG #define GPIO_PIN_CLR_REG
Yes, but how to handle both a single GPI0 output register and some GPIO {set,clear} output registers (in a nice way) ?
Two distinct gipo drivers for the two marvell variants?
If let I choose, I'd prefer two, since the register set is different.
Best regards, Lei
participants (4)
-
Ajay Bhargav
-
Albert ARIBAUD
-
Lei Wen
-
Simon Guinot