2021-11-08 20:45:27

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v3] Fix WWAN device disabled issue after S3 deep

Hi,

On 11/8/21 07:06, Slark Xiao wrote:
> When WWAN device wake from S3 deep, under thinkpad platform,
> WWAN would be disabled. This disable status could be checked
> by command 'nmcli r wwan' or 'rfkill list'.
>
> Issue analysis as below:
> When host resume from S3 deep, thinkpad_acpi driver would
> call hotkey_resume() function. Finnaly, it will use
> wan_get_status to check the current status of WWAN device.
> During this resume progress, wan_get_status would always
> return off even WWAN boot up completely.
> In patch V2, Hans said 'sw_state should be unchanged
> after a suspend/resume. It's better to drop the
> tpacpi_rfk_update_swstate call all together from the
> resume path'.
> And it's confimed by Lenovo that GWAN is no longer
> available from WHL generation because the design does not
> match with current pin control.
>
> Signed-off-by: Slark Xiao <[email protected]>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

I will merge this once 5.16-rc1 is out.

Regards,

Hans

> ---
> drivers/platform/x86/thinkpad_acpi.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 50ff04c84650..f1cbd27282e1 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -1178,15 +1178,6 @@ static int tpacpi_rfk_update_swstate(const struct tpacpi_rfk *tp_rfk)
> return status;
> }
>
> -/* Query FW and update rfkill sw state for all rfkill switches */
> -static void tpacpi_rfk_update_swstate_all(void)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < TPACPI_RFK_SW_MAX; i++)
> - tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[i]);
> -}
> -
> /*
> * Sync the HW-blocking state of all rfkill switches,
> * do notice it causes the rfkill core to schedule uevents
> @@ -3129,9 +3120,6 @@ static void tpacpi_send_radiosw_update(void)
> if (wlsw == TPACPI_RFK_RADIO_OFF)
> tpacpi_rfk_update_hwblock_state(true);
>
> - /* Sync sw blocking state */
> - tpacpi_rfk_update_swstate_all();
> -
> /* Sync hw blocking state last if it is hw-unblocked */
> if (wlsw == TPACPI_RFK_RADIO_ON)
> tpacpi_rfk_update_hwblock_state(false);
>


2021-11-09 02:21:58

by Slark Xiao

[permalink] [raw]
Subject: Re:Re: [PATCH v3] Fix WWAN device disabled issue after S3 deep



At 2021-11-08 22:32:56, "Hans de Goede" <[email protected]> wrote:
>Hi,
>
>On 11/8/21 07:06, Slark Xiao wrote:
>> When WWAN device wake from S3 deep, under thinkpad platform,
>> WWAN would be disabled. This disable status could be checked
>> by command 'nmcli r wwan' or 'rfkill list'.
>>
>> Issue analysis as below:
>> When host resume from S3 deep, thinkpad_acpi driver would
>> call hotkey_resume() function. Finnaly, it will use
>> wan_get_status to check the current status of WWAN device.
>> During this resume progress, wan_get_status would always
>> return off even WWAN boot up completely.
>> In patch V2, Hans said 'sw_state should be unchanged
>> after a suspend/resume. It's better to drop the
>> tpacpi_rfk_update_swstate call all together from the
>> resume path'.
>> And it's confimed by Lenovo that GWAN is no longer
>> available from WHL generation because the design does not
>> match with current pin control.
>>
>> Signed-off-by: Slark Xiao <[email protected]>
>
>Thanks, patch looks good to me:
>
>Reviewed-by: Hans de Goede <[email protected]>
>
>I will merge this once 5.16-rc1 is out.
>
>Regards,
>
>Hans
Hi Hans,
Thanks for that!

