* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift [not found] ` <20090717103253.GF3439@rakim.wolfsonmicro.main> @ 2009-07-17 11:12 ` Marek Vasut 2009-07-17 11:22 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2009-07-17 11:12 UTC (permalink / raw) To: linux-input; +Cc: Mark Brown, alsa-devel, Dmitry Torokhov [-- Attachment #1: Type: text/plain, Size: 1236 bytes --] Dne Pá 17. července 2009 12:32:53 Mark Brown napsal(a): > On Fri, Jul 17, 2009 at 10:01:16AM +0200, Marek Vasut wrote: > > This patch allows tweaking the behaviour of GPIO_STATUS register > > shift quirk that's in wm97xx-core. The problem with GPIO_STATUS > > register being shifted by one doesn't appear on all hardware it > > seems and causes problems with accelerated touchscreen drivers on > > Palm hardware. Therefore an accelerated touchscreen driver can select > > if the shift is/isn't happening on the hardware. > > Again, this isn't an ALSA patch. > > I've done some checking internally and I'm very suspicous that what's > going on here is actually the masking of some other bug elsewhere in the > system. Which CPU are the systems you're observing this on using? > There is at least one special case in the current code for GPIO register > readbacks in the PXA AC97 driver code and there have been a number of > interoperabilty issues in that area... About the mailer, same here, sending as attachment, sorry. The CPU used is pxa270c5 and pxa270c0. The chip is WM1613 (seems to report itself as WM9712 though). Maybe it's correction in that chip? There's no datasheet available to it though. [-- Attachment #2: 0002-Add-possibility-to-control-the-GPIO_STATUS-shift.patch --] [-- Type: text/x-patch, Size: 3449 bytes --] From e7179d9d34b0a6078324fbe2c3666937937c0fde Mon Sep 17 00:00:00 2001 From: Marek Vasut <marek.vasut@gmail.com> Date: Thu, 4 Jun 2009 21:56:59 +0200 Subject: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift This patch allows tweaking the behaviour of GPIO_STATUS register shift quirk that's in wm97xx-core. The problem with GPIO_STATUS register being shifted by one doesn't appear on all hardware it seems and causes problems with accelerated touchscreen drivers on Palm hardware. Therefore an accelerated touchscreen driver can select if the shift is/isn't happening on the hardware. Signed-off-by: Marek Vasut <marek.vasut@gmail.com> --- drivers/input/touchscreen/mainstone-wm97xx.c | 2 ++ drivers/input/touchscreen/wm97xx-core.c | 19 +++++++++---------- include/linux/wm97xx.h | 1 + 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/input/touchscreen/mainstone-wm97xx.c b/drivers/input/touchscreen/mainstone-wm97xx.c index de02f87..7b95741 100644 --- a/drivers/input/touchscreen/mainstone-wm97xx.c +++ b/drivers/input/touchscreen/mainstone-wm97xx.c @@ -198,6 +198,8 @@ static int wm97xx_acc_startup(struct wm97xx *wm) if (machine_is_palmt5() || machine_is_palmtx() || machine_is_palmld()) { pen_int = 1; irq = 27; + wm->gpio_offset = 0; /* interestingly, on Palm hardware, the + * GPIO_STATUS shift doesn't occur */ } else if (machine_is_mainstone() && pen_int) irq = 4; diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c index 2957d48..6621c0e 100644 --- a/drivers/input/touchscreen/wm97xx-core.c +++ b/drivers/input/touchscreen/wm97xx-core.c @@ -204,10 +204,7 @@ void wm97xx_set_gpio(struct wm97xx *wm, u32 gpio, else reg &= ~gpio; - if (wm->id == WM9712_ID2) - wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg << 1); - else - wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg); + wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg << wm->gpio_offset); mutex_unlock(&wm->codec_mutex); } EXPORT_SYMBOL_GPL(wm97xx_set_gpio); @@ -307,12 +304,8 @@ static void wm97xx_pen_irq_worker(struct work_struct *work) WM97XX_GPIO_13); } - if (wm->id == WM9712_ID2) - wm97xx_reg_write(wm, AC97_GPIO_STATUS, (status & - ~WM97XX_GPIO_13) << 1); - else - wm97xx_reg_write(wm, AC97_GPIO_STATUS, status & - ~WM97XX_GPIO_13); + wm97xx_reg_write(wm, AC97_GPIO_STATUS, (status & + ~WM97XX_GPIO_13) << wm->gpio_offset); mutex_unlock(&wm->codec_mutex); } @@ -582,6 +575,12 @@ static int wm97xx_probe(struct device *dev) wm->id = wm97xx_reg_read(wm, AC97_VENDOR_ID2); + /* On some versions of WM9712, the GPIO_STATUS register is shifted */ + if (wm->id == WM9712_ID2) + wm->gpio_offset = 1; + else + wm->gpio_offset = 0; + dev_info(wm->dev, "detected a wm97%02x codec\n", wm->id & 0xff); switch (wm->id & 0xff) { diff --git a/include/linux/wm97xx.h b/include/linux/wm97xx.h index 6f69968..c6dc15b 100644 --- a/include/linux/wm97xx.h +++ b/include/linux/wm97xx.h @@ -283,6 +283,7 @@ struct wm97xx { unsigned pen_is_down:1; /* Pen is down */ unsigned aux_waiting:1; /* aux measurement waiting */ unsigned pen_probably_down:1; /* used in polling mode */ + unsigned gpio_offset:1; /* gpio register offset */ u16 suspend_mode; /* PRP in suspend mode */ }; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 11:12 ` [PATCH 2/2] Add possibility to control the GPIO_STATUS shift Marek Vasut @ 2009-07-17 11:22 ` Mark Brown 2009-07-17 11:26 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2009-07-17 11:22 UTC (permalink / raw) To: Marek Vasut; +Cc: alsa-devel, Dmitry Torokhov, linux-input On Fri, Jul 17, 2009 at 01:12:15PM +0200, Marek Vasut wrote: > The CPU used is pxa270c5 and pxa270c0. The chip is WM1613 (seems to report > itself as WM9712 though). Maybe it's correction in that chip? There's no > datasheet available to it though. Ah, that is crucial information which you hadn't previously provided. If you rework the patch as adding support for WM1613 with the interface being one that enables a WM1613 mode rather than tweaking the GPIO offset that'd be OK. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 11:22 ` Mark Brown @ 2009-07-17 11:26 ` Marek Vasut 2009-07-17 11:43 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2009-07-17 11:26 UTC (permalink / raw) To: Mark Brown; +Cc: alsa-devel, Dmitry Torokhov, linux-input Dne Pá 17. července 2009 13:22:15 Mark Brown napsal(a): > On Fri, Jul 17, 2009 at 01:12:15PM +0200, Marek Vasut wrote: > > The CPU used is pxa270c5 and pxa270c0. The chip is WM1613 (seems to > > report itself as WM9712 though). Maybe it's correction in that chip? > > There's no datasheet available to it though. > > Ah, that is crucial information which you hadn't previously provided. > If you rework the patch as adding support for WM1613 with the interface > being one that enables a WM1613 mode rather than tweaking the GPIO > offset that'd be OK. But how the heck am I supposed to detect it's wm1613 if it reports itself as wm9712 ? Apparently, on treo680 it reports itself as wm9713 (even better). _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 11:26 ` Marek Vasut @ 2009-07-17 11:43 ` Mark Brown 2009-07-17 11:53 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2009-07-17 11:43 UTC (permalink / raw) To: Marek Vasut; +Cc: alsa-devel, Dmitry Torokhov, linux-input On Fri, Jul 17, 2009 at 01:26:08PM +0200, Marek Vasut wrote: > But how the heck am I supposed to detect it's wm1613 if it reports itself as > wm9712 ? Apparently, on treo680 it reports itself as wm9713 (even better). The same way your current patch does it. I'm just saying you should change the way the configuration is passed in so that it is clear that this is only useful for WM1613. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 11:43 ` Mark Brown @ 2009-07-17 11:53 ` Marek Vasut 2009-07-17 12:11 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2009-07-17 11:53 UTC (permalink / raw) To: Mark Brown; +Cc: alsa-devel, Dmitry Torokhov, linux-input Dne Pá 17. července 2009 13:43:12 Mark Brown napsal(a): > On Fri, Jul 17, 2009 at 01:26:08PM +0200, Marek Vasut wrote: > > But how the heck am I supposed to detect it's wm1613 if it reports itself > > as wm9712 ? Apparently, on treo680 it reports itself as wm9713 (even > > better). > > The same way your current patch does it. I'm just saying you should > change the way the configuration is passed in so that it is clear that > this is only useful for WM1613. But there might appear some other chips that have this issue. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 11:53 ` Marek Vasut @ 2009-07-17 12:11 ` Mark Brown 2009-07-17 14:45 ` Marek Vasut 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2009-07-17 12:11 UTC (permalink / raw) To: Marek Vasut; +Cc: linux-input, alsa-devel, Dmitry Torokhov On Fri, Jul 17, 2009 at 01:53:59PM +0200, Marek Vasut wrote: > Dne Pá 17. července 2009 13:43:12 Mark Brown napsal(a): > > The same way your current patch does it. I'm just saying you should > > change the way the configuration is passed in so that it is clear that > > this is only useful for WM1613. > But there might appear some other chips that have this issue. Then they can get handled the same way - it might be an idea to have the data that's passed in be a chip number to allow for this. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 12:11 ` Mark Brown @ 2009-07-17 14:45 ` Marek Vasut 2009-07-17 14:52 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Marek Vasut @ 2009-07-17 14:45 UTC (permalink / raw) To: Mark Brown; +Cc: linux-input, alsa-devel, Dmitry Torokhov [-- Attachment #1: Type: text/plain, Size: 633 bytes --] Dne Pá 17. července 2009 14:11:59 Mark Brown napsal(a): > On Fri, Jul 17, 2009 at 01:53:59PM +0200, Marek Vasut wrote: > > Dne Pá 17. července 2009 13:43:12 Mark Brown napsal(a): > > > The same way your current patch does it. I'm just saying you should > > > change the way the configuration is passed in so that it is clear that > > > this is only useful for WM1613. > > > > But there might appear some other chips that have this issue. > > Then they can get handled the same way - it might be an idea to have the > data that's passed in be a chip number to allow for this. Will this one be more to your liking ? [-- Attachment #2: 0002-Add-possibility-to-control-the-GPIO_STATUS-shift.patch --] [-- Type: text/x-patch, Size: 3328 bytes --] From 1ea286d088993e05b5b374a08ee6f933242d3561 Mon Sep 17 00:00:00 2001 From: Marek Vasut <marek.vasut@gmail.com> Date: Thu, 4 Jun 2009 21:56:59 +0200 Subject: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift This patch allows tweaking the behaviour of GPIO_STATUS register shift quirk that's in wm97xx-core. The problem with GPIO_STATUS register being shifted by one doesn't appear on all hardware it seems and causes problems with accelerated touchscreen drivers on Palm hardware. Therefore an accelerated touchscreen driver can select if the shift is/isn't happening on the hardware. Signed-off-by: Marek Vasut <marek.vasut@gmail.com> --- drivers/input/touchscreen/mainstone-wm97xx.c | 3 +++ drivers/input/touchscreen/wm97xx-core.c | 6 ++++-- include/linux/wm97xx.h | 7 +++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/input/touchscreen/mainstone-wm97xx.c b/drivers/input/touchscreen/mainstone-wm97xx.c index de02f87..e000ab3 100644 --- a/drivers/input/touchscreen/mainstone-wm97xx.c +++ b/drivers/input/touchscreen/mainstone-wm97xx.c @@ -198,6 +198,9 @@ static int wm97xx_acc_startup(struct wm97xx *wm) if (machine_is_palmt5() || machine_is_palmtx() || machine_is_palmld()) { pen_int = 1; irq = 27; + /* There is some obscure mutant of WM9712 interbred with WM9713 + * used on Palm HW */ + wm->variant = WM97xx_WM1613; } else if (machine_is_mainstone() && pen_int) irq = 4; diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c index 2957d48..252eb11 100644 --- a/drivers/input/touchscreen/wm97xx-core.c +++ b/drivers/input/touchscreen/wm97xx-core.c @@ -204,7 +204,7 @@ void wm97xx_set_gpio(struct wm97xx *wm, u32 gpio, else reg &= ~gpio; - if (wm->id == WM9712_ID2) + if (wm->id == WM9712_ID2 && wm->variant != WM97xx_WM1613) wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg << 1); else wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg); @@ -307,7 +307,7 @@ static void wm97xx_pen_irq_worker(struct work_struct *work) WM97XX_GPIO_13); } - if (wm->id == WM9712_ID2) + if (wm->id == WM9712_ID2 && wm->variant != WM97xx_WM1613) wm97xx_reg_write(wm, AC97_GPIO_STATUS, (status & ~WM97XX_GPIO_13) << 1); else @@ -582,6 +582,8 @@ static int wm97xx_probe(struct device *dev) wm->id = wm97xx_reg_read(wm, AC97_VENDOR_ID2); + wm->variant = WM97xx_GENERIC; + dev_info(wm->dev, "detected a wm97%02x codec\n", wm->id & 0xff); switch (wm->id & 0xff) { diff --git a/include/linux/wm97xx.h b/include/linux/wm97xx.h index 6f69968..0c98781 100644 --- a/include/linux/wm97xx.h +++ b/include/linux/wm97xx.h @@ -16,6 +16,12 @@ #include <linux/platform_device.h> /* + * WM97xx variants + */ +#define WM97xx_GENERIC 0x0000 +#define WM97xx_WM1613 0x1613 + +/* * WM97xx AC97 Touchscreen registers */ #define AC97_WM97XX_DIGITISER1 0x76 @@ -283,6 +289,7 @@ struct wm97xx { unsigned pen_is_down:1; /* Pen is down */ unsigned aux_waiting:1; /* aux measurement waiting */ unsigned pen_probably_down:1; /* used in polling mode */ + u16 variant; /* WM97xx chip variant */ u16 suspend_mode; /* PRP in suspend mode */ }; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 14:45 ` Marek Vasut @ 2009-07-17 14:52 ` Mark Brown 2009-07-21 5:45 ` Dmitry Torokhov 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2009-07-17 14:52 UTC (permalink / raw) To: Marek Vasut; +Cc: alsa-devel, Dmitry Torokhov, linux-input On Fri, Jul 17, 2009 at 04:45:21PM +0200, Marek Vasut wrote: > From 1ea286d088993e05b5b374a08ee6f933242d3561 Mon Sep 17 00:00:00 2001 > From: Marek Vasut <marek.vasut@gmail.com> > Date: Thu, 4 Jun 2009 21:56:59 +0200 > Subject: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift > > This patch allows tweaking the behaviour of GPIO_STATUS register > shift quirk that's in wm97xx-core. The problem with GPIO_STATUS > register being shifted by one doesn't appear on all hardware it > seems and causes problems with accelerated touchscreen drivers on > Palm hardware. Therefore an accelerated touchscreen driver can select > if the shift is/isn't happening on the hardware. Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com> > --- > drivers/input/touchscreen/mainstone-wm97xx.c | 3 +++ > drivers/input/touchscreen/wm97xx-core.c | 6 ++++-- > include/linux/wm97xx.h | 7 +++++++ > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/touchscreen/mainstone-wm97xx.c b/drivers/input/touchscreen/mainstone-wm97xx.c > index de02f87..e000ab3 100644 > --- a/drivers/input/touchscreen/mainstone-wm97xx.c > +++ b/drivers/input/touchscreen/mainstone-wm97xx.c > @@ -198,6 +198,9 @@ static int wm97xx_acc_startup(struct wm97xx *wm) > if (machine_is_palmt5() || machine_is_palmtx() || machine_is_palmld()) { > pen_int = 1; > irq = 27; > + /* There is some obscure mutant of WM9712 interbred with WM9713 > + * used on Palm HW */ > + wm->variant = WM97xx_WM1613; > } else if (machine_is_mainstone() && pen_int) > irq = 4; > > diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c > index 2957d48..252eb11 100644 > --- a/drivers/input/touchscreen/wm97xx-core.c > +++ b/drivers/input/touchscreen/wm97xx-core.c > @@ -204,7 +204,7 @@ void wm97xx_set_gpio(struct wm97xx *wm, u32 gpio, > else > reg &= ~gpio; > > - if (wm->id == WM9712_ID2) > + if (wm->id == WM9712_ID2 && wm->variant != WM97xx_WM1613) > wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg << 1); > else > wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg); > @@ -307,7 +307,7 @@ static void wm97xx_pen_irq_worker(struct work_struct *work) > WM97XX_GPIO_13); > } > > - if (wm->id == WM9712_ID2) > + if (wm->id == WM9712_ID2 && wm->variant != WM97xx_WM1613) > wm97xx_reg_write(wm, AC97_GPIO_STATUS, (status & > ~WM97XX_GPIO_13) << 1); > else > @@ -582,6 +582,8 @@ static int wm97xx_probe(struct device *dev) > > wm->id = wm97xx_reg_read(wm, AC97_VENDOR_ID2); > > + wm->variant = WM97xx_GENERIC; > + > dev_info(wm->dev, "detected a wm97%02x codec\n", wm->id & 0xff); > > switch (wm->id & 0xff) { > diff --git a/include/linux/wm97xx.h b/include/linux/wm97xx.h > index 6f69968..0c98781 100644 > --- a/include/linux/wm97xx.h > +++ b/include/linux/wm97xx.h > @@ -16,6 +16,12 @@ > #include <linux/platform_device.h> > > /* > + * WM97xx variants > + */ > +#define WM97xx_GENERIC 0x0000 > +#define WM97xx_WM1613 0x1613 > + > +/* > * WM97xx AC97 Touchscreen registers > */ > #define AC97_WM97XX_DIGITISER1 0x76 > @@ -283,6 +289,7 @@ struct wm97xx { > unsigned pen_is_down:1; /* Pen is down */ > unsigned aux_waiting:1; /* aux measurement waiting */ > unsigned pen_probably_down:1; /* used in polling mode */ > + u16 variant; /* WM97xx chip variant */ > u16 suspend_mode; /* PRP in suspend mode */ > }; > > -- > 1.6.3.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift 2009-07-17 14:52 ` Mark Brown @ 2009-07-21 5:45 ` Dmitry Torokhov 0 siblings, 0 replies; 9+ messages in thread From: Dmitry Torokhov @ 2009-07-21 5:45 UTC (permalink / raw) To: Mark Brown; +Cc: Marek Vasut, linux-input, alsa-devel On Fri, Jul 17, 2009 at 03:52:03PM +0100, Mark Brown wrote: > On Fri, Jul 17, 2009 at 04:45:21PM +0200, Marek Vasut wrote: > > > From 1ea286d088993e05b5b374a08ee6f933242d3561 Mon Sep 17 00:00:00 2001 > > From: Marek Vasut <marek.vasut@gmail.com> > > Date: Thu, 4 Jun 2009 21:56:59 +0200 > > Subject: [PATCH 2/2] Add possibility to control the GPIO_STATUS shift > > > > This patch allows tweaking the behaviour of GPIO_STATUS register > > shift quirk that's in wm97xx-core. The problem with GPIO_STATUS > > register being shifted by one doesn't appear on all hardware it > > seems and causes problems with accelerated touchscreen drivers on > > Palm hardware. Therefore an accelerated touchscreen driver can select > > if the shift is/isn't happening on the hardware. > > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > Applied both patches to 'next', thank you Marek. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-07-21 5:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200907171001.16889.marek.vasut@gmail.com>
[not found] ` <20090717103253.GF3439@rakim.wolfsonmicro.main>
2009-07-17 11:12 ` [PATCH 2/2] Add possibility to control the GPIO_STATUS shift Marek Vasut
2009-07-17 11:22 ` Mark Brown
2009-07-17 11:26 ` Marek Vasut
2009-07-17 11:43 ` Mark Brown
2009-07-17 11:53 ` Marek Vasut
2009-07-17 12:11 ` Mark Brown
2009-07-17 14:45 ` Marek Vasut
2009-07-17 14:52 ` Mark Brown
2009-07-21 5:45 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).