| Message ID | ec04aa1e-1ac3-edbc-ac08-eec15ec5c952@dreschner.net (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | Update whitelisted ThinkPad models with dual fan support in thinkpad_acpi | expand |
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
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
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
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
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
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) */