Regards,
Slark
>
>> ---
>> drivers/platform/x86/thinkpad_acpi.c | 12 ------------
>> 1 file changed, 12 deletions(-)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index 50ff04c84650..f1cbd27282e1 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -1178,15 +1178,6 @@ static int tpacpi_rfk_update_swstate(const struct tpacpi_rfk *tp_rfk)
>> return status;
>> }
>>
>> -/* Query FW and update rfkill sw state for all rfkill switches */
>> -static void tpacpi_rfk_update_swstate_all(void)
>> -{
>> - unsigned int i;
>> -
>> - for (i = 0; i < TPACPI_RFK_SW_MAX; i++)
>> - tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[i]);
>> -}
>> -
>> /*
>> * Sync the HW-blocking state of all rfkill switches,
>> * do notice it causes the rfkill core to schedule uevents
>> @@ -3129,9 +3120,6 @@ static void tpacpi_send_radiosw_update(void)
>> if (wlsw == TPACPI_RFK_RADIO_OFF)
>> tpacpi_rfk_update_hwblock_state(true);
>>
>> - /* Sync sw blocking state */
>> - tpacpi_rfk_update_swstate_all();
>> -
>> /* Sync hw blocking state last if it is hw-unblocked */
>> if (wlsw == TPACPI_RFK_RADIO_ON)
>> tpacpi_rfk_update_hwblock_state(false);
>>

2021-11-16 06:56:43

by Slark Xiao

[permalink] [raw]
Subject: Re:Re:Re: [PATCH v3] Fix WWAN device disabled issue after S3 deep




At 2021-11-09 10:06:21, "Slark Xiao" <[email protected]> wrote:
>
>
>At 2021-11-08 22:32:56, "Hans de Goede" <[email protected]> wrote:
>>Hi,
>>
>>On 11/8/21 07:06, Slark Xiao wrote:
>>> When WWAN device wake from S3 deep, under thinkpad platform,
>>> WWAN would be disabled. This disable status could be checked
>>> by command 'nmcli r wwan' or 'rfkill list'.
>>>
>>> Issue analysis as below:
>>> When host resume from S3 deep, thinkpad_acpi driver would
>>> call hotkey_resume() function. Finnaly, it will use
>>> wan_get_status to check the current status of WWAN device.
>>> During this resume progress, wan_get_status would always
>>> return off even WWAN boot up completely.
>>> In patch V2, Hans said 'sw_state should be unchanged
>>> after a suspend/resume. It's better to drop the
>>> tpacpi_rfk_update_swstate call all together from the
>>> resume path'.
>>> And it's confimed by Lenovo that GWAN is no longer
>>> available from WHL generation because the design does not
>>> match with current pin control.
>>>
>>> Signed-off-by: Slark Xiao <[email protected]>
>>
>>Thanks, patch looks good to me:
>>
>>Reviewed-by: Hans de Goede <[email protected]>
>>
>>I will merge this once 5.16-rc1 is out.
>>
>>Regards,
>>
Hi Hans,
5.16-rc1 is already out. Could you help merge this patch into baseline now?

Thanks!
Slark Xiao
>>
>>> ---
>>> drivers/platform/x86/thinkpad_acpi.c | 12 ------------
>>> 1 file changed, 12 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>> index 50ff04c84650..f1cbd27282e1 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -1178,15 +1178,6 @@ static int tpacpi_rfk_update_swstate(const struct tpacpi_rfk *tp_rfk)
>>> return status;
>>> }
>>>
>>> -/* Query FW and update rfkill sw state for all rfkill switches */
>>> -static void tpacpi_rfk_update_swstate_all(void)
>>> -{
>>> - unsigned int i;
>>> -
>>> - for (i = 0; i < TPACPI_RFK_SW_MAX; i++)
>>> - tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[i]);
>>> -}
>>> -
>>> /*
>>> * Sync the HW-blocking state of all rfkill switches,
>>> * do notice it causes the rfkill core to schedule uevents
>>> @@ -3129,9 +3120,6 @@ static void tpacpi_send_radiosw_update(void)
>>> if (wlsw == TPACPI_RFK_RADIO_OFF)
>>> tpacpi_rfk_update_hwblock_state(true);
>>>
>>> - /* Sync sw blocking state */
>>> - tpacpi_rfk_update_swstate_all();
>>> -
>>> /* Sync hw blocking state last if it is hw-unblocked */
>>> if (wlsw == TPACPI_RFK_RADIO_ON)
>>> tpacpi_rfk_update_hwblock_state(false);
>>>

2021-11-16 10:02:51

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v3] Fix WWAN device disabled issue after S3 deep

Hi,

On 11/16/21 07:40, Slark Xiao wrote:
>
>
>
> At 2021-11-09 10:06:21, "Slark Xiao" <[email protected]> wrote:
>>
>>
>> At 2021-11-08 22:32:56, "Hans de Goede" <[email protected]> wrote:
>>> Hi,
>>>
>>> On 11/8/21 07:06, Slark Xiao wrote:
>>>> When WWAN device wake from S3 deep, under thinkpad platform,
>>>> WWAN would be disabled. This disable status could be checked
>>>> by command 'nmcli r wwan' or 'rfkill list'.
>>>>
>>>> Issue analysis as below:
>>>> When host resume from S3 deep, thinkpad_acpi driver would
>>>> call hotkey_resume() function. Finnaly, it will use
>>>> wan_get_status to check the current status of WWAN device.
>>>> During this resume progress, wan_get_status would always
>>>> return off even WWAN boot up completely.
>>>> In patch V2, Hans said 'sw_state should be unchanged
>>>> after a suspend/resume. It's better to drop the
>>>> tpacpi_rfk_update_swstate call all together from the
>>>> resume path'.
>>>> And it's confimed by Lenovo that GWAN is no longer
>>>> available from WHL generation because the design does not
>>>> match with current pin control.
>>>>
>>>> Signed-off-by: Slark Xiao <[email protected]>
>>>
>>> Thanks, patch looks good to me:
>>>
>>> Reviewed-by: Hans de Goede <[email protected]>
>>>
>>> I will merge this once 5.16-rc1 is out.
>>>
>>> Regards,
>>>
> Hi Hans,
> 5.16-rc1 is already out. Could you help merge this patch into baseline now?

5.16-rc1 has only been out for 1 day, some patience please!

But yes I plan to send a fixes pull-req to Linus later today
including this fix.

Regards,

Hans




>>>> ---
>>>> drivers/platform/x86/thinkpad_acpi.c | 12 ------------
>>>> 1 file changed, 12 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>>> index 50ff04c84650..f1cbd27282e1 100644
>>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>>> @@ -1178,15 +1178,6 @@ static int tpacpi_rfk_update_swstate(const struct tpacpi_rfk *tp_rfk)
>>>> return status;
>>>> }
>>>>
>>>> -/* Query FW and update rfkill sw state for all rfkill switches */
>>>> -static void tpacpi_rfk_update_swstate_all(void)
>>>> -{
>>>> - unsigned int i;
>>>> -
>>>> - for (i = 0; i < TPACPI_RFK_SW_MAX; i++)
>>>> - tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[i]);
>>>> -}
>>>> -
>>>> /*
>>>> * Sync the HW-blocking state of all rfkill switches,
>>>> * do notice it causes the rfkill core to schedule uevents
>>>> @@ -3129,9 +3120,6 @@ static void tpacpi_send_radiosw_update(void)
>>>> if (wlsw == TPACPI_RFK_RADIO_OFF)
>>>> tpacpi_rfk_update_hwblock_state(true);
>>>>
>>>> - /* Sync sw blocking state */
>>>> - tpacpi_rfk_update_swstate_all();
>>>> -
>>>> /* Sync hw blocking state last if it is hw-unblocked */
>>>> if (wlsw == TPACPI_RFK_RADIO_ON)
>>>> tpacpi_rfk_update_hwblock_state(false);
>>>>