Menu
▾
▴
ibm-acpi-devel — ibm-acpi Linux driver development
You can subscribe to this list here.
| 2006 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
(74) |
Dec
(26) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2007 |
Jan
(14) |
Feb
(70) |
Mar
(116) |
Apr
(64) |
May
(118) |
Jun
(39) |
Jul
(243) |
Aug
(68) |
Sep
(77) |
Oct
(136) |
Nov
(48) |
Dec
(38) |
| 2008 |
Jan
(100) |
Feb
(84) |
Mar
(21) |
Apr
(56) |
May
(24) |
Jun
(19) |
Jul
(47) |
Aug
(27) |
Sep
(20) |
Oct
(65) |
Nov
(54) |
Dec
(14) |
| 2009 |
Jan
(45) |
Feb
(12) |
Mar
(15) |
Apr
(42) |
May
(21) |
Jun
(41) |
Jul
(15) |
Aug
(25) |
Sep
(49) |
Oct
(20) |
Nov
(61) |
Dec
(139) |
| 2010 |
Jan
(42) |
Feb
(56) |
Mar
(30) |
Apr
(27) |
May
(40) |
Jun
(16) |
Jul
(28) |
Aug
(14) |
Sep
(16) |
Oct
(11) |
Nov
(9) |
Dec
(10) |
| 2011 |
Jan
(51) |
Feb
(15) |
Mar
(29) |
Apr
(17) |
May
(70) |
Jun
(37) |
Jul
(6) |
Aug
(20) |
Sep
(7) |
Oct
(18) |
Nov
(28) |
Dec
(9) |
| 2012 |
Jan
(11) |
Feb
(12) |
Mar
(25) |
Apr
(8) |
May
(11) |
Jun
(27) |
Jul
(25) |
Aug
(22) |
Sep
(7) |
Oct
(6) |
Nov
(8) |
Dec
(29) |
| 2013 |
Jan
(11) |
Feb
(16) |
Mar
(41) |
Apr
(5) |
May
(4) |
Jun
(27) |
Jul
(10) |
Aug
(21) |
Sep
(13) |
Oct
(16) |
Nov
(31) |
Dec
(13) |
| 2014 |
Jan
(2) |
Feb
(37) |
Mar
(39) |
Apr
(62) |
May
(13) |
Jun
(6) |
Jul
(3) |
Aug
(4) |
Sep
(15) |
Oct
(13) |
Nov
(45) |
Dec
(2) |
| 2015 |
Jan
(7) |
Feb
(85) |
Mar
(66) |
Apr
(14) |
May
(24) |
Jun
(127) |
Jul
(7) |
Aug
(9) |
Sep
|
Oct
(11) |
Nov
(2) |
Dec
(12) |
| 2016 |
Jan
(71) |
Feb
(17) |
Mar
(8) |
Apr
(26) |
May
(8) |
Jun
(12) |
Jul
|
Aug
(2) |
Sep
(11) |
Oct
(25) |
Nov
(139) |
Dec
(7) |
| 2017 |
Jan
(12) |
Feb
(12) |
Mar
(4) |
Apr
(8) |
May
(37) |
Jun
(12) |
Jul
(5) |
Aug
(5) |
Sep
(18) |
Oct
(21) |
Nov
(10) |
Dec
(91) |
| 2018 |
Jan
(35) |
Feb
(31) |
Mar
(1) |
Apr
(33) |
May
(15) |
Jun
(50) |
Jul
(10) |
Aug
(14) |
Sep
(5) |
Oct
(8) |
Nov
(53) |
Dec
(9) |
| 2019 |
Jan
|
Feb
|
Mar
(7) |
Apr
(14) |
May
(14) |
Jun
(5) |
Jul
(5) |
Aug
|
Sep
(2) |
Oct
(2) |
Nov
|
Dec
(2) |
| 2020 |
Jan
(9) |
Feb
(8) |
Mar
|
Apr
(11) |
May
(14) |
Jun
(35) |
Jul
(53) |
Aug
(23) |
Sep
(15) |
Oct
(6) |
Nov
(14) |
Dec
(8) |
| 2021 |
Jan
(9) |
Feb
(6) |
Mar
(14) |
Apr
(4) |
May
(3) |
Jun
(1) |
Jul
(7) |
Aug
(2) |
Sep
(10) |
Oct
(4) |
Nov
(26) |
Dec
(9) |
| 2022 |
Jan
(13) |
Feb
(1) |
Mar
(1) |
Apr
(7) |
May
(7) |
Jun
(17) |
Jul
(6) |
Aug
(9) |
Sep
|
Oct
(8) |
Nov
(6) |
Dec
|
| 2023 |
Jan
(1) |
Feb
|
Mar
|
Apr
(8) |
May
|
Jun
|
Jul
(4) |
Aug
(1) |
Sep
(13) |
Oct
(1) |
Nov
(9) |
Dec
(4) |
| 2024 |
Jan
(9) |
Feb
|
Mar
(5) |
Apr
(118) |
May
(4) |
Jun
(13) |
Jul
|
Aug
(4) |
Sep
(4) |
Oct
(16) |
Nov
(20) |
Dec
(3) |
| 2025 |
Jan
(7) |
Feb
(1) |
Mar
(3) |
Apr
(8) |
May
(15) |
Jun
(38) |
Jul
(2) |
Aug
(9) |
Sep
(2) |
Oct
|
Nov
|
Dec
|
|
From: Hans de G. <hde...@re...> - 2024-09-03 07:59:41
|
Hi,
On 9/2/24 10:26 PM, Matthias Fetzer wrote:
> Hi Hans,
>
> Am 27.08.24 um 11:09 schrieb Hans de Goede:
>> Hi Dan,
>>
>> On 8/27/24 10:45 AM, Dan Carpenter wrote:
>>> Hello Matthias Fetzer,
>>>
>>> Commit 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge
>>> E531 fan support") from Aug 16, 2024 (linux-next), leads to the
>>> following Smatch static checker warning:
>>>
>>> drivers/platform/x86/thinkpad_acpi.c:8387 fan_set_enable()
>>> error: uninitialized symbol 's'.
>>>
>>> drivers/platform/x86/thinkpad_acpi.c
>>> 8319 static int fan_set_enable(void)
>>> 8320 {
>>> 8321 u8 s;
>>> 8322 int rc;
>>> 8323
>>> 8324 if (!fan_control_allowed)
>>> 8325 return -EPERM;
>>> 8326
>>> 8327 if (mutex_lock_killable(&fan_mutex))
>>> 8328 return -ERESTARTSYS;
>>> 8329
>>> 8330 switch (fan_control_access_mode) {
>>> 8331 case TPACPI_FAN_WR_ACPI_FANS:
>>> 8332 case TPACPI_FAN_WR_TPEC:
>>> 8333 rc = fan_get_status(&s);
>>> 8334 if (rc)
>>> 8335 break;
>>> 8336
>>> 8337 /* Don't go out of emergency fan mode */
>>> 8338 if (s != 7) {
>>> 8339 s &= 0x07;
>>> 8340 s |= TP_EC_FAN_AUTO | 4; /* min fan speed 4 */
>>> 8341 }
>>> 8342
>>> 8343 if (!acpi_ec_write(fan_status_offset, s))
>>> 8344 rc = -EIO;
>>> 8345 else {
>>> 8346 tp_features.fan_ctrl_status_undef = 0;
>>> 8347 rc = 0;
>>> 8348 }
>>> 8349 break;
>>> 8350
>>> 8351 case TPACPI_FAN_WR_ACPI_SFAN:
>>> 8352 rc = fan_get_status(&s);
>>> 8353 if (rc)
>>> 8354 break;
>>> 8355
>>> 8356 s &= 0x07;
>>> 8357
>>> 8358 /* Set fan to at least level 4 */
>>> 8359 s |= 4;
>>> 8360
>>> 8361 if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", s))
>>> 8362 rc = -EIO;
>>> 8363 else
>>> 8364 rc = 0;
>>> 8365 break;
>>> 8366
>>> 8367 case TPACPI_FAN_WR_ACPI_FANW:
>>> 8368 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) {
>>> 8369 rc = -EIO;
>>> 8370 break;
>>> 8371 }
>>> 8372 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) {
>>> 8373 rc = -EIO;
>>> 8374 break;
>>> 8375 }
>>> 8376
>>> 8377 rc = 0;
>>>
>>> s isn't set on this path
>>>
>>> 8378 break;
>>> 8379
>>> 8380 default:
>>> 8381 rc = -ENXIO;
>>> 8382 }
>>> 8383
>>> 8384 mutex_unlock(&fan_mutex);
>>> 8385
>>> 8386 if (!rc)
>>> --> 8387 vdbg_printk(TPACPI_DBG_FAN,
>>> 8388 "fan control: set fan control register to 0x%02x\n",
>>> 8389 s);
>>> ^
>>> Here
>>>
>>> 8390 return rc;
>>> 8391 }
>>
>> Thank you for reporting this.
>>
>> Since this is just a debug print I think it is best to fix this by just
>> initializing s to 0 when it is declared and then just log 0
>> in the TPACPI_FAN_WR_ACPI_FANW case.
>>
>> Anyone got any better suggestions ?
>>
>> Regards,
>>
>> Hans
>>
>>
>
> Since there do not seem to be any other suggestions I'll fix it that
> way.
> Shall I make a completely new patch for this since it is already on your
> review branch or would a fixed v5 be enough?
Please submit a new patch on top of platform-drivers-x86/for-next
or on top of platform-drivers-x86/review-hans addressing just this
warning.
Regards,
Hans
|
|
From: Nitin J. <nj...@le...> - 2024-09-02 15:14:44
|
Hello Pellaeon, >> @Mark: has the team replied anything? Mark has informed me regarding this and I am yet to check this . Sorry for this ! I will try to get hold of system and try this within this week , if I find AMD e-privacy guard machine . Thanks From: Pellaeon Lin <nfs...@gm...> Sent: Sunday, September 1, 2024 2:32 PM To: ibm...@li... Subject: [External] Re: [ibm-acpi-devel] PrivacyGuard doesn't work even by sending ACPI commands directly Hi, @Mark: has the team replied anything? @Marco: `proptest` on my system did not return any property related to privacy screen, is this normal? (I ran `proptest | grep -i privacy`, which returned empty result.) Thanks, Marco Trevisan (Treviño) <ma...@3v...<mailto:ma...@3v...>> 於 2024年6月4日 週二 上午2:07寫道： Hi, Not sure if something changed or it is different in newer models, but the privacy screen feature so far has been exposed as a KMS property you can inspect and set with tools like proptest (in libdrm-tests package for what concerns ubuntu) Cheers On giu 3 2024, at 1:46 pm, Pellaeon Lin <nfs...@gm...<mailto:nfs...@gm...>> wrote: > Hi, > > I have a ThinkPad X13 Gen 2 AMD with PrivacyGuard. But by pressing the > combination key Fn+D it does not toggle the PrivacyGuard, the > PrivacyGuard stays on. I've tested: > > - Fn+D does toggle PrivacyGuard when I'm in the BIOS > - Fn+D has no effect on Ubuntu 22.10, Ubuntu 23.04, Ubuntu 23.10, > Ubuntu 24.04 and Fedora 40 (except Ubuntu 23.10 and 24.04, all was > tested using LiveUSB) > > In all of the Linux cases, I can confirm that by pressing Fn+D, the > status value of /proc/acpi/ibm/lcdshadow actually changes. > > I've always thought this was a Ubuntu-specific issue, until I tested Fedora. > > I tested this further by installing the acpi-call-dkms package on > Ubuntu and issues the following call: > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x1' | sudo tee /proc/acpi/call > > It returned 0 (which should mean success), but PrivacyGuard is still > ON. Then I tried to call it with 0x0: > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x0' | sudo tee /proc/acpi/call > > Also getting 0 in return. PrivacyGuard is still ON. > > Based on my limited understanding of ACPI and the kernel, at this > point it might be a firmware issue? (fwupdmgr shows that there is no > available updates) But based on my reading and understanding of > thinkpad_acpi.c, the particular ACPI call that I issued is also how > the Linux driver currently operates the PrivacyGuard feature, which > means the driver might also be affected by this issue. So I'm hoping > someone could help me debug this further, or point out anything that > I'm not understanding correctly. > > Thanks! > _______________________________________________ > > ibm-acpi-devel mailing list > > ibm...@li...<mailto:ibm...@li...> > > https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel |
|
From: Pellaeon L. <nfs...@gm...> - 2024-09-01 05:32:29
|
Hi, @Mark: has the team replied anything? @Marco: `proptest` on my system did not return any property related to privacy screen, is this normal? (I ran `proptest | grep -i privacy`, which returned empty result.) Thanks, Marco Trevisan (Treviño) <ma...@3v...> 於 2024年6月4日 週二 上午2:07寫道： > Hi, > > Not sure if something changed or it is different in newer models, but > the privacy screen feature so far has been exposed as a KMS property you > can inspect and set with tools like proptest (in libdrm-tests package > for what concerns ubuntu) > > Cheers > > On giu 3 2024, at 1:46 pm, Pellaeon Lin <nfs...@gm...> wrote: > > > Hi, > > > > I have a ThinkPad X13 Gen 2 AMD with PrivacyGuard. But by pressing the > > combination key Fn+D it does not toggle the PrivacyGuard, the > > PrivacyGuard stays on. I've tested: > > > > - Fn+D does toggle PrivacyGuard when I'm in the BIOS > > - Fn+D has no effect on Ubuntu 22.10, Ubuntu 23.04, Ubuntu 23.10, > > Ubuntu 24.04 and Fedora 40 (except Ubuntu 23.10 and 24.04, all was > > tested using LiveUSB) > > > > In all of the Linux cases, I can confirm that by pressing Fn+D, the > > status value of /proc/acpi/ibm/lcdshadow actually changes. > > > > I've always thought this was a Ubuntu-specific issue, until I tested > Fedora. > > > > I tested this further by installing the acpi-call-dkms package on > > Ubuntu and issues the following call: > > > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x1' | sudo tee /proc/acpi/call > > > > It returned 0 (which should mean success), but PrivacyGuard is still > > ON. Then I tried to call it with 0x0: > > > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x0' | sudo tee /proc/acpi/call > > > > Also getting 0 in return. PrivacyGuard is still ON. > > > > Based on my limited understanding of ACPI and the kernel, at this > > point it might be a firmware issue? (fwupdmgr shows that there is no > > available updates) But based on my reading and understanding of > > thinkpad_acpi.c, the particular ACPI call that I issued is also how > > the Linux driver currently operates the PrivacyGuard feature, which > > means the driver might also be affected by this issue. So I'm hoping > > someone could help me debug this further, or point out anything that > > I'm not understanding correctly. > > > > Thanks! > > _______________________________________________ > > > > ibm-acpi-devel mailing list > > > > ibm...@li... > > > > https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel > |
|
From: Hans de G. <hde...@re...> - 2024-08-27 09:10:04
|
Hi Dan,
On 8/27/24 10:45 AM, Dan Carpenter wrote:
> Hello Matthias Fetzer,
>
> Commit 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge
> E531 fan support") from Aug 16, 2024 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/platform/x86/thinkpad_acpi.c:8387 fan_set_enable()
> error: uninitialized symbol 's'.
>
> drivers/platform/x86/thinkpad_acpi.c
> 8319 static int fan_set_enable(void)
> 8320 {
> 8321 u8 s;
> 8322 int rc;
> 8323
> 8324 if (!fan_control_allowed)
> 8325 return -EPERM;
> 8326
> 8327 if (mutex_lock_killable(&fan_mutex))
> 8328 return -ERESTARTSYS;
> 8329
> 8330 switch (fan_control_access_mode) {
> 8331 case TPACPI_FAN_WR_ACPI_FANS:
> 8332 case TPACPI_FAN_WR_TPEC:
> 8333 rc = fan_get_status(&s);
> 8334 if (rc)
> 8335 break;
> 8336
> 8337 /* Don't go out of emergency fan mode */
> 8338 if (s != 7) {
> 8339 s &= 0x07;
> 8340 s |= TP_EC_FAN_AUTO | 4; /* min fan speed 4 */
> 8341 }
> 8342
> 8343 if (!acpi_ec_write(fan_status_offset, s))
> 8344 rc = -EIO;
> 8345 else {
> 8346 tp_features.fan_ctrl_status_undef = 0;
> 8347 rc = 0;
> 8348 }
> 8349 break;
> 8350
> 8351 case TPACPI_FAN_WR_ACPI_SFAN:
> 8352 rc = fan_get_status(&s);
> 8353 if (rc)
> 8354 break;
> 8355
> 8356 s &= 0x07;
> 8357
> 8358 /* Set fan to at least level 4 */
> 8359 s |= 4;
> 8360
> 8361 if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", s))
> 8362 rc = -EIO;
> 8363 else
> 8364 rc = 0;
> 8365 break;
> 8366
> 8367 case TPACPI_FAN_WR_ACPI_FANW:
> 8368 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) {
> 8369 rc = -EIO;
> 8370 break;
> 8371 }
> 8372 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) {
> 8373 rc = -EIO;
> 8374 break;
> 8375 }
> 8376
> 8377 rc = 0;
>
> s isn't set on this path
>
> 8378 break;
> 8379
> 8380 default:
> 8381 rc = -ENXIO;
> 8382 }
> 8383
> 8384 mutex_unlock(&fan_mutex);
> 8385
> 8386 if (!rc)
> --> 8387 vdbg_printk(TPACPI_DBG_FAN,
> 8388 "fan control: set fan control register to 0x%02x\n",
> 8389 s);
> ^
> Here
>
> 8390 return rc;
> 8391 }
Thank you for reporting this.
Since this is just a debug print I think it is best to fix this by just
initializing s to 0 when it is declared and then just log 0
in the TPACPI_FAN_WR_ACPI_FANW case.
Anyone got any better suggestions ?
Regards,
Hans
|
|
From: Hans de G. <hde...@re...> - 2024-08-19 14:34:12
|
Hi, On 8/16/24 4:12 PM, Matthias Fetzer wrote: > Fan control on the E531 is done using the ACPI methods FANG and > FANW. The correct parameters and register values were found by > analyzing EC firmware as well as DSDT. This has been tested on > my Thinkpad Edge E531 (6885CTO, BIOS HEET52WW 1.33). > > Signed-off-by: Matthias Fetzer <ko...@ma...> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > > Changes in v4: > - Remove unnecessary variable > Changes in v3: > - Add missing newline > - Remove redundant code > Changes in v2: > - Fix typo in EC memory description > - Split plausibilty check for better readability > > drivers/platform/x86/thinkpad_acpi.c | 143 ++++++++++++++++++++++++++- > 1 file changed, 142 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 397b409064c9..96c58bc59018 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -7751,6 +7751,28 @@ static struct ibm_struct volume_driver_data = { > * EC 0x2f (HFSP) might be available *for reading*, but do not use > * it for writing. > * > + * TPACPI_FAN_RD_ACPI_FANG: > + * ACPI FANG method: returns fan control register > + * > + * Takes one parameter which is 0x8100 plus the offset to EC memory > + * address 0xf500 and returns the byte at this address. > + * > + * 0xf500: > + * When the value is less than 9 automatic mode is enabled > + * 0xf502: > + * Contains the current fan speed from 0-100% > + * 0xf506: > + * Bit 7 has to be set in order to enable manual control by > + * writing a value >= 9 to 0xf500 > + * > + * TPACPI_FAN_WR_ACPI_FANW: > + * ACPI FANG method: sets fan control registers > + * > + * Takes 0x8100 plus the offset to EC memory address 0xf500 and the > + * value to be written there as parameters. > + * > + * see TPACPI_FAN_RD_ACPI_FANG > + * > * TPACPI_FAN_WR_TPEC: > * ThinkPad EC register 0x2f (HFSP): fan control loop mode > * Supported on almost all ThinkPads > @@ -7884,6 +7906,7 @@ enum { /* Fan control constants */ > enum fan_status_access_mode { > TPACPI_FAN_NONE = 0, /* No fan status or control */ > TPACPI_FAN_RD_ACPI_GFAN, /* Use ACPI GFAN */ > + TPACPI_FAN_RD_ACPI_FANG, /* Use ACPI FANG */ > TPACPI_FAN_RD_TPEC, /* Use ACPI EC regs 0x2f, 0x84-0x85 */ > TPACPI_FAN_RD_TPEC_NS, /* Use non-standard ACPI EC regs (eg: L13 Yoga gen2 etc.) */ > }; > @@ -7891,6 +7914,7 @@ enum fan_status_access_mode { > enum fan_control_access_mode { > TPACPI_FAN_WR_NONE = 0, /* No fan control */ > TPACPI_FAN_WR_ACPI_SFAN, /* Use ACPI SFAN */ > + TPACPI_FAN_WR_ACPI_FANW, /* Use ACPI FANW */ > TPACPI_FAN_WR_TPEC, /* Use ACPI EC reg 0x2f */ > TPACPI_FAN_WR_ACPI_FANS, /* Use ACPI FANS and EC reg 0x2f */ > }; > @@ -7924,9 +7948,13 @@ TPACPI_HANDLE(fans, ec, "FANS"); /* X31, X40, X41 */ > TPACPI_HANDLE(gfan, ec, "GFAN", /* 570 */ > "\\FSPD", /* 600e/x, 770e, 770x */ > ); /* all others */ > +TPACPI_HANDLE(fang, ec, "FANG", /* E531 */ > + ); /* all others */ > TPACPI_HANDLE(sfan, ec, "SFAN", /* 570 */ > "JFNS", /* 770x-JL */ > ); /* all others */ > +TPACPI_HANDLE(fanw, ec, "FANW", /* E531 */ > + ); /* all others */ > > /* > * Unitialized HFSP quirk: ACPI DSDT and EC fail to initialize the > @@ -8033,6 +8061,23 @@ static int fan_get_status(u8 *status) > > break; > } > + case TPACPI_FAN_RD_ACPI_FANG: { > + /* E531 */ > + int mode, speed; > + > + if (unlikely(!acpi_evalf(fang_handle, &mode, NULL, "dd", 0x8100))) > + return -EIO; > + if (unlikely(!acpi_evalf(fang_handle, &speed, NULL, "dd", 0x8102))) > + return -EIO; > + > + if (likely(status)) { > + *status = speed * 7 / 100; > + if (mode < 9) > + *status |= TP_EC_FAN_AUTO; > + } > + > + break; > + } > case TPACPI_FAN_RD_TPEC: > /* all except 570, 600e/x, 770e, 770x */ > if (unlikely(!acpi_ec_read(fan_status_offset, &s))) > @@ -8147,6 +8192,17 @@ static int fan2_get_speed(unsigned int *speed) > if (speed) > *speed = lo ? FAN_RPM_CAL_CONST / lo : 0; > break; > + case TPACPI_FAN_RD_ACPI_FANG: { > + /* E531 */ > + int speed_tmp; > + > + if (unlikely(!acpi_evalf(fang_handle, &speed_tmp, NULL, "dd", 0x8102))) > + return -EIO; > + > + if (likely(speed)) > + *speed = speed_tmp * 65535 / 100; > + break; > + } > > default: > return -ENXIO; > @@ -8206,6 +8262,32 @@ static int fan_set_level(int level) > tp_features.fan_ctrl_status_undef = 0; > break; > > + case TPACPI_FAN_WR_ACPI_FANW: > + if (!(level & TP_EC_FAN_AUTO) && (level < 0 || level > 7)) > + return -EINVAL; > + if (level & TP_EC_FAN_FULLSPEED) > + return -EINVAL; > + > + if (level & TP_EC_FAN_AUTO) { > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) { > + return -EIO; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) { > + return -EIO; > + } > + } else { > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) { > + return -EIO; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) { > + return -EIO; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8102, level * 100 / 7)) { > + return -EIO; > + } > + } > + break; > + > default: > return -ENXIO; > } > @@ -8284,6 +8366,19 @@ static int fan_set_enable(void) > rc = 0; > break; > > + case TPACPI_FAN_WR_ACPI_FANW: > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) { > + rc = -EIO; > + break; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) { > + rc = -EIO; > + break; > + } > + > + rc = 0; > + break; > + > default: > rc = -ENXIO; > } > @@ -8326,6 +8421,22 @@ static int fan_set_disable(void) > fan_control_desired_level = 0; > break; > > + case TPACPI_FAN_WR_ACPI_FANW: > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) { > + rc = -EIO; > + break; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) { > + rc = -EIO; > + break; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8102, 0x00)) { > + rc = -EIO; > + break; > + } > + rc = 0; > + break; > + > default: > rc = -ENXIO; > } > @@ -8359,6 +8470,23 @@ static int fan_set_speed(int speed) > rc = -EINVAL; > break; > > + case TPACPI_FAN_WR_ACPI_FANW: > + if (speed >= 0 && speed <= 65535) { > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) { > + rc = -EIO; > + break; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) { > + rc = -EIO; > + break; > + } > + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", > + 0x8102, speed * 100 / 65535)) > + rc = -EIO; > + } else > + rc = -EINVAL; > + break; > + > default: > rc = -ENXIO; > } > @@ -8701,6 +8829,10 @@ static int __init fan_init(struct ibm_init_struct *iibm) > TPACPI_ACPIHANDLE_INIT(gfan); > TPACPI_ACPIHANDLE_INIT(sfan); > } > + if (tpacpi_is_lenovo()) { > + TPACPI_ACPIHANDLE_INIT(fang); > + TPACPI_ACPIHANDLE_INIT(fanw); > + } > > quirks = tpacpi_check_quirks(fan_quirk_table, > ARRAY_SIZE(fan_quirk_table)); > @@ -8720,6 +8852,9 @@ static int __init fan_init(struct ibm_init_struct *iibm) > if (gfan_handle) { > /* 570, 600e/x, 770e, 770x */ > fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN; > + } else if (fang_handle) { > + /* E531 */ > + fan_status_access_mode = TPACPI_FAN_RD_ACPI_FANG; > } else { > /* all other ThinkPads: note that even old-style > * ThinkPad ECs supports the fan control register */ > @@ -8766,6 +8901,11 @@ static int __init fan_init(struct ibm_init_struct *iibm) > fan_control_access_mode = TPACPI_FAN_WR_ACPI_SFAN; > fan_control_commands |= > TPACPI_FAN_CMD_LEVEL | TPACPI_FAN_CMD_ENABLE; > + } else if (fanw_handle) { > + /* E531 */ > + fan_control_access_mode = TPACPI_FAN_WR_ACPI_FANW; > + fan_control_commands |= > + TPACPI_FAN_CMD_LEVEL | TPACPI_FAN_CMD_SPEED | TPACPI_FAN_CMD_ENABLE; > } else { > if (!gfan_handle) { > /* gfan without sfan means no fan control */ > @@ -8917,6 +9057,7 @@ static int fan_read(struct seq_file *m) > > case TPACPI_FAN_RD_TPEC_NS: > case TPACPI_FAN_RD_TPEC: > + case TPACPI_FAN_RD_ACPI_FANG: > /* all except 570, 600e/x, 770e, 770x */ > rc = fan_get_status_safe(&status); > if (rc) > @@ -8937,7 +9078,7 @@ static int fan_read(struct seq_file *m) > * No other levels settings available > */ > seq_printf(m, "level:\t\t%s\n", status & FAN_NS_CTRL ? "unknown" : "auto"); > - } else { > + } else if (fan_status_access_mode == TPACPI_FAN_RD_TPEC) { > if (status & TP_EC_FAN_FULLSPEED) > /* Disengaged mode takes precedence */ > seq_printf(m, "level:\t\tdisengaged\n"); |
|
From: kernel t. r. <lk...@in...> - 2024-08-15 19:06:35
|
Hi Matthias, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on v6.11-rc3 next-20240815] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Matthias-Fetzer/platform-x86-thinkpad_acpi-Add-Thinkpad-Edge-E531-fan-support/20240815-054239 base: linus/master patch link: https://lore.kernel.org/r/20240814213927.49075-1-kontakt%40matthias-fetzer.de patch subject: [PATCH v3] platform/x86: thinkpad_acpi: Add Thinkpad Edge E531 fan support config: i386-randconfig-001-20240815 (https://download.01.org/0day-ci/archive/20240816/202...@in.../config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240816/202...@in.../reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lk...@in...> | Closes: https://lore.kernel.org/oe-kbuild-all/202...@in.../ All warnings (new ones prefixed by >>): drivers/platform/x86/thinkpad_acpi.c: In function 'fan_set_level': >> drivers/platform/x86/thinkpad_acpi.c:8214:13: warning: variable 'rc' set but not used [-Wunused-but-set-variable] 8214 | int rc; | ^~ vim +/rc +8214 drivers/platform/x86/thinkpad_acpi.c 8211 8212 static int fan_set_level(int level) 8213 { > 8214 int rc; 8215 8216 if (!fan_control_allowed) 8217 return -EPERM; 8218 8219 switch (fan_control_access_mode) { 8220 case TPACPI_FAN_WR_ACPI_SFAN: 8221 if ((level < 0) || (level > 7)) 8222 return -EINVAL; 8223 8224 if (tp_features.second_fan_ctl) { 8225 if (!fan_select_fan2() || 8226 !acpi_evalf(sfan_handle, NULL, NULL, "vd", level)) { 8227 pr_warn("Couldn't set 2nd fan level, disabling support\n"); 8228 tp_features.second_fan_ctl = 0; 8229 } 8230 fan_select_fan1(); 8231 } 8232 if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level)) 8233 return -EIO; 8234 break; 8235 8236 case TPACPI_FAN_WR_ACPI_FANS: 8237 case TPACPI_FAN_WR_TPEC: 8238 if (!(level & TP_EC_FAN_AUTO) && 8239 !(level & TP_EC_FAN_FULLSPEED) && 8240 ((level < 0) || (level > 7))) 8241 return -EINVAL; 8242 8243 /* safety net should the EC not support AUTO 8244 * or FULLSPEED mode bits and just ignore them */ 8245 if (level & TP_EC_FAN_FULLSPEED) 8246 level |= 7; /* safety min speed 7 */ 8247 else if (level & TP_EC_FAN_AUTO) 8248 level |= 4; /* safety min speed 4 */ 8249 8250 if (tp_features.second_fan_ctl) { 8251 if (!fan_select_fan2() || 8252 !acpi_ec_write(fan_status_offset, level)) { 8253 pr_warn("Couldn't set 2nd fan level, disabling support\n"); 8254 tp_features.second_fan_ctl = 0; 8255 } 8256 fan_select_fan1(); 8257 8258 } 8259 if (!acpi_ec_write(fan_status_offset, level)) 8260 return -EIO; 8261 else 8262 tp_features.fan_ctrl_status_undef = 0; 8263 break; 8264 8265 case TPACPI_FAN_WR_ACPI_FANW: 8266 if (!(level & TP_EC_FAN_AUTO) && (level < 0 || level > 7)) 8267 return -EINVAL; 8268 if (level & TP_EC_FAN_FULLSPEED) 8269 return -EINVAL; 8270 8271 if (level & TP_EC_FAN_AUTO) { 8272 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) { 8273 rc = -EIO; 8274 break; 8275 } 8276 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) { 8277 rc = -EIO; 8278 break; 8279 } 8280 } else { 8281 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) { 8282 rc = -EIO; 8283 break; 8284 } 8285 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) { 8286 rc = -EIO; 8287 break; 8288 } 8289 if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8102, level * 100 / 7)) { 8290 rc = -EIO; 8291 break; 8292 } 8293 } 8294 break; 8295 8296 default: 8297 return -ENXIO; 8298 } 8299 8300 vdbg_printk(TPACPI_DBG_FAN, 8301 "fan control: set fan control register to 0x%02x\n", level); 8302 return 0; 8303 } 8304 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki |
|
From: Hans de G. <hde...@re...> - 2024-08-12 12:33:40
|
Hi Matthias,
On 7/14/24 6:50 PM, Matthias Fetzer wrote:
> Fan control on the E531 is done using the ACPI methods FANG and
> FANW. The correct parameters and register values were found by
> analyzing EC firmware as well as DSDT. This has been tested on
> my Thinkpad Edge E531 (6885CTO, BIOS HEET52WW 1.33).
>
> Signed-off-by: Matthias Fetzer <ko...@ma...>
Thank you for your patch.
With Ilpo's remarks addressed this patch looks good to me:
Reviewed-by: Hans de Goede <hde...@re...>
(for v2 with remarks addressed).
Please submit a v2 with Ilpo's remarks addressed.
Regards,
Hans
> ---
> drivers/platform/x86/thinkpad_acpi.c | 159 +++++++++++++++++++++++++++
> 1 file changed, 159 insertions(+)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 397b409064c9..a171a2b39ac9 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -7751,6 +7751,28 @@ static struct ibm_struct volume_driver_data = {
> * EC 0x2f (HFSP) might be available *for reading*, but do not use
> * it for writing.
> *
> + * TPACPI_FAN_RD_ACPI_FANG:
> + * ACPI FANG method: returns fan control register
> + *
> + * Takes one parameter which is 0x8100 plus the offset to EC memory
> + * address 0xf500 and returns the byte at this address.
> + *
> + * 0xf500:
> + * When the value is less than 9 automatic mode is enabled
> + * 0xf502:
> + * Contains the current fan speed from 0-100%
> + * 0xf504:
> + * Bit 7 has to be set in order to enable manual control by
> + * writing a value >= 9 to 0xf500
> + *
> + * TPACPI_FAN_WR_ACPI_FANW:
> + * ACPI FANG method: sets fan control registers
> + *
> + * Takes 0x8100 plus the offset to EC memory address 0xf500 and the
> + * value to be written there as parameters.
> + *
> + * see TPACPI_FAN_RD_ACPI_FANG
> + *
> * TPACPI_FAN_WR_TPEC:
> * ThinkPad EC register 0x2f (HFSP): fan control loop mode
> * Supported on almost all ThinkPads
> @@ -7884,6 +7906,7 @@ enum { /* Fan control constants */
> enum fan_status_access_mode {
> TPACPI_FAN_NONE = 0, /* No fan status or control */
> TPACPI_FAN_RD_ACPI_GFAN, /* Use ACPI GFAN */
> + TPACPI_FAN_RD_ACPI_FANG, /* Use ACPI FANG */
> TPACPI_FAN_RD_TPEC, /* Use ACPI EC regs 0x2f, 0x84-0x85 */
> TPACPI_FAN_RD_TPEC_NS, /* Use non-standard ACPI EC regs (eg: L13 Yoga gen2 etc.) */
> };
> @@ -7891,6 +7914,7 @@ enum fan_status_access_mode {
> enum fan_control_access_mode {
> TPACPI_FAN_WR_NONE = 0, /* No fan control */
> TPACPI_FAN_WR_ACPI_SFAN, /* Use ACPI SFAN */
> + TPACPI_FAN_WR_ACPI_FANW, /* Use ACPI FANW */
> TPACPI_FAN_WR_TPEC, /* Use ACPI EC reg 0x2f */
> TPACPI_FAN_WR_ACPI_FANS, /* Use ACPI FANS and EC reg 0x2f */
> };
> @@ -7924,9 +7948,13 @@ TPACPI_HANDLE(fans, ec, "FANS"); /* X31, X40, X41 */
> TPACPI_HANDLE(gfan, ec, "GFAN", /* 570 */
> "\\FSPD", /* 600e/x, 770e, 770x */
> ); /* all others */
> +TPACPI_HANDLE(fang, ec, "FANG", /* E531 */
> + ); /* all others */
> TPACPI_HANDLE(sfan, ec, "SFAN", /* 570 */
> "JFNS", /* 770x-JL */
> ); /* all others */
> +TPACPI_HANDLE(fanw, ec, "FANW", /* E531 */
> + ); /* all others */
>
> /*
> * Unitialized HFSP quirk: ACPI DSDT and EC fail to initialize the
> @@ -8033,6 +8061,23 @@ static int fan_get_status(u8 *status)
>
> break;
> }
> + case TPACPI_FAN_RD_ACPI_FANG: {
> + /* E531 */
> + int mode, speed;
> +
> + if (unlikely(!acpi_evalf(fang_handle, &mode, NULL, "dd", 0x8100)))
> + return -EIO;
> + if (unlikely(!acpi_evalf(fang_handle, &speed, NULL, "dd", 0x8102)))
> + return -EIO;
> +
> + if (likely(status)) {
> + *status = speed * 7 / 100;
> + if (mode < 9)
> + *status |= TP_EC_FAN_AUTO;
> + }
> +
> + break;
> + }
> case TPACPI_FAN_RD_TPEC:
> /* all except 570, 600e/x, 770e, 770x */
> if (unlikely(!acpi_ec_read(fan_status_offset, &s)))
> @@ -8147,6 +8192,17 @@ static int fan2_get_speed(unsigned int *speed)
> if (speed)
> *speed = lo ? FAN_RPM_CAL_CONST / lo : 0;
> break;
> + case TPACPI_FAN_RD_ACPI_FANG: {
> + /* E531 */
> + int speed_tmp;
> +
> + if (unlikely(!acpi_evalf(fang_handle, &speed_tmp, NULL, "dd", 0x8102)))
> + return -EIO;
> +
> + if (likely(speed))
> + *speed = speed_tmp * 65535 / 100;
> + break;
> + }
>
> default:
> return -ENXIO;
> @@ -8157,6 +8213,7 @@ static int fan2_get_speed(unsigned int *speed)
>
> static int fan_set_level(int level)
> {
> + int rc;
> if (!fan_control_allowed)
> return -EPERM;
>
> @@ -8206,6 +8263,36 @@ static int fan_set_level(int level)
> tp_features.fan_ctrl_status_undef = 0;
> break;
>
> + case TPACPI_FAN_WR_ACPI_FANW:
> + if ((!(level & TP_EC_FAN_AUTO) &&
> + ((level < 0) || (level > 7))) ||
> + (level & TP_EC_FAN_FULLSPEED))
> + return -EINVAL;
> + if (level & TP_EC_FAN_AUTO) {
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) {
> + rc = -EIO;
> + break;
> + }
> + } else {
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8102, level * 100 / 7)) {
> + rc = -EIO;
> + break;
> + }
> + }
> + break;
> +
> default:
> return -ENXIO;
> }
> @@ -8284,6 +8371,19 @@ static int fan_set_enable(void)
> rc = 0;
> break;
>
> + case TPACPI_FAN_WR_ACPI_FANW:
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x05)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0x00)) {
> + rc = -EIO;
> + break;
> + }
> +
> + rc = 0;
> + break;
> +
> default:
> rc = -ENXIO;
> }
> @@ -8326,6 +8426,22 @@ static int fan_set_disable(void)
> fan_control_desired_level = 0;
> break;
>
> + case TPACPI_FAN_WR_ACPI_FANW:
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8102, 0x00)) {
> + rc = -EIO;
> + break;
> + }
> + rc = 0;
> + break;
> +
> default:
> rc = -ENXIO;
> }
> @@ -8359,6 +8475,23 @@ static int fan_set_speed(int speed)
> rc = -EINVAL;
> break;
>
> + case TPACPI_FAN_WR_ACPI_FANW:
> + if (speed >= 0 && speed <= 65535) {
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8106, 0x45)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd", 0x8100, 0xff)) {
> + rc = -EIO;
> + break;
> + }
> + if (!acpi_evalf(fanw_handle, NULL, NULL, "vdd",
> + 0x8102, speed * 100 / 65535))
> + rc = -EIO;
> + } else
> + rc = -EINVAL;
> + break;
> +
> default:
> rc = -ENXIO;
> }
> @@ -8701,6 +8834,10 @@ static int __init fan_init(struct ibm_init_struct *iibm)
> TPACPI_ACPIHANDLE_INIT(gfan);
> TPACPI_ACPIHANDLE_INIT(sfan);
> }
> + if (tpacpi_is_lenovo()) {
> + TPACPI_ACPIHANDLE_INIT(fang);
> + TPACPI_ACPIHANDLE_INIT(fanw);
> + }
>
> quirks = tpacpi_check_quirks(fan_quirk_table,
> ARRAY_SIZE(fan_quirk_table));
> @@ -8720,6 +8857,9 @@ static int __init fan_init(struct ibm_init_struct *iibm)
> if (gfan_handle) {
> /* 570, 600e/x, 770e, 770x */
> fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
> + } else if (fang_handle) {
> + /* E531 */
> + fan_status_access_mode = TPACPI_FAN_RD_ACPI_FANG;
> } else {
> /* all other ThinkPads: note that even old-style
> * ThinkPad ECs supports the fan control register */
> @@ -8766,6 +8906,11 @@ static int __init fan_init(struct ibm_init_struct *iibm)
> fan_control_access_mode = TPACPI_FAN_WR_ACPI_SFAN;
> fan_control_commands |=
> TPACPI_FAN_CMD_LEVEL | TPACPI_FAN_CMD_ENABLE;
> + } else if (fanw_handle) {
> + /* E531 */
> + fan_control_access_mode = TPACPI_FAN_WR_ACPI_FANW;
> + fan_control_commands |=
> + TPACPI_FAN_CMD_LEVEL | TPACPI_FAN_CMD_SPEED | TPACPI_FAN_CMD_ENABLE;
> } else {
> if (!gfan_handle) {
> /* gfan without sfan means no fan control */
> @@ -8915,6 +9060,20 @@ static int fan_read(struct seq_file *m)
> str_enabled_disabled(status), status);
> break;
>
> + case TPACPI_FAN_RD_ACPI_FANG:
> + /* E531 */
> + rc = fan_get_status_safe(&status);
> + if (rc)
> + return rc;
> +
> + seq_printf(m, "status:\t\t%s\n", str_enabled_disabled(status));
> +
> + rc = fan_get_speed(&speed);
> + if (rc < 0)
> + return rc;
> + seq_printf(m, "speed:\t\t%d\n", speed);
> + break;
> +
> case TPACPI_FAN_RD_TPEC_NS:
> case TPACPI_FAN_RD_TPEC:
> /* all except 570, 600e/x, 770e, 770x */
|
|
From: <ma...@3v...> - 2024-06-03 18:08:00
|
Hi, Not sure if something changed or it is different in newer models, but the privacy screen feature so far has been exposed as a KMS property you can inspect and set with tools like proptest (in libdrm-tests package for what concerns ubuntu) Cheers On giu 3 2024, at 1:46 pm, Pellaeon Lin <nfs...@gm...> wrote: > Hi, > > I have a ThinkPad X13 Gen 2 AMD with PrivacyGuard. But by pressing the > combination key Fn+D it does not toggle the PrivacyGuard, the > PrivacyGuard stays on. I've tested: > > - Fn+D does toggle PrivacyGuard when I'm in the BIOS > - Fn+D has no effect on Ubuntu 22.10, Ubuntu 23.04, Ubuntu 23.10, > Ubuntu 24.04 and Fedora 40 (except Ubuntu 23.10 and 24.04, all was > tested using LiveUSB) > > In all of the Linux cases, I can confirm that by pressing Fn+D, the > status value of /proc/acpi/ibm/lcdshadow actually changes. > > I've always thought this was a Ubuntu-specific issue, until I tested Fedora. > > I tested this further by installing the acpi-call-dkms package on > Ubuntu and issues the following call: > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x1' | sudo tee /proc/acpi/call > > It returned 0 (which should mean success), but PrivacyGuard is still > ON. Then I tried to call it with 0x0: > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x0' | sudo tee /proc/acpi/call > > Also getting 0 in return. PrivacyGuard is still ON. > > Based on my limited understanding of ACPI and the kernel, at this > point it might be a firmware issue? (fwupdmgr shows that there is no > available updates) But based on my reading and understanding of > thinkpad_acpi.c, the particular ACPI call that I issued is also how > the Linux driver currently operates the PrivacyGuard feature, which > means the driver might also be affected by this issue. So I'm hoping > someone could help me debug this further, or point out anything that > I'm not understanding correctly. > > Thanks! > _______________________________________________ > > ibm-acpi-devel mailing list > > ibm...@li... > > https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel |
|
From: Mark P. <mpe...@sq...> - 2024-06-03 18:01:23
|
Hi Pellaeon, On Mon, Jun 3, 2024, at 7:46 AM, Pellaeon Lin wrote: > Hi, > > I have a ThinkPad X13 Gen 2 AMD with PrivacyGuard. But by pressing the > combination key Fn+D it does not toggle the PrivacyGuard, the PrivacyGuard > stays on. I've tested: > > - Fn+D does toggle PrivacyGuard when I'm in the BIOS > - Fn+D has no effect on Ubuntu 22.10, Ubuntu 23.04, Ubuntu 23.10, Ubuntu > 24.04 and Fedora 40 (except Ubuntu 23.10 and 24.04, all was tested using > LiveUSB) > > In all of the Linux cases, I can confirm that by pressing Fn+D, the status > value of /proc/acpi/ibm/lcdshadow actually changes. > > I've always thought this was a Ubuntu-specific issue, until I tested Fedora. > > I tested this further by installing the acpi-call-dkms package on Ubuntu > and issues the following call: > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x1' | sudo tee /proc/acpi/call > > It returned 0 (which should mean success), but PrivacyGuard is still ON. > Then I tried to call it with 0x0: > > echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x0' | sudo tee /proc/acpi/call > > Also getting 0 in return. PrivacyGuard is still ON. > > Based on my limited understanding of ACPI and the kernel, at this point it > might be a firmware issue? (fwupdmgr shows that there is no available > updates) But based on my reading and understanding of thinkpad_acpi.c, the > particular ACPI call that I issued is also how the Linux driver currently > operates the PrivacyGuard feature, which means the driver might also be > affected by this issue. So I'm hoping someone could help me debug this > further, or point out anything that I'm not understanding correctly. > I've forwarded this to the team to check out (I don't have an X13 G2 AMD on hand myself). My understanding is that the kernel wasn't involved for the privacy FN+D feature - the BIOS handled it (and sent an ACPI notification so you could track the status). At least on the platforms I've tested this on - it just worked. I'll see what they reply. Mark |
|
From: Pellaeon L. <nfs...@gm...> - 2024-06-03 11:46:48
|
Hi, I have a ThinkPad X13 Gen 2 AMD with PrivacyGuard. But by pressing the combination key Fn+D it does not toggle the PrivacyGuard, the PrivacyGuard stays on. I've tested: - Fn+D does toggle PrivacyGuard when I'm in the BIOS - Fn+D has no effect on Ubuntu 22.10, Ubuntu 23.04, Ubuntu 23.10, Ubuntu 24.04 and Fedora 40 (except Ubuntu 23.10 and 24.04, all was tested using LiveUSB) In all of the Linux cases, I can confirm that by pressing Fn+D, the status value of /proc/acpi/ibm/lcdshadow actually changes. I've always thought this was a Ubuntu-specific issue, until I tested Fedora. I tested this further by installing the acpi-call-dkms package on Ubuntu and issues the following call: echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x1' | sudo tee /proc/acpi/call It returned 0 (which should mean success), but PrivacyGuard is still ON. Then I tried to call it with 0x0: echo '\_SB.PCI0.LPC0.EC0.HKEY.SSSS 0x0' | sudo tee /proc/acpi/call Also getting 0 in return. PrivacyGuard is still ON. Based on my limited understanding of ACPI and the kernel, at this point it might be a firmware issue? (fwupdmgr shows that there is no available updates) But based on my reading and understanding of thinkpad_acpi.c, the particular ACPI call that I issued is also how the Linux driver currently operates the PrivacyGuard feature, which means the driver might also be affected by this issue. So I'm hoping someone could help me debug this further, or point out anything that I'm not understanding correctly. Thanks! |
|
From: Hans de G. <hde...@re...> - 2024-06-03 09:16:50
|
Hi, On 6/2/24 10:58 AM, Andy Shevchenko wrote: > Use 2-argument strscpy(), which is not only shorter but also provides > an additional check that destination buffer is an array. > > Signed-off-by: Andy Shevchenko <and...@gm...> Since the code being modified only exists on the fixes branch I've merged this as a fix now. I know this is more of a cleanup then a pure fix, but since the DMI quirks always get updated through the fixes branch this avoids conflicts. Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Regards, Hans > --- > drivers/platform/x86/touchscreen_dmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c > index 2d9ca2292ea1..879a63e4ecd0 100644 > --- a/drivers/platform/x86/touchscreen_dmi.c > +++ b/drivers/platform/x86/touchscreen_dmi.c > @@ -1907,7 +1907,7 @@ static int __init ts_parse_props(char *str) > u32 u32val; > int i, ret; > > - strscpy(orig_str, str, sizeof(orig_str)); > + strscpy(orig_str, str); > > /* > * str is part of the static_command_line from init/main.c and poking |
|
From: Hans de G. <hde...@re...> - 2024-06-02 13:28:34
|
Hi, On 6/2/24 10:57 AM, Andy Shevchenko wrote: > Use 2-argument strscpy(), which is not only shorter but also provides > an additional check that destination buffer is an array. > > Signed-off-by: Andy Shevchenko <and...@gm...> Thanks, the entire series looks good to me: Reviewed-by: Hans de Goede <hde...@re...> for the series. Regards, Hans > --- > drivers/platform/x86/asus-tf103c-dock.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/asus-tf103c-dock.c b/drivers/platform/x86/asus-tf103c-dock.c > index 8f0f87637c5f..b441d8ca72d3 100644 > --- a/drivers/platform/x86/asus-tf103c-dock.c > +++ b/drivers/platform/x86/asus-tf103c-dock.c > @@ -490,7 +490,7 @@ static void tf103c_dock_enable_touchpad(struct tf103c_dock_data *dock) > return; > } > > - strscpy(board_info.type, "elan_i2c", I2C_NAME_SIZE); > + strscpy(board_info.type, "elan_i2c"); > board_info.addr = TF103C_DOCK_TP_ADDR; > board_info.dev_name = TF103C_DOCK_DEV_NAME "-tp"; > board_info.irq = dock->tp_irq; > @@ -795,7 +795,7 @@ static int tf103c_dock_probe(struct i2c_client *client) > */ > dock->ec_client = client; > > - strscpy(board_info.type, "tf103c-dock-intr", I2C_NAME_SIZE); > + strscpy(board_info.type, "tf103c-dock-intr"); > board_info.addr = TF103C_DOCK_INTR_ADDR; > board_info.dev_name = TF103C_DOCK_DEV_NAME "-intr"; > > @@ -803,7 +803,7 @@ static int tf103c_dock_probe(struct i2c_client *client) > if (IS_ERR(dock->intr_client)) > return dev_err_probe(dev, PTR_ERR(dock->intr_client), "creating intr client\n"); > > - strscpy(board_info.type, "tf103c-dock-kbd", I2C_NAME_SIZE); > + strscpy(board_info.type, "tf103c-dock-kbd"); > board_info.addr = TF103C_DOCK_KBD_ADDR; > board_info.dev_name = TF103C_DOCK_DEV_NAME "-kbd"; > > @@ -846,8 +846,8 @@ static int tf103c_dock_probe(struct i2c_client *client) > dock->hid->vendor = 0x0b05; /* USB_VENDOR_ID_ASUSTEK */ > dock->hid->product = 0x0103; /* From TF-103-C */ > dock->hid->version = 0x0100; /* 1.0 */ > - strscpy(dock->hid->name, "Asus TF103C Dock Keyboard", sizeof(dock->hid->name)); > - strscpy(dock->hid->phys, dev_name(dev), sizeof(dock->hid->phys)); > + strscpy(dock->hid->name, "Asus TF103C Dock Keyboard"); > + strscpy(dock->hid->phys, dev_name(dev)); > > ret = hid_add_device(dock->hid); > if (ret) |
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:29
|
Use 2-argument strscpy(), which is not only shorter but also provides
an additional check that destination buffer is an array.
Signed-off-by: Andy Shevchenko <and...@gm...>
---
drivers/platform/x86/think-lmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 0f2264bb7577..4cfb53206cb8 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -1508,7 +1508,7 @@ static struct tlmi_pwd_setting *tlmi_create_auth(const char *pwd_type,
if (!new_pwd)
return NULL;
- strscpy(new_pwd->kbdlang, "us", TLMI_LANG_MAXLEN);
+ strscpy(new_pwd->kbdlang, "us");
new_pwd->encoding = TLMI_ENCODING_ASCII;
new_pwd->pwd_type = pwd_type;
new_pwd->role = pwd_role;
@@ -1582,7 +1582,7 @@ static int tlmi_analyze(void)
goto fail_clear_attr;
}
setting->index = i;
- strscpy(setting->display_name, item, TLMI_SETTINGS_MAXLEN);
+ strscpy(setting->display_name, item);
/* If BIOS selections supported, load those */
if (tlmi_priv.can_get_bios_selections) {
ret = tlmi_get_bios_selections(setting->display_name,
--
2.45.1
|
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:24
|
Move to 2-argument strscpy() to make code shorter and have an additional check. No functional change intended. Some cases are let untouched where it looks better with the 3rd argument. Andy Shevchenko (7): platform/x86: asus-tf103c-dock: Use 2-argument strscpy() platform/x86: hp: hp-bioscfg: Use 2-argument strscpy() platform/x86: intel: chtwc_int33fe: Use 2-argument strscpy() platform/x86: serial-multi-instantiate: Use 2-argument strscpy() platform/x86: think-lmi: Use 2-argument strscpy() platform/x86: thinkpad_acpi: Use 2-argument strscpy() platform/x86: touchscreen_dmi: Use 2-argument strscpy() drivers/platform/x86/asus-tf103c-dock.c | 10 +++++----- .../x86/hp/hp-bioscfg/enum-attributes.c | 18 +++++------------- .../x86/hp/hp-bioscfg/int-attributes.c | 7 ++----- .../x86/hp/hp-bioscfg/order-list-attributes.c | 18 +++++------------- .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 19 +++++-------------- .../x86/hp/hp-bioscfg/spmobj-attributes.c | 3 +-- .../x86/hp/hp-bioscfg/string-attributes.c | 12 ++++-------- drivers/platform/x86/intel/chtwc_int33fe.c | 6 +++--- .../platform/x86/serial-multi-instantiate.c | 4 ++-- drivers/platform/x86/think-lmi.c | 4 ++-- drivers/platform/x86/thinkpad_acpi.c | 6 ++---- drivers/platform/x86/touchscreen_dmi.c | 2 +- 12 files changed, 37 insertions(+), 72 deletions(-) -- 2.45.1 |
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:23
|
Use 2-argument strscpy(), which is not only shorter but also provides
an additional check that destination buffer is an array.
Signed-off-by: Andy Shevchenko <and...@gm...>
---
drivers/platform/x86/serial-multi-instantiate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 97b9c6392230..3be016cfe601 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -131,7 +131,7 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
ctlr = spi_dev->controller;
- strscpy(spi_dev->modalias, inst_array[i].type, sizeof(spi_dev->modalias));
+ strscpy(spi_dev->modalias, inst_array[i].type);
ret = smi_get_irq(pdev, adev, &inst_array[i]);
if (ret < 0) {
@@ -205,7 +205,7 @@ static int smi_i2c_probe(struct platform_device *pdev, struct smi *smi,
for (i = 0; i < count && inst_array[i].type; i++) {
memset(&board_info, 0, sizeof(board_info));
- strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE);
+ strscpy(board_info.type, inst_array[i].type);
snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i);
board_info.dev_name = name;
--
2.45.1
|
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:22
|
Use 2-argument strscpy(), which is not only shorter but also provides
an additional check that destination buffer is an array.
Signed-off-by: Andy Shevchenko <and...@gm...>
---
drivers/platform/x86/intel/chtwc_int33fe.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/intel/chtwc_int33fe.c b/drivers/platform/x86/intel/chtwc_int33fe.c
index 93f75ba1dafd..11503b1c85f3 100644
--- a/drivers/platform/x86/intel/chtwc_int33fe.c
+++ b/drivers/platform/x86/intel/chtwc_int33fe.c
@@ -270,7 +270,7 @@ cht_int33fe_register_max17047(struct device *dev, struct cht_int33fe_data *data)
}
memset(&board_info, 0, sizeof(board_info));
- strscpy(board_info.type, "max17047", I2C_NAME_SIZE);
+ strscpy(board_info.type, "max17047");
board_info.dev_name = "max17047";
board_info.fwnode = fwnode;
data->battery_fg = i2c_acpi_new_device(dev, 1, &board_info);
@@ -361,7 +361,7 @@ static int cht_int33fe_typec_probe(struct platform_device *pdev)
}
memset(&board_info, 0, sizeof(board_info));
- strscpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
+ strscpy(board_info.type, "typec_fusb302");
board_info.dev_name = "fusb302";
board_info.fwnode = fwnode;
board_info.irq = fusb302_irq;
@@ -381,7 +381,7 @@ static int cht_int33fe_typec_probe(struct platform_device *pdev)
memset(&board_info, 0, sizeof(board_info));
board_info.dev_name = "pi3usb30532";
board_info.fwnode = fwnode;
- strscpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
+ strscpy(board_info.type, "pi3usb30532");
data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
if (IS_ERR(data->pi3usb30532)) {
--
2.45.1
|
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:21
|
Use 2-argument strscpy(), which is not only shorter but also provides
an additional check that destination buffer is an array.
Signed-off-by: Andy Shevchenko <and...@gm...>
---
.../x86/hp/hp-bioscfg/enum-attributes.c | 18 +++++-------------
.../x86/hp/hp-bioscfg/int-attributes.c | 7 ++-----
.../x86/hp/hp-bioscfg/order-list-attributes.c | 18 +++++-------------
.../x86/hp/hp-bioscfg/passwdobj-attributes.c | 19 +++++--------------
.../x86/hp/hp-bioscfg/spmobj-attributes.c | 3 +--
.../x86/hp/hp-bioscfg/string-attributes.c | 12 ++++--------
6 files changed, 22 insertions(+), 55 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index a2402d31c146..c50ad5880503 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -52,9 +52,7 @@ static void update_enumeration_value(int instance_id, char *attr_value)
{
struct enumeration_data *enum_data = &bioscfg_drv.enumeration_data[instance_id];
- strscpy(enum_data->current_value,
- attr_value,
- sizeof(enum_data->current_value));
+ strscpy(enum_data->current_value, attr_value);
}
ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name, enumeration);
@@ -174,8 +172,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
case VALUE:
break;
case PATH:
- strscpy(enum_data->common.path, str_value,
- sizeof(enum_data->common.path));
+ strscpy(enum_data->common.path, str_value);
break;
case IS_READONLY:
enum_data->common.is_readonly = int_value;
@@ -222,9 +219,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
if (ret)
return -EINVAL;
- strscpy(enum_data->common.prerequisites[reqs],
- str_value,
- sizeof(enum_data->common.prerequisites[reqs]));
+ strscpy(enum_data->common.prerequisites[reqs], str_value);
kfree(str_value);
str_value = NULL;
@@ -236,8 +231,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
break;
case ENUM_CURRENT_VALUE:
- strscpy(enum_data->current_value,
- str_value, sizeof(enum_data->current_value));
+ strscpy(enum_data->current_value, str_value);
break;
case ENUM_SIZE:
if (int_value > MAX_VALUES_SIZE) {
@@ -278,9 +272,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
* is greater than MAX_VALUES_SIZE
*/
if (size < MAX_VALUES_SIZE)
- strscpy(enum_data->possible_values[pos_values],
- str_value,
- sizeof(enum_data->possible_values[pos_values]));
+ strscpy(enum_data->possible_values[pos_values], str_value);
kfree(str_value);
str_value = NULL;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index 86b7ac63fec2..6c7f4d5fa9cb 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -192,8 +192,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
integer_data->current_value = int_value;
break;
case PATH:
- strscpy(integer_data->common.path, str_value,
- sizeof(integer_data->common.path));
+ strscpy(integer_data->common.path, str_value);
break;
case IS_READONLY:
integer_data->common.is_readonly = int_value;
@@ -240,9 +239,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
if (ret)
continue;
- strscpy(integer_data->common.prerequisites[reqs],
- str_value,
- sizeof(integer_data->common.prerequisites[reqs]));
+ strscpy(integer_data->common.prerequisites[reqs], str_value);
kfree(str_value);
str_value = NULL;
}
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 1ff09dfb7d7e..c6e57bb9d8b7 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -57,9 +57,7 @@ static void update_ordered_list_value(int instance, char *attr_value)
{
struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance];
- strscpy(ordered_list_data->current_value,
- attr_value,
- sizeof(ordered_list_data->current_value));
+ strscpy(ordered_list_data->current_value, attr_value);
}
ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name, ordered_list);
@@ -179,13 +177,11 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
/* Assign appropriate element value to corresponding field*/
switch (eloc) {
case VALUE:
- strscpy(ordered_list_data->current_value,
- str_value, sizeof(ordered_list_data->current_value));
+ strscpy(ordered_list_data->current_value, str_value);
replace_char_str(ordered_list_data->current_value, COMMA_SEP, SEMICOLON_SEP);
break;
case PATH:
- strscpy(ordered_list_data->common.path, str_value,
- sizeof(ordered_list_data->common.path));
+ strscpy(ordered_list_data->common.path, str_value);
break;
case IS_READONLY:
ordered_list_data->common.is_readonly = int_value;
@@ -227,9 +223,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
if (ret)
continue;
- strscpy(ordered_list_data->common.prerequisites[reqs],
- str_value,
- sizeof(ordered_list_data->common.prerequisites[reqs]));
+ strscpy(ordered_list_data->common.prerequisites[reqs], str_value);
kfree(str_value);
str_value = NULL;
@@ -271,9 +265,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
part = strsep(&part_tmp, COMMA_SEP);
for (olist_elem = 0; olist_elem < MAX_ELEMENTS_SIZE && part; olist_elem++) {
- strscpy(ordered_list_data->elements[olist_elem],
- part,
- sizeof(ordered_list_data->elements[olist_elem]));
+ strscpy(ordered_list_data->elements[olist_elem], part);
part = strsep(&part_tmp, COMMA_SEP);
}
ordered_list_data->elements_size = olist_elem;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index f7efe217a4bb..35936c05e45b 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -101,13 +101,9 @@ static int store_password_instance(struct kobject *kobj, const char *buf,
if (!ret) {
if (is_current)
- strscpy(bioscfg_drv.password_data[id].current_password,
- buf_cp,
- sizeof(bioscfg_drv.password_data[id].current_password));
+ strscpy(bioscfg_drv.password_data[id].current_password, buf_cp);
else
- strscpy(bioscfg_drv.password_data[id].new_password,
- buf_cp,
- sizeof(bioscfg_drv.password_data[id].new_password));
+ strscpy(bioscfg_drv.password_data[id].new_password, buf_cp);
}
kfree(buf_cp);
@@ -272,8 +268,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
case VALUE:
break;
case PATH:
- strscpy(password_data->common.path, str_value,
- sizeof(password_data->common.path));
+ strscpy(password_data->common.path, str_value);
break;
case IS_READONLY:
password_data->common.is_readonly = int_value;
@@ -315,9 +310,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
if (ret)
break;
- strscpy(password_data->common.prerequisites[reqs],
- str_value,
- sizeof(password_data->common.prerequisites[reqs]));
+ strscpy(password_data->common.prerequisites[reqs], str_value);
kfree(str_value);
str_value = NULL;
@@ -359,9 +352,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
if (ret)
break;
- strscpy(password_data->encodings[pos_values],
- str_value,
- sizeof(password_data->encodings[pos_values]));
+ strscpy(password_data->encodings[pos_values], str_value);
kfree(str_value);
str_value = NULL;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
index 86f90238750c..2b00a14792e9 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -365,8 +365,7 @@ int hp_populate_secure_platform_data(struct kobject *attr_name_kobj)
/* Populate data for Secure Platform Management */
bioscfg_drv.spm_data.attr_name_kobj = attr_name_kobj;
- strscpy(bioscfg_drv.spm_data.attribute_name, SPM_STR,
- sizeof(bioscfg_drv.spm_data.attribute_name));
+ strscpy(bioscfg_drv.spm_data.attribute_name, SPM_STR);
bioscfg_drv.spm_data.is_enabled = 0;
bioscfg_drv.spm_data.mechanism = 0;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index f0c20070094d..27758b779b2d 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -50,7 +50,7 @@ static void update_string_value(int instance_id, char *attr_value)
struct string_data *string_data = &bioscfg_drv.string_data[instance_id];
/* Write settings to BIOS */
- strscpy(string_data->current_value, attr_value, sizeof(string_data->current_value));
+ strscpy(string_data->current_value, attr_value);
}
/*
@@ -178,12 +178,10 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
/* Assign appropriate element value to corresponding field*/
switch (eloc) {
case VALUE:
- strscpy(string_data->current_value,
- str_value, sizeof(string_data->current_value));
+ strscpy(string_data->current_value, str_value);
break;
case PATH:
- strscpy(string_data->common.path, str_value,
- sizeof(string_data->common.path));
+ strscpy(string_data->common.path, str_value);
break;
case IS_READONLY:
string_data->common.is_readonly = int_value;
@@ -231,9 +229,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
if (ret)
continue;
- strscpy(string_data->common.prerequisites[reqs],
- str_value,
- sizeof(string_data->common.prerequisites[reqs]));
+ strscpy(string_data->common.prerequisites[reqs], str_value);
kfree(str_value);
str_value = NULL;
}
--
2.45.1
|
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:20
|
Use 2-argument strscpy(), which is not only shorter but also provides an additional check that destination buffer is an array. Signed-off-by: Andy Shevchenko <and...@gm...> --- drivers/platform/x86/thinkpad_acpi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 397b409064c9..f269ca1ff771 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -7416,10 +7416,8 @@ static int __init volume_create_alsa_mixer(void) data = card->private_data; data->card = card; - strscpy(card->driver, TPACPI_ALSA_DRVNAME, - sizeof(card->driver)); - strscpy(card->shortname, TPACPI_ALSA_SHRTNAME, - sizeof(card->shortname)); + strscpy(card->driver, TPACPI_ALSA_DRVNAME); + strscpy(card->shortname, TPACPI_ALSA_SHRTNAME); snprintf(card->mixername, sizeof(card->mixername), "ThinkPad EC %s", (thinkpad_id.ec_version_str) ? thinkpad_id.ec_version_str : "(unknown)"); -- 2.45.1 |
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:15
|
Use 2-argument strscpy(), which is not only shorter but also provides an additional check that destination buffer is an array. Signed-off-by: Andy Shevchenko <and...@gm...> --- drivers/platform/x86/touchscreen_dmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c index 2d9ca2292ea1..879a63e4ecd0 100644 --- a/drivers/platform/x86/touchscreen_dmi.c +++ b/drivers/platform/x86/touchscreen_dmi.c @@ -1907,7 +1907,7 @@ static int __init ts_parse_props(char *str) u32 u32val; int i, ret; - strscpy(orig_str, str, sizeof(orig_str)); + strscpy(orig_str, str); /* * str is part of the static_command_line from init/main.c and poking -- 2.45.1 |
|
From: Andy S. <and...@gm...> - 2024-06-02 09:19:11
|
Use 2-argument strscpy(), which is not only shorter but also provides an additional check that destination buffer is an array. Signed-off-by: Andy Shevchenko <and...@gm...> --- drivers/platform/x86/asus-tf103c-dock.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/platform/x86/asus-tf103c-dock.c b/drivers/platform/x86/asus-tf103c-dock.c index 8f0f87637c5f..b441d8ca72d3 100644 --- a/drivers/platform/x86/asus-tf103c-dock.c +++ b/drivers/platform/x86/asus-tf103c-dock.c @@ -490,7 +490,7 @@ static void tf103c_dock_enable_touchpad(struct tf103c_dock_data *dock) return; } - strscpy(board_info.type, "elan_i2c", I2C_NAME_SIZE); + strscpy(board_info.type, "elan_i2c"); board_info.addr = TF103C_DOCK_TP_ADDR; board_info.dev_name = TF103C_DOCK_DEV_NAME "-tp"; board_info.irq = dock->tp_irq; @@ -795,7 +795,7 @@ static int tf103c_dock_probe(struct i2c_client *client) */ dock->ec_client = client; - strscpy(board_info.type, "tf103c-dock-intr", I2C_NAME_SIZE); + strscpy(board_info.type, "tf103c-dock-intr"); board_info.addr = TF103C_DOCK_INTR_ADDR; board_info.dev_name = TF103C_DOCK_DEV_NAME "-intr"; @@ -803,7 +803,7 @@ static int tf103c_dock_probe(struct i2c_client *client) if (IS_ERR(dock->intr_client)) return dev_err_probe(dev, PTR_ERR(dock->intr_client), "creating intr client\n"); - strscpy(board_info.type, "tf103c-dock-kbd", I2C_NAME_SIZE); + strscpy(board_info.type, "tf103c-dock-kbd"); board_info.addr = TF103C_DOCK_KBD_ADDR; board_info.dev_name = TF103C_DOCK_DEV_NAME "-kbd"; @@ -846,8 +846,8 @@ static int tf103c_dock_probe(struct i2c_client *client) dock->hid->vendor = 0x0b05; /* USB_VENDOR_ID_ASUSTEK */ dock->hid->product = 0x0103; /* From TF-103-C */ dock->hid->version = 0x0100; /* 1.0 */ - strscpy(dock->hid->name, "Asus TF103C Dock Keyboard", sizeof(dock->hid->name)); - strscpy(dock->hid->phys, dev_name(dev), sizeof(dock->hid->phys)); + strscpy(dock->hid->name, "Asus TF103C Dock Keyboard"); + strscpy(dock->hid->phys, dev_name(dev)); ret = hid_add_device(dock->hid); if (ret) -- 2.45.1 |
|
From: Hans de G. <hde...@re...> - 2024-05-22 13:30:42
|
Hi Vlastimil, On 5/22/24 1:50 PM, Vlastimil Babka wrote: > On 4/24/24 2:28 PM, Hans de Goede wrote: >> Switch the hotkey keymap handling over to the sparse-keymap helpers, >> there should be no functional changes from this. >> >> Note all the mappings to KEY_UNKNOWN are removed since that is the default >> behavior of sparse_keymap_report_event() for unknown scancodes. >> >> Also drop the big comment about making changes to the keymaps since >> the contents of that comment are mostly obsolete. >> >> Tested-by: Mark Pearson <mpe...@sq...> >> Signed-off-by: Hans de Goede <hde...@re...> > > Hi, > > I believe this is what gave me the following error compiling current > Linus's master: > > ERROR: modpost: "sparse_keymap_report_event" > [drivers/platform/x86/thinkpad_acpi.ko] undefined! > ERROR: modpost: "sparse_keymap_setup" > [drivers/platform/x86/thinkpad_acpi.ko] undefined! > > probably config THINKPAD_ACPI now has to depend/select INPUT_SPARSEKMAP? > It's fixed when I configure it =m manually. Thank you for reporting this. This should be fixed by: https://lore.kernel.org/platform-driver-x86/20240522074813.379b9fc2@gandalf.local.home/T/#u Which I will include in my next pdx86 fixes pull-request to Linus. Regards, Hans |
|
From: Michael T. <mj...@tl...> - 2024-05-14 06:38:15
|
30.04.2024 10:34, Michael Tokarev wrote: > Hi! > > This is my first thinkpad, and since I use linux almost exclusively, > it is running linux too (debian bookworm). However, there are a few > probs with it which I'd love to debug and find solution to. > > One of the probs is the power button: it stops working after the first > suspend-resume cycle. > > Initially it is registered as event5 "Power Button". After fresh boot, > `input record` shows EV_KEY/KEY_POWER keypress events coming from it. > So far, so good. > > Now, I perform hibernation: `echo disk > /sys/power/state` or > `systemctl hibernate` (I have to use `shutdown` method here instead of > `platform`, since the latter does not work, which is another issue > I'm trying to fix). There's no GUI or anything fancy running, - > just plain old command line on a linux tty. > > And after resume, this button does not produce any events in linux > anymore, after a note in dmesg: > > [ 24.788054] thinkpad_acpi: acpi_evalf(STRW, vd, ...) failed: AE_NOT_FOUND > [ 24.788058] thinkpad_acpi: Cannot set adaptive keyboard mode. > > Here's the full `dmesg | grep thinkpad` output: > > [ 3.294025] thinkpad_acpi: ThinkPad ACPI Extras v0.26 > [ 3.295427] thinkpad_acpi: http://ibm-acpi.sf.net/ > [ 3.295431] thinkpad_acpi: ThinkPad BIOS R13ET55W(1.29 ), EC R13HT55W > [ 3.295433] thinkpad_acpi: Lenovo ThinkPad T495s, model 20QKS0EQ0N > [ 3.298362] thinkpad_acpi: radio switch found; radios are enabled > [ 3.300653] thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver > [ 3.303193] thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... > [ 3.318819] thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked > [ 3.364425] thinkpad_acpi: Standard ACPI backlight interface available, not loading native one > [ 3.399354] thinkpad_acpi: secondary fan control detected & enabled > [ 3.425884] thinkpad_acpi: battery 1 registered (start 95, stop 100, behaviours: 0x7) > [ 3.433515] input: ThinkPad Extra Buttons as /devices/platform/thinkpad_acpi/input/input10 > [ 24.202923] thinkpad_acpi: acpi_evalf(GTRW, dd, ...) failed: AE_NOT_FOUND > [ 24.202953] thinkpad_acpi: Cannot read adaptive keyboard mode. > [ 24.788054] thinkpad_acpi: acpi_evalf(STRW, vd, ...) failed: AE_NOT_FOUND > [ 24.788058] thinkpad_acpi: Cannot set adaptive keyboard mode. I found another mention of - apparently - this issue, on the same hardware (though the question was about something else): https://askubuntu.com/questions/1224358/thinkpad-t495s-suspend-issue so it looks like the issue with the power button losing events with linux is old(ish). Also, I verified how it works on windows (installing windows10 on this laptop) - it works exactly as expected, after various suspend/hibernate/etc cycles, so the problem isn't hardware (firmware). But in case of windows, I guess it is using equivalent of "platform" hibernation mode, while on linux I have to switch to "shutdown" or else it doesn't resume. Thanks, /mjt |
|
From: Michael T. <mj...@tl...> - 2024-05-11 06:56:36
|
30.04.2024 10:34, Michael Tokarev wrote: > Hi! > > This is my first thinkpad, and since I use linux almost exclusively, > it is running linux too (debian bookworm). However, there are a few > probs with it which I'd love to debug and find solution to. > > One of the probs is the power button: it stops working after the first > suspend-resume cycle. > > Initially it is registered as event5 "Power Button". After fresh boot, > `input record` shows EV_KEY/KEY_POWER keypress events coming from it. > So far, so good. > > Now, I perform hibernation: `echo disk > /sys/power/state` or > `systemctl hibernate` (I have to use `shutdown` method here instead of > `platform`, since the latter does not work, which is another issue > I'm trying to fix). There's no GUI or anything fancy running, - > just plain old command line on a linux tty. > > And after resume, this button does not produce any events in linux > anymore, after a note in dmesg: > > [ 24.788054] thinkpad_acpi: acpi_evalf(STRW, vd, ...) failed: AE_NOT_FOUND > [ 24.788058] thinkpad_acpi: Cannot set adaptive keyboard mode. > > Here's the full `dmesg | grep thinkpad` output: > > [ 3.294025] thinkpad_acpi: ThinkPad ACPI Extras v0.26 > [ 3.295427] thinkpad_acpi: http://ibm-acpi.sf.net/ > [ 3.295431] thinkpad_acpi: ThinkPad BIOS R13ET55W(1.29 ), EC R13HT55W > [ 3.295433] thinkpad_acpi: Lenovo ThinkPad T495s, model 20QKS0EQ0N > [ 3.298362] thinkpad_acpi: radio switch found; radios are enabled > [ 3.300653] thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver > [ 3.303193] thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... > [ 3.318819] thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked > [ 3.364425] thinkpad_acpi: Standard ACPI backlight interface available, not loading native one > [ 3.399354] thinkpad_acpi: secondary fan control detected & enabled > [ 3.425884] thinkpad_acpi: battery 1 registered (start 95, stop 100, behaviours: 0x7) > [ 3.433515] input: ThinkPad Extra Buttons as /devices/platform/thinkpad_acpi/input/input10 > [ 24.202923] thinkpad_acpi: acpi_evalf(GTRW, dd, ...) failed: AE_NOT_FOUND > [ 24.202953] thinkpad_acpi: Cannot read adaptive keyboard mode. > [ 24.788054] thinkpad_acpi: acpi_evalf(STRW, vd, ...) failed: AE_NOT_FOUND > [ 24.788058] thinkpad_acpi: Cannot set adaptive keyboard mode. I tried current 6.7 kernel for this, which shows exactly the same behavior. Also I tried removing thinkpad_acpi module before hibernate and modprobing it after, - this way, the module does not report the error above, but the power button still does nothing. > What's the problem here, any hints how to debug it further? Any debugging tips? Thanks! /mjt -- GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24. New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98 ECDF 2C8E Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0 8044 65C5 Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt |
|
From: Greg Kroah-H. <gr...@li...> - 2024-05-04 15:36:34
|
On Sat, May 04, 2024 at 04:31:42PM +0200, Lukas Wunner wrote: > Dear Greg, > > On Sat, Apr 20, 2024 at 10:00:00PM +0200, Lukas Wunner wrote: > > Introduce a generic ->show() callback to expose a string as a device > > attribute in sysfs. Deduplicate various identical callbacks across > > the tree. > > > > Result: Minus 216 LoC, minus 1576 bytes vmlinux size (x86_64 allyesconfig). > > > > This is a byproduct of my upcoming PCI device authentication v2 patches. > > > > > > Lukas Wunner (6): > > driver core: Add device_show_string() helper for sysfs attributes > > hwmon: Use device_show_string() helper for sysfs attributes > > IB/qib: Use device_show_string() helper for sysfs attributes > > perf: Use device_show_string() helper for sysfs attributes > > platform/x86: Use device_show_string() helper for sysfs attributes > > scsi: Use device_show_string() helper for sysfs attributes > > This series hasn't been applied to driver-core-next AFAICS and the > merge window is drawing closer. > > So far only patches 1, 2 and 5 have been ack'ed by the respective > subsystem maintainers. If the missing acks are the reason it hasn't > been applied, would it be possibe to apply only 1, 2 and 5? > > I would then resubmit the other ones individually to the subsystem > maintainers in the next cycle. I'll just pick it up now, thanks! greg k-h |
|
From: Michael T. <mj...@tl...> - 2024-04-30 07:54:14
|
30.04.2024 10:34, Michael Tokarev wrote: ... Forgot to mention: this is debian kernel 6.6.13-1~bpo12+1. I tried that one after observing exactly the same issue on regular bookworm 6.1.0 kernels. /mjt |
15 messages has been excluded from this view by a project administrator.
