On Tue, Jan 12, 2016 at 04:39:53PM +0900, Glen Lee wrote:
> +static void handle_set_tx_pwr(struct wilc_vif *vif, u8 tx_pwr)
> +{
> + s32 ret = 0;
s32 should almost always be changed to int. Don't initialize variables
with bogus values. GCC has a helper warning for uninitialized variables
and this disables GCC's uninitialized variable checking.
> + struct wid wid;
> +
> + wid.id = (u16)WID_TX_POWER;
> + wid.type = WID_CHAR;
> + wid.val = (s8 *)&tx_pwr;
Casting an unsigned value from the user to signed seems like a recipe
for disaster.
> + wid.size = sizeof(char);
> +
> + ret = wilc_send_config_pkt(vif->wilc, SET_CFG, &wid, 1,
> + wilc_get_vif_idx(vif));
> + if(ret)
grumble grumble... checkpatch.
> + netdev_err(vif->ndev,"Failed to set TX PWR\n");
> +}
[ snip]
> +static int set_tx_power(struct wiphy *wiphy, struct wireless_dev *wdev,
> + enum nl80211_tx_power_setting type, int mbm)
> +{
> + int ret = 0;
No need.
> + s32 tx_power = MBM_TO_DBM(mbm);
> + struct wilc_priv *priv = wiphy_priv(wiphy);
> + struct wilc_vif *vif = netdev_priv(priv->dev);
> +
> + netdev_info(vif->ndev, "Setting tx power to %d\n", tx_power);
Remove this debug output.
> +
> + if(tx_power < 0)
grumble.
> + tx_power = 0;
> + else if(tx_power > 18)
> + tx_power = 18;
> + ret = wilc_set_tx_power(vif ,(u8)tx_power);
This cast is not needed. Whitespace grumble.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
