| Message ID | 1462430663-9448-1-git-send-email-wnhuang@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: > Recent new hardware has the ability to switch between tablet mode and > clamshell mode. To optimize WiFi performance, we want to be able to > use > different power table between modes. This patch adds a new netlink > message type and cfg80211_ops function to allow userspace to trigger > a > power mode switch for a given wireless interface. > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > --- > include/net/cfg80211.h | 11 +++++++++++ > include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ > net/wireless/nl80211.c | 16 ++++++++++++++++ > net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ > net/wireless/trace.h | 20 ++++++++++++++++++++ > 5 files changed, 90 insertions(+) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index 9e1b24c..aa77fa0 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { > * @get_tx_power: store the current TX power into the dbm variable; > * return 0 if successful > * > + * @set_tx_power_mode: set the transmit power mode. Some device have > the ability > + * to transform between different mode such as clamshell and > tablet mode. > + * set_tx_power_mode allows setting of different TX power > mode at runtime. > + * @get_tx_power_mode: store the current TX power mode into the mode > variable; > + * return 0 if successful > + * > * @set_wds_peer: set the WDS peer for a WDS interface > * > * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting > @@ -2631,6 +2637,11 @@ struct cfg80211_ops { > int (*get_tx_power)(struct wiphy *wiphy, struct > wireless_dev *wdev, > int *dbm); > > + int (*set_tx_power_mode)(struct wiphy *wiphy, > + enum nl80211_tx_power_mode > mode); > + int (*get_tx_power_mode)(struct wiphy *wiphy, > + enum nl80211_tx_power_mode > *mode); > + > int (*set_wds_peer)(struct wiphy *wiphy, struct > net_device *dev, > const u8 *addr); > > diff --git a/include/uapi/linux/nl80211.h > b/include/uapi/linux/nl80211.h > index 5a30a75..9b1888a 100644 > --- a/include/uapi/linux/nl80211.h > +++ b/include/uapi/linux/nl80211.h > @@ -1796,6 +1796,9 @@ enum nl80211_commands { > * connecting to a PCP, and in %NL80211_CMD_START_AP to start > * a PCP instead of AP. Relevant for DMG networks only. > * > + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See > + * &enum nl80211_tx_power_mode for possible values. > + * > * @NUM_NL80211_ATTR: total number of nl80211_attrs available > * @NL80211_ATTR_MAX: highest attribute number currently defined > * @__NL80211_ATTR_AFTER_LAST: internal use > @@ -2172,6 +2175,8 @@ enum nl80211_attrs { > > NL80211_ATTR_PBSS, > > + NL80211_ATTR_WIPHY_TX_POWER_MODE, > + > /* add attributes here, update the policy in nl80211.c */ > > __NL80211_ATTR_AFTER_LAST, > @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { > }; > > /** > + * enum nl80211_tx_power_mode - TX power mode setting > + * @NL80211_TX_POWER_LOW: general low TX power mode > + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode > + * @NL80211_TX_POWER_HIGH: general high TX power mode > + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode > + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode > + */ > +enum nl80211_tx_power_mode { > + NL80211_TX_POWER_LOW, > + NL80211_TX_POWER_MEDIUM, > + NL80211_TX_POWER_HIGH, > + NL80211_TX_POWER_CLAMSHELL, > + NL80211_TX_POWER_TABLET, "clamshell" and "tablet" probably mean many different things to many different people with respect to whether or not they should do anything with power saving or wifi. I feel like a more generic interface is needed here. Could this be already done by: @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet mode> and then the device would be able to change its TX power as it saw fit up to that limit set by your application which figures out whether it's in clamshell or tablet mode? Dan > +}; > + > +/** > * enum nl80211_packet_pattern_attr - packet pattern attribute > * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute > * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask has > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > index 056a730..61b474d 100644 > --- a/net/wireless/nl80211.c > +++ b/net/wireless/nl80211.c > @@ -402,6 +402,7 @@ static const struct nla_policy > nl80211_policy[NUM_NL80211_ATTR] = { > [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, > [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, > [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, > + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, > }; > > /* policy for the key attributes */ > @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct sk_buff > *skb, struct genl_info *info) > return result; > } > > + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { > + enum nl80211_tx_power_mode mode; > + int idx = 0; > + > + if (!rdev->ops->set_tx_power_mode) > + return -EOPNOTSUPP; > + > + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; > + mode = nla_get_u32(info->attrs[idx]); > + > + result = rdev_set_tx_power_mode(rdev, mode); > + if (result) > + return result; > + } > + > if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && > info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { > u32 tx_ant, rx_ant; > diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h > index 8ae0c04..c3a1035 100644 > --- a/net/wireless/rdev-ops.h > +++ b/net/wireless/rdev-ops.h > @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct > cfg80211_registered_device *rdev, > return ret; > } > > +static inline int > +rdev_set_tx_power_mode(struct cfg80211_registered_device *rdev, > + enum nl80211_tx_power_mode mode) > +{ > + int ret; > + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); > + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); > + trace_rdev_return_int(&rdev->wiphy, ret); > + return ret; > +} > + > +static inline int > +rdev_get_tx_power_mode(struct cfg80211_registered_device *rdev, > + enum nl80211_tx_power_mode *mode) > +{ > + int ret; > + trace_rdev_get_tx_power_mode(&rdev->wiphy); > + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); > + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); > + return ret; > +} > + > static inline int rdev_set_wds_peer(struct > cfg80211_registered_device *rdev, > struct net_device *dev, const u8 > *addr) > { > diff --git a/net/wireless/trace.h b/net/wireless/trace.h > index 09b242b..132c8c2 100644 > --- a/net/wireless/trace.h > +++ b/net/wireless/trace.h > @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, > WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, __entry- > >mbm) > ); > > +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, > + TP_PROTO(struct wiphy *wiphy), > + TP_ARGS(wiphy) > +); > + > +TRACE_EVENT(rdev_set_tx_power_mode, > + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode > mode), > + TP_ARGS(wiphy, mode), > + TP_STRUCT__entry( > + WIPHY_ENTRY > + __field(enum nl80211_tx_power_mode, mode) > + ), > + TP_fast_assign( > + WIPHY_ASSIGN; > + __entry->mode = mode; > + ), > + TP_printk(WIPHY_PR_FMT ", mode: %d", > + WIPHY_PR_ARG, __entry->mode) > +); > + > TRACE_EVENT(rdev_return_int_int, > TP_PROTO(struct wiphy *wiphy, int func_ret, int func_fill), > TP_ARGS(wiphy, func_ret, func_fill), -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> wrote: > > On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: > > Recent new hardware has the ability to switch between tablet mode and > > clamshell mode. To optimize WiFi performance, we want to be able to > > use > > different power table between modes. This patch adds a new netlink > > message type and cfg80211_ops function to allow userspace to trigger > > a > > power mode switch for a given wireless interface. > > > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > > --- > > include/net/cfg80211.h | 11 +++++++++++ > > include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ > > net/wireless/nl80211.c | 16 ++++++++++++++++ > > net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ > > net/wireless/trace.h | 20 ++++++++++++++++++++ > > 5 files changed, 90 insertions(+) > > > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > > index 9e1b24c..aa77fa0 100644 > > --- a/include/net/cfg80211.h > > +++ b/include/net/cfg80211.h > > @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { > > * @get_tx_power: store the current TX power into the dbm variable; > > * return 0 if successful > > * > > + * @set_tx_power_mode: set the transmit power mode. Some device have > > the ability > > + * to transform between different mode such as clamshell and > > tablet mode. > > + * set_tx_power_mode allows setting of different TX power > > mode at runtime. > > + * @get_tx_power_mode: store the current TX power mode into the mode > > variable; > > + * return 0 if successful > > + * > > * @set_wds_peer: set the WDS peer for a WDS interface > > * > > * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting > > @@ -2631,6 +2637,11 @@ struct cfg80211_ops { > > int (*get_tx_power)(struct wiphy *wiphy, struct > > wireless_dev *wdev, > > int *dbm); > > > > + int (*set_tx_power_mode)(struct wiphy *wiphy, > > + enum nl80211_tx_power_mode > > mode); > > + int (*get_tx_power_mode)(struct wiphy *wiphy, > > + enum nl80211_tx_power_mode > > *mode); > > + > > int (*set_wds_peer)(struct wiphy *wiphy, struct > > net_device *dev, > > const u8 *addr); > > > > diff --git a/include/uapi/linux/nl80211.h > > b/include/uapi/linux/nl80211.h > > index 5a30a75..9b1888a 100644 > > --- a/include/uapi/linux/nl80211.h > > +++ b/include/uapi/linux/nl80211.h > > @@ -1796,6 +1796,9 @@ enum nl80211_commands { > > * connecting to a PCP, and in %NL80211_CMD_START_AP to start > > * a PCP instead of AP. Relevant for DMG networks only. > > * > > + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See > > + * &enum nl80211_tx_power_mode for possible values. > > + * > > * @NUM_NL80211_ATTR: total number of nl80211_attrs available > > * @NL80211_ATTR_MAX: highest attribute number currently defined > > * @__NL80211_ATTR_AFTER_LAST: internal use > > @@ -2172,6 +2175,8 @@ enum nl80211_attrs { > > > > NL80211_ATTR_PBSS, > > > > + NL80211_ATTR_WIPHY_TX_POWER_MODE, > > + > > /* add attributes here, update the policy in nl80211.c */ > > > > __NL80211_ATTR_AFTER_LAST, > > @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { > > }; > > > > /** > > + * enum nl80211_tx_power_mode - TX power mode setting > > + * @NL80211_TX_POWER_LOW: general low TX power mode > > + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode > > + * @NL80211_TX_POWER_HIGH: general high TX power mode > > + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode > > + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode > > + */ > > +enum nl80211_tx_power_mode { > > + NL80211_TX_POWER_LOW, > > + NL80211_TX_POWER_MEDIUM, > > + NL80211_TX_POWER_HIGH, > > + NL80211_TX_POWER_CLAMSHELL, > > + NL80211_TX_POWER_TABLET, > > "clamshell" and "tablet" probably mean many different things to many > different people with respect to whether or not they should do anything > with power saving or wifi. I feel like a more generic interface is > needed here. We could probably drop those two CLAMSHELL and TABLET constant or describing what they mean in more detail? > > Could this be already done by: > @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED > @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet mode> > > and then the device would be able to change its TX power as it saw fit > up to that limit set by your application which figures out whether it's > in clamshell or tablet mode? We usually want different power settings in different band/channel. For example, we can have three different power settings in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we can not simply set a fixed number to the power level. A power table is required to map channel/band to actual power limit. For most of the driver, changing power table requires loading a new set of calibration data from either devicetree or ACPI. Due to this, a new message type is required to allow the driver to switch between tables. Wei-Ning > > > Dan > > > +}; > > + > > +/** > > * enum nl80211_packet_pattern_attr - packet pattern attribute > > * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute > > * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask has > > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > > index 056a730..61b474d 100644 > > --- a/net/wireless/nl80211.c > > +++ b/net/wireless/nl80211.c > > @@ -402,6 +402,7 @@ static const struct nla_policy > > nl80211_policy[NUM_NL80211_ATTR] = { > > [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, > > [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, > > [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, > > + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, > > }; > > > > /* policy for the key attributes */ > > @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct sk_buff > > *skb, struct genl_info *info) > > return result; > > } > > > > + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { > > + enum nl80211_tx_power_mode mode; > > + int idx = 0; > > + > > + if (!rdev->ops->set_tx_power_mode) > > + return -EOPNOTSUPP; > > + > > + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; > > + mode = nla_get_u32(info->attrs[idx]); > > + > > + result = rdev_set_tx_power_mode(rdev, mode); > > + if (result) > > + return result; > > + } > > + > > if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && > > info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { > > u32 tx_ant, rx_ant; > > diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h > > index 8ae0c04..c3a1035 100644 > > --- a/net/wireless/rdev-ops.h > > +++ b/net/wireless/rdev-ops.h > > @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct > > cfg80211_registered_device *rdev, > > return ret; > > } > > > > +static inline int > > +rdev_set_tx_power_mode(struct cfg80211_registered_device *rdev, > > + enum nl80211_tx_power_mode mode) > > +{ > > + int ret; > > + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); > > + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); > > + trace_rdev_return_int(&rdev->wiphy, ret); > > + return ret; > > +} > > + > > +static inline int > > +rdev_get_tx_power_mode(struct cfg80211_registered_device *rdev, > > + enum nl80211_tx_power_mode *mode) > > +{ > > + int ret; > > + trace_rdev_get_tx_power_mode(&rdev->wiphy); > > + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); > > + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); > > + return ret; > > +} > > + > > static inline int rdev_set_wds_peer(struct > > cfg80211_registered_device *rdev, > > struct net_device *dev, const u8 > > *addr) > > { > > diff --git a/net/wireless/trace.h b/net/wireless/trace.h > > index 09b242b..132c8c2 100644 > > --- a/net/wireless/trace.h > > +++ b/net/wireless/trace.h > > @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, > > WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, __entry- > > >mbm) > > ); > > > > +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, > > + TP_PROTO(struct wiphy *wiphy), > > + TP_ARGS(wiphy) > > +); > > + > > +TRACE_EVENT(rdev_set_tx_power_mode, > > + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode > > mode), > > + TP_ARGS(wiphy, mode), > > + TP_STRUCT__entry( > > + WIPHY_ENTRY > > + __field(enum nl80211_tx_power_mode, mode) > > + ), > > + TP_fast_assign( > > + WIPHY_ASSIGN; > > + __entry->mode = mode; > > + ), > > + TP_printk(WIPHY_PR_FMT ", mode: %d", > > + WIPHY_PR_ARG, __entry->mode) > > +); > > + > > TRACE_EVENT(rdev_return_int_int, > > TP_PROTO(struct wiphy *wiphy, int func_ret, int func_fill), > > TP_ARGS(wiphy, func_ret, func_fill),
On Fri, May 6, 2016 at 4:19 PM, Wei-Ning Huang <wnhuang@google.com> wrote: > On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> wrote: >> >> On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: >> > Recent new hardware has the ability to switch between tablet mode and >> > clamshell mode. To optimize WiFi performance, we want to be able to >> > use >> > different power table between modes. This patch adds a new netlink >> > message type and cfg80211_ops function to allow userspace to trigger >> > a >> > power mode switch for a given wireless interface. >> > >> > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> >> > --- >> > include/net/cfg80211.h | 11 +++++++++++ >> > include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ >> > net/wireless/nl80211.c | 16 ++++++++++++++++ >> > net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ >> > net/wireless/trace.h | 20 ++++++++++++++++++++ >> > 5 files changed, 90 insertions(+) >> > >> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h >> > index 9e1b24c..aa77fa0 100644 >> > --- a/include/net/cfg80211.h >> > +++ b/include/net/cfg80211.h >> > @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { >> > * @get_tx_power: store the current TX power into the dbm variable; >> > * return 0 if successful >> > * >> > + * @set_tx_power_mode: set the transmit power mode. Some device have >> > the ability >> > + * to transform between different mode such as clamshell and >> > tablet mode. >> > + * set_tx_power_mode allows setting of different TX power >> > mode at runtime. >> > + * @get_tx_power_mode: store the current TX power mode into the mode >> > variable; >> > + * return 0 if successful >> > + * >> > * @set_wds_peer: set the WDS peer for a WDS interface >> > * >> > * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting >> > @@ -2631,6 +2637,11 @@ struct cfg80211_ops { >> > int (*get_tx_power)(struct wiphy *wiphy, struct >> > wireless_dev *wdev, >> > int *dbm); >> > >> > + int (*set_tx_power_mode)(struct wiphy *wiphy, >> > + enum nl80211_tx_power_mode >> > mode); >> > + int (*get_tx_power_mode)(struct wiphy *wiphy, >> > + enum nl80211_tx_power_mode >> > *mode); >> > + >> > int (*set_wds_peer)(struct wiphy *wiphy, struct >> > net_device *dev, >> > const u8 *addr); >> > >> > diff --git a/include/uapi/linux/nl80211.h >> > b/include/uapi/linux/nl80211.h >> > index 5a30a75..9b1888a 100644 >> > --- a/include/uapi/linux/nl80211.h >> > +++ b/include/uapi/linux/nl80211.h >> > @@ -1796,6 +1796,9 @@ enum nl80211_commands { >> > * connecting to a PCP, and in %NL80211_CMD_START_AP to start >> > * a PCP instead of AP. Relevant for DMG networks only. >> > * >> > + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See >> > + * &enum nl80211_tx_power_mode for possible values. >> > + * >> > * @NUM_NL80211_ATTR: total number of nl80211_attrs available >> > * @NL80211_ATTR_MAX: highest attribute number currently defined >> > * @__NL80211_ATTR_AFTER_LAST: internal use >> > @@ -2172,6 +2175,8 @@ enum nl80211_attrs { >> > >> > NL80211_ATTR_PBSS, >> > >> > + NL80211_ATTR_WIPHY_TX_POWER_MODE, >> > + >> > /* add attributes here, update the policy in nl80211.c */ >> > >> > __NL80211_ATTR_AFTER_LAST, >> > @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { >> > }; >> > >> > /** >> > + * enum nl80211_tx_power_mode - TX power mode setting >> > + * @NL80211_TX_POWER_LOW: general low TX power mode >> > + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode >> > + * @NL80211_TX_POWER_HIGH: general high TX power mode >> > + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode >> > + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode >> > + */ >> > +enum nl80211_tx_power_mode { >> > + NL80211_TX_POWER_LOW, >> > + NL80211_TX_POWER_MEDIUM, >> > + NL80211_TX_POWER_HIGH, >> > + NL80211_TX_POWER_CLAMSHELL, >> > + NL80211_TX_POWER_TABLET, >> > >> "clamshell" and "tablet" probably mean many different things to many >> different people with respect to whether or not they should do anything >> with power saving or wifi. I feel like a more generic interface is >> needed here. > We could probably drop those two CLAMSHELL and TABLET constant or > describing what they mean > in more detail? > >> >> Could this be already done by: >> @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED >> @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet mode> >> >> and then the device would be able to change its TX power as it saw fit >> up to that limit set by your application which figures out whether it's >> in clamshell or tablet mode? > > We usually want different power settings in different band/channel. > For example, we can have three different power settings > in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we > can not simply set a fixed number to the power level. > A power table is required to map channel/band to actual power limit. > For most of the driver, changing power table requires loading > a new set of calibration data from either devicetree or ACPI. Due to > this, a new message type is required to allow the driver to > switch between tables. > > Wei-Ning Hi Dan, Does this make sense to you? If so I'll send another patch to clarify the term clamshell / tablet mode. Thanks for reviewing. Wei-Ning >> >> >> Dan >> >> > +}; >> > + >> > +/** >> > * enum nl80211_packet_pattern_attr - packet pattern attribute >> > * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute >> > * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask has >> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c >> > index 056a730..61b474d 100644 >> > --- a/net/wireless/nl80211.c >> > +++ b/net/wireless/nl80211.c >> > @@ -402,6 +402,7 @@ static const struct nla_policy >> > nl80211_policy[NUM_NL80211_ATTR] = { >> > [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, >> > [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, >> > [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, >> > + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, >> > }; >> > >> > /* policy for the key attributes */ >> > @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct sk_buff >> > *skb, struct genl_info *info) >> > return result; >> > } >> > >> > + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { >> > + enum nl80211_tx_power_mode mode; >> > + int idx = 0; >> > + >> > + if (!rdev->ops->set_tx_power_mode) >> > + return -EOPNOTSUPP; >> > + >> > + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; >> > + mode = nla_get_u32(info->attrs[idx]); >> > + >> > + result = rdev_set_tx_power_mode(rdev, mode); >> > + if (result) >> > + return result; >> > + } >> > + >> > if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && >> > info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { >> > u32 tx_ant, rx_ant; >> > diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h >> > index 8ae0c04..c3a1035 100644 >> > --- a/net/wireless/rdev-ops.h >> > +++ b/net/wireless/rdev-ops.h >> > @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct >> > cfg80211_registered_device *rdev, >> > return ret; >> > } >> > >> > +static inline int >> > +rdev_set_tx_power_mode(struct cfg80211_registered_device *rdev, >> > + enum nl80211_tx_power_mode mode) >> > +{ >> > + int ret; >> > + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); >> > + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); >> > + trace_rdev_return_int(&rdev->wiphy, ret); >> > + return ret; >> > +} >> > + >> > +static inline int >> > +rdev_get_tx_power_mode(struct cfg80211_registered_device *rdev, >> > + enum nl80211_tx_power_mode *mode) >> > +{ >> > + int ret; >> > + trace_rdev_get_tx_power_mode(&rdev->wiphy); >> > + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); >> > + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); >> > + return ret; >> > +} >> > + >> > static inline int rdev_set_wds_peer(struct >> > cfg80211_registered_device *rdev, >> > struct net_device *dev, const u8 >> > *addr) >> > { >> > diff --git a/net/wireless/trace.h b/net/wireless/trace.h >> > index 09b242b..132c8c2 100644 >> > --- a/net/wireless/trace.h >> > +++ b/net/wireless/trace.h >> > @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, >> > WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, __entry- >> > >mbm) >> > ); >> > >> > +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, >> > + TP_PROTO(struct wiphy *wiphy), >> > + TP_ARGS(wiphy) >> > +); >> > + >> > +TRACE_EVENT(rdev_set_tx_power_mode, >> > + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode >> > mode), >> > + TP_ARGS(wiphy, mode), >> > + TP_STRUCT__entry( >> > + WIPHY_ENTRY >> > + __field(enum nl80211_tx_power_mode, mode) >> > + ), >> > + TP_fast_assign( >> > + WIPHY_ASSIGN; >> > + __entry->mode = mode; >> > + ), >> > + TP_printk(WIPHY_PR_FMT ", mode: %d", >> > + WIPHY_PR_ARG, __entry->mode) >> > +); >> > + >> > TRACE_EVENT(rdev_return_int_int, >> > TP_PROTO(struct wiphy *wiphy, int func_ret, int func_fill), >> > TP_ARGS(wiphy, func_ret, func_fill), > > > > > -- > Wei-Ning Huang, ??? | Software Engineer, Google Inc., Taiwan | > wnhuang@google.com | Cell: +886 910-380678
On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: > Recent new hardware has the ability to switch between tablet mode and > clamshell mode. To optimize WiFi performance, we want to be able to > use different power table between modes. This patch adds a new > netlink message type and cfg80211_ops function to allow userspace to > trigger a power mode switch for a given wireless interface. I'm not a fan of this at all. Too much magic. FWIW, we implemented something similar, currently with vendor commands, that simply allows to set the TX power in a more controlled fashion, i.e. allows setting TX power per band/channel range. For our device it's only 2.4 GHz vs. 5.2 GHz, but I can imagine that you might want to split it a bit further, so perhaps we need to advertise those ranges somehow. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-05-11 at 13:03 +0800, Wei-Ning Huang wrote: > On Fri, May 6, 2016 at 4:19 PM, Wei-Ning Huang <wnhuang@google.com> > wrote: > > > > On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> > > wrote: > > > > > > > > > On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: > > > > > > > > Recent new hardware has the ability to switch between tablet > > > > mode and > > > > clamshell mode. To optimize WiFi performance, we want to be > > > > able to > > > > use > > > > different power table between modes. This patch adds a new > > > > netlink > > > > message type and cfg80211_ops function to allow userspace to > > > > trigger > > > > a > > > > power mode switch for a given wireless interface. > > > > > > > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> > > > > --- > > > > include/net/cfg80211.h | 11 +++++++++++ > > > > include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ > > > > net/wireless/nl80211.c | 16 ++++++++++++++++ > > > > net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ > > > > net/wireless/trace.h | 20 ++++++++++++++++++++ > > > > 5 files changed, 90 insertions(+) > > > > > > > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > > > > index 9e1b24c..aa77fa0 100644 > > > > --- a/include/net/cfg80211.h > > > > +++ b/include/net/cfg80211.h > > > > @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { > > > > * @get_tx_power: store the current TX power into the dbm > > > > variable; > > > > * return 0 if successful > > > > * > > > > + * @set_tx_power_mode: set the transmit power mode. Some > > > > device have > > > > the ability > > > > + * to transform between different mode such as clamshell and > > > > tablet mode. > > > > + * set_tx_power_mode allows setting of different TX power > > > > mode at runtime. > > > > + * @get_tx_power_mode: store the current TX power mode into > > > > the mode > > > > variable; > > > > + * return 0 if successful > > > > + * > > > > * @set_wds_peer: set the WDS peer for a WDS interface > > > > * > > > > * @rfkill_poll: polls the hw rfkill line, use cfg80211 > > > > reporting > > > > @@ -2631,6 +2637,11 @@ struct cfg80211_ops { > > > > int (*get_tx_power)(struct wiphy *wiphy, struct > > > > wireless_dev *wdev, > > > > int *dbm); > > > > > > > > + int (*set_tx_power_mode)(struct wiphy *wiphy, > > > > + enum nl80211_tx_power_mode > > > > mode); > > > > + int (*get_tx_power_mode)(struct wiphy *wiphy, > > > > + enum nl80211_tx_power_mode > > > > *mode); > > > > + > > > > int (*set_wds_peer)(struct wiphy *wiphy, struct > > > > net_device *dev, > > > > const u8 *addr); > > > > > > > > diff --git a/include/uapi/linux/nl80211.h > > > > b/include/uapi/linux/nl80211.h > > > > index 5a30a75..9b1888a 100644 > > > > --- a/include/uapi/linux/nl80211.h > > > > +++ b/include/uapi/linux/nl80211.h > > > > @@ -1796,6 +1796,9 @@ enum nl80211_commands { > > > > * connecting to a PCP, and in %NL80211_CMD_START_AP to > > > > start > > > > * a PCP instead of AP. Relevant for DMG networks only. > > > > * > > > > + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See > > > > + * &enum nl80211_tx_power_mode for possible values. > > > > + * > > > > * @NUM_NL80211_ATTR: total number of nl80211_attrs available > > > > * @NL80211_ATTR_MAX: highest attribute number currently > > > > defined > > > > * @__NL80211_ATTR_AFTER_LAST: internal use > > > > @@ -2172,6 +2175,8 @@ enum nl80211_attrs { > > > > > > > > NL80211_ATTR_PBSS, > > > > > > > > + NL80211_ATTR_WIPHY_TX_POWER_MODE, > > > > + > > > > /* add attributes here, update the policy in nl80211.c */ > > > > > > > > __NL80211_ATTR_AFTER_LAST, > > > > @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { > > > > }; > > > > > > > > /** > > > > + * enum nl80211_tx_power_mode - TX power mode setting > > > > + * @NL80211_TX_POWER_LOW: general low TX power mode > > > > + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode > > > > + * @NL80211_TX_POWER_HIGH: general high TX power mode > > > > + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode > > > > + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode > > > > + */ > > > > +enum nl80211_tx_power_mode { > > > > + NL80211_TX_POWER_LOW, > > > > + NL80211_TX_POWER_MEDIUM, > > > > + NL80211_TX_POWER_HIGH, > > > > + NL80211_TX_POWER_CLAMSHELL, > > > > + NL80211_TX_POWER_TABLET, > > > > > > "clamshell" and "tablet" probably mean many different things to > > > many > > > different people with respect to whether or not they should do > > > anything > > > with power saving or wifi. I feel like a more generic interface > > > is > > > needed here. > > We could probably drop those two CLAMSHELL and TABLET constant or > > describing what they mean > > in more detail? > > > > > > > > > > > Could this be already done by: > > > @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED > > > @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet > > > mode> > > > > > > and then the device would be able to change its TX power as it > > > saw fit > > > up to that limit set by your application which figures out > > > whether it's > > > in clamshell or tablet mode? > > We usually want different power settings in different band/channel. > > For example, we can have three different power settings > > in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we > > can not simply set a fixed number to the power level. > > A power table is required to map channel/band to actual power > > limit. > > For most of the driver, changing power table requires loading > > a new set of calibration data from either devicetree or ACPI. Due > > to > > this, a new message type is required to allow the driver to > > switch between tables. > > > > Wei-Ning > Hi Dan, > > Does this make sense to you? If so I'll send another patch to clarify > the term clamshell / tablet mode. > Thanks for reviewing. Yes, you're right that NL80211_ATTR_WIPHY_TX_POWER_SETTING is probably too limiting. But I feel that the constants that you've proposed are too "magic" and will mean many different things to different driver writers and userspace clients. I think it would be better implemented as a request to simply load specific power calibration data or a new power table, instead of attempting to classify power tables through kernel constants that could mean something different to everyone. Dan > Wei-Ning > > > > > > > > > > > > > > Dan > > > > > > > > > > > +}; > > > > + > > > > +/** > > > > * enum nl80211_packet_pattern_attr - packet pattern attribute > > > > * @__NL80211_PKTPAT_INVALID: invalid number for nested > > > > attribute > > > > * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask > > > > has > > > > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > > > > index 056a730..61b474d 100644 > > > > --- a/net/wireless/nl80211.c > > > > +++ b/net/wireless/nl80211.c > > > > @@ -402,6 +402,7 @@ static const struct nla_policy > > > > nl80211_policy[NUM_NL80211_ATTR] = { > > > > [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, > > > > [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, > > > > [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, > > > > + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, > > > > }; > > > > > > > > /* policy for the key attributes */ > > > > @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct > > > > sk_buff > > > > *skb, struct genl_info *info) > > > > return result; > > > > } > > > > > > > > + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { > > > > + enum nl80211_tx_power_mode mode; > > > > + int idx = 0; > > > > + > > > > + if (!rdev->ops->set_tx_power_mode) > > > > + return -EOPNOTSUPP; > > > > + > > > > + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; > > > > + mode = nla_get_u32(info->attrs[idx]); > > > > + > > > > + result = rdev_set_tx_power_mode(rdev, mode); > > > > + if (result) > > > > + return result; > > > > + } > > > > + > > > > if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && > > > > info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { > > > > u32 tx_ant, rx_ant; > > > > diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h > > > > index 8ae0c04..c3a1035 100644 > > > > --- a/net/wireless/rdev-ops.h > > > > +++ b/net/wireless/rdev-ops.h > > > > @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct > > > > cfg80211_registered_device *rdev, > > > > return ret; > > > > } > > > > > > > > +static inline int > > > > +rdev_set_tx_power_mode(struct cfg80211_registered_device > > > > *rdev, > > > > + enum nl80211_tx_power_mode mode) > > > > +{ > > > > + int ret; > > > > + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); > > > > + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); > > > > + trace_rdev_return_int(&rdev->wiphy, ret); > > > > + return ret; > > > > +} > > > > + > > > > +static inline int > > > > +rdev_get_tx_power_mode(struct cfg80211_registered_device > > > > *rdev, > > > > + enum nl80211_tx_power_mode *mode) > > > > +{ > > > > + int ret; > > > > + trace_rdev_get_tx_power_mode(&rdev->wiphy); > > > > + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); > > > > + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); > > > > + return ret; > > > > +} > > > > + > > > > static inline int rdev_set_wds_peer(struct > > > > cfg80211_registered_device *rdev, > > > > struct net_device *dev, const > > > > u8 > > > > *addr) > > > > { > > > > diff --git a/net/wireless/trace.h b/net/wireless/trace.h > > > > index 09b242b..132c8c2 100644 > > > > --- a/net/wireless/trace.h > > > > +++ b/net/wireless/trace.h > > > > @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, > > > > WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, > > > > __entry- > > > > > > > > > > mbm) > > > > ); > > > > > > > > +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, > > > > + TP_PROTO(struct wiphy *wiphy), > > > > + TP_ARGS(wiphy) > > > > +); > > > > + > > > > +TRACE_EVENT(rdev_set_tx_power_mode, > > > > + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode > > > > mode), > > > > + TP_ARGS(wiphy, mode), > > > > + TP_STRUCT__entry( > > > > + WIPHY_ENTRY > > > > + __field(enum nl80211_tx_power_mode, mode) > > > > + ), > > > > + TP_fast_assign( > > > > + WIPHY_ASSIGN; > > > > + __entry->mode = mode; > > > > + ), > > > > + TP_printk(WIPHY_PR_FMT ", mode: %d", > > > > + WIPHY_PR_ARG, __entry->mode) > > > > +); > > > > + > > > > TRACE_EVENT(rdev_return_int_int, > > > > TP_PROTO(struct wiphy *wiphy, int func_ret, int > > > > func_fill), > > > > TP_ARGS(wiphy, func_ret, func_fill), > > > > > > > > -- > > Wei-Ning Huang, ??? | Software Engineer, Google Inc., Taiwan | > > wnhuang@google.com | Cell: +886 910-380678 > > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 12, 2016 at 2:33 AM, Dan Williams <dcbw@redhat.com> wrote: > On Wed, 2016-05-11 at 13:03 +0800, Wei-Ning Huang wrote: >> On Fri, May 6, 2016 at 4:19 PM, Wei-Ning Huang <wnhuang@google.com> >> wrote: >> > >> > On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> >> > wrote: >> > > >> > > >> > > On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: >> > > > >> > > > Recent new hardware has the ability to switch between tablet >> > > > mode and >> > > > clamshell mode. To optimize WiFi performance, we want to be >> > > > able to >> > > > use >> > > > different power table between modes. This patch adds a new >> > > > netlink >> > > > message type and cfg80211_ops function to allow userspace to >> > > > trigger >> > > > a >> > > > power mode switch for a given wireless interface. >> > > > >> > > > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> >> > > > --- >> > > > include/net/cfg80211.h | 11 +++++++++++ >> > > > include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ >> > > > net/wireless/nl80211.c | 16 ++++++++++++++++ >> > > > net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ >> > > > net/wireless/trace.h | 20 ++++++++++++++++++++ >> > > > 5 files changed, 90 insertions(+) >> > > > >> > > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h >> > > > index 9e1b24c..aa77fa0 100644 >> > > > --- a/include/net/cfg80211.h >> > > > +++ b/include/net/cfg80211.h >> > > > @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { >> > > > * @get_tx_power: store the current TX power into the dbm >> > > > variable; >> > > > * return 0 if successful >> > > > * >> > > > + * @set_tx_power_mode: set the transmit power mode. Some >> > > > device have >> > > > the ability >> > > > + * to transform between different mode such as clamshell and >> > > > tablet mode. >> > > > + * set_tx_power_mode allows setting of different TX power >> > > > mode at runtime. >> > > > + * @get_tx_power_mode: store the current TX power mode into >> > > > the mode >> > > > variable; >> > > > + * return 0 if successful >> > > > + * >> > > > * @set_wds_peer: set the WDS peer for a WDS interface >> > > > * >> > > > * @rfkill_poll: polls the hw rfkill line, use cfg80211 >> > > > reporting >> > > > @@ -2631,6 +2637,11 @@ struct cfg80211_ops { >> > > > int (*get_tx_power)(struct wiphy *wiphy, struct >> > > > wireless_dev *wdev, >> > > > int *dbm); >> > > > >> > > > + int (*set_tx_power_mode)(struct wiphy *wiphy, >> > > > + enum nl80211_tx_power_mode >> > > > mode); >> > > > + int (*get_tx_power_mode)(struct wiphy *wiphy, >> > > > + enum nl80211_tx_power_mode >> > > > *mode); >> > > > + >> > > > int (*set_wds_peer)(struct wiphy *wiphy, struct >> > > > net_device *dev, >> > > > const u8 *addr); >> > > > >> > > > diff --git a/include/uapi/linux/nl80211.h >> > > > b/include/uapi/linux/nl80211.h >> > > > index 5a30a75..9b1888a 100644 >> > > > --- a/include/uapi/linux/nl80211.h >> > > > +++ b/include/uapi/linux/nl80211.h >> > > > @@ -1796,6 +1796,9 @@ enum nl80211_commands { >> > > > * connecting to a PCP, and in %NL80211_CMD_START_AP to >> > > > start >> > > > * a PCP instead of AP. Relevant for DMG networks only. >> > > > * >> > > > + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See >> > > > + * &enum nl80211_tx_power_mode for possible values. >> > > > + * >> > > > * @NUM_NL80211_ATTR: total number of nl80211_attrs available >> > > > * @NL80211_ATTR_MAX: highest attribute number currently >> > > > defined >> > > > * @__NL80211_ATTR_AFTER_LAST: internal use >> > > > @@ -2172,6 +2175,8 @@ enum nl80211_attrs { >> > > > >> > > > NL80211_ATTR_PBSS, >> > > > >> > > > + NL80211_ATTR_WIPHY_TX_POWER_MODE, >> > > > + >> > > > /* add attributes here, update the policy in nl80211.c */ >> > > > >> > > > __NL80211_ATTR_AFTER_LAST, >> > > > @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { >> > > > }; >> > > > >> > > > /** >> > > > + * enum nl80211_tx_power_mode - TX power mode setting >> > > > + * @NL80211_TX_POWER_LOW: general low TX power mode >> > > > + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode >> > > > + * @NL80211_TX_POWER_HIGH: general high TX power mode >> > > > + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode >> > > > + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode >> > > > + */ >> > > > +enum nl80211_tx_power_mode { >> > > > + NL80211_TX_POWER_LOW, >> > > > + NL80211_TX_POWER_MEDIUM, >> > > > + NL80211_TX_POWER_HIGH, >> > > > + NL80211_TX_POWER_CLAMSHELL, >> > > > + NL80211_TX_POWER_TABLET, >> > > >> > > "clamshell" and "tablet" probably mean many different things to >> > > many >> > > different people with respect to whether or not they should do >> > > anything >> > > with power saving or wifi. I feel like a more generic interface >> > > is >> > > needed here. >> > We could probably drop those two CLAMSHELL and TABLET constant or >> > describing what they mean >> > in more detail? >> > >> > > >> > > >> > > Could this be already done by: >> > > @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED >> > > @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet >> > > mode> >> > > >> > > and then the device would be able to change its TX power as it >> > > saw fit >> > > up to that limit set by your application which figures out >> > > whether it's >> > > in clamshell or tablet mode? >> > We usually want different power settings in different band/channel. >> > For example, we can have three different power settings >> > in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we >> > can not simply set a fixed number to the power level. >> > A power table is required to map channel/band to actual power >> > limit. >> > For most of the driver, changing power table requires loading >> > a new set of calibration data from either devicetree or ACPI. Due >> > to >> > this, a new message type is required to allow the driver to >> > switch between tables. >> > >> > Wei-Ning >> Hi Dan, >> >> Does this make sense to you? If so I'll send another patch to clarify >> the term clamshell / tablet mode. >> Thanks for reviewing. > > Yes, you're right that NL80211_ATTR_WIPHY_TX_POWER_SETTING is probably > too limiting. But I feel that the constants that you've proposed are > too "magic" and will mean many different things to different driver > writers and userspace clients. I think it would be better implemented > as a request to simply load specific power calibration data or a new > power table, instead of attempting to classify power tables through > kernel constants that could mean something different to everyone. Dan, Thanks for the pointer. Passing the power table/calibration data name instead of constants sounds reasonable. Johannes, I feel like being able to set calibration data at runtime is something common to all wireless drivers, so instead of using vendor commands what do you think if I pass the calibration data name instead of using those magic constants? This way, userspace does not need to know the details of what band/range power limit the driver supports. It allows for flexible driver side implementation and easier for userspace to control. Wei-Ning > > Dan > >> Wei-Ning >> > >> > > >> > > >> > > >> > > Dan >> > > >> > > > >> > > > +}; >> > > > + >> > > > +/** >> > > > * enum nl80211_packet_pattern_attr - packet pattern attribute >> > > > * @__NL80211_PKTPAT_INVALID: invalid number for nested >> > > > attribute >> > > > * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask >> > > > has >> > > > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c >> > > > index 056a730..61b474d 100644 >> > > > --- a/net/wireless/nl80211.c >> > > > +++ b/net/wireless/nl80211.c >> > > > @@ -402,6 +402,7 @@ static const struct nla_policy >> > > > nl80211_policy[NUM_NL80211_ATTR] = { >> > > > [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, >> > > > [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, >> > > > [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, >> > > > + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, >> > > > }; >> > > > >> > > > /* policy for the key attributes */ >> > > > @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct >> > > > sk_buff >> > > > *skb, struct genl_info *info) >> > > > return result; >> > > > } >> > > > >> > > > + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { >> > > > + enum nl80211_tx_power_mode mode; >> > > > + int idx = 0; >> > > > + >> > > > + if (!rdev->ops->set_tx_power_mode) >> > > > + return -EOPNOTSUPP; >> > > > + >> > > > + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; >> > > > + mode = nla_get_u32(info->attrs[idx]); >> > > > + >> > > > + result = rdev_set_tx_power_mode(rdev, mode); >> > > > + if (result) >> > > > + return result; >> > > > + } >> > > > + >> > > > if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && >> > > > info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { >> > > > u32 tx_ant, rx_ant; >> > > > diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h >> > > > index 8ae0c04..c3a1035 100644 >> > > > --- a/net/wireless/rdev-ops.h >> > > > +++ b/net/wireless/rdev-ops.h >> > > > @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct >> > > > cfg80211_registered_device *rdev, >> > > > return ret; >> > > > } >> > > > >> > > > +static inline int >> > > > +rdev_set_tx_power_mode(struct cfg80211_registered_device >> > > > *rdev, >> > > > + enum nl80211_tx_power_mode mode) >> > > > +{ >> > > > + int ret; >> > > > + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); >> > > > + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); >> > > > + trace_rdev_return_int(&rdev->wiphy, ret); >> > > > + return ret; >> > > > +} >> > > > + >> > > > +static inline int >> > > > +rdev_get_tx_power_mode(struct cfg80211_registered_device >> > > > *rdev, >> > > > + enum nl80211_tx_power_mode *mode) >> > > > +{ >> > > > + int ret; >> > > > + trace_rdev_get_tx_power_mode(&rdev->wiphy); >> > > > + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); >> > > > + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); >> > > > + return ret; >> > > > +} >> > > > + >> > > > static inline int rdev_set_wds_peer(struct >> > > > cfg80211_registered_device *rdev, >> > > > struct net_device *dev, const >> > > > u8 >> > > > *addr) >> > > > { >> > > > diff --git a/net/wireless/trace.h b/net/wireless/trace.h >> > > > index 09b242b..132c8c2 100644 >> > > > --- a/net/wireless/trace.h >> > > > +++ b/net/wireless/trace.h >> > > > @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, >> > > > WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, >> > > > __entry- >> > > > > >> > > > > mbm) >> > > > ); >> > > > >> > > > +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, >> > > > + TP_PROTO(struct wiphy *wiphy), >> > > > + TP_ARGS(wiphy) >> > > > +); >> > > > + >> > > > +TRACE_EVENT(rdev_set_tx_power_mode, >> > > > + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode >> > > > mode), >> > > > + TP_ARGS(wiphy, mode), >> > > > + TP_STRUCT__entry( >> > > > + WIPHY_ENTRY >> > > > + __field(enum nl80211_tx_power_mode, mode) >> > > > + ), >> > > > + TP_fast_assign( >> > > > + WIPHY_ASSIGN; >> > > > + __entry->mode = mode; >> > > > + ), >> > > > + TP_printk(WIPHY_PR_FMT ", mode: %d", >> > > > + WIPHY_PR_ARG, __entry->mode) >> > > > +); >> > > > + >> > > > TRACE_EVENT(rdev_return_int_int, >> > > > TP_PROTO(struct wiphy *wiphy, int func_ret, int >> > > > func_fill), >> > > > TP_ARGS(wiphy, func_ret, func_fill), >> > >> > >> > >> > -- >> > Wei-Ning Huang, ??? | Software Engineer, Google Inc., Taiwan | >> > wnhuang@google.com | Cell: +886 910-380678 >> >>
On 12-05-16 11:34, Wei-Ning Huang wrote: > On Thu, May 12, 2016 at 2:33 AM, Dan Williams <dcbw@redhat.com> wrote: >> On Wed, 2016-05-11 at 13:03 +0800, Wei-Ning Huang wrote: >>> On Fri, May 6, 2016 at 4:19 PM, Wei-Ning Huang <wnhuang@google.com> >>> wrote: >>>> >>>> On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> >>>> wrote: >>>>> >>>>> >>>>> On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: >>>>>> >>>>>> Recent new hardware has the ability to switch between tablet >>>>>> mode and >>>>>> clamshell mode. To optimize WiFi performance, we want to be >>>>>> able to >>>>>> use >>>>>> different power table between modes. This patch adds a new >>>>>> netlink >>>>>> message type and cfg80211_ops function to allow userspace to >>>>>> trigger >>>>>> a >>>>>> power mode switch for a given wireless interface. >>>>>> >>>>>> Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> >>>>>> --- >>>>>> include/net/cfg80211.h | 11 +++++++++++ >>>>>> include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ >>>>>> net/wireless/nl80211.c | 16 ++++++++++++++++ >>>>>> net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ >>>>>> net/wireless/trace.h | 20 ++++++++++++++++++++ >>>>>> 5 files changed, 90 insertions(+) >>>>>> >>>>>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h >>>>>> index 9e1b24c..aa77fa0 100644 >>>>>> --- a/include/net/cfg80211.h >>>>>> +++ b/include/net/cfg80211.h >>>>>> @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { >>>>>> * @get_tx_power: store the current TX power into the dbm >>>>>> variable; >>>>>> * return 0 if successful >>>>>> * >>>>>> + * @set_tx_power_mode: set the transmit power mode. Some >>>>>> device have >>>>>> the ability >>>>>> + * to transform between different mode such as clamshell and >>>>>> tablet mode. >>>>>> + * set_tx_power_mode allows setting of different TX power >>>>>> mode at runtime. >>>>>> + * @get_tx_power_mode: store the current TX power mode into >>>>>> the mode >>>>>> variable; >>>>>> + * return 0 if successful >>>>>> + * >>>>>> * @set_wds_peer: set the WDS peer for a WDS interface >>>>>> * >>>>>> * @rfkill_poll: polls the hw rfkill line, use cfg80211 >>>>>> reporting >>>>>> @@ -2631,6 +2637,11 @@ struct cfg80211_ops { >>>>>> int (*get_tx_power)(struct wiphy *wiphy, struct >>>>>> wireless_dev *wdev, >>>>>> int *dbm); >>>>>> >>>>>> + int (*set_tx_power_mode)(struct wiphy *wiphy, >>>>>> + enum nl80211_tx_power_mode >>>>>> mode); >>>>>> + int (*get_tx_power_mode)(struct wiphy *wiphy, >>>>>> + enum nl80211_tx_power_mode >>>>>> *mode); >>>>>> + >>>>>> int (*set_wds_peer)(struct wiphy *wiphy, struct >>>>>> net_device *dev, >>>>>> const u8 *addr); >>>>>> >>>>>> diff --git a/include/uapi/linux/nl80211.h >>>>>> b/include/uapi/linux/nl80211.h >>>>>> index 5a30a75..9b1888a 100644 >>>>>> --- a/include/uapi/linux/nl80211.h >>>>>> +++ b/include/uapi/linux/nl80211.h >>>>>> @@ -1796,6 +1796,9 @@ enum nl80211_commands { >>>>>> * connecting to a PCP, and in %NL80211_CMD_START_AP to >>>>>> start >>>>>> * a PCP instead of AP. Relevant for DMG networks only. >>>>>> * >>>>>> + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See >>>>>> + * &enum nl80211_tx_power_mode for possible values. >>>>>> + * >>>>>> * @NUM_NL80211_ATTR: total number of nl80211_attrs available >>>>>> * @NL80211_ATTR_MAX: highest attribute number currently >>>>>> defined >>>>>> * @__NL80211_ATTR_AFTER_LAST: internal use >>>>>> @@ -2172,6 +2175,8 @@ enum nl80211_attrs { >>>>>> >>>>>> NL80211_ATTR_PBSS, >>>>>> >>>>>> + NL80211_ATTR_WIPHY_TX_POWER_MODE, >>>>>> + >>>>>> /* add attributes here, update the policy in nl80211.c */ >>>>>> >>>>>> __NL80211_ATTR_AFTER_LAST, >>>>>> @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { >>>>>> }; >>>>>> >>>>>> /** >>>>>> + * enum nl80211_tx_power_mode - TX power mode setting >>>>>> + * @NL80211_TX_POWER_LOW: general low TX power mode >>>>>> + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode >>>>>> + * @NL80211_TX_POWER_HIGH: general high TX power mode >>>>>> + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode >>>>>> + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode >>>>>> + */ >>>>>> +enum nl80211_tx_power_mode { >>>>>> + NL80211_TX_POWER_LOW, >>>>>> + NL80211_TX_POWER_MEDIUM, >>>>>> + NL80211_TX_POWER_HIGH, >>>>>> + NL80211_TX_POWER_CLAMSHELL, >>>>>> + NL80211_TX_POWER_TABLET, >>>>> >>>>> "clamshell" and "tablet" probably mean many different things to >>>>> many >>>>> different people with respect to whether or not they should do >>>>> anything >>>>> with power saving or wifi. I feel like a more generic interface >>>>> is >>>>> needed here. >>>> We could probably drop those two CLAMSHELL and TABLET constant or >>>> describing what they mean >>>> in more detail? >>>> >>>>> >>>>> >>>>> Could this be already done by: >>>>> @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED >>>>> @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet >>>>> mode> >>>>> >>>>> and then the device would be able to change its TX power as it >>>>> saw fit >>>>> up to that limit set by your application which figures out >>>>> whether it's >>>>> in clamshell or tablet mode? >>>> We usually want different power settings in different band/channel. >>>> For example, we can have three different power settings >>>> in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we >>>> can not simply set a fixed number to the power level. >>>> A power table is required to map channel/band to actual power >>>> limit. >>>> For most of the driver, changing power table requires loading >>>> a new set of calibration data from either devicetree or ACPI. Due >>>> to >>>> this, a new message type is required to allow the driver to >>>> switch between tables. >>>> >>>> Wei-Ning >>> Hi Dan, >>> >>> Does this make sense to you? If so I'll send another patch to clarify >>> the term clamshell / tablet mode. >>> Thanks for reviewing. >> >> Yes, you're right that NL80211_ATTR_WIPHY_TX_POWER_SETTING is probably >> too limiting. But I feel that the constants that you've proposed are >> too "magic" and will mean many different things to different driver >> writers and userspace clients. I think it would be better implemented >> as a request to simply load specific power calibration data or a new >> power table, instead of attempting to classify power tables through >> kernel constants that could mean something different to everyone. > > Dan, Thanks for the pointer. Passing the power table/calibration data > name instead of > constants sounds reasonable. > > Johannes, I feel like being able to set calibration data at runtime is > something common > to all wireless drivers Not really. Only implicitly, eg. when changing to another frequency band etc. > so instead of using vendor commands what do > you think if I pass > the calibration data name instead of using those magic constants? This > way, userspace > does not need to know the details of what band/range power limit the > driver supports. user-space still need to have knowledge about what calibration data name to pick for different scenarios. Also this means the driver will use request_firmware() api to obtain the actual calibration data? I think you then want that signed like the crda database. Regards, Arend > It allows for flexible driver side implementation and easier for > userspace to control. > > Wei-Ning > >> >> Dan >> >>> Wei-Ning >>>> >>>>> >>>>> >>>>> >>>>> Dan >>>>> >>>>>> >>>>>> +}; >>>>>> + >>>>>> +/** >>>>>> * enum nl80211_packet_pattern_attr - packet pattern attribute >>>>>> * @__NL80211_PKTPAT_INVALID: invalid number for nested >>>>>> attribute >>>>>> * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask >>>>>> has >>>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c >>>>>> index 056a730..61b474d 100644 >>>>>> --- a/net/wireless/nl80211.c >>>>>> +++ b/net/wireless/nl80211.c >>>>>> @@ -402,6 +402,7 @@ static const struct nla_policy >>>>>> nl80211_policy[NUM_NL80211_ATTR] = { >>>>>> [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, >>>>>> [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, >>>>>> [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, >>>>>> + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, >>>>>> }; >>>>>> >>>>>> /* policy for the key attributes */ >>>>>> @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct >>>>>> sk_buff >>>>>> *skb, struct genl_info *info) >>>>>> return result; >>>>>> } >>>>>> >>>>>> + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { >>>>>> + enum nl80211_tx_power_mode mode; >>>>>> + int idx = 0; >>>>>> + >>>>>> + if (!rdev->ops->set_tx_power_mode) >>>>>> + return -EOPNOTSUPP; >>>>>> + >>>>>> + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; >>>>>> + mode = nla_get_u32(info->attrs[idx]); >>>>>> + >>>>>> + result = rdev_set_tx_power_mode(rdev, mode); >>>>>> + if (result) >>>>>> + return result; >>>>>> + } >>>>>> + >>>>>> if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && >>>>>> info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { >>>>>> u32 tx_ant, rx_ant; >>>>>> diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h >>>>>> index 8ae0c04..c3a1035 100644 >>>>>> --- a/net/wireless/rdev-ops.h >>>>>> +++ b/net/wireless/rdev-ops.h >>>>>> @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct >>>>>> cfg80211_registered_device *rdev, >>>>>> return ret; >>>>>> } >>>>>> >>>>>> +static inline int >>>>>> +rdev_set_tx_power_mode(struct cfg80211_registered_device >>>>>> *rdev, >>>>>> + enum nl80211_tx_power_mode mode) >>>>>> +{ >>>>>> + int ret; >>>>>> + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); >>>>>> + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); >>>>>> + trace_rdev_return_int(&rdev->wiphy, ret); >>>>>> + return ret; >>>>>> +} >>>>>> + >>>>>> +static inline int >>>>>> +rdev_get_tx_power_mode(struct cfg80211_registered_device >>>>>> *rdev, >>>>>> + enum nl80211_tx_power_mode *mode) >>>>>> +{ >>>>>> + int ret; >>>>>> + trace_rdev_get_tx_power_mode(&rdev->wiphy); >>>>>> + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); >>>>>> + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); >>>>>> + return ret; >>>>>> +} >>>>>> + >>>>>> static inline int rdev_set_wds_peer(struct >>>>>> cfg80211_registered_device *rdev, >>>>>> struct net_device *dev, const >>>>>> u8 >>>>>> *addr) >>>>>> { >>>>>> diff --git a/net/wireless/trace.h b/net/wireless/trace.h >>>>>> index 09b242b..132c8c2 100644 >>>>>> --- a/net/wireless/trace.h >>>>>> +++ b/net/wireless/trace.h >>>>>> @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, >>>>>> WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, >>>>>> __entry- >>>>>>> >>>>>>> mbm) >>>>>> ); >>>>>> >>>>>> +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, >>>>>> + TP_PROTO(struct wiphy *wiphy), >>>>>> + TP_ARGS(wiphy) >>>>>> +); >>>>>> + >>>>>> +TRACE_EVENT(rdev_set_tx_power_mode, >>>>>> + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode >>>>>> mode), >>>>>> + TP_ARGS(wiphy, mode), >>>>>> + TP_STRUCT__entry( >>>>>> + WIPHY_ENTRY >>>>>> + __field(enum nl80211_tx_power_mode, mode) >>>>>> + ), >>>>>> + TP_fast_assign( >>>>>> + WIPHY_ASSIGN; >>>>>> + __entry->mode = mode; >>>>>> + ), >>>>>> + TP_printk(WIPHY_PR_FMT ", mode: %d", >>>>>> + WIPHY_PR_ARG, __entry->mode) >>>>>> +); >>>>>> + >>>>>> TRACE_EVENT(rdev_return_int_int, >>>>>> TP_PROTO(struct wiphy *wiphy, int func_ret, int >>>>>> func_fill), >>>>>> TP_ARGS(wiphy, func_ret, func_fill), >>>> >>>> >>>> >>>> -- >>>> Wei-Ning Huang, ??? | Software Engineer, Google Inc., Taiwan | >>>> wnhuang@google.com | Cell: +886 910-380678 >>> >>> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 13, 2016 at 6:02 AM, Arend van Spriel <arend.vanspriel@broadcom.com> wrote: > On 12-05-16 11:34, Wei-Ning Huang wrote: >> On Thu, May 12, 2016 at 2:33 AM, Dan Williams <dcbw@redhat.com> wrote: >>> On Wed, 2016-05-11 at 13:03 +0800, Wei-Ning Huang wrote: >>>> On Fri, May 6, 2016 at 4:19 PM, Wei-Ning Huang <wnhuang@google.com> >>>> wrote: >>>>> >>>>> On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> >>>>> wrote: >>>>>> >>>>>> >>>>>> On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: >>>>>>> >>>>>>> Recent new hardware has the ability to switch between tablet >>>>>>> mode and >>>>>>> clamshell mode. To optimize WiFi performance, we want to be >>>>>>> able to >>>>>>> use >>>>>>> different power table between modes. This patch adds a new >>>>>>> netlink >>>>>>> message type and cfg80211_ops function to allow userspace to >>>>>>> trigger >>>>>>> a >>>>>>> power mode switch for a given wireless interface. >>>>>>> >>>>>>> Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> >>>>>>> --- >>>>>>> include/net/cfg80211.h | 11 +++++++++++ >>>>>>> include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ >>>>>>> net/wireless/nl80211.c | 16 ++++++++++++++++ >>>>>>> net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ >>>>>>> net/wireless/trace.h | 20 ++++++++++++++++++++ >>>>>>> 5 files changed, 90 insertions(+) >>>>>>> >>>>>>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h >>>>>>> index 9e1b24c..aa77fa0 100644 >>>>>>> --- a/include/net/cfg80211.h >>>>>>> +++ b/include/net/cfg80211.h >>>>>>> @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { >>>>>>> * @get_tx_power: store the current TX power into the dbm >>>>>>> variable; >>>>>>> * return 0 if successful >>>>>>> * >>>>>>> + * @set_tx_power_mode: set the transmit power mode. Some >>>>>>> device have >>>>>>> the ability >>>>>>> + * to transform between different mode such as clamshell and >>>>>>> tablet mode. >>>>>>> + * set_tx_power_mode allows setting of different TX power >>>>>>> mode at runtime. >>>>>>> + * @get_tx_power_mode: store the current TX power mode into >>>>>>> the mode >>>>>>> variable; >>>>>>> + * return 0 if successful >>>>>>> + * >>>>>>> * @set_wds_peer: set the WDS peer for a WDS interface >>>>>>> * >>>>>>> * @rfkill_poll: polls the hw rfkill line, use cfg80211 >>>>>>> reporting >>>>>>> @@ -2631,6 +2637,11 @@ struct cfg80211_ops { >>>>>>> int (*get_tx_power)(struct wiphy *wiphy, struct >>>>>>> wireless_dev *wdev, >>>>>>> int *dbm); >>>>>>> >>>>>>> + int (*set_tx_power_mode)(struct wiphy *wiphy, >>>>>>> + enum nl80211_tx_power_mode >>>>>>> mode); >>>>>>> + int (*get_tx_power_mode)(struct wiphy *wiphy, >>>>>>> + enum nl80211_tx_power_mode >>>>>>> *mode); >>>>>>> + >>>>>>> int (*set_wds_peer)(struct wiphy *wiphy, struct >>>>>>> net_device *dev, >>>>>>> const u8 *addr); >>>>>>> >>>>>>> diff --git a/include/uapi/linux/nl80211.h >>>>>>> b/include/uapi/linux/nl80211.h >>>>>>> index 5a30a75..9b1888a 100644 >>>>>>> --- a/include/uapi/linux/nl80211.h >>>>>>> +++ b/include/uapi/linux/nl80211.h >>>>>>> @@ -1796,6 +1796,9 @@ enum nl80211_commands { >>>>>>> * connecting to a PCP, and in %NL80211_CMD_START_AP to >>>>>>> start >>>>>>> * a PCP instead of AP. Relevant for DMG networks only. >>>>>>> * >>>>>>> + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See >>>>>>> + * &enum nl80211_tx_power_mode for possible values. >>>>>>> + * >>>>>>> * @NUM_NL80211_ATTR: total number of nl80211_attrs available >>>>>>> * @NL80211_ATTR_MAX: highest attribute number currently >>>>>>> defined >>>>>>> * @__NL80211_ATTR_AFTER_LAST: internal use >>>>>>> @@ -2172,6 +2175,8 @@ enum nl80211_attrs { >>>>>>> >>>>>>> NL80211_ATTR_PBSS, >>>>>>> >>>>>>> + NL80211_ATTR_WIPHY_TX_POWER_MODE, >>>>>>> + >>>>>>> /* add attributes here, update the policy in nl80211.c */ >>>>>>> >>>>>>> __NL80211_ATTR_AFTER_LAST, >>>>>>> @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { >>>>>>> }; >>>>>>> >>>>>>> /** >>>>>>> + * enum nl80211_tx_power_mode - TX power mode setting >>>>>>> + * @NL80211_TX_POWER_LOW: general low TX power mode >>>>>>> + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode >>>>>>> + * @NL80211_TX_POWER_HIGH: general high TX power mode >>>>>>> + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode >>>>>>> + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode >>>>>>> + */ >>>>>>> +enum nl80211_tx_power_mode { >>>>>>> + NL80211_TX_POWER_LOW, >>>>>>> + NL80211_TX_POWER_MEDIUM, >>>>>>> + NL80211_TX_POWER_HIGH, >>>>>>> + NL80211_TX_POWER_CLAMSHELL, >>>>>>> + NL80211_TX_POWER_TABLET, >>>>>> >>>>>> "clamshell" and "tablet" probably mean many different things to >>>>>> many >>>>>> different people with respect to whether or not they should do >>>>>> anything >>>>>> with power saving or wifi. I feel like a more generic interface >>>>>> is >>>>>> needed here. >>>>> We could probably drop those two CLAMSHELL and TABLET constant or >>>>> describing what they mean >>>>> in more detail? >>>>> >>>>>> >>>>>> >>>>>> Could this be already done by: >>>>>> @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED >>>>>> @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet >>>>>> mode> >>>>>> >>>>>> and then the device would be able to change its TX power as it >>>>>> saw fit >>>>>> up to that limit set by your application which figures out >>>>>> whether it's >>>>>> in clamshell or tablet mode? >>>>> We usually want different power settings in different band/channel. >>>>> For example, we can have three different power settings >>>>> in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we >>>>> can not simply set a fixed number to the power level. >>>>> A power table is required to map channel/band to actual power >>>>> limit. >>>>> For most of the driver, changing power table requires loading >>>>> a new set of calibration data from either devicetree or ACPI. Due >>>>> to >>>>> this, a new message type is required to allow the driver to >>>>> switch between tables. >>>>> >>>>> Wei-Ning >>>> Hi Dan, >>>> >>>> Does this make sense to you? If so I'll send another patch to clarify >>>> the term clamshell / tablet mode. >>>> Thanks for reviewing. >>> >>> Yes, you're right that NL80211_ATTR_WIPHY_TX_POWER_SETTING is probably >>> too limiting. But I feel that the constants that you've proposed are >>> too "magic" and will mean many different things to different driver >>> writers and userspace clients. I think it would be better implemented >>> as a request to simply load specific power calibration data or a new >>> power table, instead of attempting to classify power tables through >>> kernel constants that could mean something different to everyone. >> >> Dan, Thanks for the pointer. Passing the power table/calibration data >> name instead of >> constants sounds reasonable. >> >> Johannes, I feel like being able to set calibration data at runtime is >> something common >> to all wireless drivers > > Not really. Only implicitly, eg. when changing to another frequency band > etc. > >> so instead of using vendor commands what do >> you think if I pass >> the calibration data name instead of using those magic constants? This >> way, userspace >> does not need to know the details of what band/range power limit the >> driver supports. > > user-space still need to have knowledge about what calibration data name > to pick for different scenarios. Also this means the driver will use > request_firmware() api to obtain the actual calibration data? I think > you then want that signed like the crda database. We usually store the calibration data in device tree, so it's only a matter of selecting the right name. Wei-Ning > > Regards, > Arend > >> It allows for flexible driver side implementation and easier for >> userspace to control. >> >> Wei-Ning >> >>> >>> Dan >>> >>>> Wei-Ning >>>>> >>>>>> >>>>>> >>>>>> >>>>>> Dan >>>>>> >>>>>>> >>>>>>> +}; >>>>>>> + >>>>>>> +/** >>>>>>> * enum nl80211_packet_pattern_attr - packet pattern attribute >>>>>>> * @__NL80211_PKTPAT_INVALID: invalid number for nested >>>>>>> attribute >>>>>>> * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask >>>>>>> has >>>>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c >>>>>>> index 056a730..61b474d 100644 >>>>>>> --- a/net/wireless/nl80211.c >>>>>>> +++ b/net/wireless/nl80211.c >>>>>>> @@ -402,6 +402,7 @@ static const struct nla_policy >>>>>>> nl80211_policy[NUM_NL80211_ATTR] = { >>>>>>> [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, >>>>>>> [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, >>>>>>> [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, >>>>>>> + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, >>>>>>> }; >>>>>>> >>>>>>> /* policy for the key attributes */ >>>>>>> @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct >>>>>>> sk_buff >>>>>>> *skb, struct genl_info *info) >>>>>>> return result; >>>>>>> } >>>>>>> >>>>>>> + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { >>>>>>> + enum nl80211_tx_power_mode mode; >>>>>>> + int idx = 0; >>>>>>> + >>>>>>> + if (!rdev->ops->set_tx_power_mode) >>>>>>> + return -EOPNOTSUPP; >>>>>>> + >>>>>>> + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; >>>>>>> + mode = nla_get_u32(info->attrs[idx]); >>>>>>> + >>>>>>> + result = rdev_set_tx_power_mode(rdev, mode); >>>>>>> + if (result) >>>>>>> + return result; >>>>>>> + } >>>>>>> + >>>>>>> if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && >>>>>>> info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { >>>>>>> u32 tx_ant, rx_ant; >>>>>>> diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h >>>>>>> index 8ae0c04..c3a1035 100644 >>>>>>> --- a/net/wireless/rdev-ops.h >>>>>>> +++ b/net/wireless/rdev-ops.h >>>>>>> @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct >>>>>>> cfg80211_registered_device *rdev, >>>>>>> return ret; >>>>>>> } >>>>>>> >>>>>>> +static inline int >>>>>>> +rdev_set_tx_power_mode(struct cfg80211_registered_device >>>>>>> *rdev, >>>>>>> + enum nl80211_tx_power_mode mode) >>>>>>> +{ >>>>>>> + int ret; >>>>>>> + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); >>>>>>> + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); >>>>>>> + trace_rdev_return_int(&rdev->wiphy, ret); >>>>>>> + return ret; >>>>>>> +} >>>>>>> + >>>>>>> +static inline int >>>>>>> +rdev_get_tx_power_mode(struct cfg80211_registered_device >>>>>>> *rdev, >>>>>>> + enum nl80211_tx_power_mode *mode) >>>>>>> +{ >>>>>>> + int ret; >>>>>>> + trace_rdev_get_tx_power_mode(&rdev->wiphy); >>>>>>> + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); >>>>>>> + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); >>>>>>> + return ret; >>>>>>> +} >>>>>>> + >>>>>>> static inline int rdev_set_wds_peer(struct >>>>>>> cfg80211_registered_device *rdev, >>>>>>> struct net_device *dev, const >>>>>>> u8 >>>>>>> *addr) >>>>>>> { >>>>>>> diff --git a/net/wireless/trace.h b/net/wireless/trace.h >>>>>>> index 09b242b..132c8c2 100644 >>>>>>> --- a/net/wireless/trace.h >>>>>>> +++ b/net/wireless/trace.h >>>>>>> @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, >>>>>>> WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, >>>>>>> __entry- >>>>>>>> >>>>>>>> mbm) >>>>>>> ); >>>>>>> >>>>>>> +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, >>>>>>> + TP_PROTO(struct wiphy *wiphy), >>>>>>> + TP_ARGS(wiphy) >>>>>>> +); >>>>>>> + >>>>>>> +TRACE_EVENT(rdev_set_tx_power_mode, >>>>>>> + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode >>>>>>> mode), >>>>>>> + TP_ARGS(wiphy, mode), >>>>>>> + TP_STRUCT__entry( >>>>>>> + WIPHY_ENTRY >>>>>>> + __field(enum nl80211_tx_power_mode, mode) >>>>>>> + ), >>>>>>> + TP_fast_assign( >>>>>>> + WIPHY_ASSIGN; >>>>>>> + __entry->mode = mode; >>>>>>> + ), >>>>>>> + TP_printk(WIPHY_PR_FMT ", mode: %d", >>>>>>> + WIPHY_PR_ARG, __entry->mode) >>>>>>> +); >>>>>>> + >>>>>>> TRACE_EVENT(rdev_return_int_int, >>>>>>> TP_PROTO(struct wiphy *wiphy, int func_ret, int >>>>>>> func_fill), >>>>>>> TP_ARGS(wiphy, func_ret, func_fill), >>>>> >>>>> >>>>> >>>>> -- >>>>> Wei-Ning Huang, ??? | Software Engineer, Google Inc., Taiwan | >>>>> wnhuang@google.com | Cell: +886 910-380678 >>>> >>>> >> >> >>
On Thu, 2016-05-12 at 17:34 +0800, Wei-Ning Huang wrote: > > Johannes, I feel like being able to set calibration data at runtime > is something common to all wireless drivers, so instead of using > vendor commands what do you think if I pass the calibration data name > instead of using those magic constants? This way, userspace does not > need to know the details of what band/range power limit the driver > supports. It allows for flexible driver side implementation and > easier for userspace to control. > Sorry - I dropped this thread accidentally. I'm not really sure I understand the situation fully, but right now to me this seems very strange. The physical antennas probably don't really change between "clamshell" and "tablet" mode, do the physical radiation properties change enough to actually require different *calibration*? To me, that sounds very strange. Assuming they don't really change fundamentally, then I understand the need to set different power levels, per band/channel/whatever granularity. But that can be achieved in very different ways, and in fact if you look at Chrome then for our iwl7000 driver there we do have a command to do something similar (currently a vendor command, but that can be changed) without ever changing the *calibration*. So to me, the whole premise of the patch is confusing and/or wrong. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Johannes, Thanks for the reply. You are right that the physical antenna does not change. When I refer to 'calibration data', it actually corresponding to how mwifiex adjust the per-band tx power. For mwifiex, the per-band tx power is pre-calculated based on need, and stored in DT, a vendor command or std nl80211 message is sent to tell the driver to switch between two set of "calibration data". I'm aware that iwl7000 is using a vendor command to do this as well, but instead of pre-calculate required tx power info, the tx value can be passed along with the vendor command message. This patch was sent originally to standardize the requirement of sending a vendor command to the driver (so it'll be a standard nl80211 message). However, we have decided to move along with vendor command for both mwifiex and nl80211, so this patch is not needed anymore. Thanks for the comments! Wei-Ning On Tue, Jun 28, 2016 at 6:57 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Thu, 2016-05-12 at 17:34 +0800, Wei-Ning Huang wrote: >> >> Johannes, I feel like being able to set calibration data at runtime >> is something common to all wireless drivers, so instead of using >> vendor commands what do you think if I pass the calibration data name >> instead of using those magic constants? This way, userspace does not >> need to know the details of what band/range power limit the driver >> supports. It allows for flexible driver side implementation and >> easier for userspace to control. >> > > Sorry - I dropped this thread accidentally. > > I'm not really sure I understand the situation fully, but right now to > me this seems very strange. > > The physical antennas probably don't really change between "clamshell" > and "tablet" mode, do the physical radiation properties change enough > to actually require different *calibration*? To me, that sounds very > strange. > > Assuming they don't really change fundamentally, then I understand the > need to set different power levels, per band/channel/whatever > granularity. But that can be achieved in very different ways, and in > fact if you look at Chrome then for our iwl7000 driver there we do have > a command to do something similar (currently a vendor command, but that > can be changed) without ever changing the *calibration*. > > So to me, the whole premise of the patch is confusing and/or wrong. > > johannes
Hi, I'm revisiting this since we're asked to do the same for iwlwifi. On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote: > Recent new hardware has the ability to switch between tablet mode and > clamshell mode. To optimize WiFi performance, we want to be able to > use different power table between modes. This patch adds a new > netlink message type and cfg80211_ops function to allow userspace to > trigger a power mode switch for a given wireless interface. I've thought about this a bit more, and also heard this in different contexts now, and I'm actually not convinced that the wifi subsystem exposing this *in any way* is the right thing to do. Why don't we add a minimal "form factor" subsystem to the kernel? Something that allows 1) sensor/BIOS/... drivers to call a function to switch form factor 2) consumers to register and get callbacks when switching happens If the sensor driver is in the kernel (some kind of driver probably has to be anyway), or form factor switch ends up being a BIOS notification, then this gets rid of a lot of complexity - no more need to bump this through userspace etc. If, for some reason, the sensor driver really has to be in userspace, then some kind of API *to this subsystem* can be implemented to set the form factor mode from userspace, and all the other stuff can be left as is. This would also allow other clients to know about this information, let's say, for the sake of an argument that doesn't seem *too* far- fetched, that the compass driver needs to adjust the direction if you switch modes. It seems to me that this would be more robust, which can only be a good thing if we start using it for things that might be regulatory relevant. Additionally, the subsystem could allow userspace to also get an event if it wants to know about this, e.g. to switch to a more touch-friendly UI on switching to tablet mode, or something. Thoughts? johannes
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 9e1b24c..aa77fa0 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map { * @get_tx_power: store the current TX power into the dbm variable; * return 0 if successful * + * @set_tx_power_mode: set the transmit power mode. Some device have the ability + * to transform between different mode such as clamshell and tablet mode. + * set_tx_power_mode allows setting of different TX power mode at runtime. + * @get_tx_power_mode: store the current TX power mode into the mode variable; + * return 0 if successful + * * @set_wds_peer: set the WDS peer for a WDS interface * * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting @@ -2631,6 +2637,11 @@ struct cfg80211_ops { int (*get_tx_power)(struct wiphy *wiphy, struct wireless_dev *wdev, int *dbm); + int (*set_tx_power_mode)(struct wiphy *wiphy, + enum nl80211_tx_power_mode mode); + int (*get_tx_power_mode)(struct wiphy *wiphy, + enum nl80211_tx_power_mode *mode); + int (*set_wds_peer)(struct wiphy *wiphy, struct net_device *dev, const u8 *addr); diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 5a30a75..9b1888a 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -1796,6 +1796,9 @@ enum nl80211_commands { * connecting to a PCP, and in %NL80211_CMD_START_AP to start * a PCP instead of AP. Relevant for DMG networks only. * + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See + * &enum nl80211_tx_power_mode for possible values. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2172,6 +2175,8 @@ enum nl80211_attrs { NL80211_ATTR_PBSS, + NL80211_ATTR_WIPHY_TX_POWER_MODE, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting { }; /** + * enum nl80211_tx_power_mode - TX power mode setting + * @NL80211_TX_POWER_LOW: general low TX power mode + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode + * @NL80211_TX_POWER_HIGH: general high TX power mode + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode + */ +enum nl80211_tx_power_mode { + NL80211_TX_POWER_LOW, + NL80211_TX_POWER_MEDIUM, + NL80211_TX_POWER_HIGH, + NL80211_TX_POWER_CLAMSHELL, + NL80211_TX_POWER_TABLET, +}; + +/** * enum nl80211_packet_pattern_attr - packet pattern attribute * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask has diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 056a730..61b474d 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -402,6 +402,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, [NL80211_ATTR_PBSS] = { .type = NLA_FLAG }, + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 }, }; /* policy for the key attributes */ @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info) return result; } + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) { + enum nl80211_tx_power_mode mode; + int idx = 0; + + if (!rdev->ops->set_tx_power_mode) + return -EOPNOTSUPP; + + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE; + mode = nla_get_u32(info->attrs[idx]); + + result = rdev_set_tx_power_mode(rdev, mode); + if (result) + return result; + } + if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] && info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) { u32 tx_ant, rx_ant; diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h index 8ae0c04..c3a1035 100644 --- a/net/wireless/rdev-ops.h +++ b/net/wireless/rdev-ops.h @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct cfg80211_registered_device *rdev, return ret; } +static inline int +rdev_set_tx_power_mode(struct cfg80211_registered_device *rdev, + enum nl80211_tx_power_mode mode) +{ + int ret; + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode); + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode); + trace_rdev_return_int(&rdev->wiphy, ret); + return ret; +} + +static inline int +rdev_get_tx_power_mode(struct cfg80211_registered_device *rdev, + enum nl80211_tx_power_mode *mode) +{ + int ret; + trace_rdev_get_tx_power_mode(&rdev->wiphy); + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode); + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode); + return ret; +} + static inline int rdev_set_wds_peer(struct cfg80211_registered_device *rdev, struct net_device *dev, const u8 *addr) { diff --git a/net/wireless/trace.h b/net/wireless/trace.h index 09b242b..132c8c2 100644 --- a/net/wireless/trace.h +++ b/net/wireless/trace.h @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power, WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, __entry->mbm) ); +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode, + TP_PROTO(struct wiphy *wiphy), + TP_ARGS(wiphy) +); + +TRACE_EVENT(rdev_set_tx_power_mode, + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode mode), + TP_ARGS(wiphy, mode), + TP_STRUCT__entry( + WIPHY_ENTRY + __field(enum nl80211_tx_power_mode, mode) + ), + TP_fast_assign( + WIPHY_ASSIGN; + __entry->mode = mode; + ), + TP_printk(WIPHY_PR_FMT ", mode: %d", + WIPHY_PR_ARG, __entry->mode) +); + TRACE_EVENT(rdev_return_int_int, TP_PROTO(struct wiphy *wiphy, int func_ret, int func_fill), TP_ARGS(wiphy, func_ret, func_fill),
Recent new hardware has the ability to switch between tablet mode and clamshell mode. To optimize WiFi performance, we want to be able to use different power table between modes. This patch adds a new netlink message type and cfg80211_ops function to allow userspace to trigger a power mode switch for a given wireless interface. Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org> --- include/net/cfg80211.h | 11 +++++++++++ include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++ net/wireless/nl80211.c | 16 ++++++++++++++++ net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++ net/wireless/trace.h | 20 ++++++++++++++++++++ 5 files changed, 90 insertions(+)