________________________________________ From: linux-wireless-owner@xxxxxxxxxxxxxxx [linux-wireless-owner@xxxxxxxxxxxxxxx] On Behalf Of Johannes Berg [johannes@xxxxxxxxxxxxxxxx] Sent: Monday, January 05, 2015 7:17 PM To: Avinash Patil Cc: linux-wireless@xxxxxxxxxxxxxxx; Amitkumar Karwar; Cathy Luo Subject: Re: [PATCH v3] cfg80211: check for carrier state only when offchanel CAC supported On Mon, 2015-01-05 at 05:28 -0800, Avinash Patil wrote: > > - if (netif_carrier_ok(dev)) > > + if (wiphy_ext_feature_isset(&rdev->wiphy, > > + NL80211_EXT_FEATURE_OFFCHAN_CAC) && > > + netif_carrier_ok(dev)) > > return -EBUSY; > > >Wait - doesn't that have to be !feature_isset()? > > >johannes > > If Offchannel CAC is supported (driver has set this bit in wiphy's > extended features) & carrier is ON, return EBUSY as offchannel CAC may > be ongoing, isnt it? >Well, my thinking is this - a new feature flag should allow something >new. >Therefore, the patch should essentially be this: >+ if (!new_feature) >if (do_old_check) >Now wrapping that into a single if gives >- if (do_old_check) >+ if (!new_feature && do_old_check) >so the patch looks wrong to me. I think here we want to do_old_check only when new_feature is supported; because old check is causing issue for device where new feature is not supported. Earlier it was: - if (do_old_check) Now it is: +if (new_feature && do_old_check) I think check is still correct. As you suggested we can nest it... Please correct me if I am wrong. -Avinash.-- 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