[PATCH] libertas: provide reset_card() callback on OLPC

[David Woodhouse]

On Tue, 2008-05-20 at 12:19 -0400, Andres Salomon wrote:
> Awesome, I'll be happy to see this patch upstream.  Comments below.
> 
> 
> On Tue, 20 May 2008 16:48:52 +0100
> David Woodhouse <dwmw2 at infradead.org> wrote:
> 
> > Signed-off-by: David Woodhouse <dwmw2 at infradead.org>
> > 
> > diff --git a/drivers/net/wireless/libertas/if_usb.c
> > b/drivers/net/wireless/libertas/if_usb.c index 8032df7..8a7eb63 100644
> > --- a/drivers/net/wireless/libertas/if_usb.c
> > +++ b/drivers/net/wireless/libertas/if_usb.c
> > @@ -7,6 +7,10 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/usb.h>
> >  
> > +#ifdef CONFIG_OLPC
> > +#include <asm/olpc.h>
> > +#endif
> > +
> >  #define DRV_NAME "usb8xxx"
> >  
> >  #include "host.h"
> > @@ -146,6 +150,14 @@ static void if_usb_fw_timeo(unsigned long priv)
> >  	wake_up(&cardp->fw_wq);
> >  }
> >  
> > +#ifdef CONFIG_OLPC
> > +static int if_usb_reset_olpc_card(struct lbs_private *priv)
> > +{
> > +	printk(KERN_CRIT "Resetting OLPC wireless via EC...\n");
> > +	olpc_ec_cmd(0x25, NULL, 0, NULL, 0);
> > +}
> > +#endif
> 
> 
> Can you please add '#define EC_WIRELESS_RESET 0x25' (or whatever you want
> to call it) to include/asm-x86/olpc.h, and use EC_WIRELESS_RESET here
> rather than 0x25?  All EC commands should end up there.

Makes sense. Want to do it?

> > +
> >  /**
> >   *  @brief sets the configuration values
> >   *  @param ifnum	interface number
> > @@ -231,6 +243,11 @@ static int if_usb_probe(struct usb_interface
> > *intf, cardp->priv->fw_ready = 1;
> >  
> >  	priv->hw_host_to_card = if_usb_host_to_card;
> > +#ifdef CONFIG_OLPC
> > +	if (machine_is_olpc())
> > +		priv->reset_card = if_usb_reset_olpc_card;
> > +#endif
> > +
> 
> So, um, what happens if I plug a libertas dongle into an XO?  Is
> machine_is_olpc() really enough to ensure that we want to reset this
> specific libertas device?

It's enough in 99% of cases. We could perhaps check which USB port it's
in (and remember at what board rev it changed ports).

> Any particular reason why we need the ifdef?  I'd prefer the following, if
> it's a safe assumption to make:
> 
> if (ret && priv->reset_card)
> 	priv->reset_card(NULL);

We haven't got 'priv' and it hasn't got ->reset_card set yet anyway.
This happens earlier during init. Neat trick if you can manage it
though...

-- 
dwmw2