From: Jaya K. <jay...@gm...> - 2008-12-27 16:24:24
|
Hi friends, This is v2 of my attempt to add batch support to gpiolib. Thanks to David Brownell, Eric Miao, Paulius Zaleckas, Geert, and Sam for prior feedback. I have tested set_batch using the am300 driver. get_batch is not tested. Please let me know your feedback. Thanks, jaya am300epd was doing 800*600*16*gpio_set_value for each framebuffer transfer (it uses 16-pins of gpio as its data bus). I found this caused a wee performance limitation. This patch adds an API for gpio_s/get_batch which allows drivers to s/get batches of gpio together in a single call. I have done a test implementation on gumstix (pxa255) with am300epd and it provides a nice improvement in performance. Signed-off-by: Jaya Kumar <jay...@gm...> Cc: David Brownell <dbr...@us...> Cc: Eric Miao <eri...@ma...> Cc: Paulius Zaleckas <pau...@te...> Cc: Geert Uytterhoeven <ge...@li...> Cc: David Brownell <da...@pa...> Cc: Sam Ravnborg <sa...@ra...> Cc: Haavard Skinnemoen <hsk...@at...> Cc: Philipp Zabel <phi...@gm...> Cc: Russell King <rm...@ar...> Cc: Ben Gardner <bga...@wa...> CC: Greg KH <gr...@kr...> Cc: lin...@li... Cc: lin...@li... Cc: lin...@vg... --- arch/arm/mach-pxa/Kconfig | 1 + arch/arm/mach-pxa/am300epd.c | 9 ++- arch/arm/mach-pxa/gpio.c | 72 +++++++++++++++++++ arch/arm/mach-pxa/include/mach/gpio.h | 47 +++++++++++++ drivers/gpio/Kconfig | 5 ++ drivers/gpio/gpiolib.c | 122 +++++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 15 ++++- 7 files changed, 269 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig index 768618b..a5a306e 100644 --- a/arch/arm/mach-pxa/Kconfig +++ b/arch/arm/mach-pxa/Kconfig @@ -41,6 +41,7 @@ config GUMSTIX_AM200EPD bool "Enable AM200EPD board support" config GUMSTIX_AM300EPD + select GPIOLIB_BATCH bool "Enable AM300EPD board support" endchoice diff --git a/arch/arm/mach-pxa/am300epd.c b/arch/arm/mach-pxa/am300epd.c index 4bd10a1..9594cf0 100644 --- a/arch/arm/mach-pxa/am300epd.c +++ b/arch/arm/mach-pxa/am300epd.c @@ -190,9 +190,12 @@ static u16 am300_get_hdb(struct broadsheetfb_par *par) u16 res = 0; int i; +#ifdef CONFIG_GPIOLIB_BATCH + res = gpio_get_batch(DB0_GPIO_PIN, 0xFFFF, 16); +#else for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) res |= (gpio_get_value(DB0_GPIO_PIN + i)) ? (1 << i) : 0; - +#endif return res; } @@ -200,8 +203,12 @@ static void am300_set_hdb(struct broadsheetfb_par *par, u16 data) { int i; +#ifdef CONFIG_GPIOLIB_BATCH + gpio_set_batch(DB0_GPIO_PIN, data, 0xFFFF, 16); +#else for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01); +#endif } diff --git a/arch/arm/mach-pxa/gpio.c b/arch/arm/mach-pxa/gpio.c index 5fec1e4..d85cf2e 100644 --- a/arch/arm/mach-pxa/gpio.c +++ b/arch/arm/mach-pxa/gpio.c @@ -162,6 +162,76 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) __raw_writel(mask, pxa->regbase + GPCR_OFFSET); } +#ifdef CONFIG_GPIOLIB_BATCH +/* + * Set output GPIO level in batches + */ +static void pxa_gpio_set_batch(struct gpio_chip *chip, unsigned offset, + u32 values, u32 bitmask) +{ + struct pxa_gpio_chip *pxa; + + /* we're guaranteed by the caller that offset + bitmask remains + * in this chip. + */ + pxa = container_of(chip, struct pxa_gpio_chip, chip); + + /* shift the bits into our register specific position */ + values <<= offset; + bitmask <<= offset; + + values &= bitmask; + if (values) + __raw_writel(values, pxa->regbase + GPSR_OFFSET); + + values = ~values; + values &= bitmask; + if (values) + __raw_writel(values, pxa->regbase + GPCR_OFFSET); +} + +/* + * Get output GPIO level in batches + */ +static u32 pxa_gpio_get_batch(struct gpio_chip *chip, unsigned offset, + u32 bitmask) +{ + u32 values; + struct pxa_gpio_chip *pxa; + + /* we're guaranteed by the caller that offset + bitmask remains + * in this chip. + */ + pxa = container_of(chip, struct pxa_gpio_chip, chip); + + values = __raw_readl(pxa->regbase + GPLR_OFFSET); + + /* shift the result back into original position */ + values >>= offset; + /* no need to shift bitmask since we've already shifted values */ + values &= bitmask; + + return values; +} +#endif + +#ifdef CONFIG_GPIOLIB_BATCH +#define GPIO_CHIP(_n) \ + [_n] = { \ + .regbase = GPIO##_n##_BASE, \ + .chip = { \ + .label = "gpio-" #_n, \ + .direction_input = pxa_gpio_direction_input, \ + .direction_output = pxa_gpio_direction_output, \ + .get = pxa_gpio_get, \ + .set = pxa_gpio_set, \ + .base = (_n) * 32, \ + .ngpio = 32, \ + .set_batch = pxa_gpio_set_batch, \ + .get_batch = pxa_gpio_get_batch, \ + }, \ + } +#else #define GPIO_CHIP(_n) \ [_n] = { \ .regbase = GPIO##_n##_BASE, \ @@ -175,6 +245,8 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) .ngpio = 32, \ }, \ } +#endif + static struct pxa_gpio_chip pxa_gpio_chip[] = { GPIO_CHIP(0), diff --git a/arch/arm/mach-pxa/include/mach/gpio.h b/arch/arm/mach-pxa/include/mach/gpio.h index 2c538d8..e66e752 100644 --- a/arch/arm/mach-pxa/include/mach/gpio.h +++ b/arch/arm/mach-pxa/include/mach/gpio.h @@ -56,6 +56,53 @@ static inline void gpio_set_value(unsigned gpio, int value) } } +#ifdef CONFIG_GPIOLIB_BATCH +static inline void gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, + int bitwidth) +{ + if (__builtin_constant_p(gpio) && + (gpio + bitwidth < NR_BUILTIN_GPIO) && + ((gpio + bitwidth) <= roundup(gpio+1, 32))) { + int shift; + + shift = gpio % 32; + values <<= shift; + bitmask <<= shift; + + values &= bitmask; + if (values) + GPSR(gpio) = values; + + values = ~values; + values &= bitmask; + if (values) + GPCR(gpio) = values; + } else { + __gpio_set_batch(gpio, values, bitmask, bitwidth); + } +} + +static inline u32 gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth) +{ + if (__builtin_constant_p(gpio) && + (gpio + bitwidth < NR_BUILTIN_GPIO) && + ((gpio + bitwidth) <= roundup(gpio+1, 32))) { + int shift; + u32 values; + + shift = gpio % 32; + + values = GPLR(gpio); + values >>= shift; + values &= bitmask; + + return values; + } else { + return __gpio_get_batch(gpio, bitmask, bitwidth); + } +} +#endif + #define gpio_cansleep __gpio_cansleep #define gpio_to_irq(gpio) IRQ_GPIO(gpio) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 48f49d9..aeeb83c 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -37,6 +37,11 @@ menuconfig GPIOLIB if GPIOLIB +config GPIOLIB_BATCH + bool "Batch GPIO support" + help + Say Y here to add the capability to batch set/get GPIOs. + config DEBUG_GPIO bool "Debug GPIO calls" depends on DEBUG_KERNEL diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 82020ab..7b36573 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1056,6 +1056,128 @@ void __gpio_set_value(unsigned gpio, int value) } EXPORT_SYMBOL_GPL(__gpio_set_value); +#ifdef CONFIG_GPIOLIB_BATCH +/** + * __gpio_set_batch() - assign a batch of gpio pins together + * @gpio: starting gpio pin + * @values: values to assign, sequential and including masked bits + * @bitmask: bitmask to be applied to values + * @bitwidth: width inclusive of masked-off bits + * Context: any + * + * This is used directly or indirectly to implement gpio_set_value(). + * It invokes the associated gpio_chip.set_batch() method. If that + * method does not exist for any segment that is involved, then it drops + * back down to standard gpio_chip.set() + */ +void __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, int bitwidth) +{ + struct gpio_chip *chip; + int i = 0; + int value, width, remwidth; + u32 mask; + + do { + chip = gpio_to_chip(gpio + i); + WARN_ON(extra_checks && chip->can_sleep); + + if (!chip->set_batch) { + while (((gpio + i) < (chip->base + chip->ngpio)) + && bitwidth) { + mask = 1 << i; + value = values & mask; + if (bitmask & mask) + chip->set(chip, gpio + i - chip->base, + value); + i++; + bitwidth--; + } + } else { + value = values >> i; /* shift off the used data bits */ + remwidth = ((chip->base + (int) chip->ngpio) - + ((int) gpio + i)); + width = min(bitwidth, remwidth); + + /* shift off the used mask bits */ + mask = bitmask >> i; + /* now adjust mask by width of set */ + mask &= ((1 << width) - 1); + + chip->set_batch(chip, gpio + i - chip->base, value, + mask); + i += width; + bitwidth -= width; + } + } while (bitwidth); +} +EXPORT_SYMBOL_GPL(__gpio_set_batch); + +/** + * __gpio_get_batch() - get a batch of gpio pins together + * @gpio: starting gpio pin + * @bitmask: bitmask to be applied to values + * @bitwidth: width inclusive of masked-off bits + * Context: any + * + * This is used directly or indirectly to implement gpio_get_value(). + * It invokes the associated gpio_chip.get_batch() method and returns + * zero if no such method is provided. + */ +u32 __gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth) +{ + struct gpio_chip *chip; + int i = 0; + int width, remwidth; + u32 mask; + u32 values = 0; + u32 value; + + do { + chip = gpio_to_chip(gpio + i); + WARN_ON(extra_checks && chip->can_sleep); + + /* + * check if we have batch capability on this chip. + * if not, then we use the single bit gets + * any masked or inaccessible bits are already 0 + */ + if (!chip->get_batch) { + while (((gpio + i) < (chip->base + chip->ngpio)) + && bitwidth) { + mask = 1 << i; + /* skip if no get or masked out */ + if (chip->get && (bitmask & mask)) { + if (chip->get(chip, + gpio + i - chip->base)) + values |= mask; + } + i++; + bitwidth--; + } + } else { + remwidth = ((chip->base + (int) chip->ngpio) - + ((int) gpio + i)); + width = min(bitwidth, remwidth); + + /* shift off the used mask bits */ + mask = bitmask >> i; + /* now adjust mask by width of get */ + mask &= ((1 << width) - 1); + + value = chip->get_batch(chip, gpio + i - chip->base, + mask); + /* shift result back into caller position */ + values |= value << i; + i += width; + bitwidth -= width; + } + } while (bitwidth); + + return values; +} +EXPORT_SYMBOL_GPL(__gpio_get_batch); +#endif + /** * __gpio_cansleep() - report whether gpio value access will sleep * @gpio: gpio in question diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 81797ec..fa716a7 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -44,6 +44,8 @@ struct module; * returns either the value actually sensed, or zero * @direction_output: configures signal "offset" as output, or returns error * @set: assigns output value for signal "offset" + * @set_batch: batch assigns output values for consecutive signals starting at + * "offset" with mask in "bitmask" all within this gpio_chip * @to_irq: optional hook supporting non-static gpio_to_irq() mappings; * implementation may not sleep * @dbg_show: optional routine to show contents in debugfs; default code @@ -84,7 +86,13 @@ struct gpio_chip { unsigned offset, int value); void (*set)(struct gpio_chip *chip, unsigned offset, int value); - +#ifdef CONFIG_GPIOLIB_BATCH + void (*set_batch)(struct gpio_chip *chip, + unsigned offset, u32 values, + u32 bitmask); + u32 (*get_batch)(struct gpio_chip *chip, + unsigned offset, u32 bitmask); +#endif int (*to_irq)(struct gpio_chip *chip, unsigned offset); @@ -124,6 +132,11 @@ extern void gpio_set_value_cansleep(unsigned gpio, int value); */ extern int __gpio_get_value(unsigned gpio); extern void __gpio_set_value(unsigned gpio, int value); +#ifdef CONFIG_GPIOLIB_BATCH +extern void __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, + int bitwidth); +extern u32 __gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth); +#endif extern int __gpio_cansleep(unsigned gpio); -- 1.5.2.3 |
From: Jaya K. <jay...@gm...> - 2009-01-25 09:55:04
|
Hi friends, This is v5 of batch support for gpiolib. Thanks to Uwe Kleine-König, Ryan Mallon and others for prior feedback. The changes I've made are: - split the patches into generic, arch specific and am300epd - adjusting the API to remove width (note, the actual API call where width was dropped is in the arch specific code, not here.) - updating documentation of this API in gpio.txt - cleanup of the width, mask terms Please let me know your thoughts and feedback. Thanks, jaya Cc: David Brownell <da...@pa...> Cc: Eric Miao <eri...@ma...> Cc: Paulius Zaleckas <pau...@te...> Cc: Geert Uytterhoeven <ge...@li...> Cc: Sam Ravnborg <sa...@ra...> Cc: lin...@li... Cc: lin...@li... Cc: lin...@vg... Cc: lin...@vg... Signed-off-by: Jaya Kumar <jay...@gm...> --- Documentation/gpio.txt | 72 ++++++++++++ drivers/gpio/Kconfig | 5 + drivers/gpio/gpiolib.c | 272 ++++++++++++++++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 17 +++- 4 files changed, 365 insertions(+), 1 deletions(-) diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt index b1b9887..d249b5c 100644 --- a/Documentation/gpio.txt +++ b/Documentation/gpio.txt @@ -185,6 +185,78 @@ and not to need spinlocks. Such optimized calls can make bitbanging applications a lot more efficient (in both space and time) than spending dozens of instructions on subroutine calls. +[OPTIONAL] Spinlock-Safe GPIO Batch access +------------------------------------------ +The original GPIO API provides single bit access to GPIO pins. However, +some devices treat GPIO as a mechanism for bulk data transfer. In this type +of system, the performance of the single bit API may be inadequate. As such, +the user can consider enabling the batch access API. The batch access API +allows access of up-to 32-bits of GPIO at a time. This API is as follows: + + /* BATCH GPIO INPUT */ +int gpio_get_batch(unsigned startpin, u32 mask, u32 *result); + +The following examples help explain how this function is to be used. + Q: How do I get gpio pins 0 through 7? (8 bits) + A: gpio_get_batch(startpin=0, mask=0xFF, &result) result=0xnn + Q: How do I get gpio pins 58 through 73? (16 bits) + A: gpio_get_batch(startpin=58, mask=0xFFFF, &result) result=0xnnnn + Q: How do I get gpio pins 16 through 47? (32 bits) + A: gpio_get_batch(startpin=16, mask=0xFFFFFFFF, &result) + A: result=0xnnnnnnnn + Q: How do I get non-consecutive gpio pins 5 and 9? + A: Use the mask to mask out 6, 7 and 8 + A: So mask in binary is 10001 which is 0x11 + A: gpio_get_batch(startpin=5, mask=0x11, &result) + A: result is in the same form, binary n000n + + /* BATCH GPIO OUTPUT */ +int gpio_set_batch(unsigned startpin, u32 mask, u32 values); + +The following examples help explain how this function is to be used. + Q: How to set gpio pins 0 through 7 to all 0? (8 bits) + A: gpio_set_batch(startpin=0, mask=0xFF, values=0x0); + Q: How to set gpio pins 58 through 73 to all 1? (16 bits) + A: gpio_set_batch(startpin=58, mask=0xFFFF, values=0xFFFF); + Q: How to set gpio pins 16 through 47 to 0xCAFEC001? (32 bits) + A: gpio_set_batch(startpin=16, mask=0xFFFFFFFF, values=0xCAFEC001); + Q: How do I set non-consecutive gpio pins 5 and 9 to both 1? + A: Use the mask to mask out 6, 7 and 8 + A: So mask in binary is 10001 which is 0x11 + A: gpio_set_batch(startpin=5, mask=0x11, values=0x11) + +The following example shows the use of the batch API and a comparison with +the original single bit API: + +Original input method which loops through a set of pins: + for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) + res |= (gpio_get_value(DB0_GPIO_PIN + i)) ? (1 << i) : 0; + +Batch input method: + u32 val; + err = gpio_get_batch(DB0_GPIO_PIN, 0xFFFF, &val); + +Original output method: + for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) + gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01); + +Batch output method: + int err; + err = gpio_set_batch(DB0_GPIO_PIN, 0xFFFF, data); + +Using these calls for GPIOs that can't safely be accessed without sleeping +(see below) is an error. + +Platform-specific implementations are encouraged to optimize the two +calls by checking if the batch get/set requested can be achieved within the +platform's fast path access to gpio registers. For example, if the starting +gpio and width of bits to be written is contained within a single register +then the platform specific implementation may choose to execute it. If the +request is more complex, then the platform specific implementation can +choose to call upon the platform independent __gpio_get/set_batch functions +which are able to cross gpio_chip boundaries. Implementations are also +encouraged to use the bitops macros. These will also optimize for use cases +where the masks are constants. GPIO access that may sleep -------------------------- diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 3d25654..474070b 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -37,6 +37,11 @@ menuconfig GPIOLIB if GPIOLIB +config GPIOLIB_BATCH + bool "Batch GPIO support" + help + Say Y here to add the capability to batch set/get GPIOs. + config DEBUG_GPIO bool "Debug GPIO calls" depends on DEBUG_KERNEL diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 35e7aea..12e1e30 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -643,6 +643,272 @@ static inline void gpiochip_unexport(struct gpio_chip *chip) #endif /* CONFIG_GPIO_SYSFS */ +#ifdef CONFIG_GPIOLIB_BATCH +/** + * __gpio_set_batch_generic() - Set batch of gpio pins in provided gpio_chip. + * @chip: gpio_chip containing this set of pins + * @startpin: starting gpio pin + * @mask: the mask to be applied to values + * @width: the width to the last set bit of the mask + * @values: values to assign including masked bits + * Context: any + * + * This provides a generic platform independent set_batch capability. + * It invokes the associated gpio_chip.set() method to actually set the + * value. gpio_chip-s that don't implement their own optimized + * set_batch function are assigned this function as a default. + * + * Returns a negative errno if the caller supplied bad data, such as + * startpin or width in excess of this chips gpio. Otherwise, we return zero + * as a success code. + */ +static int __gpio_set_batch_generic(struct gpio_chip *chip, unsigned startpin, + u32 mask, int width, u32 values) +{ + int i; + u32 pin_mask; + int value; + + /* + * If the caller attempted to exceed the number of gpios that + * are in this chip, then we flag that as an invalid value for + * either the startpin or the width supplied. + */ + if (startpin + width > chip->ngpio) + return -EINVAL; + + /* + * We start the loop and continue till we reach the width + * of the mask that is provided to us. + */ + for (i = 0; i < width; i++) { + /* + * If this bit is enabled by the mask then + * we perform the set. If it is disabled we leave + * it alone. + */ + pin_mask = 1 << i; + if (mask & pin_mask) { + value = values & pin_mask; + chip->set(chip, startpin + i, value); + } + } + + return 0; +} + +/** + * __gpio_get_batch_generic() - Get batch of gpio pins in provided gpio_chip. + * @chip: gpio_chip containing this set of pins + * @startpin: starting gpio pin + * @mask: the mask to be applied to values + * @width: the width to the last set bit of the mask + * @result: the result to be returned + * Context: any + * + * This provides a generic platform independent get_batch capability. + * It invokes the associated gpio_chip.get() method to actually set the + * value. gpio_chip-s that don't implement their own optimized + * get_batch function are assigned this function as a default. + * + * Returns a negative errno if the caller supplied bad data, such as + * startpin or width in excess of this chips gpio. Otherwise, we return zero + * as a success code. + * Context: any + */ +static int __gpio_get_batch_generic(struct gpio_chip *chip, unsigned startpin, + u32 mask, int width, u32 *result) +{ + int i; + u32 pin_mask; + u32 values = 0; + + /* + * If the caller attempted to exceed the number of gpios that + * are in this chip, then we flag that as an invalid value for + * either the startpin or the width supplied. + */ + if (startpin + width > chip->ngpio) + return -EINVAL; + + /* + * We start the loop and continue till we reach the width + * of the mask that is provided to us. + */ + for (i = 0; i < width; i++) { + /* + * If this bit is enabled by the mask then + * we perform the get. If it is disabled we leave + * it alone thus leaving it as 0 because we initialized + * values. + */ + pin_mask = 1 << i; + if (mask & pin_mask) { + if (chip->get(chip, startpin + i)) + values |= pin_mask; + } + } + + *result = values; + return 0; +} + +/** + * __gpio_set_batch() - set batch of gpio pins across multiple gpio_chip-s + * @startpin: starting gpio pin + * @mask: the mask to be applied to values + * @width: the width to the last set bit of the mask + * @values: values to assign including masked bits + * Context: any + * + * This function is platform independent and uses the starting gpio and + * width information to iterate through the set of required gpio_chips + * to use their set_batch capability in order to set a batch of gpio pins. + * This function handles going across gpio_chip boundaries. It is intended + * to be called from arch/mach specific code if they detect that the caller + * requires functionality outside the fast-path. + * + * Returns a negative errno if the caller supplied bad data, such as + * startpin or width in excess of the platforms max gpio. Otherwise, we return + * zero as a success code. + * + */ +int __gpio_set_batch(unsigned startpin, u32 mask, int width, u32 values) +{ + struct gpio_chip *chip; + int i = 0; + int subwidth, remwidth; + u32 subvalue; + u32 submask; + int ret; + + /* + * If the caller attempted to exceed the number of gpios that + * are available here, then we flag that as an invalid value for + * either the startpin or the width supplied. + */ + if ((width > 32) || (startpin + width > ARCH_NR_GPIOS)) + return -EINVAL; + + do { + chip = gpio_to_chip(startpin + i); + WARN_ON(extra_checks && chip->can_sleep); + + subvalue = values >> i; /* shift off the used data bits */ + + /* Work out the remaining width in this chip. */ + remwidth = ((chip->base + (int) chip->ngpio) - + ((int) startpin + i)); + + /* + * Check if the remaining bits to be handled are less than + * the remaining width in this chip. That is the width of + * the subset that we are about to handle. + */ + subwidth = min(width, remwidth); + + /* Shift off the used mask bits. */ + submask = mask >> i; + + /* Now adjust mask by width of this subset. */ + submask &= ((1 << subwidth) - 1); + + /* If the mask is empty, then we can skip this chip. */ + if (submask) { + ret = chip->set_batch(chip, startpin + i - chip->base, + submask, subwidth, subvalue); + if (ret) + return ret; + } + + /* deduct the used bits from our todolist */ + i += subwidth; + width -= subwidth; + } while (width); + + return 0; +} +EXPORT_SYMBOL_GPL(__gpio_set_batch); + +/** + * __gpio_get_batch() - get batch of gpio pins across multiple gpio_chip-s + * @startpin: starting gpio pin + * @mask: the mask to be applied to values + * @width: the width to the last set bit of the mask + * @result: returned values + * Context: any + * + * This function is platform independent and uses the starting gpio and + * width information to iterate through the set of required gpio_chips + * to use their get_batch capability in order to get a batch of gpio pins. + * This function handles going across gpio_chip boundaries. It is intended + * to be called from arch/mach specific code if they detect that the caller + * requires functionality outside the fast-path. + * + * Returns a negative errno if the caller supplied bad data, such as + * startpin or width in excess of the platforms max gpio. Otherwise, we return + * zero as a success code + * + */ +int __gpio_get_batch(unsigned startpin, u32 mask, int width, u32 *result) +{ + struct gpio_chip *chip; + int i = 0; + int subwidth, remwidth; + u32 submask; + u32 values = 0; + u32 subvalue; + int ret; + + /* + * If the caller attempted to exceed the number of gpios that + * are available here, then we flag that as an invalid value for + * either the startpin or the width supplied. + */ + if ((width > 32) || (startpin + width > ARCH_NR_GPIOS)) + return -EINVAL; + + do { + chip = gpio_to_chip(startpin + i); + WARN_ON(extra_checks && chip->can_sleep); + + /* Work out the remaining width in this chip. */ + remwidth = ((chip->base + (int) chip->ngpio) - + ((int) startpin + i)); + + /* + * Check if the remaining bits to be handled are less than + * the remaining width in this chip. + */ + subwidth = min(width, remwidth); + + /* shift off the used mask bits */ + submask = mask >> i; + /* now adjust mask by width of get */ + submask &= ((1 << width) - 1); + + /* If the mask is empty, then we can skip this chip. */ + if (submask) { + ret = chip->get_batch(chip, startpin + i - chip->base, + submask, subwidth, &subvalue); + if (ret) + return ret; + + /* shift result back into correct position */ + values |= subvalue << i; + } + + /* deduct the used bits from our todolist */ + i += subwidth; + width -= subwidth; + } while (width); + + *result = values; + return 0; +} +EXPORT_SYMBOL_GPL(__gpio_get_batch); +#endif + /** * gpiochip_add() - register a gpio_chip * @chip: the chip to register, with chip->base initialized @@ -683,6 +949,12 @@ int gpiochip_add(struct gpio_chip *chip) } chip->base = base; } +#ifdef CONFIG_GPIOLIB_BATCH + if (!chip->set_batch) + chip->set_batch = __gpio_set_batch_generic; + if (!chip->get_batch) + chip->get_batch = __gpio_get_batch_generic; +#endif /* these GPIO numbers must not be managed by another gpio_chip */ for (id = base; id < base + chip->ngpio; id++) { diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 81797ec..4e92ccf 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -44,6 +44,10 @@ struct module; * returns either the value actually sensed, or zero * @direction_output: configures signal "offset" as output, or returns error * @set: assigns output value for signal "offset" + * @set_batch: batch assigns output values for signals starting at + * "startpin" with mask in "mask" all within this gpio_chip + * @get_batch: batch fetches values for consecutive signals starting at + * "startpin" with mask in "mask" all within this gpio_chip * @to_irq: optional hook supporting non-static gpio_to_irq() mappings; * implementation may not sleep * @dbg_show: optional routine to show contents in debugfs; default code @@ -84,7 +88,14 @@ struct gpio_chip { unsigned offset, int value); void (*set)(struct gpio_chip *chip, unsigned offset, int value); - +#ifdef CONFIG_GPIOLIB_BATCH + int (*set_batch)(struct gpio_chip *chip, + unsigned startpin, u32 mask, + int width, u32 values); + int (*get_batch)(struct gpio_chip *chip, + unsigned startpin, u32 mask, + int width, u32 *result); +#endif int (*to_irq)(struct gpio_chip *chip, unsigned offset); @@ -124,6 +135,10 @@ extern void gpio_set_value_cansleep(unsigned gpio, int value); */ extern int __gpio_get_value(unsigned gpio); extern void __gpio_set_value(unsigned gpio, int value); +#ifdef CONFIG_GPIOLIB_BATCH +extern int __gpio_set_batch(unsigned gpio, u32 mask, int width, u32 values); +extern int __gpio_get_batch(unsigned gpio, u32 mask, int width, u32 *result); +#endif extern int __gpio_cansleep(unsigned gpio); -- 1.5.2.3 |
From: Jaya K. <jay...@gm...> - 2009-01-25 09:55:05
|
This patch adds support for pxa specific batch set/get of gpio. This is exported to gpiolib via the gpio_chip interface. The API used conforms with the gpiolib batch set/get API described in Documentation/gpio.txt. Cc: David Brownell <da...@pa...> Cc: Eric Miao <eri...@ma...> Cc: Paulius Zaleckas <pau...@te...> Cc: Geert Uytterhoeven <ge...@li...> Cc: Sam Ravnborg <sa...@ra...> Cc: lin...@li... Cc: lin...@li... Cc: lin...@vg... Cc: lin...@vg... Signed-off-by: Jaya Kumar <jay...@gm...> --- arch/arm/mach-pxa/Kconfig | 1 + arch/arm/mach-pxa/gpio.c | 63 +++++++++++++++++++++++++++++++++ arch/arm/mach-pxa/include/mach/gpio.h | 51 ++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig index f844425..1b3e631 100644 --- a/arch/arm/mach-pxa/Kconfig +++ b/arch/arm/mach-pxa/Kconfig @@ -41,6 +41,7 @@ config GUMSTIX_AM200EPD bool "Enable AM200EPD board support" config GUMSTIX_AM300EPD + select GPIOLIB_BATCH bool "Enable AM300EPD board support" endchoice diff --git a/arch/arm/mach-pxa/gpio.c b/arch/arm/mach-pxa/gpio.c index 5fec1e4..0e466e2 100644 --- a/arch/arm/mach-pxa/gpio.c +++ b/arch/arm/mach-pxa/gpio.c @@ -162,6 +162,67 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) __raw_writel(mask, pxa->regbase + GPCR_OFFSET); } +#ifdef CONFIG_GPIOLIB_BATCH +/* + * Set output GPIO level in batches + */ +static int pxa_gpio_set_batch(struct gpio_chip *chip, unsigned startpin, + u32 mask, int width, u32 values) +{ + struct pxa_gpio_chip *pxa; + + /* we're guaranteed by the caller that startpin + mask remains + * in this chip. + */ + pxa = container_of(chip, struct pxa_gpio_chip, chip); + + /* mask is used twice in shifted form */ + mask <<= startpin; + + values = (values << startpin) & mask; + if (values) + __raw_writel(values, pxa->regbase + GPSR_OFFSET); + + values = ~values & mask; + if (values) + __raw_writel(values, pxa->regbase + GPCR_OFFSET); + + return 0; +} + +/* + * Get output GPIO level in batches + */ +static int pxa_gpio_get_batch(struct gpio_chip *chip, unsigned startpin, + u32 mask, int width, u32 *result) +{ + u32 values; + struct pxa_gpio_chip *pxa; + + /* we're guaranteed by the caller that startpin + mask remains + * in this chip. + */ + pxa = container_of(chip, struct pxa_gpio_chip, chip); + + values = __raw_readl(pxa->regbase + GPLR_OFFSET); + + /* shift the result back into original position */ + *result = (values >> startpin) & mask; + return 0; +} +#endif + +/* this is done this way so that we can insert an ifdef-able value + * into the following GPIO_CHIP macro + */ +#ifdef CONFIG_GPIOLIB_BATCH +#define SET_BATCH_MACRO .set_batch = pxa_gpio_set_batch, +#define GET_BATCH_MACRO .get_batch = pxa_gpio_get_batch, +#else +#define SET_BATCH_MACRO +#define GET_BATCH_MACRO +#endif + #define GPIO_CHIP(_n) \ [_n] = { \ .regbase = GPIO##_n##_BASE, \ @@ -173,6 +234,8 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) .set = pxa_gpio_set, \ .base = (_n) * 32, \ .ngpio = 32, \ + SET_BATCH_MACRO \ + GET_BATCH_MACRO \ }, \ } diff --git a/arch/arm/mach-pxa/include/mach/gpio.h b/arch/arm/mach-pxa/include/mach/gpio.h index 2c538d8..7fa84fe 100644 --- a/arch/arm/mach-pxa/include/mach/gpio.h +++ b/arch/arm/mach-pxa/include/mach/gpio.h @@ -24,6 +24,7 @@ #ifndef __ASM_ARCH_PXA_GPIO_H #define __ASM_ARCH_PXA_GPIO_H +#include <linux/bitops.h> #include <mach/pxa-regs.h> #include <asm/irq.h> #include <mach/hardware.h> @@ -56,6 +57,56 @@ static inline void gpio_set_value(unsigned gpio, int value) } } +#ifdef CONFIG_GPIOLIB_BATCH +static inline int gpio_set_batch(unsigned startpin, u32 mask, u32 values) +{ + int err = 0; + int width = fls(mask); + + if (__builtin_constant_p(startpin) && + (startpin + width < NR_BUILTIN_GPIO) && + ((startpin + width) <= roundup(startpin + 1, 32))) { + int shift; + + shift = startpin % 32; + mask <<= shift; + + values = (values << shift) & mask; + if (values) + GPSR(startpin) = values; + + values = ~values & mask; + if (values) + GPCR(startpin) = values; + } else { + err = __gpio_set_batch(startpin, mask, width, values); + } + return err; +} + +static inline int gpio_get_batch(unsigned startpin, u32 mask, u32 *result) +{ + int ret = 0; + int width = fls(mask); + + if (__builtin_constant_p(startpin) && + (startpin + width < NR_BUILTIN_GPIO) && + ((startpin + width) <= roundup(startpin+1, 32))) { + int shift; + u32 values; + + shift = startpin % 32; + + values = GPLR(startpin); + + *result = (values >> shift) & mask; + } else { + ret = __gpio_get_batch(startpin, mask, width, result); + } + return ret; +} +#endif + #define gpio_cansleep __gpio_cansleep #define gpio_to_irq(gpio) IRQ_GPIO(gpio) -- 1.5.2.3 |
From: Jaya K. <jay...@gm...> - 2009-01-25 09:55:09
|
This patch to am300epd uses the batch gpiolib set/get API in order to significantly improve performance when transferring framebuffer data. Cc: David Brownell <da...@pa...> Cc: Eric Miao <eri...@ma...> Cc: Paulius Zaleckas <pau...@te...> Cc: Geert Uytterhoeven <ge...@li...> Cc: Sam Ravnborg <sa...@ra...> Cc: lin...@li... Cc: lin...@li... Cc: lin...@vg... Cc: lin...@vg... Signed-off-by: Jaya Kumar <jay...@gm...> --- arch/arm/mach-pxa/am300epd.c | 23 ++++++++++++----------- 1 files changed, 12 insertions(+), 11 deletions(-) diff --git a/arch/arm/mach-pxa/am300epd.c b/arch/arm/mach-pxa/am300epd.c index 4bd10a1..237e3be 100644 --- a/arch/arm/mach-pxa/am300epd.c +++ b/arch/arm/mach-pxa/am300epd.c @@ -187,24 +187,25 @@ static void am300_cleanup(struct broadsheetfb_par *par) static u16 am300_get_hdb(struct broadsheetfb_par *par) { - u16 res = 0; - int i; - - for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) - res |= (gpio_get_value(DB0_GPIO_PIN + i)) ? (1 << i) : 0; + int err; + u32 val; - return res; + err = gpio_get_batch(DB0_GPIO_PIN, 0xFFFF, &val); + if (err) { + dev_err(&am300_device->dev, "get failed: %d\n", err); + return 0; + } + return (u16) val; } static void am300_set_hdb(struct broadsheetfb_par *par, u16 data) { - int i; - - for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) - gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01); + int err; + err = gpio_set_batch(DB0_GPIO_PIN, 0xFFFF, data); + if (err) + dev_err(&am300_device->dev, "set failed: %d\n", err); } - static void am300_set_ctl(struct broadsheetfb_par *par, unsigned char bit, u8 state) { -- 1.5.2.3 |
From: Uwe Kleine-K. <u.k...@pe...> - 2009-01-25 11:20:57
|
Hello Jaya, On Sun, Jan 25, 2009 at 05:54:47PM +0800, Jaya Kumar wrote: > - split the patches into generic, arch specific and am300epd I would swap the order to have: generic am300epd pxa specific This way the tree of the second commit is a test case for the generic implementation. > - adjusting the API to remove width (note, the actual API call where > width was dropped is in the arch specific code, not here.) Nevertheless I would document the "generic" per arch specific implementation in gpio.txt. For the functions like __gpio_get_value you can just do #define gpio_get_value(gpio) __gpio_get_value(gpio) but for your batch functions you need something like #define gpio_set_batch(startpin, mask, values) \ ({ u32 __mask = mask; __gpio_set_batch(startpin, __mask, fls(__mask), values);}) Maybe better use/recommend an inline function? > Cc: David Brownell <da...@pa...> > Cc: Eric Miao <eri...@ma...> > Cc: Paulius Zaleckas <pau...@te...> > Cc: Geert Uytterhoeven <ge...@li...> > Cc: Sam Ravnborg <sa...@ra...> > Cc: lin...@li... > Cc: lin...@li... > Cc: lin...@vg... > Cc: lin...@vg... > Signed-off-by: Jaya Kumar <jay...@gm...> Note you didn't Cc: me. -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | |
From: Eric M. <eri...@gm...> - 2008-12-29 03:38:13
|
On Sun, Dec 28, 2008 at 12:24 AM, Jaya Kumar <jay...@gm...> wrote: > Hi friends, > > This is v2 of my attempt to add batch support to gpiolib. Thanks to > David Brownell, Eric Miao, Paulius Zaleckas, Geert, and Sam for prior > feedback. I have tested set_batch using the am300 driver. get_batch > is not tested. > > Please let me know your feedback. > > Thanks, > jaya > > am300epd was doing 800*600*16*gpio_set_value for each framebuffer transfer > (it uses 16-pins of gpio as its data bus). I found this caused a wee > performance limitation. This patch adds an API for gpio_s/get_batch > which allows drivers to s/get batches of gpio together in a single call. > I have done a test implementation on gumstix (pxa255) with am300epd > and it provides a nice improvement in performance. > > Signed-off-by: Jaya Kumar <jay...@gm...> > Cc: David Brownell <dbr...@us...> > Cc: Eric Miao <eri...@ma...> > Cc: Paulius Zaleckas <pau...@te...> > Cc: Geert Uytterhoeven <ge...@li...> > Cc: David Brownell <da...@pa...> > Cc: Sam Ravnborg <sa...@ra...> > Cc: Haavard Skinnemoen <hsk...@at...> > Cc: Philipp Zabel <phi...@gm...> > Cc: Russell King <rm...@ar...> > Cc: Ben Gardner <bga...@wa...> > CC: Greg KH <gr...@kr...> > Cc: lin...@li... > Cc: lin...@li... > Cc: lin...@vg... > --- > arch/arm/mach-pxa/Kconfig | 1 + > arch/arm/mach-pxa/am300epd.c | 9 ++- > arch/arm/mach-pxa/gpio.c | 72 +++++++++++++++++++ > arch/arm/mach-pxa/include/mach/gpio.h | 47 +++++++++++++ > drivers/gpio/Kconfig | 5 ++ > drivers/gpio/gpiolib.c | 122 +++++++++++++++++++++++++++++++++ > include/asm-generic/gpio.h | 15 ++++- > 7 files changed, 269 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig > index 768618b..a5a306e 100644 > --- a/arch/arm/mach-pxa/Kconfig > +++ b/arch/arm/mach-pxa/Kconfig > @@ -41,6 +41,7 @@ config GUMSTIX_AM200EPD > bool "Enable AM200EPD board support" > > config GUMSTIX_AM300EPD > + select GPIOLIB_BATCH > bool "Enable AM300EPD board support" > > endchoice > diff --git a/arch/arm/mach-pxa/am300epd.c b/arch/arm/mach-pxa/am300epd.c > index 4bd10a1..9594cf0 100644 > --- a/arch/arm/mach-pxa/am300epd.c > +++ b/arch/arm/mach-pxa/am300epd.c > @@ -190,9 +190,12 @@ static u16 am300_get_hdb(struct broadsheetfb_par *par) > u16 res = 0; > int i; > > +#ifdef CONFIG_GPIOLIB_BATCH > + res = gpio_get_batch(DB0_GPIO_PIN, 0xFFFF, 16); > +#else > for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) > res |= (gpio_get_value(DB0_GPIO_PIN + i)) ? (1 << i) : 0; > - > +#endif > return res; > } > > @@ -200,8 +203,12 @@ static void am300_set_hdb(struct broadsheetfb_par *par, u16 data) > { > int i; > > +#ifdef CONFIG_GPIOLIB_BATCH > + gpio_set_batch(DB0_GPIO_PIN, data, 0xFFFF, 16); > +#else > for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) > gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01); > +#endif Well, if AM300 selects GPIOLIB_BATCH, I don't think we need the gpio_set_value() stuffs, and get rid of this #ifdef completely. > } > > > diff --git a/arch/arm/mach-pxa/gpio.c b/arch/arm/mach-pxa/gpio.c > index 5fec1e4..d85cf2e 100644 > --- a/arch/arm/mach-pxa/gpio.c > +++ b/arch/arm/mach-pxa/gpio.c > @@ -162,6 +162,76 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) > __raw_writel(mask, pxa->regbase + GPCR_OFFSET); > } > > +#ifdef CONFIG_GPIOLIB_BATCH > +/* > + * Set output GPIO level in batches > + */ > +static void pxa_gpio_set_batch(struct gpio_chip *chip, unsigned offset, > + u32 values, u32 bitmask) > +{ > + struct pxa_gpio_chip *pxa; > + > + /* we're guaranteed by the caller that offset + bitmask remains > + * in this chip. > + */ > + pxa = container_of(chip, struct pxa_gpio_chip, chip); > + > + /* shift the bits into our register specific position */ > + values <<= offset; > + bitmask <<= offset; > + > + values &= bitmask; or a single 'values = (values & bitmask) << offset' ? > + if (values) > + __raw_writel(values, pxa->regbase + GPSR_OFFSET); > + > + values = ~values; > + values &= bitmask; ditto > + if (values) > + __raw_writel(values, pxa->regbase + GPCR_OFFSET); > +} > + > +/* > + * Get output GPIO level in batches > + */ > +static u32 pxa_gpio_get_batch(struct gpio_chip *chip, unsigned offset, > + u32 bitmask) > +{ > + u32 values; > + struct pxa_gpio_chip *pxa; > + > + /* we're guaranteed by the caller that offset + bitmask remains > + * in this chip. > + */ > + pxa = container_of(chip, struct pxa_gpio_chip, chip); > + > + values = __raw_readl(pxa->regbase + GPLR_OFFSET); > + > + /* shift the result back into original position */ > + values >>= offset; > + /* no need to shift bitmask since we've already shifted values */ > + values &= bitmask; > + > + return values; or a single 'return (values >> offset) & bitmask;' should be enough. > +} > +#endif > + > +#ifdef CONFIG_GPIOLIB_BATCH > +#define GPIO_CHIP(_n) \ > + [_n] = { \ > + .regbase = GPIO##_n##_BASE, \ > + .chip = { \ > + .label = "gpio-" #_n, \ > + .direction_input = pxa_gpio_direction_input, \ > + .direction_output = pxa_gpio_direction_output, \ > + .get = pxa_gpio_get, \ > + .set = pxa_gpio_set, \ > + .base = (_n) * 32, \ > + .ngpio = 32, \ > + .set_batch = pxa_gpio_set_batch, \ > + .get_batch = pxa_gpio_get_batch, \ This is a bit ugly, define pxa_gpio_set_batch to NULL #ifndef GPIOLIB_BATCH in the above code, and force .{set,get}_batch assignment anyway, this will look a bit better, the same way as PM. However, this requires a modification to gpio_chip to always allow these two pointers, which might be a concern. > + }, \ > + } > +#else > #define GPIO_CHIP(_n) \ > [_n] = { \ > .regbase = GPIO##_n##_BASE, \ > @@ -175,6 +245,8 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) > .ngpio = 32, \ > }, \ > } > +#endif > + > > static struct pxa_gpio_chip pxa_gpio_chip[] = { > GPIO_CHIP(0), > diff --git a/arch/arm/mach-pxa/include/mach/gpio.h b/arch/arm/mach-pxa/include/mach/gpio.h > index 2c538d8..e66e752 100644 > --- a/arch/arm/mach-pxa/include/mach/gpio.h > +++ b/arch/arm/mach-pxa/include/mach/gpio.h > @@ -56,6 +56,53 @@ static inline void gpio_set_value(unsigned gpio, int value) > } > } > > +#ifdef CONFIG_GPIOLIB_BATCH > +static inline void gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, > + int bitwidth) > +{ > + if (__builtin_constant_p(gpio) && > + (gpio + bitwidth < NR_BUILTIN_GPIO) && > + ((gpio + bitwidth) <= roundup(gpio+1, 32))) { > + int shift; > + > + shift = gpio % 32; > + values <<= shift; > + bitmask <<= shift; > + > + values &= bitmask; > + if (values) > + GPSR(gpio) = values; > + > + values = ~values; > + values &= bitmask; > + if (values) > + GPCR(gpio) = values; I doubt the optimization of GPIO being a constant is essential for batch operation ... anyway, this is plus. > + } else { > + __gpio_set_batch(gpio, values, bitmask, bitwidth); > + } > +} > + > +static inline u32 gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth) > +{ > + if (__builtin_constant_p(gpio) && > + (gpio + bitwidth < NR_BUILTIN_GPIO) && > + ((gpio + bitwidth) <= roundup(gpio+1, 32))) { > + int shift; > + u32 values; > + > + shift = gpio % 32; > + > + values = GPLR(gpio); > + values >>= shift; > + values &= bitmask; > + > + return values; > + } else { > + return __gpio_get_batch(gpio, bitmask, bitwidth); > + } > +} > +#endif > + > #define gpio_cansleep __gpio_cansleep > > #define gpio_to_irq(gpio) IRQ_GPIO(gpio) > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 48f49d9..aeeb83c 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -37,6 +37,11 @@ menuconfig GPIOLIB > > if GPIOLIB > > +config GPIOLIB_BATCH > + bool "Batch GPIO support" > + help > + Say Y here to add the capability to batch set/get GPIOs. > + > config DEBUG_GPIO > bool "Debug GPIO calls" > depends on DEBUG_KERNEL > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 82020ab..7b36573 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1056,6 +1056,128 @@ void __gpio_set_value(unsigned gpio, int value) > } > EXPORT_SYMBOL_GPL(__gpio_set_value); > > +#ifdef CONFIG_GPIOLIB_BATCH > +/** > + * __gpio_set_batch() - assign a batch of gpio pins together > + * @gpio: starting gpio pin > + * @values: values to assign, sequential and including masked bits > + * @bitmask: bitmask to be applied to values > + * @bitwidth: width inclusive of masked-off bits > + * Context: any > + * > + * This is used directly or indirectly to implement gpio_set_value(). > + * It invokes the associated gpio_chip.set_batch() method. If that > + * method does not exist for any segment that is involved, then it drops > + * back down to standard gpio_chip.set() > + */ > +void __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, int bitwidth) > +{ > + struct gpio_chip *chip; > + int i = 0; > + int value, width, remwidth; > + u32 mask; > + > + do { > + chip = gpio_to_chip(gpio + i); > + WARN_ON(extra_checks && chip->can_sleep); > + > + if (!chip->set_batch) { > + while (((gpio + i) < (chip->base + chip->ngpio)) > + && bitwidth) { > + mask = 1 << i; > + value = values & mask; > + if (bitmask & mask) > + chip->set(chip, gpio + i - chip->base, > + value); > + i++; > + bitwidth--; I recommend this being put into something like 'default_gpio_set_batch', and assign this to 'chip->set_batch' when the gpio chip is being registered and found 'chip->set_batch == NULL', so to keep this block consistent. Same comment to the 'get_batch' implementation below. > + } > + } else { > + value = values >> i; /* shift off the used data bits */ > + remwidth = ((chip->base + (int) chip->ngpio) - > + ((int) gpio + i)); > + width = min(bitwidth, remwidth); > + > + /* shift off the used mask bits */ > + mask = bitmask >> i; > + /* now adjust mask by width of set */ > + mask &= ((1 << width) - 1); > + > + chip->set_batch(chip, gpio + i - chip->base, value, > + mask); > + i += width; > + bitwidth -= width; > + } > + } while (bitwidth); > +} > +EXPORT_SYMBOL_GPL(__gpio_set_batch); > + > +/** > + * __gpio_get_batch() - get a batch of gpio pins together > + * @gpio: starting gpio pin > + * @bitmask: bitmask to be applied to values > + * @bitwidth: width inclusive of masked-off bits > + * Context: any > + * > + * This is used directly or indirectly to implement gpio_get_value(). > + * It invokes the associated gpio_chip.get_batch() method and returns > + * zero if no such method is provided. > + */ > +u32 __gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth) > +{ > + struct gpio_chip *chip; > + int i = 0; > + int width, remwidth; > + u32 mask; > + u32 values = 0; > + u32 value; > + > + do { > + chip = gpio_to_chip(gpio + i); > + WARN_ON(extra_checks && chip->can_sleep); > + > + /* > + * check if we have batch capability on this chip. > + * if not, then we use the single bit gets > + * any masked or inaccessible bits are already 0 > + */ > + if (!chip->get_batch) { > + while (((gpio + i) < (chip->base + chip->ngpio)) > + && bitwidth) { > + mask = 1 << i; > + /* skip if no get or masked out */ > + if (chip->get && (bitmask & mask)) { > + if (chip->get(chip, > + gpio + i - chip->base)) > + values |= mask; > + } > + i++; > + bitwidth--; > + } > + } else { > + remwidth = ((chip->base + (int) chip->ngpio) - > + ((int) gpio + i)); > + width = min(bitwidth, remwidth); > + > + /* shift off the used mask bits */ > + mask = bitmask >> i; > + /* now adjust mask by width of get */ > + mask &= ((1 << width) - 1); > + > + value = chip->get_batch(chip, gpio + i - chip->base, > + mask); > + /* shift result back into caller position */ > + values |= value << i; > + i += width; > + bitwidth -= width; > + } > + } while (bitwidth); > + > + return values; > +} > +EXPORT_SYMBOL_GPL(__gpio_get_batch); > +#endif > + > /** > * __gpio_cansleep() - report whether gpio value access will sleep > * @gpio: gpio in question > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h > index 81797ec..fa716a7 100644 > --- a/include/asm-generic/gpio.h > +++ b/include/asm-generic/gpio.h > @@ -44,6 +44,8 @@ struct module; > * returns either the value actually sensed, or zero > * @direction_output: configures signal "offset" as output, or returns error > * @set: assigns output value for signal "offset" > + * @set_batch: batch assigns output values for consecutive signals starting at > + * "offset" with mask in "bitmask" all within this gpio_chip > * @to_irq: optional hook supporting non-static gpio_to_irq() mappings; > * implementation may not sleep > * @dbg_show: optional routine to show contents in debugfs; default code > @@ -84,7 +86,13 @@ struct gpio_chip { > unsigned offset, int value); > void (*set)(struct gpio_chip *chip, > unsigned offset, int value); > - > +#ifdef CONFIG_GPIOLIB_BATCH > + void (*set_batch)(struct gpio_chip *chip, > + unsigned offset, u32 values, > + u32 bitmask); > + u32 (*get_batch)(struct gpio_chip *chip, > + unsigned offset, u32 bitmask); > +#endif > int (*to_irq)(struct gpio_chip *chip, > unsigned offset); > > @@ -124,6 +132,11 @@ extern void gpio_set_value_cansleep(unsigned gpio, int value); > */ > extern int __gpio_get_value(unsigned gpio); > extern void __gpio_set_value(unsigned gpio, int value); > +#ifdef CONFIG_GPIOLIB_BATCH > +extern void __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, > + int bitwidth); > +extern u32 __gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth); > +#endif > > extern int __gpio_cansleep(unsigned gpio); > > -- > 1.5.2.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to maj...@vg... > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- Cheers - eric |
From: Jaya K. <jay...@gm...> - 2008-12-29 04:12:10
|
On Sun, Dec 28, 2008 at 10:38 PM, Eric Miao <eri...@gm...> wrote: > On Sun, Dec 28, 2008 at 12:24 AM, Jaya Kumar <jay...@gm...> wrote: >> +#ifdef CONFIG_GPIOLIB_BATCH >> + gpio_set_batch(DB0_GPIO_PIN, data, 0xFFFF, 16); >> +#else >> for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) >> gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01); >> +#endif > > Well, if AM300 selects GPIOLIB_BATCH, I don't think we need the > gpio_set_value() stuffs, and get rid of this #ifdef completely. Good point. Will do. >> + /* shift the bits into our register specific position */ >> + values <<= offset; >> + bitmask <<= offset; >> + >> + values &= bitmask; > > or a single 'values = (values & bitmask) << offset' ? Yup, good point. Will do. > >> + if (values) >> + __raw_writel(values, pxa->regbase + GPSR_OFFSET); >> + >> + values = ~values; >> + values &= bitmask; > > ditto Will do. > >> + if (values) >> + __raw_writel(values, pxa->regbase + GPCR_OFFSET); >> +} >> + >> +/* >> + * Get output GPIO level in batches >> + */ >> +static u32 pxa_gpio_get_batch(struct gpio_chip *chip, unsigned offset, >> + u32 bitmask) >> +{ >> + u32 values; >> + struct pxa_gpio_chip *pxa; >> + >> + /* we're guaranteed by the caller that offset + bitmask remains >> + * in this chip. >> + */ >> + pxa = container_of(chip, struct pxa_gpio_chip, chip); >> + >> + values = __raw_readl(pxa->regbase + GPLR_OFFSET); >> + >> + /* shift the result back into original position */ >> + values >>= offset; >> + /* no need to shift bitmask since we've already shifted values */ >> + values &= bitmask; >> + >> + return values; > > or a single 'return (values >> offset) & bitmask;' should be enough. Agreed. > >> +} >> +#endif >> + >> +#ifdef CONFIG_GPIOLIB_BATCH >> +#define GPIO_CHIP(_n) \ >> + [_n] = { \ >> + .regbase = GPIO##_n##_BASE, \ >> + .chip = { \ >> + .label = "gpio-" #_n, \ >> + .direction_input = pxa_gpio_direction_input, \ >> + .direction_output = pxa_gpio_direction_output, \ >> + .get = pxa_gpio_get, \ >> + .set = pxa_gpio_set, \ >> + .base = (_n) * 32, \ >> + .ngpio = 32, \ >> + .set_batch = pxa_gpio_set_batch, \ >> + .get_batch = pxa_gpio_get_batch, \ > > This is a bit ugly, define pxa_gpio_set_batch to NULL #ifndef GPIOLIB_BATCH > in the above code, and force .{set,get}_batch assignment anyway, this will > look a bit better, the same way as PM. However, this requires a modification > to gpio_chip to always allow these two pointers, which might be a concern. I think I tried that but then encountered the problem that I can't put ifdefs within the define GPIO_CHIP macro. Will try to find a different way. How about if I do: #ifdef GPIOLIB_BATCH #define SET_BATCH_MACRO .set_batch = pxa_gpio_set_batch \ #else #define SET_BATCH_MACRO #endif then leave SET_BATCH_MACRO in the GPIO_CHIP macro. I think that would work. >> + if (!chip->set_batch) { >> + while (((gpio + i) < (chip->base + chip->ngpio)) >> + && bitwidth) { >> + mask = 1 << i; >> + value = values & mask; >> + if (bitmask & mask) >> + chip->set(chip, gpio + i - chip->base, >> + value); >> + i++; >> + bitwidth--; > > I recommend this being put into something like 'default_gpio_set_batch', and > assign this to 'chip->set_batch' when the gpio chip is being registered and > found 'chip->set_batch == NULL', so to keep this block consistent. > > Same comment to the 'get_batch' implementation below. Ok, that should also make the code nicer, will do. Thanks, jaya |
From: David B. <da...@pa...> - 2008-12-29 20:34:34
|
I'm a bit surprised to see patches against 2.6.27, rather than a 2.6.28 (or 2.6.28-rc) kernel. ;) On Sunday 28 December 2008, Eric Miao wrote: > > @@ -200,8 +203,12 @@ static void am300_set_hdb(struct broadsheetfb_par *par, u16 data) > > { > > int i; > > > > +#ifdef CONFIG_GPIOLIB_BATCH > > + gpio_set_batch(DB0_GPIO_PIN, data, 0xFFFF, 16); > > +#else > > for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++) > > gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01); > > +#endif > > Well, if AM300 selects GPIOLIB_BATCH, I don't think we need the > gpio_set_value() stuffs, and get rid of this #ifdef completely. Right ... although we don't *have* a GPIOLIB_BATCH, so that's not (yet?) an option. > > @@ -1056,6 +1056,128 @@ void __gpio_set_value(unsigned gpio, int value) > > } > > EXPORT_SYMBOL_GPL(__gpio_set_value); > > > > +#ifdef CONFIG_GPIOLIB_BATCH > > +/** > > + * __gpio_set_batch() - assign a batch of gpio pins together > > + * @gpio: starting gpio pin > > + * @values: values to assign, sequential and including masked bits > > + * @bitmask: bitmask to be applied to values > > + * @bitwidth: width inclusive of masked-off bits > > + * Context: any > > + * > > + * This is used directly or indirectly to implement gpio_set_value(). > > + * It invokes the associated gpio_chip.set_batch() method. If that > > + * method does not exist for any segment that is involved, then it drops > > + * back down to standard gpio_chip.set() > > + */ > > +void __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, int bitwidth) > > +{ > > + struct gpio_chip *chip; > > + int i = 0; > > + int value, width, remwidth; > > + u32 mask; > > + > > + do { > > + chip = gpio_to_chip(gpio + i); > > + WARN_ON(extra_checks && chip->can_sleep); > > + > > + if (!chip->set_batch) { > > + while (((gpio + i) < (chip->base + chip->ngpio)) > > + && bitwidth) { > > + mask = 1 << i; > > + value = values & mask; > > + if (bitmask & mask) > > + chip->set(chip, gpio + i - chip->base, > > + value); > > + i++; > > + bitwidth--; > > I recommend this being put into something like 'default_gpio_set_batch', and > assign this to 'chip->set_batch' when the gpio chip is being registered and > found 'chip->set_batch == NULL', so to keep this block consistent. > > Same comment to the 'get_batch' implementation below. Right ... this is something I had suggested earlier: make sure that the (renamed) "batch" interfaces don't depend on some TBD extension to gpio_chip. Those extensions should be just an optimization. |