RFC: Add module parameter to override regioncode?

[Dan Williams]

On Fri, 2008-06-06 at 10:57 +0200, Johan Adolfsson wrote:
> Hi,
> 
> We need to be able to override the wlan region
> code read from the module since that setting is
> configured late in the logistics chain and not updated
> in the module but the "normal" flash memory.

Sounds like a good idea.  In the future we should also add a sysfs
parameter (probably a good idea to synchronize it with whatever the
mac80211 regulatory framework uses) to change it at runtime by doing
something like "echo 0x30 > /sys/class/net/eth2/device/region" or
whatever.

Dan

> I was thinking of adding a wlan_region_code module parameter
> to libertas module (main.c) and
> using that value to initialize priv->regioncode
> before calling cmd.c:lbs_update_hw_spec()
> and that function would do sanity check of the value
> if it is set and use that instead of the cmd response
> if it's ok.
> (Not sure if setting priv->regioncode before calling lbs_update_hw_spec() or 
> letting that function check the
> module parameter i best)
> 
> Would this be acceptable and something you would consider
> pushing upstream?
> Basically something like the below (beware of whitespace damage)
> 
> Best regards
> /Johan
> 
> 
> --- 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
> +        * returns non-zero high 8 bits here.
> +        */
> +       /* First use and sanity check the module parameter that may
> +        * already initialised priv->regioncode
> +        */
> +       if (priv->regioncode) {
> +               for (i = 0; i < MRVDRV_MAX_REGION_CODE; i++) {
> +                       /* use the region code to search for the index */
> +                       if (priv->regioncode == lbs_region_code_to_index[i])
> +                               break;
> +               }
> +               if (i >= MRVDRV_MAX_REGION_CODE) {
> +                       /* Not sane, use response from command */
> +                       priv->regioncode = le16_to_cpu(cmd.regioncode) & 
> 0xFF;
> +               }
> +       } else {
> +               priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> +       }
> +
>         /* Some firmware capabilities:
>          * CF card    firmware 5.0.16p0:   cap 0x00000303
>          * USB dongle firmware 5.110.17p2: cap 0x00000303
>          */
> -       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);
>         lbs_deb_cmd("GET_HW_SPEC: hardware interface 0x%x, hardware spec 
> 0x%04x\
>                     cmd.hwifversion, cmd.version);
> 
> -       /* 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
> -        * returns non-zero high 8 bits here.
> -        */
> -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> 
> 
> +       /* Sanity check regioncode */
>         for (i = 0; i < MRVDRV_MAX_REGION_CODE; i++) {
>                 /* use the region code to search for the index */
>                 if (priv->regioncode == lbs_region_code_to_index[i])
> 
> 
> --- 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);
> +
> 
> @@ -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;
> +       } 
> 
> 
> _______________________________________________
> libertas-dev mailing list
> libertas-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev