Hi 2021. február 12., péntek 6:58 keltezéssel, Nitin Joshi írta: > Some newer Thinkpads have the WLAN antenna placed close to the WWAN > antenna. In these cases FCC certification requires that when WWAN is > active we reduce WLAN transmission power. A new Dynamic Power > Reduction Control (DPRC) method is available from the BIOS on these > platforms to reduce or restore WLAN Tx power. > > This patch provides a sysfs interface that userspace can use to adjust the > WLAN power appropriately. > > Reviewed-by: Mark Pearson <markpearson@xxxxxxxxxx> > Signed-off-by: Nitin Joshi <njoshi1@xxxxxxxxxx> > --- > .../admin-guide/laptops/thinkpad-acpi.rst | 18 +++ > drivers/platform/x86/thinkpad_acpi.c | 136 ++++++++++++++++++ > 2 files changed, 154 insertions(+) > > diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst > index 5fe1ade88c17..10410811b990 100644 > --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst > +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst > @@ -51,6 +51,7 @@ detailed description): > - UWB enable and disable > - LCD Shadow (PrivacyGuard) enable and disable > - Lap mode sensor > + - WLAN transmission power control > > A compatibility table by model and feature is maintained on the web > site, http://ibm-acpi.sf.net/. I appreciate any success or failure > @@ -1447,6 +1448,23 @@ they differ between desk and lap mode. > The property is read-only. If the platform doesn't have support the sysfs > class is not created. > > +WLAN transmission power control > +-------------------------------- > + > +sysfs: wlan_tx_strength_reduce > + > +Some newer Thinkpads have the WLAN antenna placed close to the WWAN antenna. > +This interface will be used by userspace to reduce the WLAN Tx power strength > +when WWAN is active, as is required for FCC certification. > + > +The available commands are:: > + > + echo '0' > /sys/devices/platform/thinkpad_acpi/wlan_tx_strength_reduce > + echo '1' > /sys/devices/platform/thinkpad_acpi/wlan_tx_strength_reduce > + > +The first command restores the wlan transmission power and the latter one > +reduces wlan transmission power. > + > EXPERIMENTAL: UWB > ----------------- > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index f3e8eca8d86d..6dbbd4a14264 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -9983,6 +9983,138 @@ static struct ibm_struct proxsensor_driver_data = { > .exit = proxsensor_exit, > }; > > +/************************************************************************* > + * DPRC(Dynamic Power Reduction Control) subdriver, for the Lenovo WWAN > + * and WLAN feature. > + */ > +#define DPRC_GET_WLAN_STATE 0x20000 > +#define DPRC_SET_WLAN_POWER_REDUCE 0x00030010 > +#define DPRC_SET_WLAN_POWER_FULL 0x00030100 > +static int has_wlantxreduce; I think `bool` would be better. > +static int wlan_txreduce; > + > +static int dprc_command(int command, int *output) > +{ > + acpi_handle dprc_handle; > + > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DPRC", &dprc_handle))) { > + /* Platform doesn't support DPRC */ > + return -ENODEV; > + } > + > + if (!acpi_evalf(dprc_handle, output, NULL, "dd", command)) > + return -EIO; > + > + /* > + * METHOD_ERR gets returned on devices where few commands are not supported > + * for example WLAN power reduce command is not supported on some devices. > + */ > + if (*output & METHOD_ERR) > + return -ENODEV; > + > + return 0; > +} > + > +static int get_wlan_state(int *has_wlantxreduce, int *wlan_txreduce) > +{ > + int output, err; > + > + /* Get current WLAN Power Transmission state */ > + err = dprc_command(DPRC_GET_WLAN_STATE, &output); > + if (err) > + return err; > + > + if (output & BIT(4)) I believe it'd be preferable to name `BIT(4)` and `BIT(8)`. E.g.: #define DPRC_GET_WLAN_STATE_RES_REDUCED BIT(4) #define DPRC_GET_WLAN_STATE_RES_FULL BIT(8) (or any name you like). > + *wlan_txreduce = 1; > + else if (output & BIT(8)) > + *wlan_txreduce = 0; > + else > + *wlan_txreduce = -1; Can you elaborate what -1 means here? Unknown/not available/invalid/error? > + > + *has_wlantxreduce = 1; Is it necessary for the getter to set this? Couldn't it be set in `tpacpi_dprc_init()` once during initialization? > + return 0; > +} > + > +/* sysfs wlan entry */ > +static ssize_t wlan_tx_strength_reduce_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) Please align the arguments: ..._show(struct device *dev, struct device_attribute ... ...); > +{ > + int err; > + > + err = get_wlan_state(&has_wlantxreduce, &wlan_txreduce); > + if (err) > + return err; > + Wouldn't it be better to return -ENODATA or something similar when `wlan_txreduce` == -1? > + return sysfs_emit(buf, "%d\n", wlan_txreduce); > +} > + > +static ssize_t wlan_tx_strength_reduce_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) Same here. > +{ > + int output, err; > + unsigned long t; > + > + if (parse_strtoul(buf, 1, &t)) Maybe `kstrtobool`? > + return -EINVAL; > + > + tpacpi_disclose_usertask(attr->attr.name, > + "WLAN tx strength reduced %lu\n", t); > + > + switch (t) { > + case 0: > + err = dprc_command(DPRC_SET_WLAN_POWER_FULL, &output); > + break; > + case 1: > + err = dprc_command(DPRC_SET_WLAN_POWER_REDUCE, &output); > + break; > + default: > + err = -EINVAL; > + dev_err(&tpacpi_pdev->dev, "Unknown operating mode. Ignoring\n"); > + break; > + } > + If I'm not mistaken, `err` is never returned, so the write() will always seem to succeed. > + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "wlan_tx_strength_reduce"); > + return count; > +} > +static DEVICE_ATTR_RW(wlan_tx_strength_reduce); > + > +static int tpacpi_dprc_init(struct ibm_init_struct *iibm) > +{ > + int wlantx_err, err; > + > + wlantx_err = get_wlan_state(&has_wlantxreduce, &wlan_txreduce); > + /* > + * If support isn't available (ENODEV) for both devices then quit, but > + * don't return an error. > + */ > + if (wlantx_err == -ENODEV) > + return 0; > + /* Otherwise, if there was an error return it */ > + if (wlantx_err && (wlantx_err != -ENODEV)) > + return wlantx_err; > + > + if (has_wlantxreduce) { > + err = sysfs_create_file(&tpacpi_pdev->dev.kobj, > + &dev_attr_wlan_tx_strength_reduce.attr); I believe `device_create_file()` would be better. > + if (err) > + return err; > + } > + return 0; > +} > + > +static void dprc_exit(void) > +{ > + if (has_wlantxreduce) > + sysfs_remove_file(&tpacpi_pdev->dev.kobj, &dev_attr_wlan_tx_strength_reduce.attr); And similarly, `device_remove_file()`. > +} > + > +static struct ibm_struct dprc_driver_data = { > + .name = "dprc", > + .exit = dprc_exit, > +}; > + > /**************************************************************************** > **************************************************************************** > * > @@ -10475,6 +10607,10 @@ static struct ibm_init_struct ibms_init[] __initdata = { > .init = tpacpi_proxsensor_init, > .data = &proxsensor_driver_data, > }, > + { > + .init = tpacpi_dprc_init, > + .data = &dprc_driver_data, > + }, > }; > > static int __init set_ibm_param(const char *val, const struct kernel_param *kp) > -- > 2.25.1 Regards, Barnabás Pőcze
