----- "Prafulla Wadaskar" <prafu...@marvell.com> wrote: > > -----Original Message----- > > From: u-boot-boun...@lists.denx.de > [mailto:u-boot-boun...@lists.denx.de] > > On Behalf Of Ajay Bhargav > > Sent: Thursday, August 04, 2011 4:21 PM > > To: Lei Wen > > Cc: u-boot@lists.denx.de > > Subject: Re: [U-Boot] [PATCH v2 1/2] gpio: Add GPIO driver framework > for > > Marvell SoCs > > > > ----- "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 > > Let's avoid this, because there will be several SoC architectures that > uses similar GPIO register definitions, like kirkwood/orion have > similar definition and armada/mmp/pantheon/etc.. have different one. > > So we will end up having several #ifdefs. Ideally #ifdefs are > discouraged for better coding practices. > > Instead, > I would suggest to use macros for this code segments or alternatively > inlined functions and those should be defined in mvgpio.h, #ifdefed > with CPU core subversion (i.e. CONFIG_FEROCEION, > CONFIG_SHEEVA_88SV331xV5) > > Regards.. > Prafulla . .
Hi Prafulla, I think it will be better to keep two driver files. Let this patch be for Armada/mmp/pantheon and other compatible SoCs. Should the common GPIO struct for armada/mmp etc.. be moved out of GPIO.h to mvgpio.h? Regards, Ajay Bhargav _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
