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

[Lei Wen]

Hi Ajay,

On Thu, Aug 4, 2011 at 6:51 PM, Ajay Bhargav
<ajay.bhargav at einfochips.com> wrote:
> ----- "Lei Wen" <adrian.wenl at gmail.com> wrote:
>
>> On Thu, Aug 4, 2011 at 4:51 PM, Albert ARIBAUD
>> <albert.u.boot at 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 at 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
>>
>
> Hi Simon,
>
> For Armada minimum 3 registers are required and available for armada
> 1. Direction (read/write)
> 2. Pin level set (write only)
> 3. Pin level clear (write only)
>
> @lei
> How bout if we check for architecture and use specific code or defines?
> i.e.
> #ifdef CONFIG_KIRKWOOD
> //KW code
> #elif CONFIG_ARMADA100
> //Armada code
> #else
> //orion or other?
> #endif
>
I agree with Prafulla, if this not unalignment could be fixed in the
.h file which
is located in the arch directory, or ifdef there?, then I think I
would totally fine with that. Or maybe
seperate with two c file is a better solution.

Best regards,
Lei