RFC: Add module parameter to override regioncode?

[Holger Schurig]

Hi Johan !

Thanks for your contribution. I for my part would definitely
accept a patch for this.

What libertas-flavour are you using, USB, CF or SDIO (that
question is just out of curiosity, has nothing to do with
your patch).



I've made some comments inline:

> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -105,26 +105,45 @@ int lbs_update_hw_spec(struct
> lbs_private *priv) priv->fwrelease = (priv->fwrelease << 8) |
>                 (priv->fwrelease >> 24 & 0xff);
>
> +       /* Clamp region code to 8-bit since FW spec indicates that it should
> +        * only ever be 8-bit, even though the field size is 16-bit.  Some
> firmw 

Please turn off word-wrapping when sending e-mails. I see
that you're constrained by MS Outlook Express (I've pity with
you), but surely this e-mail program can also be
convinced ... :-)

With word wrap, no-one can save your e-mail and incorporate
the saved file directly into quilt/patch/git.


> +        * returns non-zero high 8 bits here.

I see the comment (this line and the lines above), I understand the
meaning, but now the comment is misplaced. I'd add insert your new
code, then the comment, and directly after the comment a line like

  priv->regioncode &= 0xff;

line.


> -       printk("libertas: %s, fw %u.%u.%up%u, cap 0x%08x\n",
> -               print_mac(mac, cmd.permanentaddr),
> -               priv->fwrelease >> 24 & 0xff,
> -               priv->fwrelease >> 16 & 0xff,
> -               priv->fwrelease >>  8 & 0xff,
> -               priv->fwrelease       & 0xff,
> -               priv->fwcapinfo);
> +       printk("libertas: %s, fw %u.%u.%up%u, cap 0x%08x regioncode hw 0x%02x sw
> +              print_mac(mac, cmd.permanentaddr),
> +              priv->fwrelease >> 24 & 0xff,
> +              priv->fwrelease >> 16 & 0xff,
> +              priv->fwrelease >>  8 & 0xff,
> +              priv->fwrelease       & 0xff,
> +              priv->fwcapinfo,
> +              le16_to_cpu(cmd.regioncode) & 0xFF,
> +              priv->regioncode);

The removal and addition of those "priv->fwrelease >> xx & 0xff"
lines looks like a whitespace-changing patch. Does your patch
survive scripts/checkpatch.pl?

> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -37,6 +37,10 @@ unsigned int lbs_debug;
>  EXPORT_SYMBOL_GPL(lbs_debug);
>  module_param_named(libertas_debug, lbs_debug, int, 0644);
>
> +int lbs_wlan_region_code = 0;
> +EXPORT_SYMBOL_GPL(lbs_wlan_region_code);
> +module_param_named(wlan_region_code, lbs_wlan_region_code, int, 0644);
> +

No need to export this. You only need to EXPORT some symbol
if you want to access it from another module. cmd.c and main.c
are in the same module, libertas.ko, so exporting is superfluous.

Maybe you name your module parameter "libertas_regioncode", so
that it harmonizes with "libertas_debug". But no strong feeling
about this ...  :-)

> @@ -927,6 +935,9 @@ static int lbs_setup_firmware(struct
> lbs_private *priv) * Read MAC address from HW
>          */
>         memset(priv->current_addr, 0xff, ETH_ALEN);
> +       if (lbs_wlan_region_code) {
> +               priv->regioncode = lbs_wlan_region_code;
> +       }

You don't need curly brackets here.


I note that this is a RFC, so it's ok that there is no
Signed-off-by line here, but don't forget to put that
line in.

Note that I haven't look at the functionality or
implications of this patch yet. With what region code are
you using your libertas?


Greetings, Holger