Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
On 2012年07月27日 03:30, Kevin Hilman wrote:
+ Zumeng Chen
Igor Grinberggrinb...@compulab.co.il writes:
Hi Kevin,
I've just noticed that the patch has been modified by Arnd in a way
that of course will trigger GPIO use without being requested.
I'm sorry, I was not available by that time Arnd changed the patch.
Your right, your original patch isn't the problem. I found the root
cause.
The real problem is actually introduced by the merge of your patch from
the arm-soc/cleanup branch, and this one from Zumeng Chen: commit
16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for
ads7846) from the arm-soc/boards branch.
However, looking closer at the one from Zumeng, that one is clearly not
right. It unconditionally adds a *board-specific* -get_pendown_state
function to the pdata that is common to *all* boards. That's just wrong
and has the side-effect of making -get_pendown_state() wrong on every
board except the OMAP3EVM. Oops.
So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen
GPIO working on non OMAP3EVM boards, we also need something like this as
well.
Definitely, thanks Kevin.
Igor, Zumeng, could you try this out on your boards anc confirm if it's
working? I currently don't have a board setup with a touchscreen in my
board farm.
Acked
Zumeng
Kevin
From 85516c6a3354967caf4cff434d28c3001cd411eb Mon Sep 17 00:00:00 2001
From: Kevin Hilmankhil...@ti.com
Date: Thu, 26 Jul 2012 12:15:38 -0700
Subject: [PATCH 2/2] ARM: OMAP2+: ads7846: fix -get_pendown_state() to work
on all boards
commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce
time for ads7846) introduced a common -get_pendown_state() function
into the generic code, but that function was board-specific for the
OMAP3EVM.
Fix this up to work for all boards that pass in a pendown GPIO.
Cc: Zumeng Chenzumeng.c...@windriver.com
Cc: Igor Grinberggrinb...@compulab.co.il
Signed-off-by: Kevin Hilmankhil...@ti.com
---
arch/arm/mach-omap2/board-omap3evm.c |1 +
arch/arm/mach-omap2/common-board-devices.c |4 +++-
arch/arm/mach-omap2/common-board-devices.h |1 -
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c
b/arch/arm/mach-omap2/board-omap3evm.c
index ef230a0..0d362e9 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -58,6 +58,7 @@
#include hsmmc.h
#include common-board-devices.h
+#define OMAP3_EVM_TS_GPIO 175
#define OMAP3_EVM_EHCI_VBUS 22
#define OMAP3_EVM_EHCI_SELECT 61
diff --git a/arch/arm/mach-omap2/common-board-devices.c
b/arch/arm/mach-omap2/common-board-devices.c
index 0fee95f..06f176f 100644
--- a/arch/arm/mach-omap2/common-board-devices.c
+++ b/arch/arm/mach-omap2/common-board-devices.c
@@ -40,9 +40,10 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config
= {
* of pdata-get_pendown_state, but we have done this. So set
* get_pendown_state to avoid twice gpio requesting.
*/
+static int omap3_gpio_pendown;
static int omap3_get_pendown_state(void)
{
- return !gpio_get_value(OMAP3_EVM_TS_GPIO);
+ return !gpio_get_value(omap3_gpio_pendown);
}
static struct ads7846_platform_data ads7846_config = {
@@ -74,6 +75,7 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown,
int gpio_debounce,
struct spi_board_info *spi_bi =ads7846_spi_board_info;
int err;
+ omap3_gpio_pendown = gpio_pendown;
err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown);
if (err) {
pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err);
diff --git a/arch/arm/mach-omap2/common-board-devices.h
b/arch/arm/mach-omap2/common-board-devices.h
index 4c4ef6a..a0b4a428 100644
--- a/arch/arm/mach-omap2/common-board-devices.h
+++ b/arch/arm/mach-omap2/common-board-devices.h
@@ -4,7 +4,6 @@
#include twl-common.h
#define NAND_BLOCK_SIZE SZ_128K
-#define OMAP3_EVM_TS_GPIO 175
struct mtd_partition;
struct ads7846_platform_data;
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
On 2012年07月27日 05:58, Kevin Hilman wrote: zumeng.chenzc...@windriver.com writes: On 2012年07月27日 03:30, Kevin Hilman wrote: + Zumeng Chen Igor Grinberggrinb...@compulab.co.il writes: Hi Kevin, I've just noticed that the patch has been modified by Arnd in a way that of course will trigger GPIO use without being requested. I'm sorry, I was not available by that time Arnd changed the patch. Your right, your original patch isn't the problem. I found the root cause. The real problem is actually introduced by the merge of your patch from the arm-soc/cleanup branch, and this one from Zumeng Chen: commit 16aced80f6 (ARM: OMAP3530evm: set pendown_state and debounce time for ads7846) from the arm-soc/boards branch. However, looking closer at the one from Zumeng, that one is clearly not right. It unconditionally adds a *board-specific* -get_pendown_state function to the pdata that is common to *all* boards. That's just wrong and has the side-effect of making -get_pendown_state() wrong on every board except the OMAP3EVM. Oops. So, IMO, in addition to $SUBJECT patch, in order to get the touchscreen GPIO working on non OMAP3EVM boards, we also need something like this as well. Definitely, thanks Kevin. Igor, Zumeng, could you try this out on your boards anc confirm if it's working? I currently don't have a board setup with a touchscreen in my board farm. Acked Did you test this on your board? No, I just read/compiled it. If so, could you respond with a Tested-by tag? Thanks NP, Kevin, I'll test it after being office, and it's about ten o'clock this morning. Regards, Zumeng -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND V3 0/3] Watchdog: OMAP3: bootstatus fix and changes for the current watchdog framework
On 2012年07月16日 21:10, Hiremath, Vaibhav wrote: On Sun, Jul 15, 2012 at 13:14:03, Zumeng Chen wrote: Hello, The following patches based on the 3.5-rc6 from Wim, which focus on: 1. bootstatus fix for omap3, 2. omap-wdt framework update cater for the current framework as Shubhrajyoti comments mentioned. V3 changes: 1. New comments updated as Kevin mentioned in the third patch; 2. 3530evm works well, AM33xx seems work well with the following changes: + if (cpu_is_am335x()) + return omap2_prm_read_mod_reg(AM33XX_PRM_DEVICE_MOD, + AM33XX_PRM_RSTST_OFFSET) 0x7f; But since some definitions not ready for am33xx, so I don't give the patch, if they have been updated, feel free to take these. Did you test it on any of AM33xx platform? Yes. I ever did in v2 as follows shown: The same case works well on am335xevm but with a new patch in mach-omap2/prcm.c I'll send it later. root@ti-omap3:~# uname -a Linux ti-omap3 3.4.3-00635-g82d1d26-dirty #32 Wed Jul 11 16:02:12 CST 2012 armv7l GNU/Linux root@ti-omap3:~# dmesg|grep WDT [ 1.921173] omap_wdt: OMAP WDTimer Rev 0x01: Initial timeout 0sec status= 0x1 root@ti-omap3:~# ./a.out -i 20; for i in `seq 1 20`; do echo $i ; sleep 1;done Set watchdog interval to 20 Current watchdog interval is 20 Last boot is caused by : Watchdog Use: w to kick through writing over device file i to kick through IOCTL x to exit the program x 1 2 [snip] U-Boot SPL 2011.09 (Feb 09 2012 - 15:38:59) Texas Instruments Revision detection unimplemented U-Boot 2011.09 (Feb 09 2012 - 15:11:31) I2C: ready DRAM: 256 MiB WARNING: Caches not enabled Found a daughter card connected NAND: HW ECC Hamming Code selected 256 MiB MMC: OMAP SD/MMC: 0 Net: cpsw Hit any key to stop autoboot: 0 U-Boot# If you use linux-omap/master pr linux-next branch as a baseline you should have basic things (except hwmod data) available. I will test it and send a patch for this on your behalf (if you are ok with it). I'm OK for this, thanks Vaibhav. Note: Change in convention, cpu_is_am335x() = soc_is_am335x() Yes, agreed. Regards, Zumeng Thanks, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Watchdog: OMAP3: fix wrong boot status from wdt reboot
On 2012年07月05日 21:05, Bedia, Vaibhav wrote:
Hello,
On Wed, Jul 04, 2012 at 21:24:01, Zumeng Chen wrote:
Does the following fix make sense?
WDIOC_GETBOOTSTATUS always return 0 even if the machine
comes from omap-wdt reboot. Because WKUP_MOD is not right
for OMAP3, so give the right addr 0xA00 of PRM_RSTST for
get_reset_sources, which inputs the signal from omap-wdt
reboot, and return 1 when coming from omap-wdt reboot for
WDIOC_GETBOOTSTATUS.
Signed-off-by: Zumeng Chenzumeng.c...@windriver.com
---
arch/arm/mach-omap2/prcm.c |4 +++-
drivers/watchdog/omap_wdt.c |3 +++
drivers/watchdog/omap_wdt.h |3 +++
3 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 480f40a..43f3feb 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -49,8 +49,10 @@ void __iomem *prcm_mpu_base;
u32 omap_prcm_get_reset_sources(void)
{
/* XXX This presumably needs modification for 34XX */
- if (cpu_is_omap24xx() || cpu_is_omap34xx())
+ if (cpu_is_omap24xx())
return omap2_prm_read_mod_reg(WKUP_MOD, OMAP2_RM_RSTST) 0x7f;
+ if (cpu_is_omap34xx())
+ return omap2_prm_read_mod_reg(0xA00, OMAP2_RM_RSTST) 0x7f;
if (cpu_is_omap44xx())
return omap2_prm_read_mod_reg(WKUP_MOD, OMAP4_RM_RSTST) 0x7f;
Instead of adding more cpu_is_* checks maybe you could switch to a
function pointers based approach here?
I don't see any more checks VS before like ( cpu_is_omap24xx() ||
cpu_is_omap34xx())
Actually what we want is just to read a register with different offset
responding to the different SOC.
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 8285d65..ea57078 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -234,6 +234,9 @@ static long omap_wdt_ioctl(struct file *file, unsigned int
cmd,
if (cpu_is_omap24xx())
return put_user(omap_prcm_get_reset_sources(),
(int __user *)arg);
+ if (cpu_is_omap34xx())
+ return put_user(omap_prcm_get_reset_sources() 0x10
+ OMAP3_PRM_RSTST_BIT, (int __user *)arg);
return put_user(0, (int __user *)arg);
Usage of PRCM bit masks in the driver looks wrong. Why not
Maybe the not proper definition causes looks wrong.
It should be MPU_WD_RST_BIT, so you see, it is about
watchdog bit.
Anyway, I'll try to send V2 patch with Hubhrajyoti's and your
comments
Regards,
Zumeng
introduce an API like omap_prcm_check_reset_reason() which
returns true or false based on the reset reason being checked?
In case of WDT, the driver can then pass the right flag to
userspace.
Regards,
Vaibhav B.
case WDIOC_KEEPALIVE:
pm_runtime_get_sync(wdev-dev);
diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
index 09b774c..d8d5daa 100644
--- a/drivers/watchdog/omap_wdt.h
+++ b/drivers/watchdog/omap_wdt.h
@@ -40,6 +40,9 @@
#define OMAP_WATCHDOG_WPS (0x34)
#define OMAP_WATCHDOG_SPR (0x48)
+/* PRM_RSTST MPU_WD_RST bit */
+#define OMAP3_PRM_RSTST_BIT4
+
/* Using the prescaler, the OMAP watchdog could go for many
* months before firing. These limits work without scaling,
* with the 60 second default assumed by most tools and docs.
--
1.7.5.4
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Watchdog: OMAP3: fix wrong boot status from wdt reboot
To: Shubhrajyoti
On 2012年07月04日 23:54, Zumeng Chen wrote:
Does the following fix make sense?
WDIOC_GETBOOTSTATUS always return 0 even if the machine
comes from omap-wdt reboot. Because WKUP_MOD is not right
for OMAP3, so give the right addr 0xA00 of PRM_RSTST for
get_reset_sources, which inputs the signal from omap-wdt
reboot, and return 1 when coming from omap-wdt reboot for
WDIOC_GETBOOTSTATUS.
Signed-off-by: Zumeng Chenzumeng.c...@windriver.com
---
arch/arm/mach-omap2/prcm.c |4 +++-
drivers/watchdog/omap_wdt.c |3 +++
drivers/watchdog/omap_wdt.h |3 +++
3 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 480f40a..43f3feb 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -49,8 +49,10 @@ void __iomem *prcm_mpu_base;
u32 omap_prcm_get_reset_sources(void)
{
/* XXX This presumably needs modification for 34XX */
- if (cpu_is_omap24xx() || cpu_is_omap34xx())
+ if (cpu_is_omap24xx())
return omap2_prm_read_mod_reg(WKUP_MOD, OMAP2_RM_RSTST) 0x7f;
+ if (cpu_is_omap34xx())
+ return omap2_prm_read_mod_reg(0xA00, OMAP2_RM_RSTST) 0x7f;
if (cpu_is_omap44xx())
return omap2_prm_read_mod_reg(WKUP_MOD, OMAP4_RM_RSTST) 0x7f;
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 8285d65..ea57078 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -234,6 +234,9 @@ static long omap_wdt_ioctl(struct file *file, unsigned int
cmd,
if (cpu_is_omap24xx())
return put_user(omap_prcm_get_reset_sources(),
(int __user *)arg);
+ if (cpu_is_omap34xx())
+ return put_user(omap_prcm_get_reset_sources() 0x10
+ OMAP3_PRM_RSTST_BIT, (int __user *)arg);
return put_user(0, (int __user *)arg);
case WDIOC_KEEPALIVE:
pm_runtime_get_sync(wdev-dev);
diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
index 09b774c..d8d5daa 100644
--- a/drivers/watchdog/omap_wdt.h
+++ b/drivers/watchdog/omap_wdt.h
@@ -40,6 +40,9 @@
#define OMAP_WATCHDOG_WPS (0x34)
#define OMAP_WATCHDOG_SPR (0x48)
+/* PRM_RSTST MPU_WD_RST bit */
+#define OMAP3_PRM_RSTST_BIT4
+
/* Using the prescaler, the OMAP watchdog could go for many
* months before firing. These limits work without scaling,
* with the 60 second default assumed by most tools and docs.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level
On 2012年06月14日 14:59, Zumeng Chen wrote: 于 2012年06月14日 14:31, Hiremath, Vaibhav 写道: On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote: 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道: On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote: From: Zumeng Chenzumeng.c...@windriver.com If we don't set proper debouce time for ads7846, then there are flooded interrupt counters of ads7846 responding to one time touch on screen, so the driver couldn't work well. And since most OMAP3 series boards pass NULL pointer of board_pdata to omap_ads7846_init, so it's more proper to set it in driver level after having gpio_request done. This patch has been validated on 3530evm. Signed-off-by: Zumeng Chenzumeng.c...@windriver.com Signed-off-by: Syed Mohammed Khasimkha...@ti.com --- drivers/input/touchscreen/ads7846.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index f02028e..459ff29 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 } ts-gpio_pendown = pdata-gpio_pendown; +#ifdef CONFIG_ARCH_OMAP3 + /* 310 means about 10 microsecond for omap3 */ + gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Zumeng, With my sign-off you are changing the original code, that too without my sign-off and ack. I wouldn't mind you to submit patches from my tree, but the expectation is if you are changing the original code, it should be under your sign-off. Thanks, good to learn. I'll remove in next time. Coming to the patch, #ifdef in driver is not recommended, and this 10msec delay is specific to OMAP GPIO and driver should not be aware of this, that's where you will find the original patch handling it in board file. According to the git blame of the board-omap3evm.c I think 96974a24 did a good thing to all the related codes for omap3 boards. So I think we can call board and driver as BSP level :-) If #ifdef in driver can save many codes, I guess it's deserved. No, not really. In the same commit, the debounce time is already handled as an argument to the function omap_ads7846_init(), and that’s the way it should be. That means you'd like to implement the same get_pendown_state for every omap3 board? Currently, board_pdata is NULL. And actually, the reason why I agree 96974a24 is that get_pendown_state for all omap3 boards is the common chip level gpio operations. so I think we should set debounce for them in one common point. But since Igor and you don't like them, I have created and tested the attachment patch, and I'd like Mike to check if convenience too But obviously there are more codes than mine before :-) Tony Mike, How about your comments on this issue? Do you think the following is acceptable VS my last email _attachment_. +#ifdef CONFIG_ARCH_OMAP3 + /* 310 means about 10 microsecond for omap3 */ + gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Regards, Zumeng Regards, Zumeng So no need for #ifdefs in driver... Thanks, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
