[PATCH] cxacru: ADSL state management

[PATCH] cxacru: ADSL state management

[Simon Arlott]

The device has commands to start/stop the ADSL function, so this adds 
a sysfs attribute to allow it to be started/stopped/restarted. It also 
stops polling the device for status when the ADSL function is disabled.

There are no problems with sending multiple start or stop commands, 
even with a fast loop of them the device still works. There is no 
need to protect the restart process from further user actions while 
it's waiting for 1.5s.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Duncan Sands <duncan.sands@math.u-psud.fr>
---
This patch requires usb-cxacru-export-detailed-device-info-through-sysfs 
from gregkh-2.6.

 drivers/usb/atm/cxacru.c |  215 ++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 206 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index c8b69bf..a89c484 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2004 David Woodhouse, Duncan Sands, Roman Kagan
  *  Copyright (C) 2005 Duncan Sands, Roman Kagan (rkagan % mail ! ru)
+ *  Copyright (C) 2007 Simon Arlott
  *
  *  This program is free software; you can redistribute it and/or modify it
  *  under the terms of the GNU General Public License as published by the Free
@@ -145,6 +146,13 @@ enum cxacru_info_idx {
 	/* dunno what the missing two mean */
 	CXINF_MAX = 0x1c,
 };
+ 
+enum cxacru_poll_state {
+	CXPOLL_STOPPING,
+	CXPOLL_STOPPED,
+	CXPOLL_POLLING,
+	CXPOLL_SHUTDOWN
+};
 
 struct cxacru_modem_type {
 	u32 pll_f_clk;
@@ -158,8 +166,11 @@ struct cxacru_data {
 	const struct cxacru_modem_type *modem_type;
 
 	int line_status;
+	int adsl_status;
 	struct delayed_work poll_work;
 	u32 card_info[CXINF_MAX];
+	struct mutex poll_state_serialize;
+	int poll_state;
 
 	/* contol handles */
 	struct mutex cm_serialize;
@@ -171,10 +182,18 @@ struct cxacru_data {
 	struct completion snd_done;
 };
 
+static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
+	u8 *wdata, int wsize, u8 *rdata, int rsize);
+static void cxacru_poll_status(struct work_struct *work);
+
 /* Card info exported through sysfs */
 #define CXACRU__ATTR_INIT(_name) \
 static DEVICE_ATTR(_name, S_IRUGO, cxacru_sysfs_show_##_name, NULL)
 
+#define CXACRU_CMD_INIT(_name) \
+static DEVICE_ATTR(_name, S_IWUSR | S_IRUGO, \
+	cxacru_sysfs_show_##_name, cxacru_sysfs_store_##_name)
+
 #define CXACRU_ATTR_INIT(_value, _type, _name) \
 static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
 	struct device_attribute *attr, char *buf) \
@@ -187,9 +206,11 @@ static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
 CXACRU__ATTR_INIT(_name)
 
 #define CXACRU_ATTR_CREATE(_v, _t, _name) CXACRU_DEVICE_CREATE_FILE(_name)
+#define CXACRU_CMD_CREATE(_name)          CXACRU_DEVICE_CREATE_FILE(_name)
 #define CXACRU__ATTR_CREATE(_name)        CXACRU_DEVICE_CREATE_FILE(_name)
 
 #define CXACRU_ATTR_REMOVE(_v, _t, _name) CXACRU_DEVICE_REMOVE_FILE(_name)
+#define CXACRU_CMD_REMOVE(_name)          CXACRU_DEVICE_REMOVE_FILE(_name)
 #define CXACRU__ATTR_REMOVE(_name)        CXACRU_DEVICE_REMOVE_FILE(_name)
 
 static ssize_t cxacru_sysfs_showattr_u32(u32 value, char *buf)
@@ -278,6 +299,105 @@ static ssize_t cxacru_sysfs_show_mac_address(struct device *dev,
 			atm_dev->esi[3], atm_dev->esi[4], atm_dev->esi[5]);
 }
 
+static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
+	struct cxacru_data *instance = usbatm_instance->driver_data;
+	u32 value = instance->card_info[CXINF_LINE_STARTABLE];
+
+	switch (value) {
+	case 0: return snprintf(buf, PAGE_SIZE, "running\n");
+	case 1: return snprintf(buf, PAGE_SIZE, "stopped\n");
+	default: return snprintf(buf, PAGE_SIZE, "unknown (%u)\n", value);
+	}
+}
+
+static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
+	struct cxacru_data *instance = usbatm_instance->driver_data;
+	int ret = 0;
+	int poll = -1;
+
+	if (!capable(CAP_NET_ADMIN))
+		return -EACCES;
+
+	if (!strcmp(buf, "stop") || !strcmp(buf, "restart")) {
+		ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_STOP, NULL, 0, NULL, 0);
+		if (ret < 0) {
+			atm_err(usbatm_instance, "change adsl state:"
+				" CHIP_ADSL_LINE_STOP returned %d\n", ret);
+
+			ret = -EIO;
+		} else {
+			ret = strlen(buf);
+			poll = CXPOLL_STOPPED;
+		}
+	}
+
+	/* Line status is only updated every second
+	 * and the device appears to only react to
+	 * START/STOP every second too. Wait 1.5s to
+	 * be sure that restart will have an effect. */
+	if (!strcmp(buf, "restart"))
+		msleep(1500);
+
+	if (!strcmp(buf, "start") || !strcmp(buf, "restart")) {
+		ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
+		if (ret < 0) {
+			atm_err(usbatm_instance, "change adsl state:"
+				" CHIP_ADSL_LINE_START returned %d\n", ret);
+
+			ret = -EIO;
+		} else {
+			ret = strlen(buf);
+			poll = CXPOLL_POLLING;
+		}
+	}
+
+	if (!strcmp(buf, "poll")) {
+		ret = strlen(buf);
+		poll = CXPOLL_POLLING;
+	}
+
+	if (!ret)
+		return -EINVAL;
+
+	if (poll == CXPOLL_POLLING) {
+		mutex_lock(&instance->poll_state_serialize);
+		switch (instance->poll_state) {
+		case CXPOLL_STOPPED:
+			/* start polling */
+			instance->poll_state = CXPOLL_POLLING;
+			break;
+
+		case CXPOLL_STOPPING:
+			/* abort stop request */
+			instance->poll_state = CXPOLL_POLLING;
+		case CXPOLL_POLLING:
+		case CXPOLL_SHUTDOWN:
+			/* don't start polling */
+			poll = -1;
+		}
+		mutex_unlock(&instance->poll_state_serialize);
+	} else if (poll == CXPOLL_STOPPED) {
+		mutex_lock(&instance->poll_state_serialize);
+		/* request stop */
+		if (instance->poll_state == CXPOLL_POLLING)
+			instance->poll_state = CXPOLL_STOPPING;
+		mutex_unlock(&instance->poll_state_serialize);
+	}
+
+	if (poll == CXPOLL_POLLING)
+		cxacru_poll_status(&instance->poll_work.work);
+
+	return ret;
+}
+
 /*
  * All device attributes are included in CXACRU_ALL_FILES
  * so that the same list can be used multiple times:
@@ -312,7 +432,8 @@ CXACRU_ATTR_##_action(CXINF_LINE_STARTABLE,            bool, line_startable); \
 CXACRU_ATTR_##_action(CXINF_MODULATION,                MODU, modulation); \
 CXACRU_ATTR_##_action(CXINF_ADSL_HEADEND,              u32,  adsl_headend); \
 CXACRU_ATTR_##_action(CXINF_ADSL_HEADEND_ENVIRONMENT,  u32,  adsl_headend_environment); \
-CXACRU_ATTR_##_action(CXINF_CONTROLLER_VERSION,        u32,  adsl_controller_version);
+CXACRU_ATTR_##_action(CXINF_CONTROLLER_VERSION,        u32,  adsl_controller_version); \
+CXACRU_CMD_##_action(                                        adsl_state);
 
 CXACRU_ALL_FILES(INIT);
 
@@ -493,8 +614,6 @@ static int cxacru_card_status(struct cxacru_data *instance)
 	return 0;
 }
 
-static void cxacru_poll_status(struct work_struct *work);
-
 static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 		struct atm_dev *atm_dev)
 {
@@ -503,6 +622,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
 	*/
 	int ret;
