Re: [PATCH 00/49] spi: davinci: re-write existing driver, DM355 DMA problem

2010-11-23 Thread Pierre

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

2010-11-23 Thread Nori, Sekhar
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

2010-11-23 Thread Nori, Sekhar
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

2010-11-23 Thread Nori, Sekhar
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Pierre
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

2010-11-23 Thread Brian Niebuhr
 -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

2010-11-23 Thread Kevin Hilman
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

2010-11-23 Thread Pierre

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

2010-11-23 Thread Kevin Hilman
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Ben Gardiner
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

2010-11-23 Thread Paul Mundt
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