* (no subject)
@ 2011-01-03 16:38 castet.matthieu
2011-01-03 16:41 ` [PATCH 19/32] usb/ueagle-atm: use system_wq instead of dedicated workqueues castet.matthieu
2011-01-03 17:03 ` your mail Stanislaw Gruszka
0 siblings, 2 replies; 4+ messages in thread
From: castet.matthieu @ 2011-01-03 16:38 UTC (permalink / raw)
To: linux-kernel, linux-usb; +Cc: stf_xl, tj
Hi,
could you CC me on ueagle-atm.c patches.
>From what I remind we sleep in the workqueue, that's why we couldn't use the
system one (freeze keyboard...). But may be the code changed.
Matthieu
Tejun Heo a écrit :
> With cmwq, there's no reason to use separate workqueues. Drop
> uea_softc->work_q and use system_wq instead. The used work item is
> sync flushed on driver detach.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Stanislaw Gruszka <stf_xl@wp.pl>
> Cc: linux-usb@vger.kernel.org
> ---
> Only compile tested. Please feel free to take it into the subsystem
> tree or simply ack - I'll route it through the wq tree.
>
> Thanks.
>
> drivers/usb/atm/ueagle-atm.c | 19 +++++--------------
> 1 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
> index 44447f5..55c1d3b 100644
> --- a/drivers/usb/atm/ueagle-atm.c
> +++ b/drivers/usb/atm/ueagle-atm.c
> @@ -168,7 +168,6 @@ struct uea_softc {
> union cmv_dsc cmv_dsc;
>
> struct work_struct task;
> - struct workqueue_struct *work_q;
> u16 pageno;
> u16 ovl;
>
> @@ -1879,7 +1878,7 @@ static int uea_start_reset(struct uea_softc *sc)
> /* start loading DSP */
> sc->pageno = 0;
> sc->ovl = 0;
> - queue_work(sc->work_q, &sc->task);
> + schedule_work(&sc->task);
>
> /* wait for modem ready CMV */
> ret = wait_cmv_ack(sc);
> @@ -2091,14 +2090,14 @@ static void uea_schedule_load_page_e1(struct uea_softc
*sc,
> {
> sc->pageno = intr->e1_bSwapPageNo;
> sc->ovl = intr->e1_bOvl >> 4 | intr->e1_bOvl << 4;
> - queue_work(sc->work_q, &sc->task);
> + schedule_work(&sc->task);
> }
>
> static void uea_schedule_load_page_e4(struct uea_softc *sc,
> struct intr_pkt *intr)
> {
> sc->pageno = intr->e4_bSwapPageNo;
> - queue_work(sc->work_q, &sc->task);
> + schedule_work(&sc->task);
> }
>
> /*
> @@ -2170,13 +2169,6 @@ static int uea_boot(struct uea_softc *sc)
>
> init_waitqueue_head(&sc->sync_q);
>
> - sc->work_q = create_workqueue("ueagle-dsp");
> - if (!sc->work_q) {
> - uea_err(INS_TO_USBDEV(sc), "cannot allocate workqueue\n");
> - uea_leaves(INS_TO_USBDEV(sc));
> - return -ENOMEM;
> - }
> -
> if (UEA_CHIP_VERSION(sc) == ADI930)
> load_XILINX_firmware(sc);
>
> @@ -2222,7 +2214,6 @@ err1:
> sc->urb_int = NULL;
> kfree(intr);
> err0:
> - destroy_workqueue(sc->work_q);
> uea_leaves(INS_TO_USBDEV(sc));
> return -ENOMEM;
> }
> @@ -2243,8 +2234,8 @@ static void uea_stop(struct uea_softc *sc)
> kfree(sc->urb_int->transfer_buffer);
> usb_free_urb(sc->urb_int);
>
> - /* stop any pending boot process, when no one can schedule work */
> - destroy_workqueue(sc->work_q);
> + /* flush the work item, when no one can schedule it */
> + flush_work_sync(&sc->task);
>
> if (sc->dsp_firm)
> release_firmware(sc->dsp_firm);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 19/32] usb/ueagle-atm: use system_wq instead of dedicated workqueues
2011-01-03 16:38 castet.matthieu
@ 2011-01-03 16:41 ` castet.matthieu
2011-01-03 17:03 ` your mail Stanislaw Gruszka
1 sibling, 0 replies; 4+ messages in thread
From: castet.matthieu @ 2011-01-03 16:41 UTC (permalink / raw)
To: tj; +Cc: linux-kernel, linux-usb, stf_xl
Quoting castet.matthieu@free.fr:
> Hi,
>
> could you CC me on ueagle-atm.c patches.
>
> From what I remind we sleep in the workqueue, that's why we couldn't use the
> system one (freeze keyboard...). But may be the code changed.
For the record we still do blocking usb transfert on the workqueue.
>
>
> Matthieu
>
> Tejun Heo a écrit :
> > With cmwq, there's no reason to use separate workqueues. Drop
> > uea_softc->work_q and use system_wq instead. The used work item is
> > sync flushed on driver detach.
> >
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > Cc: Stanislaw Gruszka <stf_xl@wp.pl>
> > Cc: linux-usb@vger.kernel.org
> > ---
> > Only compile tested. Please feel free to take it into the subsystem
> > tree or simply ack - I'll route it through the wq tree.
> >
> > Thanks.
> >
> > drivers/usb/atm/ueagle-atm.c | 19 +++++--------------
> > 1 files changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
> > index 44447f5..55c1d3b 100644
> > --- a/drivers/usb/atm/ueagle-atm.c
> > +++ b/drivers/usb/atm/ueagle-atm.c
> > @@ -168,7 +168,6 @@ struct uea_softc {
> > union cmv_dsc cmv_dsc;
> >
> > struct work_struct task;
> > - struct workqueue_struct *work_q;
> > u16 pageno;
> > u16 ovl;
> >
> > @@ -1879,7 +1878,7 @@ static int uea_start_reset(struct uea_softc *sc)
> > /* start loading DSP */
> > sc->pageno = 0;
> > sc->ovl = 0;
> > - queue_work(sc->work_q, &sc->task);
> > + schedule_work(&sc->task);
> >
> > /* wait for modem ready CMV */
> > ret = wait_cmv_ack(sc);
> > @@ -2091,14 +2090,14 @@ static void uea_schedule_load_page_e1(struct
> uea_softc
> *sc,
> > {
> > sc->pageno = intr->e1_bSwapPageNo;
> > sc->ovl = intr->e1_bOvl >> 4 | intr->e1_bOvl << 4;
> > - queue_work(sc->work_q, &sc->task);
> > + schedule_work(&sc->task);
> > }
> >
> > static void uea_schedule_load_page_e4(struct uea_softc *sc,
> > struct intr_pkt *intr)
> > {
> > sc->pageno = intr->e4_bSwapPageNo;
> > - queue_work(sc->work_q, &sc->task);
> > + schedule_work(&sc->task);
> > }
> >
> > /*
> > @@ -2170,13 +2169,6 @@ static int uea_boot(struct uea_softc *sc)
> >
> > init_waitqueue_head(&sc->sync_q);
> >
> > - sc->work_q = create_workqueue("ueagle-dsp");
> > - if (!sc->work_q) {
> > - uea_err(INS_TO_USBDEV(sc), "cannot allocate workqueue\n");
> > - uea_leaves(INS_TO_USBDEV(sc));
> > - return -ENOMEM;
> > - }
> > -
> > if (UEA_CHIP_VERSION(sc) == ADI930)
> > load_XILINX_firmware(sc);
> >
> > @@ -2222,7 +2214,6 @@ err1:
> > sc->urb_int = NULL;
> > kfree(intr);
> > err0:
> > - destroy_workqueue(sc->work_q);
> > uea_leaves(INS_TO_USBDEV(sc));
> > return -ENOMEM;
> > }
> > @@ -2243,8 +2234,8 @@ static void uea_stop(struct uea_softc *sc)
> > kfree(sc->urb_int->transfer_buffer);
> > usb_free_urb(sc->urb_int);
> >
> > - /* stop any pending boot process, when no one can schedule work */
> > - destroy_workqueue(sc->work_q);
> > + /* flush the work item, when no one can schedule it */
> > + flush_work_sync(&sc->task);
> >
> > if (sc->dsp_firm)
> > release_firmware(sc->dsp_firm);
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: your mail
2011-01-03 16:38 castet.matthieu
2011-01-03 16:41 ` [PATCH 19/32] usb/ueagle-atm: use system_wq instead of dedicated workqueues castet.matthieu
@ 2011-01-03 17:03 ` Stanislaw Gruszka
2011-01-04 5:17 ` Tejun Heo
1 sibling, 1 reply; 4+ messages in thread
From: Stanislaw Gruszka @ 2011-01-03 17:03 UTC (permalink / raw)
To: castet.matthieu; +Cc: linux-kernel, linux-usb, stf_xl, tj
On Mon, Jan 03, 2011 at 05:38:00PM +0100, castet.matthieu@free.fr wrote:
> Hi,
>
> could you CC me on ueagle-atm.c patches.
>
> From what I remind we sleep in the workqueue, that's why we couldn't use the
> system one (freeze keyboard...). But may be the code changed.
In case when firmware is not available we can sleep for a few seconds in
work function. That's block keyboard driver who also use common workqueue.
If recent Tejun workqueue rewrite allow to long sleep in work func and
not hurt other workqueue users, patch is ok.
Unfortunately I'm not able to test the patch, my ueagle device was physically
damaged a few months ago.
Stanislaw
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: your mail
2011-01-03 17:03 ` your mail Stanislaw Gruszka
@ 2011-01-04 5:17 ` Tejun Heo
0 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2011-01-04 5:17 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: castet.matthieu, linux-kernel, linux-usb, stf_xl
On Mon, Jan 03, 2011 at 06:03:17PM +0100, Stanislaw Gruszka wrote:
> On Mon, Jan 03, 2011 at 05:38:00PM +0100, castet.matthieu@free.fr wrote:
> > could you CC me on ueagle-atm.c patches.
Will try to, but maybe it's a good idea to add a MAINTAINERS entry?
> > From what I remind we sleep in the workqueue, that's why we couldn't use the
> > system one (freeze keyboard...). But may be the code changed.
> In case when firmware is not available we can sleep for a few seconds in
> work function. That's block keyboard driver who also use common workqueue.
> If recent Tejun workqueue rewrite allow to long sleep in work func and
> not hurt other workqueue users, patch is ok.
Yeap, work items can sleep all they want on the system_wq. It won't
delay execution of other work items.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-01-04 5:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-03 16:38 castet.matthieu
2011-01-03 16:41 ` [PATCH 19/32] usb/ueagle-atm: use system_wq instead of dedicated workqueues castet.matthieu
2011-01-03 17:03 ` your mail Stanislaw Gruszka
2011-01-04 5:17 ` Tejun Heo
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).