Re: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMA problem
Hi, I have tested the new driver using DMA mode on our custom DM355 board. I replaced the linux 2.6.32 spi driver by this driver and it seemed to work except for on point. The dm355 spi polling mode was correctly working but the DMA mode was causing some communication problems. It appeared that when dm355 was reading on the spi, it was actually resending received data to our slave device on the spi bus, i didn't fully checked what i am saying i am just supposing that it's what occured. Anyway the behaviour was different in DMA and polling mode. I didn't dig deep into the driver i just solved the problem (quick and dirty) by setting tx buffer to zero in the spidev driver. The problem can be caused by using SPIDAT as dma buffer when the tx buffer is empty. (If it helps i have tested before the spi patch V5 from Brian Niebuhr and the problem was already there) So is this a bug or a feature ? I don't have time right know to work on that problem but i can test solutions if anyone have. Regards. Pierre ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMA problem
Hi Pierre, On Tue, Nov 23, 2010 at 16:43:27, Pierre wrote: Hi, I have tested the new driver using DMA mode on our custom DM355 board. I replaced the linux 2.6.32 spi driver by this driver and it seemed to work except for on point. The dm355 spi polling mode was correctly working but the DMA mode was causing some communication problems. It appeared that when dm355 was reading on the spi, it was actually resending received data to our slave device on the spi bus, i didn't Hmm, didn't really see this before. fully checked what i am saying i am just supposing that it's what occured. Anyway the behaviour was different in DMA and polling mode. I While working on the patches I noticed that DM355 has an errata regarding behavior of CSHOLD. In the patches, I take care of it by having DM355 define cshold_bug to true. Can you please make sure you have taken care of this as well while porting to 2.6.32? http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2010-November/021045.html didn't dig deep into the driver i just solved the problem (quick and dirty) by setting tx buffer to zero in the spidev driver. The problem can be caused by using SPIDAT as dma buffer when the tx buffer is empty. (If it helps i have tested before the spi patch V5 from Brian Niebuhr and the problem was already there) How about with Brian's patch applied? Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH v2 2/4] da850-evm: add UI Expander pushbuttons
Hi Ben, On Mon, Nov 22, 2010 at 19:20:48, Ben Gardiner wrote: On Mon, Nov 22, 2010 at 6:49 AM, Nori, Sekhar nsek...@ti.com wrote: On Fri, Nov 19, 2010 at 21:08:10, Ben Gardiner wrote: [...] By setting INPUT_POLLDEV default for the da850-evm users will get functioning pushbuttons and switches with the default config but they will be able to disable INPUT_POLLDEV or gpio-keys drivers in their defconfig at their convenience. I guess we could also just modify the defconfig to switch on INPUT_POLLDEV? Since only gpio-keys functionality is affected by not enabling the correct options in kernel, it should be OK. Yes -- only gpio-keys is affected but enabling the option does introduce an additional .o file: drivers/input/Makefile:obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o I agree that in its current state a user couls inadvertently disable the gpio-keys support on da850-evm simply by disabling INPUT_POLLDEV -- which is less than Ideal. How about I replace the current changes to arch/arm/mach-davinci/Kconfig with: config KEYBOARD_GPIO default MACH_DAVINCI_DA850_EVM select INPUT_POLLDEV So 1) gpio-keys functionality is default for the da850evm and 2) whenever gpio-keys is enabled so is INPUT_POLLDEV. This looks better than what was posted earlier, but I am not sure if platforms should influence driver configuration this way. I guess I am just afraid that this makes a precedent for many driver config symbols to get replicated in the platform Kconfig files. Lets see if others have an opinion on this. Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH v2 4/4] da850-evm: add baseboard UI expander buttons, switches and LEDs
Hi Ben, On Mon, Nov 22, 2010 at 19:45:46, Ben Gardiner wrote: Hi Sekhar, On Mon, Nov 22, 2010 at 7:00 AM, Nori, Sekhar nsek...@ti.com wrote: Thanks for the explanation. I should have probably asked earlier, why do we need to prevent sysfs access of deep sleep enable and sw reset pins? We don't seem to be using them in the kernel either. You're welcome. I was assuming that those pins were not exported as gpio pins on purpose; I was taking the prudent approach to prevent haphazard toggling of sw_rst and deep_sleep_en from userspace. sw_rst because it could initiate a reset of the cpu when toggled and deep_sleep_en because it can override the behaviour of davinci_pm_enter(). I'm not sure how they would be used by existing kernel classes either. The sw_rst pin could be used for reset but since it is on the other end of an i2c bus and there is an existing implementation of reset using the on chip watchdog I don't think it would be benficial to switch. Deep_sleep_en could override the behaviour in davinci_pm_enter() -- _maybe_ (I don't really know) it could be used as a hardware-assisted suspend-blocker? But I totally guessing here. My preference would be to leave these pins as is (don't call a gpio_request() on them) till someone comes up with a use case for them. From what you described, sysfs access cannot happen accidently so someone accessing these pins from sysfs surely knows what he is doing. Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/4] da850-evm: add UI Expander pushbuttons
Hi Sekhar, On Tue, Nov 23, 2010 at 7:38 AM, Nori, Sekhar nsek...@ti.com wrote: On Mon, Nov 22, 2010 at 19:20:48, Ben Gardiner wrote: [...] config KEYBOARD_GPIO default MACH_DAVINCI_DA850_EVM select INPUT_POLLDEV So 1) gpio-keys functionality is default for the da850evm and 2) whenever gpio-keys is enabled so is INPUT_POLLDEV. This looks better than what was posted earlier, but I am not sure if platforms should influence driver configuration this way. I guess I am just afraid that this makes a precedent for many driver config symbols to get replicated in the platform Kconfig files. Ok . I understand your concerns. Lets see if others have an opinion on this. Yes, good idea. I would welcome more opinions on this and any other aspects of the patch series. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 4/4] da850-evm: add baseboard UI expander buttons, switches and LEDs
Hi Sekhar, On Tue, Nov 23, 2010 at 7:42 AM, Nori, Sekhar nsek...@ti.com wrote: On Mon, Nov 22, 2010 at 19:45:46, Ben Gardiner wrote: [...] I was assuming that those pins were not exported as gpio pins on purpose; I was taking the prudent approach to prevent haphazard toggling of sw_rst and deep_sleep_en from userspace. sw_rst because it could initiate a reset of the cpu when toggled and deep_sleep_en because it can override the behaviour of davinci_pm_enter(). I'm not sure how they would be used by existing kernel classes either. The sw_rst pin could be used for reset but since it is on the other end of an i2c bus and there is an existing implementation of reset using the on chip watchdog I don't think it would be benficial to switch. Deep_sleep_en could override the behaviour in davinci_pm_enter() -- _maybe_ (I don't really know) it could be used as a hardware-assisted suspend-blocker? But I totally guessing here. My preference would be to leave these pins as is (don't call a gpio_request() on them) till someone comes up with a use case for them. From what you described, sysfs access cannot happen accidently so someone accessing these pins from sysfs surely knows what he is doing. No problem. I will re-spin the patch shortly with the deep_sleep_en and sw_rst pins free for use by client code. Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMA problem
I took the tree version of the davinci_spi.c file after every patches have been applied ( the 49 spi patches ). I tested with the csbug on and off before and if i remember well, it was not working. I'll try again as soon as i have time. The weird thing is that the problem doesn't occur at each transfer. The details of what i noticed is: -Chipselect to the client is a GPIO, chip_sel array is a 1 byte array containing the gpio pin. -The DM355 application uses spidev char driver to read and write data to spidev0.0 -When dm355 was reading a 16 bytes buffer, the client sent the data correctly but afterwards it received a byte : the 15th byte of the sent buffer. -When I added to spidev_sync_read a tx zeroed buffer everything worked perfectly. (-The problem was there even if cshold_bug was true, but i'll check that again) -The spi was correctly working in polling mode. Pierre Le 11/23/2010 01:32 PM, Nori, Sekhar a écrit : Hi Pierre, On Tue, Nov 23, 2010 at 16:43:27, Pierre wrote: Hi, I have tested the new driver using DMA mode on our custom DM355 board. I replaced the linux 2.6.32 spi driver by this driver and it seemed to work except for on point. The dm355 spi polling mode was correctly working but the DMA mode was causing some communication problems. It appeared that when dm355 was reading on the spi, it was actually resending received data to our slave device on the spi bus, i didn't Hmm, didn't really see this before. fully checked what i am saying i am just supposing that it's what occured. Anyway the behaviour was different in DMA and polling mode. I While working on the patches I noticed that DM355 has an errata regarding behavior of CSHOLD. In the patches, I take care of it by having DM355 define cshold_bug to true. Can you please make sure you have taken care of this as well while porting to 2.6.32? http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2010-November/021045.html didn't dig deep into the driver i just solved the problem (quick and dirty) by setting tx buffer to zero in the spidev driver. The problem can be caused by using SPIDAT as dma buffer when the tx buffer is empty. (If it helps i have tested before the spi patch V5 from Brian Niebuhr and the problem was already there) How about with Brian's patch applied? Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMAproblem
-Original Message- From: davinci-linux-open-source-boun...@linux.davincidsp.com [mailto:davinci-linux-open-source-boun...@linux.davincidsp.com] On Behalf Of Pierre Sent: Tuesday, November 23, 2010 5:13 AM To: davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMAproblem Hi, I have tested the new driver using DMA mode on our custom DM355 board. I replaced the linux 2.6.32 spi driver by this driver and it seemed to work except for on point. The dm355 spi polling mode was correctly working but the DMA mode was causing some communication problems. It appeared that when dm355 was reading on the spi, it was actually resending received data to our slave device on the spi bus, i didn't fully checked what i am saying i am just supposing that it's what occured. If I understand you correctly, what you are saying is that when you are trying to read data from your SPI device, the data on the DM355 output signal (MOSI) is not what you expected, and is causing issues. If that is the case, I can say that I made no effort to make sure that the inactive portion of a half-duplex SPI transfer had any particular data pattern. In other words, if you have been getting a certain pattern of outbound data it is purely coincidence, and the different implementations of polled and DMA mode expose the fact that you had been getting lucky. I am not a SPI expert by any means, but it has been my approach that SPI is a full-duplex protocol and you need to specify outbound data if you want certain outbound data sent. As it turns out, most SPI devices have a protocol for determining when they are supposed to be sending or receiving, and if the device is sending it usually ignores whatever it is receiving. You mention in your later email that using a zeroed Tx buffer makes everything work OK. I think that's actually the most correct solution. The driver could be modified to ensure that it transmits zeros on a half-duplex read, and maybe it should, but I think the best solution is for the application to specify what it wants to be sent and not rely on non-portable behavior from a driver. If I'm misunderstanding the problem, then just ignore all of the above :-) Anyway the behaviour was different in DMA and polling mode. I didn't dig deep into the driver i just solved the problem (quick and dirty) by setting tx buffer to zero in the spidev driver. The problem can be caused by using SPIDAT as dma buffer when the tx buffer is empty. (If it helps i have tested before the spi patch V5 from Brian Niebuhr and the problem was already there) So is this a bug or a feature ? I don't have time right know to work on that problem but i can test solutions if anyone have. Regards. Pierre ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] da850-evm, trivial: use da850_evm prefix for consistency
Nori, Sekhar nsek...@ti.com writes: On Sat, Nov 20, 2010 at 03:13:04, Ben Gardiner wrote: There was a single case of 'da850evm' prefix in the board-da850-evm.c file where the reset of the prefixes were 'da850_evm'; change it to 'da850_evm' for consistency. Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca --- @Sekhar, you asked me to prefix all the static symbols I added to board-da850-evm.c with 'da850evm' -- but I noticed that the current convention is a prefix of 'da850_evm' so I decided to stick with the convention and replace the only outlier with this patch. Thanks. I personally prefer da850evm, but consistency is more important so that a search-replace is possible later on. So I am okay with this too. I'll take that as an Ack. Applying, queuing for 2.6.38. Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMAproblem
You are correctly understanding my problem. Ok, So spidev should specify the out buffer in case of a read or the application should specify the out buffer. By the way, thanks for this new driver that is fully satisfying. Regards Le 11/23/2010 03:47 PM, Brian Niebuhr a écrit : -Original Message- From: davinci-linux-open-source-boun...@linux.davincidsp.com [mailto:davinci-linux-open-source-boun...@linux.davincidsp.com] On Behalf Of Pierre Sent: Tuesday, November 23, 2010 5:13 AM To: davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMAproblem Hi, I have tested the new driver using DMA mode on our custom DM355 board. I replaced the linux 2.6.32 spi driver by this driver and it seemed to work except for on point. The dm355 spi polling mode was correctly working but the DMA mode was causing some communication problems. It appeared that when dm355 was reading on the spi, it was actually resending received data to our slave device on the spi bus, i didn't fully checked what i am saying i am just supposing that it's what occured. If I understand you correctly, what you are saying is that when you are trying to read data from your SPI device, the data on the DM355 output signal (MOSI) is not what you expected, and is causing issues. If that is the case, I can say that I made no effort to make sure that the inactive portion of a half-duplex SPI transfer had any particular data pattern. In other words, if you have been getting a certain pattern of outbound data it is purely coincidence, and the different implementations of polled and DMA mode expose the fact that you had been getting lucky. I am not a SPI expert by any means, but it has been my approach that SPI is a full-duplex protocol and you need to specify outbound data if you want certain outbound data sent. As it turns out, most SPI devices have a protocol for determining when they are supposed to be sending or receiving, and if the device is sending it usually ignores whatever it is receiving. You mention in your later email that using a zeroed Tx buffer makes everything work OK. I think that's actually the most correct solution. The driver could be modified to ensure that it transmits zeros on a half-duplex read, and maybe it should, but I think the best solution is for the application to specify what it wants to be sent and not rely on non-portable behavior from a driver. If I'm misunderstanding the problem, then just ignore all of the above :-) Anyway the behaviour was different in DMA and polling mode. I didn't dig deep into the driver i just solved the problem (quick and dirty) by setting tx buffer to zero in the spidev driver. The problem can be caused by using SPIDAT as dma buffer when the tx buffer is empty. (If it helps i have tested before the spi patch V5 from Brian Niebuhr and the problem was already there) So is this a bug or a feature ? I don't have time right know to work on that problem but i can test solutions if anyone have. Regards. Pierre ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/4] da850-evm: add UI Expander pushbuttons
Nori, Sekhar nsek...@ti.com writes: Hi Ben, On Mon, Nov 22, 2010 at 19:20:48, Ben Gardiner wrote: On Mon, Nov 22, 2010 at 6:49 AM, Nori, Sekhar nsek...@ti.com wrote: On Fri, Nov 19, 2010 at 21:08:10, Ben Gardiner wrote: [...] By setting INPUT_POLLDEV default for the da850-evm users will get functioning pushbuttons and switches with the default config but they will be able to disable INPUT_POLLDEV or gpio-keys drivers in their defconfig at their convenience. I guess we could also just modify the defconfig to switch on INPUT_POLLDEV? Since only gpio-keys functionality is affected by not enabling the correct options in kernel, it should be OK. Yes -- only gpio-keys is affected but enabling the option does introduce an additional .o file: drivers/input/Makefile:obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o I agree that in its current state a user couls inadvertently disable the gpio-keys support on da850-evm simply by disabling INPUT_POLLDEV -- which is less than Ideal. How about I replace the current changes to arch/arm/mach-davinci/Kconfig with: config KEYBOARD_GPIO default MACH_DAVINCI_DA850_EVM select INPUT_POLLDEV So 1) gpio-keys functionality is default for the da850evm and 2) whenever gpio-keys is enabled so is INPUT_POLLDEV. This looks better than what was posted earlier, but I am not sure if platforms should influence driver configuration this way. Agreed. In general, we should not have machine/platform specific conditionals in generic Kconfigs. Generally, this should be handled in machine/platform specific Kconfigs. Kevin I guess I am just afraid that this makes a precedent for many driver config symbols to get replicated in the platform Kconfig files. Lets see if others have an opinion on this. Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/4] da850-evm: add UI Expander pushbuttons
Hi Kevin, Thank you for weighing in. On Tue, Nov 23, 2010 at 10:48 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Nori, Sekhar nsek...@ti.com writes: [...] -- which is less than Ideal. How about I replace the current changes to arch/arm/mach-davinci/Kconfig with: config KEYBOARD_GPIO default MACH_DAVINCI_DA850_EVM select INPUT_POLLDEV So 1) gpio-keys functionality is default for the da850evm and 2) whenever gpio-keys is enabled so is INPUT_POLLDEV. This looks better than what was posted earlier, but I am not sure if platforms should influence driver configuration this way. Agreed. In general, we should not have machine/platform specific conditionals in generic Kconfigs. Generally, this should be handled in machine/platform specific Kconfigs. My understanding is that Sekhar was expressing concern over putting generic config conditionals into machine/platform specific Kconfigs. It sounds like you are OK with generic config conditionals in machine/platform specific Kconfigs ala recent pca953x module build changes [1]. To be clear: I am proposing the following additions to arch/arm/mach-davinci/Kconfig: (slightly different than above). config KEYBOARD_GPIO default MACH_DAVINCI_DA850_EVM select INPUT_POLLDEV if MACH_DAVINCI_DA850_EVM I know it is always better to show the code: I will extract these contentious Kconfig changes into their own patch in the series when I post a new version. Best Regards, Ben Gardiner [1] http://article.gmane.org/gmane.linux.davinci/20918 --- Nanometrics Inc. http://www.nanometrics.ca ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v4 1/5] input: gpio_keys: polling mode support
From: Alexander Clouter a...@digriz.org.uk This implements an optional polling mode for the gpio_keys driver, necessary for GPIOs that are not able to generate IRQs. gpio_keys_platform_data has been extended with poll_interval which specifies the polling interval in ms, this is passed onto input-polldev. This work is a rebase of the patch by Alex Clouter [1] which was based on the patch [2] originally conceived by Paul Mundt. Signed-off-by: Paul Mundt let...@linux-sh.org Signed-off-by: Alexander Clouter a...@digriz.org.uk Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca Reviewed-by: Chris Cordahi christophercord...@nanometrics.ca CC: Paul Mundt let...@linux-sh.org [1] http://article.gmane.org/gmane.linux.kernel.input/13919 [2] http://article.gmane.org/gmane.linux.kernel.input/5814 --- Changes since v3: * no changes to this patch in the series Changes since v2: * rebased to 083eae3e28643e0eefc5243719f8b1572cf98299 of git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git Changes since v1: * use locally defined functions that are no-ops/error checkers when INPUT_POLLDEV is not defined. * disable polling mode support when input-polldev is a module and gpio_keys is builtin Changes since [1]: * rebased to 0b1c3ef1072f2b97c86351d3736d2b2d00293a11 of git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git * use _cansleep variant of gpio_get_value in the polling task to avoid WARN_ON when using I2C GPIO expanders * prevent unitialized access to 'input' in gpio_keys_close() Changes since [2]: * absolute dependency on INPUT_POLLDEV removed Tested with CONFIG_INPUT_POLLDEV={n,m,y} (gpio_keys as module for 'm'). --- drivers/input/keyboard/gpio_keys.c | 120 ++-- include/linux/gpio_keys.h |1 + 2 files changed, 103 insertions(+), 18 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 6069abe..d2f23d9 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -1,7 +1,9 @@ /* - * Driver for keys on GPIO lines capable of generating interrupts. + * Driver for keys on GPIO lines, either IRQ-driven or polled. * * Copyright 2005 Phil Blundell + * Copyright 2008 Paul Mundt + * Copyright 2010 Alexander Clouter a...@digriz.org.uk * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -25,6 +27,7 @@ #include linux/gpio_keys.h #include linux/workqueue.h #include linux/gpio.h +#include linux/input-polldev.h struct gpio_button_data { struct gpio_keys_button *button; @@ -37,6 +40,7 @@ struct gpio_button_data { struct gpio_keys_drvdata { struct input_dev *input; + struct input_polled_dev *poll_dev; struct mutex disable_lock; unsigned int n_buttons; int (*enable)(struct device *dev); @@ -44,6 +48,30 @@ struct gpio_keys_drvdata { struct gpio_button_data data[0]; }; +#if (!defined(CONFIG_INPUT_POLLDEV) !defined(CONFIG_INPUT_POLLDEV_MODULE)) \ + || (defined(CONFIG_INPUT_POLLDEV_MODULE) \ +!defined(CONFIG_KEYBOARD_GPIO_MODULE)) + +static inline struct input_polled_dev *allocate_polled_device( + const struct device *dev) +{ + dev_err(dev, device needs polling (enable INPUT_POLLDEV)\n); + return NULL; +} + +#define free_polled_device(x) do { } while (0) +#define register_polled_device(x) (-ENXIO) +#define unregister_polled_device(x)do { } while (0) + +#else + +#define allocate_polled_device(x) input_allocate_polled_device() +#define free_polled_device(x) input_free_polled_device(x) +#define register_polled_device(x) input_register_polled_device(x) +#define unregister_polled_device(x)input_unregister_polled_device(x) + +#endif + /* * SYSFS interface for enabling/disabling keys and switches: * @@ -322,7 +350,8 @@ static void gpio_keys_report_event(struct gpio_button_data *bdata) struct gpio_keys_button *button = bdata-button; struct input_dev *input = bdata-input; unsigned int type = button-type ?: EV_KEY; - int state = (gpio_get_value(button-gpio) ? 1 : 0) ^ button-active_low; + int state = (gpio_get_value_cansleep(button-gpio) ? 1 : 0) + ^ button-active_low; input_event(input, type, button-code, !!state); input_sync(input); @@ -343,6 +372,16 @@ static void gpio_keys_timer(unsigned long _data) schedule_work(data-work); } +static void gpio_handle_button_event(struct gpio_keys_button *button, +struct gpio_button_data *bdata) +{ + if (bdata-timer_debounce) + mod_timer(bdata-timer, + jiffies + msecs_to_jiffies(bdata-timer_debounce)); + else + gpio_keys_report_event(bdata); +} +
[PATCH v4 0/5] da850-evm: add gpio-{keys, leds} for UI and BB expanders
The da850-evm baseboard (BB) and its UI board both have tca6416 IO expanders. They are bootstrapped to different I2C addresses so they can be used concurrently. The expander on the UI board is currently used to enable/disable the peripherals that are available on the UI board. In addition to this functionality the expander is also connected to 8 pushbuttons. The expander on the baseboard is not currently used; it is connected to deep sleep enable, sw reset, a push button, some switches and LEDs. This proposed patch series enables the push buttons and switches on the UI and BB expanders using the gpio-keys polling mode patch by Alexander Clouter. Some work was performed to test irq-based gpio-keys support on the expanders (a WIP patch can be posted on request) but I believe that it is not possible to use irq-based gpio-keys on IO expanders for arm systems at this time. The attempt started when I noticed the patch of Alek Du and Alan Cox [1] which was recently committed [2]; a stab at integrating irq-based gpio-keys support based on that patch was attempted. I found that I either got a warning that the irq could not be mapped for the given gpio ; or, when N_IRQ was increased, a system freeze. From what I have read (particularly the message by Grant Likely [3]) IRQs on IO expanders are not ready in ARM yet. I _think_ that the sparse IRQ rework by Thomas Gleixner [4] will resolve the blocker to irq-based gpio-keys support. In the meantime we have buttons and switches that we would like to excersise in our prototyping development. The patch to convert this series to irq-based gpio-keys will be straighforward once the support in arch/arm is there. There is an existing tca6416-keypad driver with polling support which I did not employ because it isn't possible to keep the gpio's used for peripheral enable/disable on the UI board or the LEDs on the baseboard registered while simultaneously registering the pushbuttons or switches as a tca6416-keypad instance. I tested this patch series using evtest on the resulting /dev/input/eventN devices and also on the event node of a non-polling gpio-keys instance to ensure that irq-based input handling is not broken by the introduction of the polling-mode gpio-keys patch. The non-polling instance creation and registration is not included in this series since it uses one of the boot-mode DIP switches and woult not (I think) be suitable for mainline. Disclaimer: I'm not an expert in irq's or gpio-keys; this is, in fact, my first proposed feature. Please feel free to correct me -- I welcome the chance to learn from your expertise. [1] http://article.gmane.org/gmane.linux.kernel/1052551 [2] http://article.gmane.org/gmane.linux.kernel.commits.head/260919 [3] http://www.mail-archive.com/devicetree-disc...@lists.ozlabs.org/msg01974.html [4] http://article.gmane.org/gmane.linux.kernel.cross-arch/7786 Alexander Clouter (1): input: gpio_keys: polling mode support Ben Gardiner (4): da850-evm: add UI Expander pushbuttons da850-evm: extract defines for SEL{A,B,C} pins in UI expander da850-evm: add baseboard GPIO expander buttons, switches and LEDs da850-evm: KEYBOARD_GPIO and INPUT_POLLDEV Kconfig conditionals arch/arm/mach-davinci/Kconfig |4 + arch/arm/mach-davinci/board-da850-evm.c | 312 +-- drivers/input/keyboard/gpio_keys.c | 120 ++-- include/linux/gpio_keys.h |1 + 4 files changed, 406 insertions(+), 31 deletions(-) --- Changes since v3: * introduced patch 5 in the series by extracting the Kconfig changes proposed in patch 2 of v3. * not gpio_request()'ing the sw_rst and deep_sleep_en lines as requested (Sekhar Nori) Changes since v2: * register a single input device for switches and keys on the baseboard since there is no benefit to separate devices with different polling intervals (Dmitry Torokhov) * use static array intialization and range intialization for platform data structure to minimize the amount of runtime intialization needed: (Sekhar Nori) * Use the da850_evm variable name prefix for static symbols in board-da850-evm.c Changes since v1: * use locally defined functions that are no-ops/error checkers when INPUT_POLLDEV is not defined. * disable polling mode support when input-polldev is a module and gpio_keys is builtin * set INPUT_POLLDEV default for DA850_EVM machine, but don't select it unconditionally * adding note to description about why tca6416-keypad was not used ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v4 5/5] da850-evm: KEYBOARD_GPIO and INPUT_POLLDEV Kconfig conditionals
Use the mach-davinci/Kconfig to enable gpio-keys as default when da850-evm machine is enabled and to also select INPUT_POLLDEV when gpio-keys is enabled. INPUT_POLLDEV needs to be enabled for gpio-keys devices to function properly on da850-evm. Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca CC: Kevin Hilman khil...@deeprootsystems.com CC: Nori, Sekhar nsek...@ti.com --- Changes since v3: * no changes in this patch of the series / this patch was introduced in v4 of the series --- arch/arm/mach-davinci/Kconfig |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-davinci/Kconfig b/arch/arm/mach-davinci/Kconfig index 84066e8..70d1758 100644 --- a/arch/arm/mach-davinci/Kconfig +++ b/arch/arm/mach-davinci/Kconfig @@ -180,6 +180,10 @@ endchoice config GPIO_PCA953X default MACH_DAVINCI_DA850_EVM +config KEYBOARD_GPIO + default MACH_DAVINCI_DA850_EVM + select INPUT_POLLDEV if MACH_DAVINCI_DA850_EVM + config MACH_TNETV107X bool TI TNETV107X Reference Platform default ARCH_DAVINCI_TNETV107X -- 1.7.0.4 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v4 3/5] da850-evm: extract defines for SEL{A, B, C} pins in UI expander
The setup and teardown methods of the UI expander reference the SEL_{A,B,C} pins by 'magic number' in each function. This uses the common enum for their offsets in the expander setup and teardown functions. Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca Reviewed-by: Chris Cordahi christophercord...@nanometrics.ca Reviewed-by: Sekhar Nori nsek...@ti.com Signed-off-by: Sekhar Nori nsek...@ti.com CC: Victor Rodriguez vm.ro...@gmail.com --- Changes since v3: * no changes in this patch of the series Changes since v2: * rebased to 083eae3e28643e0eefc5243719f8b1572cf98299 of git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git * integrated the static array initialization patch provided by Sekhar Nori Changes since v1: * No changes since v1 --- arch/arm/mach-davinci/board-da850-evm.c | 24 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 51f5ffa..420b628 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -361,23 +361,23 @@ static int da850_evm_ui_expander_setup(struct i2c_client *client, unsigned gpio, { int sel_a, sel_b, sel_c, ret; - sel_a = gpio + 7; - sel_b = gpio + 6; - sel_c = gpio + 5; + sel_a = gpio + DA850_EVM_UI_EXP_SEL_A; + sel_b = gpio + DA850_EVM_UI_EXP_SEL_B; + sel_c = gpio + DA850_EVM_UI_EXP_SEL_C; - ret = gpio_request(sel_a, sel_a); + ret = gpio_request(sel_a, da850_evm_ui_exp[DA850_EVM_UI_EXP_SEL_A]); if (ret) { pr_warning(Cannot open UI expander pin %d\n, sel_a); goto exp_setup_sela_fail; } - ret = gpio_request(sel_b, sel_b); + ret = gpio_request(sel_b, da850_evm_ui_exp[DA850_EVM_UI_EXP_SEL_B]); if (ret) { pr_warning(Cannot open UI expander pin %d\n, sel_b); goto exp_setup_selb_fail; } - ret = gpio_request(sel_c, sel_c); + ret = gpio_request(sel_c, da850_evm_ui_exp[DA850_EVM_UI_EXP_SEL_C]); if (ret) { pr_warning(Cannot open UI expander pin %d\n, sel_c); goto exp_setup_selc_fail; @@ -420,13 +420,13 @@ static int da850_evm_ui_expander_teardown(struct i2c_client *client, platform_device_unregister(da850_evm_ui_keys_device); /* deselect all functionalities */ - gpio_set_value_cansleep(gpio + 5, 1); - gpio_set_value_cansleep(gpio + 6, 1); - gpio_set_value_cansleep(gpio + 7, 1); + gpio_set_value_cansleep(gpio + DA850_EVM_UI_EXP_SEL_C, 1); + gpio_set_value_cansleep(gpio + DA850_EVM_UI_EXP_SEL_B, 1); + gpio_set_value_cansleep(gpio + DA850_EVM_UI_EXP_SEL_A, 1); - gpio_free(gpio + 5); - gpio_free(gpio + 6); - gpio_free(gpio + 7); + gpio_free(gpio + DA850_EVM_UI_EXP_SEL_C); + gpio_free(gpio + DA850_EVM_UI_EXP_SEL_B); + gpio_free(gpio + DA850_EVM_UI_EXP_SEL_A); return 0; } -- 1.7.0.4 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v4 4/5] da850-evm: add baseboard GPIO expander buttons, switches and LEDs
This patch adds a pca953x platform device for the tca6416 found on the evm baseboard. The tca6416 is a GPIO expander, also found on the UI board at a separate I2C address. The pins of the baseboard IO expander are connected to software reset, deep sleep enable, test points, a push button, DIP switches and LEDs. Add support for the push button, DIP switches and LEDs and test points (as free GPIOs). The reset and deep sleep enable connections are reserved by the setup routine so that userspace can't toggle those lines. The existing tca6416-keypad driver was not employed because there was no apararent way to register the LEDs connected to gpio's on the tca6416 while simultaneously registering the tca6416-keypad instance. Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca Reviewed-by: Chris Cordahi christophercord...@nanometrics.ca CC: Govindarajan, Sriramakrishnan s...@ti.com Reviewed-by: Sekhar Nori nsek...@ti.com Reviewed-by: Dmitry Torokhov dmitry.torok...@gmail.com --- Changes since v3: * don't request sw_rst and deep_sleep_en gpio pins -- let clients use them freely Changes since v2: * rebased to 083eae3e28643e0eefc5243719f8b1572cf98299 of git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git * remove the TODO : populate at runtime using in 1/4 instead of this patch (Nori, Sekhar) * ui_expander_names was renamed to da850_evm_ui_exp * DA850_SW_POLL_MS definition moved to this patch from 3/4 * use indexed array initialization pattern introduced by Sekhar Nori in 3/4 * shorter names prefixed with da850_evm * static array range intializers * using only a single gpio-keys instance for the pushbutton and switches on baseboard since there is no advantage to separate device instances with different polling intervals (Dmitry Torokhov) Changes since v1: * adding note about why the tca6416-keypad driver was not used. * adding Govindarajan, Sriramakrishnan, the author of the tca6416-keypad driver --- arch/arm/mach-davinci/board-da850-evm.c | 190 +++ 1 files changed, 190 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 420b628..3cff221 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -431,6 +431,185 @@ static int da850_evm_ui_expander_teardown(struct i2c_client *client, return 0; } +/* assign the baseboard expander's GPIOs after the UI board's */ +#define DA850_UI_EXPANDER_N_GPIOS ARRAY_SIZE(da850_evm_ui_exp) +#define DA850_BB_EXPANDER_GPIO_BASE (DAVINCI_N_GPIO + DA850_UI_EXPANDER_N_GPIOS) + +enum da850_evm_bb_exp_pins { + DA850_EVM_BB_EXP_DEEP_SLEEP_EN = 0, + DA850_EVM_BB_EXP_SW_RST, + DA850_EVM_BB_EXP_TP_23, + DA850_EVM_BB_EXP_TP_22, + DA850_EVM_BB_EXP_TP_21, + DA850_EVM_BB_EXP_USER_PB1, + DA850_EVM_BB_EXP_USER_LED2, + DA850_EVM_BB_EXP_USER_LED1, + DA850_EVM_BB_EXP_USER_SW1, + DA850_EVM_BB_EXP_USER_SW2, + DA850_EVM_BB_EXP_USER_SW3, + DA850_EVM_BB_EXP_USER_SW4, + DA850_EVM_BB_EXP_USER_SW5, + DA850_EVM_BB_EXP_USER_SW6, + DA850_EVM_BB_EXP_USER_SW7, + DA850_EVM_BB_EXP_USER_SW8 +}; + +static const char const *da850_evm_bb_exp[] = { + [DA850_EVM_BB_EXP_DEEP_SLEEP_EN]= deep_sleep_en, + [DA850_EVM_BB_EXP_SW_RST] = sw_rst, + [DA850_EVM_BB_EXP_TP_23]= tp_23, + [DA850_EVM_BB_EXP_TP_22]= tp_22, + [DA850_EVM_BB_EXP_TP_21]= tp_21, + [DA850_EVM_BB_EXP_USER_PB1] = user_pb1, + [DA850_EVM_BB_EXP_USER_LED2]= user_led2, + [DA850_EVM_BB_EXP_USER_LED1]= user_led1, + [DA850_EVM_BB_EXP_USER_SW1] = user_sw1, + [DA850_EVM_BB_EXP_USER_SW2] = user_sw2, + [DA850_EVM_BB_EXP_USER_SW3] = user_sw3, + [DA850_EVM_BB_EXP_USER_SW4] = user_sw4, + [DA850_EVM_BB_EXP_USER_SW5] = user_sw5, + [DA850_EVM_BB_EXP_USER_SW6] = user_sw6, + [DA850_EVM_BB_EXP_USER_SW7] = user_sw7, + [DA850_EVM_BB_EXP_USER_SW8] = user_sw8, +}; + +#define DA850_N_BB_USER_SW 8 + +static struct gpio_keys_button da850_evm_bb_keys[] = { + [0] = { + .type = EV_KEY, + .active_low = 1, + .wakeup = 0, + .debounce_interval = DA850_KEYS_DEBOUNCE_MS, + .code = KEY_PROG1, + .desc = NULL, /* assigned at runtime */ + .gpio = -1, /* assigned at runtime */ + }, + [1 ... DA850_N_BB_USER_SW] = { + .type = EV_SW, + .active_low = 1, + .wakeup = 0, +
[PATCH v4 2/5] da850-evm: add UI Expander pushbuttons
This patch adds EV_KEYs for each of the 8 pushbuttons on the UI board via a gpio-key device. The expander is a tca6416; it controls the SEL_{A,B,C} lines which enable and disable the peripherals found on the UI board in addition to the 8 pushbuttons mentioned above. The reason the existing tca6416-keypad driver is not employed is because there was no aparent way to keep the gpio lines used as SEL_{A,B,C} registered while simultaneously registering the pushbuttons as a tca6416-keypad instance. Some experimentation with the polling interval was performed; we were searching for the largest polling interval that did not affect the feel of the responsiveness of the buttons. It is very subjective but 200ms seems to be a good value that accepts firm pushes but rejects very light ones. The key values assigned to the buttons were arbitrarily chosen to be F1-F8. Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca Reviewed-by: Chris Cordahi christophercord...@nanometrics.ca CC: Govindarajan, Sriramakrishnan s...@ti.com Reviewed-by: Sekhar Nori nsek...@ti.com Signed-off-by: Sekhar Nori nsek...@ti.com CC: Kevin Hilman khil...@deeprootsystems.com --- Changes since v3: * extracted Kconfig changes to patch 5/5 * fixed leading whitespace problem Changes since v2: * rebased to 083eae3e28643e0eefc5243719f8b1572cf98299 of git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git * remove the TODO : populate at runtime using in this patch instead of 4/4 (Nori, Sekhar) * integrated the static array initialization patch of Sekhar Nori * use static array initialization ranges * rename DA850_PB_POLL_MS to DA850_GPIO_KEYS_POLL_MS * use shorter names prefixed with da850_evm Changes since v1: * set INPUT_POLLDEV default for DA850_EVM machine, but don't select it unconditionally * adding note to description about why tca6416-keypad was not used * adding Govindarajan, Sriramakrishnan, the author of the tca6416-keypad driver --- arch/arm/mach-davinci/board-da850-evm.c | 98 ++- 1 files changed, 97 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index f89b0b7..51f5ffa 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -17,8 +17,10 @@ #include linux/i2c.h #include linux/i2c/at24.h #include linux/i2c/pca953x.h +#include linux/input.h #include linux/mfd/tps6507x.h #include linux/gpio.h +#include linux/gpio_keys.h #include linux/platform_device.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -272,6 +274,88 @@ static inline void da850_evm_setup_emac_rmii(int rmii_sel) static inline void da850_evm_setup_emac_rmii(int rmii_sel) { } #endif + +#define DA850_KEYS_DEBOUNCE_MS 10 +/* + * At 200ms polling interval it is possible to miss an + * event by tapping very lightly on the push button but most + * pushes do result in an event; longer intervals require the + * user to hold the button whereas shorter intervals require + * more CPU time for polling. + */ +#define DA850_GPIO_KEYS_POLL_MS200 + +enum da850_evm_ui_exp_pins { + DA850_EVM_UI_EXP_SEL_C = 5, + DA850_EVM_UI_EXP_SEL_B, + DA850_EVM_UI_EXP_SEL_A, + DA850_EVM_UI_EXP_PB8, + DA850_EVM_UI_EXP_PB7, + DA850_EVM_UI_EXP_PB6, + DA850_EVM_UI_EXP_PB5, + DA850_EVM_UI_EXP_PB4, + DA850_EVM_UI_EXP_PB3, + DA850_EVM_UI_EXP_PB2, + DA850_EVM_UI_EXP_PB1, +}; + +static const char const *da850_evm_ui_exp[] = { + [DA850_EVM_UI_EXP_SEL_C]= sel_c, + [DA850_EVM_UI_EXP_SEL_B]= sel_b, + [DA850_EVM_UI_EXP_SEL_A]= sel_a, + [DA850_EVM_UI_EXP_PB8] = pb8, + [DA850_EVM_UI_EXP_PB7] = pb7, + [DA850_EVM_UI_EXP_PB6] = pb6, + [DA850_EVM_UI_EXP_PB5] = pb5, + [DA850_EVM_UI_EXP_PB4] = pb4, + [DA850_EVM_UI_EXP_PB3] = pb3, + [DA850_EVM_UI_EXP_PB2] = pb2, + [DA850_EVM_UI_EXP_PB1] = pb1, +}; + +#define DA850_N_UI_PB 8 + +static struct gpio_keys_button da850_evm_ui_keys[] = { + [0 ... DA850_N_UI_PB - 1] = { + .type = EV_KEY, + .active_low = 1, + .wakeup = 0, + .debounce_interval = DA850_KEYS_DEBOUNCE_MS, + .code = -1, /* assigned at runtime */ + .gpio = -1, /* assigned at runtime */ + .desc = NULL, /* assigned at runtime */ + }, +}; + +static struct gpio_keys_platform_data da850_evm_ui_keys_pdata = { + .buttons = da850_evm_ui_keys, + .nbuttons = ARRAY_SIZE(da850_evm_ui_keys), + .rep = 0, /* disable auto-repeat */ + .poll_interval = DA850_GPIO_KEYS_POLL_MS, +}; + +static struct platform_device da850_evm_ui_keys_device = { + .name =
Re: [PATCH v2 2/4] da850-evm: add UI Expander pushbuttons
On Tue, Nov 23, 2010 at 07:48:21AM -0800, Kevin Hilman wrote: Nori, Sekhar nsek...@ti.com writes: On Mon, Nov 22, 2010 at 19:20:48, Ben Gardiner wrote: Yes -- only gpio-keys is affected but enabling the option does introduce an additional .o file: drivers/input/Makefile:obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o I agree that in its current state a user couls inadvertently disable the gpio-keys support on da850-evm simply by disabling INPUT_POLLDEV -- which is less than Ideal. How about I replace the current changes to arch/arm/mach-davinci/Kconfig with: config KEYBOARD_GPIO default MACH_DAVINCI_DA850_EVM select INPUT_POLLDEV So 1) gpio-keys functionality is default for the da850evm and 2) whenever gpio-keys is enabled so is INPUT_POLLDEV. This looks better than what was posted earlier, but I am not sure if platforms should influence driver configuration this way. Agreed. In general, we should not have machine/platform specific conditionals in generic Kconfigs. Generally, this should be handled in machine/platform specific Kconfigs. The patch that I originally wrote for this had the select under the Kconfig option for the driver itself, with the decision to use it or not being dynamically determined based on the platform data. I maintain that this is the only sensible way to deal with things, but this was rejected by the input folks at the time who felt that it was adding in extra overhead for a corner case. The alternatives then are to either make an identical copy of the driver that uses a polled interface, come up with lame INPUT_POLLDEV wrapper shims, or simply accept the fact that drivers using optional interfaces are going to have to have those interfaces built in to the kernel. There has been zero progress on getting this functionality integrated for years now because of this issue, and I don't really see that changing until people accept that drivers will sometimes require additional functionality to be built-in, or provide a suitable alternative. Personally I don't care enough about this particular problem to put any more cycles in to it, and the hardware that I wrote this patch for initially will be end-of-lifed long before there's any coherent consensus in driver land. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source