* [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init @ 2011-10-24 13:30 David Herrmann 2011-10-24 13:30 ` [PATCH 2/2] Bluetooth: Replace rfcomm tty tasklet by workqueue David Herrmann 2011-10-24 13:57 ` [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init Anderson Briglia 0 siblings, 2 replies; 5+ messages in thread From: David Herrmann @ 2011-10-24 13:30 UTC (permalink / raw) To: linux-bluetooth; +Cc: padovan, David Herrmann Forward error codes from tty core to the rfcomm_init caller instead of using generic -1 errors. Signed-off-by: David Herrmann <dh.herrmann@googlemail.com> --- net/bluetooth/rfcomm/tty.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c index c258796..2b753a3 100644 --- a/net/bluetooth/rfcomm/tty.c +++ b/net/bluetooth/rfcomm/tty.c @@ -1155,9 +1155,11 @@ static const struct tty_operations rfcomm_ops = { int __init rfcomm_init_ttys(void) { + int error; + rfcomm_tty_driver = alloc_tty_driver(RFCOMM_TTY_PORTS); if (!rfcomm_tty_driver) - return -1; + return -ENOMEM; rfcomm_tty_driver->owner = THIS_MODULE; rfcomm_tty_driver->driver_name = "rfcomm"; @@ -1172,10 +1174,11 @@ int __init rfcomm_init_ttys(void) rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON; tty_set_operations(rfcomm_tty_driver, &rfcomm_ops); - if (tty_register_driver(rfcomm_tty_driver)) { + error = tty_register_driver(rfcomm_tty_driver); + if (error) { BT_ERR("Can't register RFCOMM TTY driver"); put_tty_driver(rfcomm_tty_driver); - return -1; + return error; } BT_INFO("RFCOMM TTY layer initialized"); -- 1.7.7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Bluetooth: Replace rfcomm tty tasklet by workqueue 2011-10-24 13:30 [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init David Herrmann @ 2011-10-24 13:30 ` David Herrmann 2011-10-31 19:29 ` Gustavo Padovan 2011-10-24 13:57 ` [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init Anderson Briglia 1 sibling, 1 reply; 5+ messages in thread From: David Herrmann @ 2011-10-24 13:30 UTC (permalink / raw) To: linux-bluetooth; +Cc: padovan, David Herrmann Remove old tasklets and replace by workqueue. To avoid reentrancy (which tasklets always avoid) we use the system_nrt_wq. Signed-off-by: David Herrmann <dh.herrmann@googlemail.com> --- net/bluetooth/rfcomm/tty.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c index 2b753a3..947f1b3 100644 --- a/net/bluetooth/rfcomm/tty.c +++ b/net/bluetooth/rfcomm/tty.c @@ -34,6 +34,7 @@ #include <linux/capability.h> #include <linux/slab.h> #include <linux/skbuff.h> +#include <linux/workqueue.h> #include <net/bluetooth/bluetooth.h> #include <net/bluetooth/hci_core.h> @@ -65,7 +66,7 @@ struct rfcomm_dev { struct rfcomm_dlc *dlc; struct tty_struct *tty; wait_queue_head_t wait; - struct tasklet_struct wakeup_task; + struct work_struct wakeup_task; struct device *tty_dev; @@ -81,7 +82,7 @@ static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc, struct sk_buff *skb); static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err); static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig); -static void rfcomm_tty_wakeup(unsigned long arg); +static void rfcomm_tty_wakeup(struct work_struct *work); /* ---- Device functions ---- */ static void rfcomm_dev_destruct(struct rfcomm_dev *dev) @@ -257,7 +258,7 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc) atomic_set(&dev->opened, 0); init_waitqueue_head(&dev->wait); - tasklet_init(&dev->wakeup_task, rfcomm_tty_wakeup, (unsigned long) dev); + INIT_WORK(&dev->wakeup_task, rfcomm_tty_wakeup); skb_queue_head_init(&dev->pending); @@ -351,7 +352,7 @@ static void rfcomm_wfree(struct sk_buff *skb) struct rfcomm_dev *dev = (void *) skb->sk; atomic_sub(skb->truesize, &dev->wmem_alloc); if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags)) - tasklet_schedule(&dev->wakeup_task); + queue_work(system_nrt_wq, &dev->wakeup_task); rfcomm_dev_put(dev); } @@ -635,9 +636,10 @@ static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig) } /* ---- TTY functions ---- */ -static void rfcomm_tty_wakeup(unsigned long arg) +static void rfcomm_tty_wakeup(struct work_struct *work) { - struct rfcomm_dev *dev = (void *) arg; + struct rfcomm_dev *dev = container_of(work, struct rfcomm_dev, + wakeup_task); struct tty_struct *tty = dev->tty; if (!tty) return; @@ -762,7 +764,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp) rfcomm_dlc_close(dev->dlc, 0); clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags); - tasklet_kill(&dev->wakeup_task); + cancel_work_sync(&dev->wakeup_task); rfcomm_dlc_lock(dev->dlc); tty->driver_data = NULL; -- 1.7.7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Replace rfcomm tty tasklet by workqueue 2011-10-24 13:30 ` [PATCH 2/2] Bluetooth: Replace rfcomm tty tasklet by workqueue David Herrmann @ 2011-10-31 19:29 ` Gustavo Padovan 0 siblings, 0 replies; 5+ messages in thread From: Gustavo Padovan @ 2011-10-31 19:29 UTC (permalink / raw) To: David Herrmann; +Cc: linux-bluetooth Hi David, * David Herrmann <dh.herrmann@googlemail.com> [2011-10-24 15:30:58 +0200]: > Remove old tasklets and replace by workqueue. To avoid reentrancy (which > tasklets always avoid) we use the system_nrt_wq. > > Signed-off-by: David Herrmann <dh.herrmann@googlemail.com> > --- > net/bluetooth/rfcomm/tty.c | 16 +++++++++------- > 1 files changed, 9 insertions(+), 7 deletions(-) Both patches have been applied, thanks. Gustavo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init 2011-10-24 13:30 [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init David Herrmann 2011-10-24 13:30 ` [PATCH 2/2] Bluetooth: Replace rfcomm tty tasklet by workqueue David Herrmann @ 2011-10-24 13:57 ` Anderson Briglia 2011-10-24 14:03 ` David Herrmann 1 sibling, 1 reply; 5+ messages in thread From: Anderson Briglia @ 2011-10-24 13:57 UTC (permalink / raw) To: David Herrmann; +Cc: linux-bluetooth, padovan Hi David, On Mon, Oct 24, 2011 at 3:30 PM, David Herrmann <dh.herrmann@googlemail.com> wrote: > Forward error codes from tty core to the rfcomm_init caller instead of using > generic -1 errors. > > Signed-off-by: David Herrmann <dh.herrmann@googlemail.com> > --- > net/bluetooth/rfcomm/tty.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c > index c258796..2b753a3 100644 > --- a/net/bluetooth/rfcomm/tty.c > +++ b/net/bluetooth/rfcomm/tty.c > @@ -1155,9 +1155,11 @@ static const struct tty_operations rfcomm_ops = { > > int __init rfcomm_init_ttys(void) > { > + int error; > + > rfcomm_tty_driver = alloc_tty_driver(RFCOMM_TTY_PORTS); > if (!rfcomm_tty_driver) > - return -1; > + return -ENOMEM; > > rfcomm_tty_driver->owner = THIS_MODULE; > rfcomm_tty_driver->driver_name = "rfcomm"; > @@ -1172,10 +1174,11 @@ int __init rfcomm_init_ttys(void) > rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON; > tty_set_operations(rfcomm_tty_driver, &rfcomm_ops); > > - if (tty_register_driver(rfcomm_tty_driver)) { > + error = tty_register_driver(rfcomm_tty_driver); > + if (error) { > BT_ERR("Can't register RFCOMM TTY driver"); > put_tty_driver(rfcomm_tty_driver); > - return -1; > + return error; Since you are defining a new variable (error), how about use it on other error paths and add a "goto out" and have just one return path? Regards, Anderson Briglia > } > > BT_INFO("RFCOMM TTY layer initialized"); > -- > 1.7.7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- INdT - Instituto Nokia de tecnologia +55 92 2126 1122 +55 92 8423 3183 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init 2011-10-24 13:57 ` [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init Anderson Briglia @ 2011-10-24 14:03 ` David Herrmann 0 siblings, 0 replies; 5+ messages in thread From: David Herrmann @ 2011-10-24 14:03 UTC (permalink / raw) To: Anderson Briglia; +Cc: linux-bluetooth, padovan On Mon, Oct 24, 2011 at 3:57 PM, Anderson Briglia <anderson.briglia@openbossa.org> wrote: > Hi David, > > On Mon, Oct 24, 2011 at 3:30 PM, David Herrmann > <dh.herrmann@googlemail.com> wrote: >> Forward error codes from tty core to the rfcomm_init caller instead of using >> generic -1 errors. >> >> Signed-off-by: David Herrmann <dh.herrmann@googlemail.com> >> --- >> net/bluetooth/rfcomm/tty.c | 9 ++++++--- >> 1 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c >> index c258796..2b753a3 100644 >> --- a/net/bluetooth/rfcomm/tty.c >> +++ b/net/bluetooth/rfcomm/tty.c >> @@ -1155,9 +1155,11 @@ static const struct tty_operations rfcomm_ops = { >> >> int __init rfcomm_init_ttys(void) >> { >> + int error; >> + >> rfcomm_tty_driver = alloc_tty_driver(RFCOMM_TTY_PORTS); >> if (!rfcomm_tty_driver) >> - return -1; >> + return -ENOMEM; >> >> rfcomm_tty_driver->owner = THIS_MODULE; >> rfcomm_tty_driver->driver_name = "rfcomm"; >> @@ -1172,10 +1174,11 @@ int __init rfcomm_init_ttys(void) >> rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON; >> tty_set_operations(rfcomm_tty_driver, &rfcomm_ops); >> >> - if (tty_register_driver(rfcomm_tty_driver)) { >> + error = tty_register_driver(rfcomm_tty_driver); >> + if (error) { >> BT_ERR("Can't register RFCOMM TTY driver"); >> put_tty_driver(rfcomm_tty_driver); >> - return -1; >> + return error; > > Since you are defining a new variable (error), how about use it on > other error paths and add a "goto out" and have just one return path? Isn't it quite common style to have the first error path return directly? And then the other error paths use goto. However, there is only one additional error path left so I don't see any way to make this look nicer. With more than 2 error paths I agree, a goto would be more readable, but I cannot see this making this function look nicer. Anyway, if you guys want, I can resend with goto's. > Regards, > > Anderson Briglia Regards David ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-10-31 19:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-24 13:30 [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init David Herrmann 2011-10-24 13:30 ` [PATCH 2/2] Bluetooth: Replace rfcomm tty tasklet by workqueue David Herrmann 2011-10-31 19:29 ` Gustavo Padovan 2011-10-24 13:57 ` [PATCH 1/2] Bluetooth: Return proper error codes on rfcomm tty init Anderson Briglia 2011-10-24 14:03 ` David Herrmann
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).