On 07/20/2013 03:10 AM, Gianluca Anzolin wrote:
On Tue, Jul 16, 2013 at 04:48:22PM -0400, Peter Hurley wrote:
+static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
+{
+ DECLARE_WAITQUEUE(wait, current);
+ struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+ struct rfcomm_dlc *dlc = dev->dlc;
+ int err;
+
+ err = rfcomm_dlc_open(dlc, &dev->src, &dev->dst, dev->channel);
+ if (err < 0)
+ goto error_no_dlc;
+
+ /* Wait for DLC to connect */
+ add_wait_queue(&dev->wait, &wait);
+ while (1) {
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (dlc->state == BT_CLOSED) {
+ err = -dev->err;
+ break;
+ }
+
+ if (dlc->state == BT_CONNECTED)
+ break;
Please consider moving these dlc->state tests into a
.carrier_raised() port method (this is what the gsm
driver does). Then this wait loop could go away.
I have a question about this: how do I signal an error condition with
carrier_raised?
Sorry Gianluca, I should have been more specific here.
There's no need to test for dlc->state == BT_CLOSED in carrier_raised().
At the point where port->carrier_raised is called, the tty will have been
linked with the file descriptor, so if the dlc->state goes to BT_CLOSED,
then rfcomm_dev_state_change() will call
tty_hangup() -> driver hangup() -> tty_port_hangup() -> tty_port_shutdown()
This call chain will
1. set the file_ops to hung_up_tty_fops which will cause tty_hung_up_p() to
return true
2. clear ASYNCB_INITIALIZED in port->flags
3. wakeup port->open_wait
So an open() parked in the schedule loop of tty_port_block_til_ready()
will wake and exit the loop with either -EAGAIN or -ERESTARTSYS.
rfcomm_dev_state_change() should only do a wakeup on port->open_wait when
dlc->state == BT_CONNECTED.
In case of success I should also call some device_move,
rfcomm_tty_copy_pending and rfcomm_dlc_unthrottle. Could I do it in
carrier_raised directly?
I wouldn't. That would be a nasty hack and a potential problem if a
signal occurred.
The device_move() isn't dependent on success, and can stay in .activate().
The rfcomm_tty_copy_pending() looks to me like a hack for a lack
of flow control when receiving data for the rfcomm_dev.
[Note how rfcomm_dev_data_ready() doesn't bother to test the return
value from tty_insert_flip_string().]
For simplicity, you could leave
rfcomm_tty_copy_pending(dev);
rfcomm_dlc_unthrottle(dlc);
in rfcomm_tty_open(), eg.:
err = tty_port_open(&dev->port, tty, filp);
if (err)
return err;
/*
* FIXME: rfcomm should use proper flow control for
* received data. This hack will be unnecessary and can
* be removed when that's implemented.
*/
rfcomm_tty_copy_pending(dev);
rfcomm_dlc_unthrottle(dlc);
return 0;
Regards,
Peter Hurley
+ goto error_no_connection;
+
+ device_move(dev->tty_dev, rfcomm_get_device(dev),
+ DPM_ORDER_DEV_AFTER_PARENT);
+
+ rfcomm_tty_copy_pending(dev);
+ rfcomm_dlc_unthrottle(dlc);
+ return 0;
+
+error_no_connection:
+ rfcomm_dlc_close(dlc, err);
+error_no_dlc:
+ return err;
+}
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