+	int start_polling = 1;
 
 	dbg("cxacru_atm_start");
 
@@ -522,7 +642,25 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	}
 
 	/* Start status polling */
-	cxacru_poll_status(&instance->poll_work.work);
+	mutex_lock(&instance->poll_state_serialize);
+	switch (instance->poll_state) {
+	case CXPOLL_STOPPED:
+		/* start polling */
+		instance->poll_state = CXPOLL_POLLING;
+		break;
+
+	case CXPOLL_STOPPING:
+		/* abort stop request */
+		instance->poll_state = CXPOLL_POLLING;
+	case CXPOLL_POLLING:
+	case CXPOLL_SHUTDOWN:
+		/* don't start polling */
+		start_polling = 0;
+	}
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (start_polling)
+		cxacru_poll_status(&instance->poll_work.work);
 	return 0;
 }
 
@@ -533,16 +671,46 @@ static void cxacru_poll_status(struct work_struct *work)
 	u32 buf[CXINF_MAX] = {};
 	struct usbatm_data *usbatm = instance->usbatm;
 	struct atm_dev *atm_dev = usbatm->atm_dev;
+	int keep_polling = 1;
 	int ret;
 
 	ret = cxacru_cm_get_array(instance, CM_REQUEST_CARD_INFO_GET, buf, CXINF_MAX);
 	if (ret < 0) {
-		atm_warn(usbatm, "poll status: error %d\n", ret);
+		if (ret != -ESHUTDOWN)
+			atm_warn(usbatm, "poll status: error %d\n", ret);
+		
+		mutex_lock(&instance->poll_state_serialize);
+		if (instance->poll_state != CXPOLL_SHUTDOWN) {
+			instance->poll_state = CXPOLL_STOPPED;
+
+			if (ret != -ESHUTDOWN)
+				atm_warn(usbatm, "polling disabled, set adsl_state"
+						" to 'start' or 'poll' to resume\n");
+		}
+		mutex_unlock(&instance->poll_state_serialize);
 		goto reschedule;
 	}
 
 	memcpy(instance->card_info, buf, sizeof(instance->card_info));
 
+	if (instance->adsl_status != buf[CXINF_LINE_STARTABLE]) {
+		instance->adsl_status = buf[CXINF_LINE_STARTABLE];
+
+		switch (instance->adsl_status) {
+		case 0:
+			atm_printk(KERN_INFO, usbatm, "ADSL state: running\n");
+			break;
+
+		case 1:
+			atm_printk(KERN_INFO, usbatm, "ADSL state: stopped\n");
+			break;
+
+		default:
+			atm_printk(KERN_INFO, usbatm, "Unknown adsl status %02x\n", instance->adsl_status);
+			break;
+		}
+	}
+
 	if (instance->line_status == buf[CXINF_LINE_STATUS])
 		goto reschedule;
 
@@ -597,8 +765,20 @@ static void cxacru_poll_status(struct work_struct *work)
 		break;
 	}
 reschedule:
