On Tue, Sep 06, 2011 at 09:58:35AM +0300, Kalle Valo wrote:
> On 09/05/2011 05:38 PM, Jouni Malinen wrote:
> > --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> > +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> > +static unsigned int ath6kl_p2p;
> > +
> > +module_param(ath6kl_p2p, uint, 0644);
>
> Currently all module parameters are in init.c. It's not the best place
> and most likely will move to core.c soon, but I still would prefer to
> have them all in one place. I think moving this to init.c would make sense.
I started with this in init.c, but that causes checkpatch warnings when
the variable needs to be accessed as extern in cfg80211.c. Or well, I
guess I could hide that by defining the extern in a header file, but
this parameter should not really be used in any other file than
cfg80211.c to initialize the flag in struct ath6kl. As such, I would
prefer not to make its visibility any larger that necessary. Why would
all module parameters need to be defined in the same C file?
> > --- a/drivers/net/wireless/ath/ath6kl/core.h
> > +++ b/drivers/net/wireless/ath/ath6kl/core.h
> > @@ -471,6 +471,8 @@ struct ath6kl {
> > + bool p2p;
>
> We have struct ath6kl::conf_flags exactly for this purpose, see
> ATH6KL_CONF_ENABLE_11N for an example. I think we should use it also
> with P2P.
Sounds reasonable.
> > + if (ar->p2p) {
> > + ret = ath6kl_wmi_info_req_cmd(ar->wmi,
> > + P2P_FLAG_CAPABILITIES_REQ |
> > + P2P_FLAG_MACADDR_REQ |
> > + P2P_FLAG_HMODEL_REQ);
> > + if (ret) {
> > + ath6kl_dbg(ATH6KL_DBG_TRC, "failed to request P2P "
> > + "capabilities (%d) - assuming P2P not "
> > + "supported\n", ret);
> > + ar->p2p = 0;
> > + }
> > + }
> > +
> > + if (ar->p2p) {
>
> Can we combine this if block with the first one?
Sure, but that would add extra indentation level for the following code,
so this separate check-if-P2P-is-supported followed by
if-P2P-is-supported looks cleaner to me.
> If it's ok for you, I'll commit the first three patches and we can talk
> more about this patch.
Yes, please do.
--
Jouni Malinen PGP id EFC895FA
--
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
