From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vipin Kumar Date: Mon, 1 Jul 2013 11:51:46 +0530 Subject: [U-Boot] [PATCH] gpio: spear_gpio: Fix gpio_set_value() implementation In-Reply-To: References: <1371712419.4200.1.camel@phoenix> <201306201544.30751.marex@denx.de> <51C3D94D.1030507@st.com> <51C3F5EA.2050506@amarulasolutions.com> <51D10545.1030902@st.com> Message-ID: <51D11FFA.6000203@st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 7/1/2013 11:02 AM, Axel Lin wrote: >> >> The questions raised here are valid and it forced me to re-read the >> datasheet. For your convenience, I must tell you that the device is actually >> pl061 from ARM, so the driver can also be named so. >> >> The datasheet is here >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0190b/I1002697.html >> >> Quoting from the datasheet >> "The GPIODATA register is the data register. In software control mode, >> values written in the GPIODATA register are transferred onto the GPOUT pins >> if the respective pins have been configured as outputs through the GPIODIR >> register. >> >> In order to write to GPIODATA, the corresponding bits in the mask, resulting >> from the address bus, PADDR[9:2], must be HIGH. Otherwise the bit values >> remain unchanged by the write. >> >> Similarly, the values read from this register are determined for each bit, >> by the mask bit derived from the address used to access the data register, >> PADDR[9:2]. Bits that are 1 in the address mask cause the corresponding bits >> in GPIODATA to be read, and bits that are 0 in the address mask cause the >> corresponding bits in GPIODATA to be read as 0, regardless of their value. >> >> A read from GPIODATA returns the last bit value written if the respective >> pins are configured as output, or it returns the value on the corresponding >> input GPIN bit when these are configured as inputs. All bits are cleared by >> a reset." >> >> After reading all this I am confused about numbering of the gpio's. I think >> the numbering should be from 1 to 8 for a device. And this would mean that >> we should write to *®s->datareg[1<< (gpio - 1)]* instead of the present >> code which is _®s->datareg[1<< (gpio + 2)]_ > > Hi Vipin, Hello Alex, > Thanks for the review and providing the datasheet information. > You mentioned that this is PL061. > So... I just checked the gpio-pl061 driver in linux kernel. > It's writing to _®s->datareg[1<< (gpio + 2)]. and seems no bug > report for this. > Yes, I see it now. The difference is that we are using a writel and the datareg is a u32 array. > And the gpio_get/set implementation in linux kernel has the same behavior as > this patch does: > > ( below is from linux/drivers/gpio/gpio-pl061.c ) > > static int pl061_get_value(struct gpio_chip *gc, unsigned offset) > { > struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > > return !!readb(chip->base + (1<< (offset + 2))); > } > > static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value) > { > struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > > writeb(!!value<< offset, chip->base + (1<< (offset + 2))); > } > > BTW, it would be great if you have the hardware to test. > I am sorry about this. I have now moved to a different group and I have no access to the hardware Regards Vipin > Regards, > Axel > . >