* [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 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 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 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