* Re: [External] Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi
2022-01-11 11:09 ` Hans de Goede
@ 2022-01-11 19:34 ` Mark Pearson
2022-01-17 9:55 ` Hans de Goede
2022-01-17 9:55 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Mark Pearson @ 2022-01-11 19:34 UTC (permalink / raw)
To: Hans de Goede, David Dreschner, ibm-acpi-devel; +Cc: platform-driver-x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi
2022-01-11 11:09 ` Hans de Goede
2022-01-11 19:34 ` [External] " Mark Pearson
@ 2022-01-17 9:55 ` Hans de Goede
2022-02-01 14:17 ` Hans de Goede
2022-01-17 9:55 ` Hans de Goede
2 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2022-01-17 9:55 UTC (permalink / raw)
To: David Dreschner, ibm-acpi-devel, Mark Pearson; +Cc: platform-driver-x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi
2022-01-17 9:55 ` Hans de Goede
@ 2022-02-01 14:17 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2022-02-01 14:17 UTC (permalink / raw)
To: David Dreschner, ibm-acpi-devel, Mark Pearson; +Cc: platform-driver-x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi
2022-01-11 11:09 ` Hans de Goede
2022-01-11 19:34 ` [External] " Mark Pearson
2022-01-17 9:55 ` Hans de Goede
@ 2022-01-17 9:55 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2022-01-17 9:55 UTC (permalink / raw)
To: David Dreschner, ibm-acpi-devel, Mark Pearson; +Cc: platform-driver-x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread