Hello Hans, >-----Original Message----- >From: Hans de Goede <hdegoede@xxxxxxxxxx> >Sent: Monday, February 15, 2021 11:48 PM >To: Nitin Joshi <nitjoshi@xxxxxxxxx> >Cc: ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx; platform-driver- >x86@xxxxxxxxxxxxxxx; Nitin Joshi1 <njoshi1@xxxxxxxxxx>; Mark RH Pearson ><markpearson@xxxxxxxxxx> >Subject: [External] Re: [PATCH 1/2] platorm/x86: thinkpad_acpi: sysfs >interface to reduce wlan tx power > >Hi Nitin, > >On 2/12/21 6:58 AM, Nitin Joshi wrote: >> 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> > >This patch as well as patch 2/2 generally look ok to me. > >The one thing which stands out is the: > > *wlan_txreduce = -1; > >Resp: > > *wwan_antennatype = -1; > > default: > return sysfs_emit(buf, "unknown type\n"); > >Bits, which Barnabás already pointed out. > >If you can prepare a v2 of this patch-set addressing all the review remarks >which you have received so far then that would be great. > Thank you for your comment. I have already prepared patch and will send patch soon. >Regards, > >Hans Thanks & Regards, Nitin Joshi > > > > >> --- >> .../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; >> +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)) >> + *wlan_txreduce = 1; >> + else if (output & BIT(8)) >> + *wlan_txreduce = 0; >> + else >> + *wlan_txreduce = -1; >> + >> + *has_wlantxreduce = 1; >> + return 0; >> +} >> + >> +/* sysfs wlan entry */ >> +static ssize_t wlan_tx_strength_reduce_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + int err; >> + >> + err = get_wlan_state(&has_wlantxreduce, &wlan_txreduce); >> + if (err) >> + return err; >> + >> + 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) >> +{ >> + int output, err; >> + unsigned long t; >> + >> + if (parse_strtoul(buf, 1, &t)) >> + 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; >> + } >> + >> + 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); >> + 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); >> +} >> + >> +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) >>
