Hello Hans,
>-----Original Message-----
>From: Hans de Goede <hde...@re...>
>Sent: Monday, February 15, 2021 11:48 PM
>To: Nitin Joshi <nit...@gm...>
>Cc: ibm...@li...; platform-driver-
>x8...@vg...; Nitin Joshi1 <nj...@le...>; Mark RH Pearson
><mar...@le...>
>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 <mar...@le...>
>> Signed-off-by: Nitin Joshi <nj...@le...>
>
>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)
>>
|