-	schedule_delayed_work(&instance->poll_work,
-			round_jiffies_relative(msecs_to_jiffies(POLL_INTERVAL*1000)));
+
+	mutex_lock(&instance->poll_state_serialize);
+	if (instance->poll_state == CXPOLL_STOPPING &&
+				instance->adsl_status == 1 && /* stopped */
+				instance->line_status == 0) /* down */
+		instance->poll_state = CXPOLL_STOPPED;
+
+	if (instance->poll_state == CXPOLL_STOPPED)
+		keep_polling = 0;
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (keep_polling)
+		schedule_delayed_work(&instance->poll_work,
+				round_jiffies_relative(POLL_INTERVAL*HZ));
 }
 
 static int cxacru_fw(struct usb_device *usb_dev, enum cxacru_fw_request fw,
@@ -834,6 +1014,10 @@ static int cxacru_bind(struct usbatm_data *usbatm_instance,
 	instance->usbatm = usbatm_instance;
 	instance->modem_type = (struct cxacru_modem_type *) id->driver_info;
 	memset(instance->card_info, 0, sizeof(instance->card_info));
+	mutex_init(&instance->poll_state_serialize);
+	instance->poll_state = CXPOLL_STOPPED;
+	instance->line_status = -1;
+	instance->adsl_status = -1;
 
 	instance->rcv_buf = (u8 *) __get_free_page(GFP_KERNEL);
 	if (!instance->rcv_buf) {
@@ -909,6 +1093,7 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
 		struct usb_interface *intf)
 {
 	struct cxacru_data *instance = usbatm_instance->driver_data;
+	int is_polling = 1;
 
 	dbg("cxacru_unbind entered");
 
@@ -917,8 +1102,20 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
 		return;
 	}
 
-	while (!cancel_delayed_work(&instance->poll_work))
-	       flush_scheduled_work();
+	mutex_lock(&instance->poll_state_serialize);
+	BUG_ON(instance->poll_state == CXPOLL_SHUTDOWN);
+
+	/* ensure that status polling continues unless
+	 * it has already stopped */
+	if (instance->poll_state == CXPOLL_STOPPED)
+		is_polling = 0;
+		
+	/* stop polling from being stopped or started */
+	instance->poll_state = CXPOLL_SHUTDOWN;
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (is_polling)
+		cancel_rearming_delayed_work(&instance->poll_work);
 
 	usb_kill_urb(instance->snd_urb);
 	usb_kill_urb(instance->rcv_urb);
-- 
1.5.0.1

-- 
Simon Arlott

Re: [PATCH] cxacru: ADSL state management

[Duncan Sands]

Hi Simon,

> +static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
> +	struct cxacru_data *instance = usbatm_instance->driver_data;

instance could be NULL if cxacru_sysfs_show_adsl_state is called after
the device has been unbound.  What prevents this?

> +static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)

This is not serialized by a lock, so if two or more processes call it
simultaneously then the commands to the modem can be interleaved, which
could (a) leave it in a funky state, and (b) means that the value of poll
could be wrong when you write it to instance->poll_state, eg because the
processes could reach the poll_state writing step in reverse order to the
order in which they reached the sending-commands-to-the-modem step.

> @@ -503,6 +622,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
>  	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
>  	*/
>  	int ret;
> +	int start_polling = 1;
>  
>  	dbg("cxacru_atm_start");
>  
> @@ -522,7 +642,25 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
>  	}
>  
>  	/* Start status polling */
> -	cxacru_poll_status(&instance->poll_work.work);
> +	mutex_lock(&instance->poll_state_serialize);
> +	switch (instance->poll_state) {

Can poll_state really have anything else than its initial
value here?

> +	mutex_unlock(&instance->poll_state_serialize);
> +
> +	if (start_polling)
> +		cxacru_poll_status(&instance->poll_work.work);

Between releasing the lock and calling cxacru_poll_status, another
process could have called cxacru_sysfs_store_adsl_state and changed
whether you want to poll or not.  I don't know if it matters.

> +	mutex_unlock(&instance->poll_state_serialize);
> +
> +	if (keep_polling)
> +		schedule_delayed_work(&instance->poll_work,
> +				round_jiffies_relative(POLL_INTERVAL*HZ));

Likewise.

> @@ -917,8 +1102,20 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
>  		return;
>  	}
>  
> -	while (!cancel_delayed_work(&instance->poll_work))
> -	       flush_scheduled_work();
> +	mutex_lock(&instance->poll_state_serialize);
> +	BUG_ON(instance->poll_state == CXPOLL_SHUTDOWN);
> +
> +	/* ensure that status polling continues unless
> +	 * it has already stopped */

Your device has been removed.  Why would you want to go on
polling?  Especially as your device structures are about to
be freed...  By the way, where do you remove the sysfs files?

> +	mutex_unlock(&instance->poll_state_serialize);
> +
> +	if (is_polling)
> +		cancel_rearming_delayed_work(&instance->poll_work);

This looks unreliable, again due to race conditions.  Better to always
shoot it down.

Ciao,

Duncan.

Re: [PATCH] cxacru: ADSL state management

[Simon Arlott]

On 15/04/07 11:08, Duncan Sands wrote:
> Hi Simon,
> 
>> +static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
>> +	struct device_attribute *attr, char *buf)
>> +{
>> +	struct usb_interface *intf = to_usb_interface(dev);
>> +	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
>> +	struct cxacru_data *instance = usbatm_instance->driver_data;
> 
> instance could be NULL if cxacru_sysfs_show_adsl_state is called after
> the device has been unbound.  What prevents this?

When read() or write() is used after sysfs_remove_file(), it doesn't call 
the function and returns -ENODEV. I would assume that it will wait for a 
running show() or store() before leaving sysfs_remove_file() too - if it 
doesn't then it should...

>> +static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t count)
> 
> This is not serialized by a lock, so if two or more processes call it
> simultaneously then the commands to the modem can be interleaved, which
> could (a) leave it in a funky state, and (b) means that the value of poll

a) The commands sent to the modem are serialised so there's no problem 
there.

> could be wrong when you write it to instance->poll_state, eg because the
> processes could reach the poll_state writing step in reverse order to the
> order in which they reached the sending-commands-to-the-modem step.

b) You're right here, I'll need to add a mutex for that whole function.

>> @@ -503,6 +622,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
>>  	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
>>  	*/
>>  	int ret;
>> +	int start_polling = 1;
>>  
>>  	dbg("cxacru_atm_start");
>>  
>> @@ -522,7 +642,25 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
>>  	}
>>  
>>  	/* Start status polling */
>> -	cxacru_poll_status(&instance->poll_work.work);
>> +	mutex_lock(&instance->poll_state_serialize);
>> +	switch (instance->poll_state) {
> 
> Can poll_state really have anything else than its initial
> value here?

Yes, the sysfs attributes are set up in bind so the user can change the polling before 
atm_start is called. Also the device could be removed while usbatm is still preparing 
to call atm_start.

>> +	mutex_unlock(&instance->poll_state_serialize);
>> +
>> +	if (start_polling)
>> +		cxacru_poll_status(&instance->poll_work.work);
> 
> Between releasing the lock and calling cxacru_poll_status, another
> process could have called cxacru_sysfs_store_adsl_state and changed
> whether you want to poll or not.  I don't know if it matters.

Yes, this could happen and I designed it so that whichever process sets it to POLLING 
MUST then start polling - if something else wants it to stop it will stop at the end 
of the poll function. The only way to allow something other than the poll function to 
stop it would be to reschedule with the mutex held which I'd rather avoid doing.
 
>> +	mutex_unlock(&instance->poll_state_serialize);
>> +
>> +	if (keep_polling)
>> +		schedule_delayed_work(&instance->poll_work,
>> +				round_jiffies_relative(POLL_INTERVAL*HZ));
> 
> Likewise.

Again, it will be stopped at the next poll - it's safer than trying to cancel work 
that may not be scheduled.
 
>> @@ -917,8 +1102,20 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
>>  		return;
>>  	}
>>  
>> -	while (!cancel_delayed_work(&instance->poll_work))
>> -	       flush_scheduled_work();
>> +	mutex_lock(&instance->poll_state_serialize);
>> +	BUG_ON(instance->poll_state == CXPOLL_SHUTDOWN);
>> +
>> +	/* ensure that status polling continues unless
>> +	 * it has already stopped */
> 
> Your device has been removed.  Why would you want to go on
> polling?  Especially as your device structures are about to
> be freed...  By the way, where do you remove the sysfs files?

They are removed further down in unbind. Polling is safe to continue because 
it is stopped below, and it must continue because:

>> +	mutex_unlock(&instance->poll_state_serialize);
>> +
>> +	if (is_polling)
>> +		cancel_rearming_delayed_work(&instance->poll_work);
> 
> This looks unreliable, again due to race conditions.  Better to always
> shoot it down.

Cancelling the work causes an infinite loop if the polling is never rescheduled, 
I've written it so that the polling is always either enabled/disabled at this 
point. The SHUTDOWN state prevents anything else changing it.

If it's scheduled to run then it will be cancelled, and if it's running it will 
be cancelled as soon as it gets rescheduled.

I could repeatedly acquire the mutex and check if it's shutdown itself, but 
there would be no point since it won't be any faster.

> Ciao,
> 
> Duncan.


-- 
Simon Arlott

Re: [PATCH (rev 2)] cxacru: ADSL state management

[Simon Arlott]

The device has commands to start/stop the ADSL function, so this adds 
a sysfs attribute to allow it to be started/stopped/restarted. It also 
stops polling the device for status when the ADSL function is disabled.

There are no problems with sending multiple start or stop commands, 
even with a fast loop of them the device still works.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Duncan Sands <duncan.sands@math.u-psud.fr>
---
This patch requires usb-cxacru-export-detailed-device-info-through-sysfs 
from gregkh-2.6.

Revision 2: Add adsl_state_serialize mutex and use it around ADSL 
start/stop commands followed by deciding on the poll state.

 drivers/usb/atm/cxacru.c |  226 ++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 217 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index c8b69bf..f203507 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2004 David Woodhouse, Duncan Sands, Roman Kagan
  *  Copyright (C) 2005 Duncan Sands, Roman Kagan (rkagan % mail ! ru)
+ *  Copyright (C) 2007 Simon Arlott
  *
  *  This program is free software; you can redistribute it and/or modify it
  *  under the terms of the GNU General Public License as published by the Free
@@ -145,6 +146,13 @@ enum cxacru_info_idx {
 	/* dunno what the missing two mean */
 	CXINF_MAX = 0x1c,
 };
+ 
+enum cxacru_poll_state {
+	CXPOLL_STOPPING,
+	CXPOLL_STOPPED,
+	CXPOLL_POLLING,
+	CXPOLL_SHUTDOWN
+};
 
 struct cxacru_modem_type {
 	u32 pll_f_clk;
@@ -158,8 +166,12 @@ struct cxacru_data {
 	const struct cxacru_modem_type *modem_type;
 
 	int line_status;
+	struct mutex adsl_state_serialize;
+	int adsl_status;
 	struct delayed_work poll_work;
 	u32 card_info[CXINF_MAX];
+	struct mutex poll_state_serialize;
+	int poll_state;
 
 	/* contol handles */
 	struct mutex cm_serialize;
@@ -171,10 +183,18 @@ struct cxacru_data {
 	struct completion snd_done;
 };
 
+static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
+	u8 *wdata, int wsize, u8 *rdata, int rsize);
+static void cxacru_poll_status(struct work_struct *work);
+
 /* Card info exported through sysfs */
 #define CXACRU__ATTR_INIT(_name) \
 static DEVICE_ATTR(_name, S_IRUGO, cxacru_sysfs_show_##_name, NULL)
 
+#define CXACRU_CMD_INIT(_name) \
+static DEVICE_ATTR(_name, S_IWUSR | S_IRUGO, \
+	cxacru_sysfs_show_##_name, cxacru_sysfs_store_##_name)
+
 #define CXACRU_ATTR_INIT(_value, _type, _name) \
 static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
 	struct device_attribute *attr, char *buf) \
@@ -187,9 +207,11 @@ static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
 CXACRU__ATTR_INIT(_name)
 
 #define CXACRU_ATTR_CREATE(_v, _t, _name) CXACRU_DEVICE_CREATE_FILE(_name)
+#define CXACRU_CMD_CREATE(_name)          CXACRU_DEVICE_CREATE_FILE(_name)
 #define CXACRU__ATTR_CREATE(_name)        CXACRU_DEVICE_CREATE_FILE(_name)
 
 #define CXACRU_ATTR_REMOVE(_v, _t, _name) CXACRU_DEVICE_REMOVE_FILE(_name)
+#define CXACRU_CMD_REMOVE(_name)          CXACRU_DEVICE_REMOVE_FILE(_name)
 #define CXACRU__ATTR_REMOVE(_name)        CXACRU_DEVICE_REMOVE_FILE(_name)
 
 static ssize_t cxacru_sysfs_showattr_u32(u32 value, char *buf)
@@ -278,6 +300,110 @@ static ssize_t cxacru_sysfs_show_mac_address(struct device *dev,
 			atm_dev->esi[3], atm_dev->esi[4], atm_dev->esi[5]);
 }
 
+static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
+	struct cxacru_data *instance = usbatm_instance->driver_data;
+	u32 value = instance->card_info[CXINF_LINE_STARTABLE];
+
+	switch (value) {
+	case 0: return snprintf(buf, PAGE_SIZE, "running\n");
+	case 1: return snprintf(buf, PAGE_SIZE, "stopped\n");
+	default: return snprintf(buf, PAGE_SIZE, "unknown (%u)\n", value);
+	}
+}
+
+static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
+	struct cxacru_data *instance = usbatm_instance->driver_data;
+	int ret = 0;
+	int poll = -1;
+
+	if (!capable(CAP_NET_ADMIN))
+		return -EACCES;
+
+	if (mutex_lock_interruptible(&instance->adsl_state_serialize))
+		return -ERESTARTSYS;
+
+	if (!strcmp(buf, "stop") || !strcmp(buf, "restart")) {
+		ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_STOP, NULL, 0, NULL, 0);
+		if (ret < 0) {
+			atm_err(usbatm_instance, "change adsl state:"
+				" CHIP_ADSL_LINE_STOP returned %d\n", ret);
+
+			ret = -EIO;
+		} else {
+			ret = strlen(buf);
+			poll = CXPOLL_STOPPED;
+		}
+	}
+
+	/* Line status is only updated every second
+	 * and the device appears to only react to
+	 * START/STOP every second too. Wait 1.5s to
+	 * be sure that restart will have an effect. */
+	if (!strcmp(buf, "restart"))
+		msleep(1500);
+
+	if (!strcmp(buf, "start") || !strcmp(buf, "restart")) {
+		ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
+		if (ret < 0) {
+			atm_err(usbatm_instance, "change adsl state:"
+				" CHIP_ADSL_LINE_START returned %d\n", ret);
+
+			ret = -EIO;
+		} else {
+			ret = strlen(buf);
+			poll = CXPOLL_POLLING;
+		}
+	}
+
+	if (!strcmp(buf, "poll")) {
+		ret = strlen(buf);
+		poll = CXPOLL_POLLING;
+	}
+
+	if (!ret)
+		return -EINVAL;
+
+	if (poll == CXPOLL_POLLING) {
+		mutex_lock(&instance->poll_state_serialize);
+		switch (instance->poll_state) {
+		case CXPOLL_STOPPED:
+			/* start polling */
+			instance->poll_state = CXPOLL_POLLING;
+			break;
+
+		case CXPOLL_STOPPING:
+			/* abort stop request */
+			instance->poll_state = CXPOLL_POLLING;
+		case CXPOLL_POLLING:
+		case CXPOLL_SHUTDOWN:
+			/* don't start polling */
+			poll = -1;
+		}
+		mutex_unlock(&instance->poll_state_serialize);
+	} else if (poll == CXPOLL_STOPPED) {
+		mutex_lock(&instance->poll_state_serialize);
+		/* request stop */
+		if (instance->poll_state == CXPOLL_POLLING)
+			instance->poll_state = CXPOLL_STOPPING;
+		mutex_unlock(&instance->poll_state_serialize);
+	}
+
+	mutex_unlock(&instance->adsl_state_serialize);
+
+	if (poll == CXPOLL_POLLING)
+		cxacru_poll_status(&instance->poll_work.work);
+
+	return ret;
+}
+
 /*
  * All device attributes are included in CXACRU_ALL_FILES
  * so that the same list can be used multiple times:
@@ -312,7 +438,8 @@ CXACRU_ATTR_##_action(CXINF_LINE_STARTABLE,            bool, line_startable); \
 CXACRU_ATTR_##_action(CXINF_MODULATION,                MODU, modulation); \
 CXACRU_ATTR_##_action(CXINF_ADSL_HEADEND,              u32,  adsl_headend); \
 CXACRU_ATTR_##_action(CXINF_ADSL_HEADEND_ENVIRONMENT,  u32,  adsl_headend_environment); \
-CXACRU_ATTR_##_action(CXINF_CONTROLLER_VERSION,        u32,  adsl_controller_version);
+CXACRU_ATTR_##_action(CXINF_CONTROLLER_VERSION,        u32,  adsl_controller_version); \
+CXACRU_CMD_##_action(                                        adsl_state);
 
 CXACRU_ALL_FILES(INIT);
 
@@ -493,8 +620,6 @@ static int cxacru_card_status(struct cxacru_data *instance)
 	return 0;
 }
 
-static void cxacru_poll_status(struct work_struct *work);
-
 static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 		struct atm_dev *atm_dev)
 {
@@ -503,6 +628,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
 	*/
 	int ret;
+	int start_polling = 1;
 
 	dbg("cxacru_atm_start");
 
@@ -515,6 +641,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	}
 
 	/* start ADSL */
+	mutex_lock(&instance->adsl_state_serialize);
 	ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
 	if (ret < 0) {
 		atm_err(usbatm_instance, "cxacru_atm_start: CHIP_ADSL_LINE_START returned %d\n", ret);
@@ -522,7 +649,26 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	}
 
 	/* Start status polling */
-	cxacru_poll_status(&instance->poll_work.work);
+	mutex_lock(&instance->poll_state_serialize);
+	switch (instance->poll_state) {
+	case CXPOLL_STOPPED:
+		/* start polling */
+		instance->poll_state = CXPOLL_POLLING;
+		break;
+
+	case CXPOLL_STOPPING:
+		/* abort stop request */
+		instance->poll_state = CXPOLL_POLLING;
+	case CXPOLL_POLLING:
+	case CXPOLL_SHUTDOWN:
+		/* don't start polling */
+		start_polling = 0;
+	}
+	mutex_unlock(&instance->poll_state_serialize);
+	mutex_unlock(&instance->adsl_state_serialize);
+
+	if (start_polling)
+		cxacru_poll_status(&instance->poll_work.work);
 	return 0;
 }
 
@@ -533,16 +679,46 @@ static void cxacru_poll_status(struct work_struct *work)
 	u32 buf[CXINF_MAX] = {};
 	struct usbatm_data *usbatm = instance->usbatm;
 	struct atm_dev *atm_dev = usbatm->atm_dev;
+	int keep_polling = 1;
 	int ret;
 
 	ret = cxacru_cm_get_array(instance, CM_REQUEST_CARD_INFO_GET, buf, CXINF_MAX);
 	if (ret < 0) {
-		atm_warn(usbatm, "poll status: error %d\n", ret);
+		if (ret != -ESHUTDOWN)
+			atm_warn(usbatm, "poll status: error %d\n", ret);
+		
+		mutex_lock(&instance->poll_state_serialize);
+		if (instance->poll_state != CXPOLL_SHUTDOWN) {
+			instance->poll_state = CXPOLL_STOPPED;
+
+			if (ret != -ESHUTDOWN)
+				atm_warn(usbatm, "polling disabled, set adsl_state"
+						" to 'start' or 'poll' to resume\n");
+		}
+		mutex_unlock(&instance->poll_state_serialize);
 		goto reschedule;
 	}
 
 	memcpy(instance->card_info, buf, sizeof(instance->card_info));
 
+	if (instance->adsl_status != buf[CXINF_LINE_STARTABLE]) {
+		instance->adsl_status = buf[CXINF_LINE_STARTABLE];
+
+		switch (instance->adsl_status) {
+		case 0:
+			atm_printk(KERN_INFO, usbatm, "ADSL state: running\n");
+			break;
+
+		case 1:
+			atm_printk(KERN_INFO, usbatm, "ADSL state: stopped\n");
+			break;
+
+		default:
+			atm_printk(KERN_INFO, usbatm, "Unknown adsl status %02x\n", instance->adsl_status);
+			break;
+		}
+	}
+
 	if (instance->line_status == buf[CXINF_LINE_STATUS])
 		goto reschedule;
 
@@ -597,8 +773,20 @@ static void cxacru_poll_status(struct work_struct *work)
 		break;
 	}
 reschedule:
-	schedule_delayed_work(&instance->poll_work,
-			round_jiffies_relative(msecs_to_jiffies(POLL_INTERVAL*1000)));
+
+	mutex_lock(&instance->poll_state_serialize);
+	if (instance->poll_state == CXPOLL_STOPPING &&
+				instance->adsl_status == 1 && /* stopped */
+				instance->line_status == 0) /* down */
+		instance->poll_state = CXPOLL_STOPPED;
+
+	if (instance->poll_state == CXPOLL_STOPPED)
+		keep_polling = 0;
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (keep_polling)
+		schedule_delayed_work(&instance->poll_work,
+				round_jiffies_relative(POLL_INTERVAL*HZ));
 }
 
 static int cxacru_fw(struct usb_device *usb_dev, enum cxacru_fw_request fw,
@@ -835,6 +1023,13 @@ static int cxacru_bind(struct usbatm_data *usbatm_instance,
 	instance->modem_type = (struct cxacru_modem_type *) id->driver_info;
 	memset(instance->card_info, 0, sizeof(instance->card_info));
 
+	mutex_init(&instance->poll_state_serialize);
+	instance->poll_state = CXPOLL_STOPPED;
+	instance->line_status = -1;
+	instance->adsl_status = -1;
+
+	mutex_init(&instance->adsl_state_serialize);
+
 	instance->rcv_buf = (u8 *) __get_free_page(GFP_KERNEL);
 	if (!instance->rcv_buf) {
 		dbg("cxacru_bind: no memory for rcv_buf");
@@ -909,6 +1104,7 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
 		struct usb_interface *intf)
 {
 	struct cxacru_data *instance = usbatm_instance->driver_data;
+	int is_polling = 1;
 
 	dbg("cxacru_unbind entered");
 
@@ -917,8 +1113,20 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
 		return;
 	}
 
-	while (!cancel_delayed_work(&instance->poll_work))
-	       flush_scheduled_work();
+	mutex_lock(&instance->poll_state_serialize);
+	BUG_ON(instance->poll_state == CXPOLL_SHUTDOWN);
+
+	/* ensure that status polling continues unless
+	 * it has already stopped */
+	if (instance->poll_state == CXPOLL_STOPPED)
+		is_polling = 0;
+		
+	/* stop polling from being stopped or started */
+	instance->poll_state = CXPOLL_SHUTDOWN;
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (is_polling)
+		cancel_rearming_delayed_work(&instance->poll_work);
 
 	usb_kill_urb(instance->snd_urb);
 	usb_kill_urb(instance->rcv_urb);
-- 
1.5.0.1

-- 
Simon Arlott

Re: [PATCH (rev 3)] cxacru: ADSL state management

[Simon Arlott]

The device has commands to start/stop the ADSL function, so this adds a 
sysfs attribute to allow it to be started/stopped/restarted. It also 
stops polling the device for status when the ADSL function is disabled.

There are no problems with sending multiple start or stop commands, even 
with a fast loop of them the device still works.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Duncan Sands <duncan.sands@math.u-psud.fr>
---
This patch requires usb-cxacru-export-detailed-device-info-through-sysfs 
from gregkh-2.6.

Revision 2: Add adsl_state_serialize mutex and use it around ADSL 
start/stop commands followed by deciding on the poll state.

Revision 3: Fix some stupid return while locked bugs and mishandling of 
-EIO return value. Allow newlines etc. at the end of adsl_state commands.

 drivers/usb/atm/cxacru.c |  236 ++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 227 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index c8b69bf..0c47755 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2004 David Woodhouse, Duncan Sands, Roman Kagan
  *  Copyright (C) 2005 Duncan Sands, Roman Kagan (rkagan % mail ! ru)
+ *  Copyright (C) 2007 Simon Arlott
  *
  *  This program is free software; you can redistribute it and/or modify it
  *  under the terms of the GNU General Public License as published by the Free
@@ -145,6 +146,13 @@ enum cxacru_info_idx {
 	/* dunno what the missing two mean */
 	CXINF_MAX = 0x1c,
 };
+ 
+enum cxacru_poll_state {
+	CXPOLL_STOPPING,
+	CXPOLL_STOPPED,
+	CXPOLL_POLLING,
+	CXPOLL_SHUTDOWN
+};
 
 struct cxacru_modem_type {
 	u32 pll_f_clk;
@@ -158,8 +166,12 @@ struct cxacru_data {
 	const struct cxacru_modem_type *modem_type;
 
 	int line_status;
+	struct mutex adsl_state_serialize;
+	int adsl_status;
 	struct delayed_work poll_work;
 	u32 card_info[CXINF_MAX];
+	struct mutex poll_state_serialize;
+	int poll_state;
 
 	/* contol handles */
 	struct mutex cm_serialize;
@@ -171,10 +183,18 @@ struct cxacru_data {
 	struct completion snd_done;
 };
 
+static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
+	u8 *wdata, int wsize, u8 *rdata, int rsize);
+static void cxacru_poll_status(struct work_struct *work);
+
 /* Card info exported through sysfs */
 #define CXACRU__ATTR_INIT(_name) \
 static DEVICE_ATTR(_name, S_IRUGO, cxacru_sysfs_show_##_name, NULL)
 
+#define CXACRU_CMD_INIT(_name) \
+static DEVICE_ATTR(_name, S_IWUSR | S_IRUGO, \
+	cxacru_sysfs_show_##_name, cxacru_sysfs_store_##_name)
+
 #define CXACRU_ATTR_INIT(_value, _type, _name) \
 static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
 	struct device_attribute *attr, char *buf) \
@@ -187,9 +207,11 @@ static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
 CXACRU__ATTR_INIT(_name)
 
 #define CXACRU_ATTR_CREATE(_v, _t, _name) CXACRU_DEVICE_CREATE_FILE(_name)
+#define CXACRU_CMD_CREATE(_name)          CXACRU_DEVICE_CREATE_FILE(_name)
 #define CXACRU__ATTR_CREATE(_name)        CXACRU_DEVICE_CREATE_FILE(_name)
 
 #define CXACRU_ATTR_REMOVE(_v, _t, _name) CXACRU_DEVICE_REMOVE_FILE(_name)
+#define CXACRU_CMD_REMOVE(_name)          CXACRU_DEVICE_REMOVE_FILE(_name)
 #define CXACRU__ATTR_REMOVE(_name)        CXACRU_DEVICE_REMOVE_FILE(_name)
 
 static ssize_t cxacru_sysfs_showattr_u32(u32 value, char *buf)
@@ -278,6 +300,119 @@ static ssize_t cxacru_sysfs_show_mac_address(struct device *dev,
 			atm_dev->esi[3], atm_dev->esi[4], atm_dev->esi[5]);
 }
 
+static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
+	struct cxacru_data *instance = usbatm_instance->driver_data;
+	u32 value = instance->card_info[CXINF_LINE_STARTABLE];
+
+	switch (value) {
+	case 0: return snprintf(buf, PAGE_SIZE, "running\n");
+	case 1: return snprintf(buf, PAGE_SIZE, "stopped\n");
+	default: return snprintf(buf, PAGE_SIZE, "unknown (%u)\n", value);
+	}
+}
+
+static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
+	struct cxacru_data *instance = usbatm_instance->driver_data;
+	int ret;
+	int poll = -1;
+	char str_cmd[8];
+	int len = strlen(buf);
+
+	if (!capable(CAP_NET_ADMIN))
+		return -EACCES;
+
+	ret = sscanf(buf, "%7s", str_cmd);
+	if (ret != 1)
+		return -EINVAL;
+	ret = 0;
+
+	if (mutex_lock_interruptible(&instance->adsl_state_serialize))
+		return -ERESTARTSYS;
+
+	if (!strcmp(str_cmd, "stop") || !strcmp(str_cmd, "restart")) {
+		ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_STOP, NULL, 0, NULL, 0);
+		if (ret < 0) {
+			atm_err(usbatm_instance, "change adsl state:"
+				" CHIP_ADSL_LINE_STOP returned %d\n", ret);
+
+			ret = -EIO;
+		} else {
+			ret = len;
+			poll = CXPOLL_STOPPED;
+		}
+	}
+
+	/* Line status is only updated every second
+	 * and the device appears to only react to
+	 * START/STOP every second too. Wait 1.5s to
+	 * be sure that restart will have an effect. */
+	if (!strcmp(str_cmd, "restart"))
+		msleep(1500);
+
+	if (!strcmp(str_cmd, "start") || !strcmp(str_cmd, "restart")) {
+		ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
+		if (ret < 0) {
+			atm_err(usbatm_instance, "change adsl state:"
+				" CHIP_ADSL_LINE_START returned %d\n", ret);
+
+			ret = -EIO;
+		} else {
+			ret = len;
+			poll = CXPOLL_POLLING;
+		}
+	}
+
+	if (!strcmp(str_cmd, "poll")) {
+		ret = len;
+		poll = CXPOLL_POLLING;
+	}
+
+	if (ret == 0) {
+		ret = -EINVAL;
+		poll = -1;
+	}
+
+	if (poll == CXPOLL_POLLING) {
+		mutex_lock(&instance->poll_state_serialize);
+		switch (instance->poll_state) {
+		case CXPOLL_STOPPED:
+			/* start polling */
+			instance->poll_state = CXPOLL_POLLING;
+			break;
+
+		case CXPOLL_STOPPING:
+			/* abort stop request */
+			instance->poll_state = CXPOLL_POLLING;
+		case CXPOLL_POLLING:
+		case CXPOLL_SHUTDOWN:
+			/* don't start polling */
+			poll = -1;
+		}
+		mutex_unlock(&instance->poll_state_serialize);
+	} else if (poll == CXPOLL_STOPPED) {
+		mutex_lock(&instance->poll_state_serialize);
+		/* request stop */
+		if (instance->poll_state == CXPOLL_POLLING)
+			instance->poll_state = CXPOLL_STOPPING;
+		mutex_unlock(&instance->poll_state_serialize);
+	}
+
+	mutex_unlock(&instance->adsl_state_serialize);
+
+	if (poll == CXPOLL_POLLING)
+		cxacru_poll_status(&instance->poll_work.work);
+
+	return ret;
+}
+
 /*
  * All device attributes are included in CXACRU_ALL_FILES
  * so that the same list can be used multiple times:
@@ -312,7 +447,8 @@ CXACRU_ATTR_##_action(CXINF_LINE_STARTABLE,            bool, line_startable); \
 CXACRU_ATTR_##_action(CXINF_MODULATION,                MODU, modulation); \
 CXACRU_ATTR_##_action(CXINF_ADSL_HEADEND,              u32,  adsl_headend); \
 CXACRU_ATTR_##_action(CXINF_ADSL_HEADEND_ENVIRONMENT,  u32,  adsl_headend_environment); \
-CXACRU_ATTR_##_action(CXINF_CONTROLLER_VERSION,        u32,  adsl_controller_version);
+CXACRU_ATTR_##_action(CXINF_CONTROLLER_VERSION,        u32,  adsl_controller_version); \
+CXACRU_CMD_##_action(                                        adsl_state);
 
 CXACRU_ALL_FILES(INIT);
 
@@ -493,8 +629,6 @@ static int cxacru_card_status(struct cxacru_data *instance)
 	return 0;
 }
 
-static void cxacru_poll_status(struct work_struct *work);
-
 static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 		struct atm_dev *atm_dev)
 {
@@ -503,6 +637,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
 	*/
 	int ret;
+	int start_polling = 1;
 
 	dbg("cxacru_atm_start");
 
@@ -515,14 +650,35 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
 	}
 
 	/* start ADSL */
+	mutex_lock(&instance->adsl_state_serialize);
 	ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
 	if (ret < 0) {
 		atm_err(usbatm_instance, "cxacru_atm_start: CHIP_ADSL_LINE_START returned %d\n", ret);
+		mutex_unlock(&instance->adsl_state_serialize);
 		return ret;
 	}
 
 	/* Start status polling */
-	cxacru_poll_status(&instance->poll_work.work);
+	mutex_lock(&instance->poll_state_serialize);
+	switch (instance->poll_state) {
+	case CXPOLL_STOPPED:
+		/* start polling */
+		instance->poll_state = CXPOLL_POLLING;
+		break;
+
+	case CXPOLL_STOPPING:
+		/* abort stop request */
+		instance->poll_state = CXPOLL_POLLING;
+	case CXPOLL_POLLING:
+	case CXPOLL_SHUTDOWN:
+		/* don't start polling */
+		start_polling = 0;
+	}
+	mutex_unlock(&instance->poll_state_serialize);
+	mutex_unlock(&instance->adsl_state_serialize);
+
+	if (start_polling)
+		cxacru_poll_status(&instance->poll_work.work);
 	return 0;
 }
 
