From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vikram Narayanan Date: Tue, 31 Jul 2012 21:39:51 +0530 Subject: [U-Boot] [PATCH v2 1/2] bcm: Add GPIO driver In-Reply-To: <5017FF32.2060308@wwwdotorg.org> References: <4FFDE3A9.7080804@gmail.com> <4FFDE406.4030901@gmail.com> <5002FCA2.1050706@wwwdotorg.org> <5017FDD4.9080602@gmail.com> <5017FF32.2060308@wwwdotorg.org> Message-ID: <5018034F.7050304@gmail.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/31/2012 9:22 PM, Stephen Warren wrote: > On 07/31/2012 09:46 AM, Vikram Narayanan wrote: >> On 7/15/2012 10:53 PM, Stephen Warren wrote: >>> On 07/11/2012 02:37 PM, Vikram Narayanan wrote: >>>> Driver for BCM2835 SoC. This gives the basic functionality of >>>> setting/clearing the output. >>> >>>> diff --git a/arch/arm/include/asm/arch-bcm2835/gpio.h >>>> b/arch/arm/include/asm/arch-bcm2835/gpio.h > > One more comment on the patch subject; it probably should be "gpio: > bcm2835:" not "bcm:" since (a) it's in the GPIO directory and (b) the > GPIO module is specifically for a BCM2835, and probably doesn't apply to > any/all Broadcom devices. > >> >> Linux kernel follows this naming, to be exact, it should've been >> gpio-bcm2835.c. Having a thought in mind that one day the namings would >> be made consistent with the kernel. That is the reason for this naming, >> but isn't a big deal to change it. > > Hmmm. It seems better to be internally consistent with U-Boot rather > than keeping (onyl part of) U-Boot consistent with the kernel... Yes. > >>> Shouldn't that be BCM2835_GPIO_COMMON_SHIFT not BCM2835_GPIO_COMMON_MASK? >> >> If you'd like to have naming consistency FSEL_SHIFT/COMMON_SHIFT, then >> it shall be COMMON_SHIFT. >> >> But it doesn't do any shifting like the FSEL_SHIFT, rather it does only >> masking of bits. So, it makes more sense for me to name it as MASK and >> not SHIFT. > > The full quote you're replying to was: > >>> +int gpio_get_value(unsigned gpio) >> >>> + return (val>> BCM2835_GPIO_COMMON_MASK(gpio))& 0x1; >> >> Shouldn't that be BCM2835_GPIO_COMMON_SHIFT not BCM2835_GPIO_COMMON_MASK? > > ... so that macro is being used as a shift not as a mask. Naming isn't really a problem for me. If you want it to be SHIFT, I'd go with it.