[PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[David Dreschner]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Hey guys,

the attached patch updates the list of whitelisted ThinkPad models with 
dual fan support.

The changes were tested on my ThinkPad T15g Gen 2. According to Lenovo, 
the BIOS version is the same for the P15 Gen 2 and the P17 Gen 2 ( 
https://pcsupport.lenovo.com/us/en/downloads/ds551321-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15-gen-2-p17-gen-2-t15g-gen-2 
).

I also added the P15v Gen 2 and T15p Gen 2 to the whitelist based on the 
BIOS version listed on the Lenovo homepage ( 
https://pcsupport.lenovo.com/us/en/downloads/ds551356-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15v-gen-2-t15p-gen-2 
). The first generation had two fans and where covered by the whitelist 
entry for the P15 Gen 2. As the second generation has two fans, too, I 
made that change for completeness.

To apply the changes before it's merged in the mainline linux kernel, I 
made a little dkms patch: 
https://github.com/dreschner/thinkpad_acpi-dual-fan-patch

Best reguards,
David

[-- Attachment #2: update_whitelisted_thinkpad_models.patch --]
[-- Type: text/x-patch, Size: 374 bytes --]

8845c8845
< 	TPACPI_Q_LNV3('N', '3', '0', TPACPI_FAN_2CTL),	/* P15 (1st gen) / P15v (1st gen) */
---
> 	TPACPI_Q_LNV3('N', '3', '0', TPACPI_FAN_2CTL),	/* P15 / P17 / T15g / T15p / P15v (1st gen) */
8846a8847,8848
> 	TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL),	/* P15 / P17 / T15g (2nd gen) */
> 	TPACPI_Q_LNV3('N', '3', '8', TPACPI_FAN_2CTL),	/* P15v / T15p (2nd gen) */

Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[Hans de Goede]

Hi David,

On 1/4/22 04:41, David Dreschner wrote:
> Hey guys,
> 
> the attached patch updates the list of whitelisted ThinkPad models with dual fan support.
> 
> The changes were tested on my ThinkPad T15g Gen 2. According to Lenovo, the BIOS version is the same for the P15 Gen 2 and the P17 Gen 2 ( https://pcsupport.lenovo.com/us/en/downloads/ds551321-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15-gen-2-p17-gen-2-t15g-gen-2 ).
> 
> I also added the P15v Gen 2 and T15p Gen 2 to the whitelist based on the BIOS version listed on the Lenovo homepage ( https://pcsupport.lenovo.com/us/en/downloads/ds551356-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15v-gen-2-t15p-gen-2 ). The first generation had two fans and where covered by the whitelist entry for the P15 Gen 2. As the second generation has two fans, too, I made that change for completeness.
> 
> To apply the changes before it's merged in the mainline linux kernel, I made a little dkms patch: https://github.com/dreschner/thinkpad_acpi-dual-fan-patch

Thank you for your patch submission.

If I understand things correctly then you've only tested the addition of the:

TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL),  /* P15 / P17 / T15g (2nd gen) */

quirk, correct? In that case we really only want to add that quirk, we don't
want to go and add untested quirks. But perhaps Mark from Lenovo can confirm
that this quirk:

TPACPI_Q_LNV3('N', '3', '8', TPACPI_FAN_2CTL),  /* P15v / T15p (2nd gen) */

also is correct and that those models really have 2 fans, Mark ?

Mark, more in general can you perhaps talk to the firmware team and ask
if there is a better way to detect if there are 2 fans in a thinkpad then
maintaining a quirk table for this ?

Besides the issue of the untested quirk, unfortunately your patch is not
submitted in the right format, so I cannot accept it as is, esp. the
missing of a Signed-off-by is a blocker.

Kernel patches should be in git format-patch output format and
your patch is missing a Signed-off-by line in the commit-message. I can only
accept patches with a Signed-off-by line in the commit-message like this:

Signed-off-by: Your Real Name <email@your.domain>

By adding this line you indicate that you are the author of the code and
are submitting it under the existing license for the file which you are
modifying (typically GPL-2.0) or that you have permission from the author
to submit it under this license. See:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Given that this patch is trivial and mostly data, not code, I can turn
it into a proper patch with myself as author, crediting you like this:

Reported-and-tested-by: David Dreschner <david@dreschner.net>

And then merge it with me as the author, or you can resubmit
it in the proper format if you prefer.

Please let me know how you want to proceed with this.

Regards,

Hans

Re: [External] Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[Mark Pearson]

Hi Hans

On 2022-01-11 06:09, Hans de Goede wrote:
> Hi David,
> 
> On 1/4/22 04:41, David Dreschner wrote:
>> Hey guys,
>>
>> the attached patch updates the list of whitelisted ThinkPad models with dual fan support.
>>
>> The changes were tested on my ThinkPad T15g Gen 2. According to Lenovo, the BIOS version is the same for the P15 Gen 2 and the P17 Gen 2 ( https://pcsupport.lenovo.com/us/en/downloads/ds551321-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15-gen-2-p17-gen-2-t15g-gen-2 ).
>>
>> I also added the P15v Gen 2 and T15p Gen 2 to the whitelist based on the BIOS version listed on the Lenovo homepage ( https://pcsupport.lenovo.com/us/en/downloads/ds551356-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15v-gen-2-t15p-gen-2 ). The first generation had two fans and where covered by the whitelist entry for the P15 Gen 2. As the second generation has two fans, too, I made that change for completeness.
>>
>> To apply the changes before it's merged in the mainline linux kernel, I made a little dkms patch: https://github.com/dreschner/thinkpad_acpi-dual-fan-patch>> 
> Thank you for your patch submission.
> 
> If I understand things correctly then you've only tested the addition of the:
> 
> TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL),  /* P15 / P17 / T15g (2nd gen) */
> 
> quirk, correct? In that case we really only want to add that quirk, we don't
> want to go and add untested quirks. But perhaps Mark from Lenovo can confirm
> that this quirk:
> 
> TPACPI_Q_LNV3('N', '3', '8', TPACPI_FAN_2CTL),  /* P15v / T15p (2nd gen) */
> 
> also is correct and that those models really have 2 fans, Mark ?
> 
> Mark, more in general can you perhaps talk to the firmware team and ask
> if there is a better way to detect if there are 2 fans in a thinkpad then
> maintaining a quirk table for this ?
> 

Yeah - I saw the patch and wondered the same. I will look into that
(LO-1498 for my internal tracking)

We recently fixed this for the P1G4 and I asked for that to be extended
to the other platforms and I don't think it was done :( If you don't mind
holding off for a couple of days (while the patch gets cleaned up) I'll
see if I can get confirmation in the short term of which platforms are
impacted (LO-1499)

Thanks
Mark

Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[Hans de Goede]

Hi David,

On 1/11/22 12:09, Hans de Goede wrote:
> Hi David,
> 
> On 1/4/22 04:41, David Dreschner wrote:
>> Hey guys,
>>
>> the attached patch updates the list of whitelisted ThinkPad models with dual fan support.
>>
>> The changes were tested on my ThinkPad T15g Gen 2. According to Lenovo, the BIOS version is the same for the P15 Gen 2 and the P17 Gen 2 ( https://pcsupport.lenovo.com/us/en/downloads/ds551321-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15-gen-2-p17-gen-2-t15g-gen-2 ).
>>
>> I also added the P15v Gen 2 and T15p Gen 2 to the whitelist based on the BIOS version listed on the Lenovo homepage ( https://pcsupport.lenovo.com/us/en/downloads/ds551356-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15v-gen-2-t15p-gen-2 ). The first generation had two fans and where covered by the whitelist entry for the P15 Gen 2. As the second generation has two fans, too, I made that change for completeness.
>>
>> To apply the changes before it's merged in the mainline linux kernel, I made a little dkms patch: https://github.com/dreschner/thinkpad_acpi-dual-fan-patch
> 
> Thank you for your patch submission.
> 
> If I understand things correctly then you've only tested the addition of the:
> 
> TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL),  /* P15 / P17 / T15g (2nd gen) */
> 
> quirk, correct? In that case we really only want to add that quirk, we don't
> want to go and add untested quirks. But perhaps Mark from Lenovo can confirm
> that this quirk:
> 
> TPACPI_Q_LNV3('N', '3', '8', TPACPI_FAN_2CTL),  /* P15v / T15p (2nd gen) */
> 
> also is correct and that those models really have 2 fans, Mark ?
> 
> Mark, more in general can you perhaps talk to the firmware team and ask
> if there is a better way to detect if there are 2 fans in a thinkpad then
> maintaining a quirk table for this ?
> 
> Besides the issue of the untested quirk, unfortunately your patch is not
> submitted in the right format, so I cannot accept it as is, esp. the
> missing of a Signed-off-by is a blocker.
> 
> Kernel patches should be in git format-patch output format and
> your patch is missing a Signed-off-by line in the commit-message. I can only
> accept patches with a Signed-off-by line in the commit-message like this:
> 
> Signed-off-by: Your Real Name <email@your.domain>
> 
> By adding this line you indicate that you are the author of the code and
> are submitting it under the existing license for the file which you are
> modifying (typically GPL-2.0) or that you have permission from the author
> to submit it under this license. See:
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> 
> Given that this patch is trivial and mostly data, not code, I can turn
> it into a proper patch with myself as author, crediting you like this:
> 
> Reported-and-tested-by: David Dreschner <david@dreschner.net>
> 
> And then merge it with me as the author, or you can resubmit
> it in the proper format if you prefer.

Despite Mark's answer that he will look into this, it might very well
take quite a while before something comes out of that and it would be
good to in the mean time just extend the quirk list with your
ThinkPad T15g Gen 2 addition.

So can you please let me know how you want to proceed with this
(see above) ?

Regards,

Hans

Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[Hans de Goede]

Hi David,

On 1/11/22 12:09, Hans de Goede wrote:
> Hi David,
> 
> On 1/4/22 04:41, David Dreschner wrote:
>> Hey guys,
>>
>> the attached patch updates the list of whitelisted ThinkPad models with dual fan support.
>>
>> The changes were tested on my ThinkPad T15g Gen 2. According to Lenovo, the BIOS version is the same for the P15 Gen 2 and the P17 Gen 2 ( https://pcsupport.lenovo.com/us/en/downloads/ds551321-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15-gen-2-p17-gen-2-t15g-gen-2 ).
>>
>> I also added the P15v Gen 2 and T15p Gen 2 to the whitelist based on the BIOS version listed on the Lenovo homepage ( https://pcsupport.lenovo.com/us/en/downloads/ds551356-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15v-gen-2-t15p-gen-2 ). The first generation had two fans and where covered by the whitelist entry for the P15 Gen 2. As the second generation has two fans, too, I made that change for completeness.
>>
>> To apply the changes before it's merged in the mainline linux kernel, I made a little dkms patch: https://github.com/dreschner/thinkpad_acpi-dual-fan-patch
> 
> Thank you for your patch submission.
> 
> If I understand things correctly then you've only tested the addition of the:
> 
> TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL),  /* P15 / P17 / T15g (2nd gen) */
> 
> quirk, correct? In that case we really only want to add that quirk, we don't
> want to go and add untested quirks. But perhaps Mark from Lenovo can confirm
> that this quirk:
> 
> TPACPI_Q_LNV3('N', '3', '8', TPACPI_FAN_2CTL),  /* P15v / T15p (2nd gen) */
> 
> also is correct and that those models really have 2 fans, Mark ?
> 
> Mark, more in general can you perhaps talk to the firmware team and ask
> if there is a better way to detect if there are 2 fans in a thinkpad then
> maintaining a quirk table for this ?
> 
> Besides the issue of the untested quirk, unfortunately your patch is not
> submitted in the right format, so I cannot accept it as is, esp. the
> missing of a Signed-off-by is a blocker.
> 
> Kernel patches should be in git format-patch output format and
> your patch is missing a Signed-off-by line in the commit-message. I can only
> accept patches with a Signed-off-by line in the commit-message like this:
> 
> Signed-off-by: Your Real Name <email@your.domain>
> 
> By adding this line you indicate that you are the author of the code and
> are submitting it under the existing license for the file which you are
> modifying (typically GPL-2.0) or that you have permission from the author
> to submit it under this license. See:
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> 
> Given that this patch is trivial and mostly data, not code, I can turn
> it into a proper patch with myself as author, crediting you like this:
> 
> Reported-and-tested-by: David Dreschner <david@dreschner.net>
> 
> And then merge it with me as the author, or you can resubmit
> it in the proper format if you prefer.

Despite Mark's answer that he will look into this, it might very well
take quite a while before something comes out of that and it would be
good to in the mean time just extend the quirk list with your
ThinkPad T15g Gen 2 addition.

So can you please let me know how you want to proceed with this
(see above) ?

Regards,

Hans

Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi

[Hans de Goede]

Hi David,

On 1/17/22 10:55, Hans de Goede wrote:
> Hi David,
> 
> On 1/11/22 12:09, Hans de Goede wrote:
>> Hi David,
>>
>> On 1/4/22 04:41, David Dreschner wrote:
>>> Hey guys,
>>>
>>> the attached patch updates the list of whitelisted ThinkPad models with dual fan support.
>>>
>>> The changes were tested on my ThinkPad T15g Gen 2. According to Lenovo, the BIOS version is the same for the P15 Gen 2 and the P17 Gen 2 ( https://pcsupport.lenovo.com/us/en/downloads/ds551321-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15-gen-2-p17-gen-2-t15g-gen-2 ).
>>>
>>> I also added the P15v Gen 2 and T15p Gen 2 to the whitelist based on the BIOS version listed on the Lenovo homepage ( https://pcsupport.lenovo.com/us/en/downloads/ds551356-bios-update-utility-bootable-cd-for-windows-10-64-bit-thinkpad-p15v-gen-2-t15p-gen-2 ). The first generation had two fans and where covered by the whitelist entry for the P15 Gen 2. As the second generation has two fans, too, I made that change for completeness.
>>>
>>> To apply the changes before it's merged in the mainline linux kernel, I made a little dkms patch: https://github.com/dreschner/thinkpad_acpi-dual-fan-patch
>>
>> Thank you for your patch submission.
>>
>> If I understand things correctly then you've only tested the addition of the:
>>
>> TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL),  /* P15 / P17 / T15g (2nd gen) */
>>
>> quirk, correct? In that case we really only want to add that quirk, we don't
>> want to go and add untested quirks. But perhaps Mark from Lenovo can confirm
>> that this quirk:
>>
>> TPACPI_Q_LNV3('N', '3', '8', TPACPI_FAN_2CTL),  /* P15v / T15p (2nd gen) */
>>
>> also is correct and that those models really have 2 fans, Mark ?
>>
>> Mark, more in general can you perhaps talk to the firmware team and ask
>> if there is a better way to detect if there are 2 fans in a thinkpad then
>> maintaining a quirk table for this ?
>>
>> Besides the issue of the untested quirk, unfortunately your patch is not
>> submitted in the right format, so I cannot accept it as is, esp. the
>> missing of a Signed-off-by is a blocker.
>>
>> Kernel patches should be in git format-patch output format and
>> your patch is missing a Signed-off-by line in the commit-message. I can only
>> accept patches with a Signed-off-by line in the commit-message like this:
>>
>> Signed-off-by: Your Real Name <email@your.domain>
>>
>> By adding this line you indicate that you are the author of the code and
>> are submitting it under the existing license for the file which you are
>> modifying (typically GPL-2.0) or that you have permission from the author
>> to submit it under this license. See:
>>
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
>>
>> Given that this patch is trivial and mostly data, not code, I can turn
>> it into a proper patch with myself as author, crediting you like this:
>>
>> Reported-and-tested-by: David Dreschner <david@dreschner.net>
>>
>> And then merge it with me as the author, or you can resubmit
>> it in the proper format if you prefer.
> 
> Despite Mark's answer that he will look into this, it might very well
> take quite a while before something comes out of that and it would be
> good to in the mean time just extend the quirk list with your
> ThinkPad T15g Gen 2 addition.
> 
> So can you please let me know how you want to proceed with this
> (see above) ?

ping David?

Regards,

Hans