Hi Ajay, On Thu, Aug 4, 2011 at 6:51 PM, Ajay Bhargav <ajay.bhar...@einfochips.com> wrote: > ----- "Lei Wen" <adrian.w...@gmail.com> wrote: > >> On Thu, Aug 4, 2011 at 4:51 PM, Albert ARIBAUD >> <albert.u.b...@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"<si...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot