[PATCH] libertas: remove handling for CMD_802_11_LED_GPIO_CTRL

[PATCH] libertas: remove handling for CMD_802_11_LED_GPIO_CTRL

[Holger Schurig]

... which just resided as an old-style command in cmd/cmdresp, but
was nowhere useed. If we ever need it, we can re-add it as a newstyle
command.

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

--- linux-wl.orig/drivers/net/wireless/libertas/cmd.c
+++ linux-wl/drivers/net/wireless/libertas/cmd.c
@@ -1181,29 +1181,6 @@
 
 		ret = 0;
 		break;
-	case CMD_802_11_LED_GPIO_CTRL:
-		{
-			struct mrvl_ie_ledgpio *gpio =
-			    (struct mrvl_ie_ledgpio*)
-			    cmdptr->params.ledgpio.data;
-
-			memmove(&cmdptr->params.ledgpio,
-				pdata_buf,
-				sizeof(struct cmd_ds_802_11_led_ctrl));
-
-			cmdptr->command =
-			    cpu_to_le16(CMD_802_11_LED_GPIO_CTRL);
-
-#define ACTION_NUMLED_TLVTYPE_LEN_FIELDS_LEN 8
-			cmdptr->size =
-			    cpu_to_le16(le16_to_cpu(gpio->header.len)
-				+ S_DS_GEN
-				+ ACTION_NUMLED_TLVTYPE_LEN_FIELDS_LEN);
-			gpio->header.len = gpio->header.len;
-
-			ret = 0;
-			break;
-		}
 
 	case CMD_BT_ACCESS:
 		ret = lbs_cmd_bt_access(cmdptr, cmd_action, pdata_buf);
--- linux-wl.orig/drivers/net/wireless/libertas/cmdresp.c
+++ linux-wl/drivers/net/wireless/libertas/cmdresp.c
@@ -187,12 +187,6 @@
 			sizeof(struct cmd_ds_802_11_tpc_cfg));
 		spin_unlock_irqrestore(&priv->driver_lock, flags);
 		break;
-	case CMD_RET(CMD_802_11_LED_GPIO_CTRL):
-		spin_lock_irqsave(&priv->driver_lock, flags);
-		memmove((void *)priv->cur_cmd->callback_arg, &resp->params.ledgpio,
-			sizeof(struct cmd_ds_802_11_led_ctrl));
-		spin_unlock_irqrestore(&priv->driver_lock, flags);
-		break;
 
 	case CMD_RET(CMD_GET_TSF):
 		spin_lock_irqsave(&priv->driver_lock, flags);

-- 
http://www.holgerschurig.de

Re: [PATCH] libertas: remove handling for CMD_802_11_LED_GPIO_CTRL

[Dan Williams]

On Mon, 2009-10-19 at 13:31 +0200, Holger Schurig wrote:
> ... which just resided as an old-style command in cmd/cmdresp, but
> was nowhere useed. If we ever need it, we can re-add it as a newstyle
> command.

It's actually used by the OLPC folks; but right now there's simply no
mechanism to configure that ability via WEXT.  Is there some way to do
this via the kernel LED  framework instead that libertas should be
using?

Dan

> Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>
> 
> --- linux-wl.orig/drivers/net/wireless/libertas/cmd.c
> +++ linux-wl/drivers/net/wireless/libertas/cmd.c
> @@ -1181,29 +1181,6 @@
>  
>  		ret = 0;
>  		break;
> -	case CMD_802_11_LED_GPIO_CTRL:
> -		{
> -			struct mrvl_ie_ledgpio *gpio =
> -			    (struct mrvl_ie_ledgpio*)
> -			    cmdptr->params.ledgpio.data;
> -
> -			memmove(&cmdptr->params.ledgpio,
> -				pdata_buf,
> -				sizeof(struct cmd_ds_802_11_led_ctrl));
> -
> -			cmdptr->command =
> -			    cpu_to_le16(CMD_802_11_LED_GPIO_CTRL);
> -
> -#define ACTION_NUMLED_TLVTYPE_LEN_FIELDS_LEN 8
> -			cmdptr->size =
> -			    cpu_to_le16(le16_to_cpu(gpio->header.len)
> -				+ S_DS_GEN
> -				+ ACTION_NUMLED_TLVTYPE_LEN_FIELDS_LEN);
> -			gpio->header.len = gpio->header.len;
> -
> -			ret = 0;
> -			break;
> -		}
>  
>  	case CMD_BT_ACCESS:
>  		ret = lbs_cmd_bt_access(cmdptr, cmd_action, pdata_buf);
> --- linux-wl.orig/drivers/net/wireless/libertas/cmdresp.c
> +++ linux-wl/drivers/net/wireless/libertas/cmdresp.c
> @@ -187,12 +187,6 @@
>  			sizeof(struct cmd_ds_802_11_tpc_cfg));
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		break;
> -	case CMD_RET(CMD_802_11_LED_GPIO_CTRL):
> -		spin_lock_irqsave(&priv->driver_lock, flags);
> -		memmove((void *)priv->cur_cmd->callback_arg, &resp->params.ledgpio,
> -			sizeof(struct cmd_ds_802_11_led_ctrl));
> -		spin_unlock_irqrestore(&priv->driver_lock, flags);
> -		break;
>  
>  	case CMD_RET(CMD_GET_TSF):
>  		spin_lock_irqsave(&priv->driver_lock, flags);
> 

Re: [PATCH] libertas: remove handling for CMD_802_11_LED_GPIO_CTRL

[Holger Schurig]

> It's actually used by the OLPC folks; but right now there's
> simply no mechanism to configure that ability via WEXT.  Is
> there some way to do this via the kernel LED  framework instead
> that libertas should be using?

They use it?

No one did bother to send a patch for the in-kernel Libertas 
driver during the last year. Sigh.

Also, if they use it, they need a patch on top of what is now in 
the kernel, because the current in-kernel libertas driver has no 
code to issue the CMD_802_11_LED_GPIO_CTRL. That's why I removed 
it in the first place.

It's O.K. for me to keep this code in (it doesn't harm), but a 
better approach would be

a) use a new-style commands, a.k.a. lbs_cmd_with_response()

b) use the in-kernel LED support (as you said)

-- 
http://www.holgerschurig.de

Re: [PATCH] libertas: remove handling for CMD_802_11_LED_GPIO_CTRL

[Dan Williams]

On Thu, 2009-10-22 at 09:53 +0200, Holger Schurig wrote:
> > It's actually used by the OLPC folks; but right now there's
> > simply no mechanism to configure that ability via WEXT.  Is
> > there some way to do this via the kernel LED  framework instead
> > that libertas should be using?
> 
> They use it?
> 
> No one did bother to send a patch for the in-kernel Libertas 
> driver during the last year. Sigh.
> 
> Also, if they use it, they need a patch on top of what is now in 
> the kernel, because the current in-kernel libertas driver has no 
> code to issue the CMD_802_11_LED_GPIO_CTRL. That's why I removed 
> it in the first place.

Because it was a WEXT private ioctl and I ripped those all out and told
them no.  They still patch that (because there wasn't a LED framework at
the time) into the OLPC Gen 1 kernel trees.  Not sure they are ever
going to jump to 2.6.32 or anything though.  Just letting you know.

There's quite a few commands that only the OLPC stuff uses, but because
there was no usable userland interface for it (because we said no to
IWPRIV, and because debugfs is for debugging only) they just stuck with
IWPRIV and patched that in.

> It's O.K. for me to keep this code in (it doesn't harm), but a 
> better approach would be
> 
> a) use a new-style commands, a.k.a. lbs_cmd_with_response()

I was working on that and had converted all but 7 of the old-style
commands before you started the cfg80211 work, and now I figure after
you're done I'd go back and clean them up if they still exist.

> b) use the in-kernel LED support (as you said)

Yeah, that's obviously what should be done.

Dan