remove special ioctl from hso driver, replace by rfkill

remove special ioctl from hso driver, replace by rfkill

[Oliver Neukum]

Hi,

could you test this patch?
It does away with the one off serial ioctl replacing it with a standard
mechanism.

	Regards
		Oliver

---

--- linux-2.6.25-hso/drivers/net/usb/hso.c.alt2	2008-04-16 07:24:18.000000000 +0200
+++ linux-2.6.25-hso/drivers/net/usb/hso.c	2008-04-16 14:01:22.000000000 +0200
@@ -57,6 +58,7 @@
 #include <linux/tty_driver.h>
 #include <linux/tty_flip.h>
 #include <linux/kmod.h>
+#include <linux/rfkill.h>
 #include <linux/ip.h>
 #include <linux/proc_fs.h>
 #include <linux/uaccess.h>
@@ -102,7 +104,6 @@
 #define HSO_SERIAL_FLAG_THROTTLED	0
 #define HSO_SERIAL_FLAG_TX_SENT		1
 #define HSO_SERIAL_FLAG_RX_SENT		2
-#define HSO_SERIAL_USB_GONE		3
 
 #define HSO_SERIAL_MAGIC		0x48534f31
 
@@ -241,11 +242,13 @@ struct hso_device {
 
 	u8 is_active;
 	u8 suspend_disabled;
+	u8 usb_gone;
 	struct work_struct async_get_intf;
 	struct work_struct async_put_intf;
 
 	struct usb_device *usb;
 	struct usb_interface *interface;
+	struct rfkill *rfkill;
 
 	struct device *dev;
 	struct kref ref;                                                       */
@@ -1238,7 +1216,7 @@ static void hso_serial_close(struct tty_
 	}
 
 	mutex_lock(&serial->parent->mutex);
-	usb_gone = test_bit(HSO_SERIAL_USB_GONE, &serial->flags);
+	usb_gone = serial->parent->usb_gone;
 
 	if (!usb_gone)
 		usb_autopm_get_interface(serial->parent->interface);
@@ -1407,43 +1385,6 @@ static int hso_serial_tiocmset(struct tt
 			       USB_CTRL_SET_TIMEOUT);
 }
 
-/* Toggles radio on or off ( used by ioctl ) */
-static int hso_set_radio(struct hso_device *hso_dev, int enabled)
-{
-	if (!hso_dev)
-		return -ENODEV;
-
-	return usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0),
-			       enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0,
-			       USB_CTRL_SET_TIMEOUT);
-}
-
-/* ioctl not supported */
-static int hso_serial_ioctl(struct tty_struct *tty, struct file *file,
-			    unsigned int cmd, unsigned long arg)
-{
-	struct hso_serial *serial = get_serial_by_tty(tty);
-	int ret = 0;
-	D4("IOCTL cmd: %d, arg: %ld", cmd, arg);
-
-	if (!serial)
-		return -ENODEV;
-
-	switch (cmd) {
-	case SIOCSETRADIO:
-		if (arg)
-			hso_set_radio(serial->parent, 1);
-		else
-			hso_set_radio(serial->parent, 0);
-		ret = 0;
-		break;
-	default:
-		ret = -ENOIOCTLCMD;
-		break;
-	}
-	return ret;
-}
-
 /* starts a transmit */
 static void hso_kick_transmit(struct hso_serial *serial)
 {
@@ -1871,7 +1812,6 @@ static struct tty_operations hso_serial_
 	.close = hso_serial_close,
 	.write = hso_serial_write,
 	.write_room = hso_serial_write_room,
-	.ioctl = hso_serial_ioctl,
 	.set_termios = hso_serial_set_termios,
 	.chars_in_buffer = hso_serial_chars_in_buffer,
 	.tiocmget = hso_serial_tiocmget,
@@ -2669,12 +2610,30 @@ static int hso_get_config_data(struct us
 	return result;
 }
 
+static int hso_radio_toggle(void *data, enum rfkill_state state)
+{
+	struct hso_device *hso_dev = data;
+	int enabled = (state == RFKILL_STATE_ON);
+	int rv;
+
+	mutex_lock(&hso_dev->mutex);
+	if (hso_dev->usb_gone)
+		rv = 0;
+	else
+		rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0),
+				       enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0,
+				       USB_CTRL_SET_TIMEOUT);
+	mutex_unlock(&hso_dev->mutex);
+	return rv;
+}
+
 /* called once for each interface upon device insertion */
 static int hso_probe(struct usb_interface *interface,
 		     const struct usb_device_id *id)
 {
 	int mux, i, if_num, port_spec;
 	unsigned char port_mask;
+	char *rfkn;
 	struct hso_device *hso_dev = NULL;
 	struct hso_shared_int *shared_int = NULL;
 	struct hso_device *tmp_dev = NULL;
@@ -2747,6 +2706,27 @@ static int hso_probe(struct usb_interfac
 		goto exit;
 	}
 
+	hso_dev->rfkill = rfkill_allocate(&interface_to_usbdev(interface)->dev,
+				RFKILL_TYPE_WLAN);
+	if (!hso_dev->rfkill)
+		goto exit;
+	rfkn = kzalloc(20, GFP_KERNEL);
+	if (!rfkn) {
+		rfkill_free(hso_dev->rfkill);
+		goto exit;
+	}
+	snprintf(rfkn, 20, "hso-%d", if_num);
+	hso_dev->rfkill->name = rfkn;
+	hso_dev->rfkill->state = RFKILL_STATE_ON;
+	hso_dev->rfkill->data = hso_dev;
+	hso_dev->rfkill->toggle_radio = hso_radio_toggle;
+	if (rfkill_register(hso_dev->rfkill) < 0) {
+		kfree(rfkn);
+		hso_dev->rfkill->name = NULL;
+		rfkill_free(hso_dev->rfkill);
+		goto exit;
+	}
+
 	usb_driver_claim_interface(&hso_driver, interface, hso_dev);
 
 	/* save our data pointer in this device */
@@ -2768,8 +2748,16 @@ exit:
 /* device removed, cleaning up */
 static void hso_disconnect(struct usb_interface *interface)
 {
+	struct hso_device *hso_dev = usb_get_intfdata(interface);
+
 	hso_free_interface(interface);
 
+	if (hso_dev) {
+		cancel_work_sync(&hso_dev->async_put_intf);
+		cancel_work_sync(&hso_dev->async_get_intf);
+		rfkill_unregister(hso_dev->rfkill);
+	}
+
 	/* remove reference of our private data */
 	usb_set_intfdata(interface, NULL);
 
@@ -2909,7 +2897,7 @@ static void hso_free_interface(struct us
 			if (hso_dev->tty)
 				tty_hangup(hso_dev->tty);
 			mutex_lock(&hso_dev->parent->mutex);
-			set_bit(HSO_SERIAL_USB_GONE, &hso_dev->flags);
+			hso_dev->parent->usb_gone = 1;
 			mutex_unlock(&hso_dev->parent->mutex);
 			kref_put(&serial_table[i]->ref, hso_serial_ref_free);
 		}

Re: remove special ioctl from hso driver, replace by rfkill

[Paulius Zaleckas]

Oliver Neukum wrote:
> Hi,
> 
> could you test this patch?
> It does away with the one off serial ioctl replacing it with a standard
> mechanism.
> 

Can you describe me how can I test it? I am pretty new to rfkill :)
By looking at rfkill documentation I got impression that it will work
only on laptop... and I am using desktop PC.

Paulius

Re: remove special ioctl from hso driver, replace by rfkill

[Oliver Neukum]

Am Mittwoch, 16. April 2008 15:06:35 schrieb Paulius Zaleckas:
> Oliver Neukum wrote:
> > Hi,
> > 
> > could you test this patch?
> > It does away with the one off serial ioctl replacing it with a standard
> > mechanism.
> > 
> 
> Can you describe me how can I test it? I am pretty new to rfkill :)

So am I.

> By looking at rfkill documentation I got impression that it will work
> only on laptop... and I am using desktop PC.

Indeed. it seems like you need a key. Can you enquire on the kernel list?

	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: remove special ioctl from hso driver, replace by rfkill

[Paulius Zaleckas]

Oliver Neukum wrote:
>  /* called once for each interface upon device insertion */
>  static int hso_probe(struct usb_interface *interface,
>  		     const struct usb_device_id *id)
>  {
>  	int mux, i, if_num, port_spec;
>  	unsigned char port_mask;
> +	char *rfkn;
>  	struct hso_device *hso_dev = NULL;
>  	struct hso_shared_int *shared_int = NULL;
>  	struct hso_device *tmp_dev = NULL;
> @@ -2747,6 +2706,27 @@ static int hso_probe(struct usb_interfac
>  		goto exit;
>  	}
>  
> +	hso_dev->rfkill = rfkill_allocate(&interface_to_usbdev(interface)->dev,
> +				RFKILL_TYPE_WLAN);

What will happen if rfkill is not enabled on kernel? I think HSO should
not depend on rfkill. Solution should be to do #ifdef CONFIG_RFKILL..?

> +	if (!hso_dev->rfkill)
> +		goto exit;
> +	rfkn = kzalloc(20, GFP_KERNEL);
> +	if (!rfkn) {
> +		rfkill_free(hso_dev->rfkill);
> +		goto exit;
> +	}
> +	snprintf(rfkn, 20, "hso-%d", if_num);
> +	hso_dev->rfkill->name = rfkn;
> +	hso_dev->rfkill->state = RFKILL_STATE_ON;
> +	hso_dev->rfkill->data = hso_dev;
> +	hso_dev->rfkill->toggle_radio = hso_radio_toggle;
> +	if (rfkill_register(hso_dev->rfkill) < 0) {
> +		kfree(rfkn);
> +		hso_dev->rfkill->name = NULL;
> +		rfkill_free(hso_dev->rfkill);
> +		goto exit;
> +	}
> +
>  	usb_driver_claim_interface(&hso_driver, interface, hso_dev);

Looks like it is possible to disable RF part of devices through sysfs...
Tomorrow I will upgrade to 2.6.25 and I will try to test it.

Paulius

Re: remove special ioctl from hso driver, replace by rfkill

[Greg KH]

On Wed, Apr 16, 2008 at 02:28:17PM +0200, Oliver Neukum wrote:
> Hi,
> 
> could you test this patch?
> It does away with the one off serial ioctl replacing it with a standard
> mechanism.

Looks good to me, I've applied it now :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: remove special ioctl from hso driver, replace by rfkill

[Greg KH]

On Fri, Apr 18, 2008 at 12:42:07AM +0300, Paulius Zaleckas wrote:
> Oliver Neukum wrote:
>>  /* called once for each interface upon device insertion */
>>  static int hso_probe(struct usb_interface *interface,
>>  		     const struct usb_device_id *id)
>>  {
>>  	int mux, i, if_num, port_spec;
>>  	unsigned char port_mask;
>> +	char *rfkn;
>>  	struct hso_device *hso_dev = NULL;
>>  	struct hso_shared_int *shared_int = NULL;
>>  	struct hso_device *tmp_dev = NULL;
>> @@ -2747,6 +2706,27 @@ static int hso_probe(struct usb_interfac
>>  		goto exit;
>>  	}
>>  +	hso_dev->rfkill = rfkill_allocate(&interface_to_usbdev(interface)->dev,
>> +				RFKILL_TYPE_WLAN);
>
> What will happen if rfkill is not enabled on kernel? I think HSO should
> not depend on rfkill. Solution should be to do #ifdef CONFIG_RFKILL..?

Ick, rfkill should degrade properly so that #ifdefs are not needed here.
I'll fix up rfkill.h to prevent this kind of dependancy requirement.

thanks,

greg k-h

Re: remove special ioctl from hso driver, replace by rfkill

[Marcel Holtmann]

Hi Greg,

>>> /* called once for each interface upon device insertion */
>>> static int hso_probe(struct usb_interface *interface,
>>> 		     const struct usb_device_id *id)
>>> {
>>> 	int mux, i, if_num, port_spec;
>>> 	unsigned char port_mask;
>>> +	char *rfkn;
>>> 	struct hso_device *hso_dev = NULL;
>>> 	struct hso_shared_int *shared_int = NULL;
>>> 	struct hso_device *tmp_dev = NULL;
>>> @@ -2747,6 +2706,27 @@ static int hso_probe(struct usb_interfac
>>> 		goto exit;
>>> 	}
>>> +	hso_dev->rfkill =  
>>> rfkill_allocate(&interface_to_usbdev(interface)->dev,
>>> +				RFKILL_TYPE_WLAN);
>>
>> What will happen if rfkill is not enabled on kernel? I think HSO  
>> should
>> not depend on rfkill. Solution should be to do #ifdef  
>> CONFIG_RFKILL..?
>
> Ick, rfkill should degrade properly so that #ifdefs are not needed  
> here.
> I'll fix up rfkill.h to prevent this kind of dependancy requirement.

while you are at it, can you make sure we have a RFKILL_TYPE_WWAN for  
these cards and then use it. Using RFKILL_TYPE_WLAN for UMTS/GSM  
modems is wrong.

Regards

Marcel

Re: remove special ioctl from hso driver, replace by rfkill

[Paulius Zaleckas]

Oliver Neukum wrote:
> Hi,
> 
> could you test this patch?
> It does away with the one off serial ioctl replacing it with a standard
> mechanism.
> 
> 	Regards
> 		Oliver

I have tested rfkill and it realy works, but there is two problems:

- LED on the device doesn't indicate rf state... Maybe Filip knows
   some usb command to control it?

- driver creates multiple rfkill entries on sysfs... This is because
   option device has multi endpoints (probe is called twice with GTM380):

T:  Bus=01 Lev=03 Prnt=05 Port=00 Cnt=01 Dev#=  7 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=ff(vend.) Sub=ff Prot=ff MxPS=64 #Cfgs=  1
P:  Vendor=0af0 ProdID=7211 Rev= 0.00
S:  Manufacturer=Option N.V.
S:  Product=Globetrotter HSUPA Modem
S:  SerialNumber=Serial Number
C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=hso
E:  Ad=83(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=03(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=85(I) Atr=03(Int.) MxPS=  16 Ivl=128ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=hso
E:  Ad=84(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=04(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: remove special ioctl from hso driver, replace by rfkill

[Greg KH]

On Fri, Apr 18, 2008 at 04:56:27PM +0300, Paulius Zaleckas wrote:
> Oliver Neukum wrote:
>> Hi,
>> could you test this patch?
>> It does away with the one off serial ioctl replacing it with a standard
>> mechanism.
>> 	Regards
>> 		Oliver
>
> I have tested rfkill and it realy works, but there is two problems:
>
> - LED on the device doesn't indicate rf state... Maybe Filip knows
>   some usb command to control it?
>
> - driver creates multiple rfkill entries on sysfs... This is because
>   option device has multi endpoints (probe is called twice with GTM380):

You mean "multiple interfaces" :)

And that's correct, for this device, that is the way it was created, so
either rfkill entry should work to shut off the radio...

thanks,

greg k-h

Re: remove special ioctl from hso driver, replace by rfkill

[Paulius Zaleckas]

Greg KH wrote:
> On Fri, Apr 18, 2008 at 04:56:27PM +0300, Paulius Zaleckas wrote:
>> Oliver Neukum wrote:
>>> Hi,
>>> could you test this patch?
>>> It does away with the one off serial ioctl replacing it with a standard
>>> mechanism.
>>> 	Regards
>>> 		Oliver
>> I have tested rfkill and it realy works, but there is two problems:
>>
>> - LED on the device doesn't indicate rf state... Maybe Filip knows
>>   some usb command to control it?
>>
>> - driver creates multiple rfkill entries on sysfs... This is because
>>   option device has multi endpoints (probe is called twice with GTM380):
> 
> You mean "multiple interfaces" :)

Yes, I think I meant it :)

> And that's correct, for this device, that is the way it was created, so
> either rfkill entry should work to shut off the radio...

Yes, both of them work, but there is a problem...
If you change state for one them, then another one remains not updated.
Also it doesn't make sense to have two rfkill entries for one device.
Maybe it should be created only with network device?

Paulius