@@ -533,16 +689,46 @@ static void cxacru_poll_status(struct work_struct *work)
 	u32 buf[CXINF_MAX] = {};
 	struct usbatm_data *usbatm = instance->usbatm;
 	struct atm_dev *atm_dev = usbatm->atm_dev;
+	int keep_polling = 1;
 	int ret;
 
 	ret = cxacru_cm_get_array(instance, CM_REQUEST_CARD_INFO_GET, buf, CXINF_MAX);
 	if (ret < 0) {
-		atm_warn(usbatm, "poll status: error %d\n", ret);
+		if (ret != -ESHUTDOWN)
+			atm_warn(usbatm, "poll status: error %d\n", ret);
+		
+		mutex_lock(&instance->poll_state_serialize);
+		if (instance->poll_state != CXPOLL_SHUTDOWN) {
+			instance->poll_state = CXPOLL_STOPPED;
+
+			if (ret != -ESHUTDOWN)
+				atm_warn(usbatm, "polling disabled, set adsl_state"
+						" to 'start' or 'poll' to resume\n");
+		}
+		mutex_unlock(&instance->poll_state_serialize);
 		goto reschedule;
 	}
 
 	memcpy(instance->card_info, buf, sizeof(instance->card_info));
 
+	if (instance->adsl_status != buf[CXINF_LINE_STARTABLE]) {
+		instance->adsl_status = buf[CXINF_LINE_STARTABLE];
+
+		switch (instance->adsl_status) {
+		case 0:
+			atm_printk(KERN_INFO, usbatm, "ADSL state: running\n");
+			break;
+
+		case 1:
+			atm_printk(KERN_INFO, usbatm, "ADSL state: stopped\n");
+			break;
+
+		default:
+			atm_printk(KERN_INFO, usbatm, "Unknown adsl status %02x\n", instance->adsl_status);
+			break;
+		}
+	}
+
 	if (instance->line_status == buf[CXINF_LINE_STATUS])
 		goto reschedule;
 
@@ -597,8 +783,20 @@ static void cxacru_poll_status(struct work_struct *work)
 		break;
 	}
 reschedule:
-	schedule_delayed_work(&instance->poll_work,
-			round_jiffies_relative(msecs_to_jiffies(POLL_INTERVAL*1000)));
+
+	mutex_lock(&instance->poll_state_serialize);
+	if (instance->poll_state == CXPOLL_STOPPING &&
+				instance->adsl_status == 1 && /* stopped */
+				instance->line_status == 0) /* down */
+		instance->poll_state = CXPOLL_STOPPED;
+
+	if (instance->poll_state == CXPOLL_STOPPED)
+		keep_polling = 0;
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (keep_polling)
+		schedule_delayed_work(&instance->poll_work,
+				round_jiffies_relative(POLL_INTERVAL*HZ));
 }
 
 static int cxacru_fw(struct usb_device *usb_dev, enum cxacru_fw_request fw,
@@ -835,6 +1033,13 @@ static int cxacru_bind(struct usbatm_data *usbatm_instance,
 	instance->modem_type = (struct cxacru_modem_type *) id->driver_info;
 	memset(instance->card_info, 0, sizeof(instance->card_info));
 
+	mutex_init(&instance->poll_state_serialize);
+	instance->poll_state = CXPOLL_STOPPED;
+	instance->line_status = -1;
+	instance->adsl_status = -1;
+
+	mutex_init(&instance->adsl_state_serialize);
+
 	instance->rcv_buf = (u8 *) __get_free_page(GFP_KERNEL);
 	if (!instance->rcv_buf) {
 		dbg("cxacru_bind: no memory for rcv_buf");
@@ -909,6 +1114,7 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
 		struct usb_interface *intf)
 {
 	struct cxacru_data *instance = usbatm_instance->driver_data;
+	int is_polling = 1;
 
 	dbg("cxacru_unbind entered");
 
@@ -917,8 +1123,20 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance,
 		return;
 	}
 
-	while (!cancel_delayed_work(&instance->poll_work))
-	       flush_scheduled_work();
+	mutex_lock(&instance->poll_state_serialize);
+	BUG_ON(instance->poll_state == CXPOLL_SHUTDOWN);
+
+	/* ensure that status polling continues unless
+	 * it has already stopped */
+	if (instance->poll_state == CXPOLL_STOPPED)
+		is_polling = 0;
+		
+	/* stop polling from being stopped or started */
+	instance->poll_state = CXPOLL_SHUTDOWN;
+	mutex_unlock(&instance->poll_state_serialize);
+
+	if (is_polling)
+		cancel_rearming_delayed_work(&instance->poll_work);
 
 	usb_kill_urb(instance->snd_urb);
 	usb_kill_urb(instance->rcv_urb);
-- 
1.5.0.1

-- 
Simon Arlott