On 05/14/2012 09:31 PM, Pedersen, Thomas wrote: > On Mon, May 14, 2012 at 09:09:17PM +0300, Kalle Valo wrote: >> On 05/14/2012 09:03 PM, Pedersen, Thomas wrote: >>> On Mon, May 14, 2012 at 10:56:59AM -0700, Joe Perches wrote: >>> >>>> Why 2 messages when 1 message might do? >>>> >>>> err = ath6kl_wmi_sta_bmiss_enhance_cmd(vif->ar->wmi, >>>> vif->fw_vif_idx, enable); >>>> ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, >>>> "%s enhanced fw bmiss detection: %s\n", >>>> enable ? "enable" : "disable", >>>> err ? "OK" : "failed"); >>> >>> OK that seems nicer. Should we still print the error code, or maybe it >>> doesn't really matter? >> >> I missed this in the original review, but it's actually better to not >> use ath6kl_dbg() for error messages. They are more difficult to notice >> that way. >> >> Or did you have a specific reason for using ath6kl_dbg()? > > No, you're right. However, if we consolidate these messages they will > both be under ath6kl_dbg() or ath6kl_err(), and neither one would be > correct. > > How about just keeping these separate and printing the error through > ath6kl_err()? Sounds good to me. And if you can, try to print the error value in ath6kl_err(). Kalle -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html