* [PATCH] ARM: OMAP: ISP1301 workqueue fixes
@ 2006-12-28 8:02 Dirk Behme
2006-12-28 12:30 ` Komal Shah
2006-12-28 20:05 ` David Brownell
0 siblings, 2 replies; 7+ messages in thread
From: Dirk Behme @ 2006-12-28 8:02 UTC (permalink / raw)
To: linux-omap-open-source
[-- Attachment #1: Type: text/plain, Size: 90 bytes --]
ARM: OMAP: ISP1301 workqueue fixes.
Signed-off-by: Dirk Behme <dirk.behme_at_gmail.com>
[-- Attachment #2: isp1301_workqueue_fix.txt --]
[-- Type: text/plain, Size: 893 bytes --]
ARM: OMAP: ISP1301 workqueue fixes.
Signed-off-by: Dirk Behme <dirk.behme_at_gmail.com>
Index: linux-osk/drivers/i2c/chips/isp1301_omap.c
===================================================================
--- linux-osk.orig/drivers/i2c/chips/isp1301_omap.c
+++ linux-osk/drivers/i2c/chips/isp1301_omap.c
@@ -1119,9 +1119,9 @@ static u8 isp1301_clear_latch(struct isp
}
static void
-isp1301_work(void *data)
+isp1301_work(struct work_struct *data)
{
- struct isp1301 *isp = data;
+ struct isp1301 *isp = (struct isp1301 *)data;
int stop;
/* implicit lock: we're the only task using this device */
@@ -1525,7 +1525,7 @@ static int isp1301_probe(struct i2c_adap
if (!isp)
return 0;
- INIT_WORK(&isp->work, isp1301_work, isp);
+ INIT_WORK(&isp->work, isp1301_work);
init_timer(&isp->timer);
isp->timer.function = isp1301_timer;
isp->timer.data = (unsigned long) isp;
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ARM: OMAP: ISP1301 workqueue fixes 2006-12-28 8:02 [PATCH] ARM: OMAP: ISP1301 workqueue fixes Dirk Behme @ 2006-12-28 12:30 ` Komal Shah 2006-12-28 16:19 ` Dirk Behme 2006-12-28 20:05 ` David Brownell 1 sibling, 1 reply; 7+ messages in thread From: Komal Shah @ 2006-12-28 12:30 UTC (permalink / raw) To: Dirk Behme; +Cc: linux-omap-open-source On 12/28/06, Dirk Behme <dirk.behme@googlemail.com> wrote: > > ARM: OMAP: ISP1301 workqueue fixes. > > Signed-off-by: Dirk Behme <dirk.behme_at_gmail.com> > Why not @. ? No obfuscation please. > > > ARM: OMAP: ISP1301 workqueue fixes. > > Signed-off-by: Dirk Behme <dirk.behme_at_gmail.com> > > Index: linux-osk/drivers/i2c/chips/isp1301_omap.c > =================================================================== > --- linux-osk.orig/drivers/i2c/chips/isp1301_omap.c > +++ linux-osk/drivers/i2c/chips/isp1301_omap.c > @@ -1119,9 +1119,9 @@ static u8 isp1301_clear_latch(struct isp > } > > static void > -isp1301_work(void *data) > +isp1301_work(struct work_struct *data) > { > - struct isp1301 *isp = data; > + struct isp1301 *isp = (struct isp1301 *)data; Why not use container_of ? Did you tested it? > int stop; > > /* implicit lock: we're the only task using this device */ > @@ -1525,7 +1525,7 @@ static int isp1301_probe(struct i2c_adap > if (!isp) > return 0; > > - INIT_WORK(&isp->work, isp1301_work, isp); > + INIT_WORK(&isp->work, isp1301_work); > init_timer(&isp->timer); > isp->timer.function = isp1301_timer; > isp->timer.data = (unsigned long) isp; > > And I think above should be treated as INIT_DELAYED_WORK instead. -- ---Komal Shah http://komalshah.blogspot.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ARM: OMAP: ISP1301 workqueue fixes 2006-12-28 12:30 ` Komal Shah @ 2006-12-28 16:19 ` Dirk Behme 2006-12-28 20:07 ` David Brownell 0 siblings, 1 reply; 7+ messages in thread From: Dirk Behme @ 2006-12-28 16:19 UTC (permalink / raw) To: linux-omap-open-source Komal Shah wrote: > On 12/28/06, Dirk Behme <dirk.behme@googlemail.com> wrote: ... >> Index: linux-osk/drivers/i2c/chips/isp1301_omap.c >> =================================================================== >> --- linux-osk.orig/drivers/i2c/chips/isp1301_omap.c >> +++ linux-osk/drivers/i2c/chips/isp1301_omap.c >> @@ -1119,9 +1119,9 @@ static u8 isp1301_clear_latch(struct isp >> } >> >> static void >> -isp1301_work(void *data) >> +isp1301_work(struct work_struct *data) >> { >> - struct isp1301 *isp = data; >> + struct isp1301 *isp = (struct isp1301 *)data; > > > Why not use container_of ? Did you tested it? Can anybody with H2/H3 board test this? Same with board-h3 workqueue fix from Ragner. Many thanks Dirk ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ARM: OMAP: ISP1301 workqueue fixes 2006-12-28 16:19 ` Dirk Behme @ 2006-12-28 20:07 ` David Brownell 0 siblings, 0 replies; 7+ messages in thread From: David Brownell @ 2006-12-28 20:07 UTC (permalink / raw) To: linux-omap-open-source On Thursday 28 December 2006 8:19 am, Dirk Behme wrote: > Komal Shah wrote: > > On 12/28/06, Dirk Behme <dirk.behme@googlemail.com> wrote: > ... > >> Index: linux-osk/drivers/i2c/chips/isp1301_omap.c > >> =================================================================== > >> --- linux-osk.orig/drivers/i2c/chips/isp1301_omap.c > >> +++ linux-osk/drivers/i2c/chips/isp1301_omap.c > >> @@ -1119,9 +1119,9 @@ static u8 isp1301_clear_latch(struct isp > >> } > >> > >> static void > >> -isp1301_work(void *data) > >> +isp1301_work(struct work_struct *data) > >> { > >> - struct isp1301 *isp = data; > >> + struct isp1301 *isp = (struct isp1301 *)data; > > > > > > Why not use container_of ? Did you tested it? > > Can anybody with H2/H3 board test this? Not worth testing, it's clearly incorrect. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ARM: OMAP: ISP1301 workqueue fixes 2006-12-28 8:02 [PATCH] ARM: OMAP: ISP1301 workqueue fixes Dirk Behme 2006-12-28 12:30 ` Komal Shah @ 2006-12-28 20:05 ` David Brownell 2006-12-29 8:36 ` Dirk Behme 1 sibling, 1 reply; 7+ messages in thread From: David Brownell @ 2006-12-28 20:05 UTC (permalink / raw) To: linux-omap-open-source On Thursday 28 December 2006 12:02 am, Dirk Behme wrote: > static void > -isp1301_work(void *data) > +isp1301_work(struct work_struct *data) > { > - struct isp1301 *isp = data; > + struct isp1301 *isp = (struct isp1301 *)data; > int stop; Clearly incorrect. It should use container_of(). The work_struct is NOT the first member of that struct... this will be oopsing. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ARM: OMAP: ISP1301 workqueue fixes 2006-12-28 20:05 ` David Brownell @ 2006-12-29 8:36 ` Dirk Behme 2007-01-02 20:34 ` tony 0 siblings, 1 reply; 7+ messages in thread From: Dirk Behme @ 2006-12-29 8:36 UTC (permalink / raw) To: linux-omap-open-source [-- Attachment #1: Type: text/plain, Size: 562 bytes --] David Brownell wrote: > On Thursday 28 December 2006 12:02 am, Dirk Behme wrote: > >> static void >>-isp1301_work(void *data) >>+isp1301_work(struct work_struct *data) >> { >>- struct isp1301 *isp = data; >>+ struct isp1301 *isp = (struct isp1301 *)data; >> int stop; > > > Clearly incorrect. It should use container_of(). The work_struct > is NOT the first member of that struct... this will be oopsing. Yes, I see. Thanks! So, next attempt ;) Below both, ISP1301 and board-h3. Again, only checked for compilation. Dirk [-- Attachment #2: isp1301_workqueue_fix.txt --] [-- Type: text/plain, Size: 823 bytes --] Index: linux-osk/drivers/i2c/chips/isp1301_omap.c =================================================================== --- linux-osk.orig/drivers/i2c/chips/isp1301_omap.c +++ linux-osk/drivers/i2c/chips/isp1301_omap.c @@ -1119,9 +1119,9 @@ static u8 isp1301_clear_latch(struct isp } static void -isp1301_work(void *data) +isp1301_work(struct work_struct *work) { - struct isp1301 *isp = data; + struct isp1301 *isp = container_of(work, struct isp1301, work); int stop; /* implicit lock: we're the only task using this device */ @@ -1525,7 +1525,7 @@ static int isp1301_probe(struct i2c_adap if (!isp) return 0; - INIT_WORK(&isp->work, isp1301_work, isp); + INIT_WORK(&isp->work, isp1301_work); init_timer(&isp->timer); isp->timer.function = isp1301_timer; isp->timer.data = (unsigned long) isp; [-- Attachment #3: board_h3_workqueue_fix.txt --] [-- Type: text/plain, Size: 1590 bytes --] Index: linux-osk/arch/arm/mach-omap1/board-h3.c =================================================================== --- linux-osk.orig/arch/arm/mach-omap1/board-h3.c +++ linux-osk/arch/arm/mach-omap1/board-h3.c @@ -296,9 +296,11 @@ static int h3_select_irda(struct device return err; } -static void set_trans_mode(void *data) +static void set_trans_mode(struct work_struct *work) { - int *mode = data; + struct omap_irda_config *irda_config = + container_of(work, struct omap_irda_config, gpio_expa.work); + int mode = irda_config->mode; unsigned char expa; int err = 0; @@ -308,7 +310,7 @@ static void set_trans_mode(void *data) expa &= ~0x03; - if (*mode & IR_SIRMODE) { + if (mode & IR_SIRMODE) { expa |= 0x01; } else { /* MIR/FIR */ expa |= 0x03; @@ -323,9 +325,9 @@ static int h3_transceiver_mode(struct de { struct omap_irda_config *irda_config = dev->platform_data; + irda_config->mode = mode; cancel_delayed_work(&irda_config->gpio_expa); - PREPARE_WORK(&irda_config->gpio_expa, set_trans_mode, &mode); -#error this is not permitted - mode is an argument variable + PREPARE_DELAYED_WORK(&irda_config->gpio_expa, set_trans_mode); schedule_delayed_work(&irda_config->gpio_expa, 0); return 0; Index: linux-osk/include/asm-arm/arch-omap/irda.h =================================================================== --- linux-osk.orig/include/asm-arm/arch-omap/irda.h +++ linux-osk/include/asm-arm/arch-omap/irda.h @@ -31,6 +31,7 @@ struct omap_irda_config { unsigned long src_start; int tx_trigger; int rx_trigger; + int mode; }; #endif [-- Attachment #4: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ARM: OMAP: ISP1301 workqueue fixes 2006-12-29 8:36 ` Dirk Behme @ 2007-01-02 20:34 ` tony 0 siblings, 0 replies; 7+ messages in thread From: tony @ 2007-01-02 20:34 UTC (permalink / raw) To: Dirk Behme; +Cc: linux-omap-open-source * Dirk Behme <dirk.behme@googlemail.com> [061229 00:44]: > David Brownell wrote: > >On Thursday 28 December 2006 12:02 am, Dirk Behme wrote: > > > >>static void > >>-isp1301_work(void *data) > >>+isp1301_work(struct work_struct *data) > >>{ > >>- struct isp1301 *isp = data; > >>+ struct isp1301 *isp = (struct isp1301 *)data; > >> int stop; > > > > > >Clearly incorrect. It should use container_of(). The work_struct > >is NOT the first member of that struct... this will be oopsing. > > Yes, I see. Thanks! > > So, next attempt ;) Below both, ISP1301 and board-h3. Again, > only checked for compilation. Pushing as two patches. Tony ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-01-02 20:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-12-28 8:02 [PATCH] ARM: OMAP: ISP1301 workqueue fixes Dirk Behme 2006-12-28 12:30 ` Komal Shah 2006-12-28 16:19 ` Dirk Behme 2006-12-28 20:07 ` David Brownell 2006-12-28 20:05 ` David Brownell 2006-12-29 8:36 ` Dirk Behme 2007-01-02 20:34 ` tony
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox