| Message ID | 20250103051657.211966-1-rha051117@gmail.com |
|---|---|
| Headers | show |
| Series | Force I2C bus freq to 100KHz for ELAN06FA touchpad | expand |
Hello, Thanks for reading my patch! On Fri, Jan 3, 2025 at 3:33 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > What are those "some devices" and "some controllers"? The "Some Devices" are the Lenovo V15 G4 IRU, which I use, and potentially the Lenovo V15 G4 AMN and Lenovo Ideapad Slim 3 15IAH8 as well (based on issue reports from other users [1]). The "Some Controllers" are the Designware I2C controller. Sorry for not putting this in the commit message; I had tried to follow the comments for the quirk I copied in Commit 7574c0db2e68c ("i2c: acpi: Force bus speed to 400KHz if a Silead touchscreen is present"), which left them out. On Fri, Jan 3, 2025 at 3:33 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > Can you add the ACPI table snippet here too for reference? I believe this is the correct snippet in my ACPI table (Again, V15 G4 IRU). Tried to edit it down as much as I could, hopefully this tells everything. Please let me know how I should attach a longer snippet or the full ACPI table if needed. Scope (_SB.PC00.I2C1) { [...] Device (TPD0) { [...] CreateWordField (SBFB, \_SB.PC00.I2C1.TPD0._Y53._ADR, BADR) // _ADR: Address CreateDWordField (SBFB, \_SB.PC00.I2C1.TPD0._Y53._SPE, SPED) // _SPE: Speed CreateWordField (SBFG, 0x17, INT1) CreateDWordField (SBFI, \_SB.PC00.I2C1.TPD0._Y54._INT, INT2) // _INT: Interrupts Method (_INI, 0, NotSerialized) // _INI: Initialize { If ((OSYS < 0x07DC)) { SRXO (0x09080011, One) } INT1 = GNUM (0x09080011) INT2 = INUM (0x09080011) If ((TPTY == One)) { _HID = "ELAN06FA" _SUB = "ELAN0001" BADR = 0x15 HID2 = One Return (Zero) } [...] } Name (_HID, "XXXX0000") // _HID: Hardware ID Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // _CID: Compatible ID Name (_SUB, "XXXX0000") // _SUB: Subsystem ID Name (_S0W, 0x03) // _S0W: S0 Device Wake State Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { If ((Arg0 == HIDG)) { Return (HIDD (Arg0, Arg1, Arg2, Arg3, HID2)) } If ((Arg0 == TP7G)) { Return (TP7D (Arg0, Arg1, Arg2, Arg3, SBFB, SBFG)) } Return (Buffer (One) { 0x00 // . }) } [...] Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { If ((OSYS < 0x07DC)) { Return (SBFI) /* \_SB_.PC00.I2C1.TPD0.SBFI */ } If ((TPDM == Zero)) { Return (ConcatenateResTemplate (I2CM (I2CX, BADR, SPED), SBFG)) } Return (ConcatenateResTemplate (I2CM (I2CX, BADR, SPED), SBFI)) } [...] } } For comparison, the properties for a device that I think did set a proper speed was like this: If ((TPNP == 0xD64D)) { _HID = "GTCH7503" HID2 = One BADR = 0x10 SPED = 0x000F4240 Return (Zero) } [1]: https://bbs.archlinux.org/viewtopic.php?id=297092
On Fri, Jan 03, 2025 at 05:46:27PM -0600, R Ha wrote: > Hello, > > Thanks for reading my patch! > > On Fri, Jan 3, 2025 at 3:33 AM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > What are those "some devices" and "some controllers"? > > The "Some Devices" are the Lenovo V15 G4 IRU, which I use, and > potentially the Lenovo V15 G4 AMN and Lenovo Ideapad Slim 3 15IAH8 as > well (based on issue reports from other users [1]). > The "Some Controllers" are the Designware I2C controller. > > Sorry for not putting this in the commit message; I had tried to > follow the comments for the quirk I copied in Commit 7574c0db2e68c > ("i2c: acpi: Force bus speed to 400KHz if a Silead touchscreen is > present"), which left them out. In general it is good to follow the existing changelogs but in this case I would prefer to add the details of the system in question (so we know what systems the quirk is applied to). > On Fri, Jan 3, 2025 at 3:33 AM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > Can you add the ACPI table snippet here too for reference? > > I believe this is the correct snippet in my ACPI table (Again, V15 G4 > IRU). Tried to edit it down as much as I could, hopefully this tells > everything. Please let me know how I should attach a longer snippet or > the full ACPI table if needed. Okay thanks for sharing. I don't see the "SPED" beeing assigned in the below snipped though. I would expect this works in Windows? Have you checked if it uses 100 kHz or 400kHz there? > Scope (_SB.PC00.I2C1) > { > [...] > Device (TPD0) > { > [...] > CreateWordField (SBFB, \_SB.PC00.I2C1.TPD0._Y53._ADR, BADR) > // _ADR: Address > CreateDWordField (SBFB, \_SB.PC00.I2C1.TPD0._Y53._SPE, SPED) > // _SPE: Speed > CreateWordField (SBFG, 0x17, INT1) > CreateDWordField (SBFI, \_SB.PC00.I2C1.TPD0._Y54._INT, INT2) > // _INT: Interrupts > Method (_INI, 0, NotSerialized) // _INI: Initialize > { > If ((OSYS < 0x07DC)) > { > SRXO (0x09080011, One) > } > > INT1 = GNUM (0x09080011) > INT2 = INUM (0x09080011) > If ((TPTY == One)) > { > _HID = "ELAN06FA" > _SUB = "ELAN0001" > BADR = 0x15 > HID2 = One > Return (Zero) > } > [...] > } > > Name (_HID, "XXXX0000") // _HID: Hardware ID > Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // > _CID: Compatible ID > Name (_SUB, "XXXX0000") // _SUB: Subsystem ID > Name (_S0W, 0x03) // _S0W: S0 Device Wake State > Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method > { > If ((Arg0 == HIDG)) > { > Return (HIDD (Arg0, Arg1, Arg2, Arg3, HID2)) > } > > If ((Arg0 == TP7G)) > { > Return (TP7D (Arg0, Arg1, Arg2, Arg3, SBFB, SBFG)) > } > > Return (Buffer (One) > { > 0x00 // . > }) > } > [...] > Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings > { > If ((OSYS < 0x07DC)) > { > Return (SBFI) /* \_SB_.PC00.I2C1.TPD0.SBFI */ > } > > If ((TPDM == Zero)) > { > Return (ConcatenateResTemplate (I2CM (I2CX, BADR, SPED), SBFG)) > } > > Return (ConcatenateResTemplate (I2CM (I2CX, BADR, SPED), SBFI)) > } > [...] > } > } > > For comparison, the properties for a device that I think did set a > proper speed was like this: > If ((TPNP == 0xD64D)) > { > _HID = "GTCH7503" > HID2 = One > BADR = 0x10 > SPED = 0x000F4240 > Return (Zero) > } > > [1]: https://bbs.archlinux.org/viewtopic.php?id=297092
On Sun, Jan 5, 2025 at 2:34 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > In general it is good to follow the existing changelogs but in this case I > would prefer to add the details of the system in question (so we know what > systems the quirk is applied to). Alright, I sent an updated patch with a commit message that specifies the devices affected. On Sun, Jan 5, 2025 at 2:34 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > Okay thanks for sharing. I don't see the "SPED" beeing assigned in the > below snipped though. I believe "SPED" is left unassigned. There are two reasons for this. 1. I could not find a place where it was assigned in the ACPI table (in the snippet, every line with the word "SPED" was already included). 2. In the file drivers/i2c/busses/i2c-designware-common.c, the code in the function "i2c_dw_adjust_bus_speed" falls through to the "else" case. For (2), here is the relevant function where the control flow falls to the "else" case. I found this by adding a print-debugging statement after the last "else" statement. static void i2c_dw_adjust_bus_speed(struct dw_i2c_dev *dev) { u32 acpi_speed = i2c_dw_acpi_round_bus_speed(dev->dev); struct i2c_timings *t = &dev->timings; /* * Find bus speed from the "clock-frequency" device property, ACPI * or by using fast mode if neither is set. */ if (acpi_speed && t->bus_freq_hz) t->bus_freq_hz = min(t->bus_freq_hz, acpi_speed); else if (acpi_speed || t->bus_freq_hz) t->bus_freq_hz = max(t->bus_freq_hz, acpi_speed); else t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ; } Actually, after some further investigation, I found that I missed a few lines in my previous snippet. Specifically the line concerning the method "I2CSerialBusV2". Here is the full snippet pasted below since I don't want to miss anything else, I'm sorry for the length but want to make sure everything is included. Scope (_SB.PC00.I2C1) { Name (I2CN, Zero) Name (I2CX, Zero) Name (I2CI, One) Method (_INI, 0, NotSerialized) // _INI: Initialize { I2CN = SDS1 /* \SDS1 */ I2CX = One } Device (TPD0) { Name (HID2, Zero) Name (SBFB, ResourceTemplate () { I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\\_SB.PC00.I2C1", 0x00, ResourceConsumer, _Y53, Exclusive, ) }) Name (SBFG, ResourceTemplate () { GpioInt (Level, ActiveLow, ExclusiveAndWake, PullDefault, 0x0000, "\\_SB.GPI0", 0x00, ResourceConsumer, , ) { // Pin list 0x0000 } }) Name (SBFI, ResourceTemplate () { Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, _Y54) { 0x00000000, } }) CreateWordField (SBFB, \_SB.PC00.I2C1.TPD0._Y53._ADR, BADR) // _ADR: Address CreateDWordField (SBFB, \_SB.PC00.I2C1.TPD0._Y53._SPE, SPED) // _SPE: Speed CreateWordField (SBFG, 0x17, INT1) CreateDWordField (SBFI, \_SB.PC00.I2C1.TPD0._Y54._INT, INT2) // _INT: Interrupts Method (_INI, 0, NotSerialized) // _INI: Initialize { If ((OSYS < 0x07DC)) { SRXO (0x09080011, One) } INT1 = GNUM (0x09080011) INT2 = INUM (0x09080011) If ((TPTY == One)) { _HID = "ELAN06FA" _SUB = "ELAN0001" BADR = 0x15 HID2 = One Return (Zero) } If ((TPTY == 0x02)) { _HID = "SYNA2BA6" _SUB = "SYNA0001" BADR = 0x2C HID2 = 0x20 Return (Zero) } If ((TPTY == 0x04)) { _HID = "CRQ1080" _SUB = "CRQ0001" BADR = 0x2C HID2 = 0x20 Return (Zero) } If ((TPTY == 0x05)) { _HID = "FTCS0038" _SUB = "FTCS0001" BADR = 0x38 HID2 = One Return (Zero) } } Name (_HID, "XXXX0000") // _HID: Hardware ID Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // _CID: Compatible ID Name (_SUB, "XXXX0000") // _SUB: Subsystem ID Name (_S0W, 0x03) // _S0W: S0 Device Wake State Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { If ((Arg0 == HIDG)) { Return (HIDD (Arg0, Arg1, Arg2, Arg3, HID2)) } If ((Arg0 == TP7G)) { Return (TP7D (Arg0, Arg1, Arg2, Arg3, SBFB, SBFG)) } Return (Buffer (One) { 0x00 // . }) } Method (_STA, 0, NotSerialized) // _STA: Status { If ((TPTY == Zero)) { Return (Zero) } Else { Return (0x0F) } } Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { If ((OSYS < 0x07DC)) { Return (SBFI) /* \_SB_.PC00.I2C1.TPD0.SBFI */ } If ((TPDM == Zero)) { Return (ConcatenateResTemplate (I2CM (I2CX, BADR, SPED), SBFG)) } Return (ConcatenateResTemplate (I2CM (I2CX, BADR, SPED), SBFI)) } Method (TPRD, 0, Serialized) { Return (^^^LPCB.EC0.ECTP) /* \_SB_.PC00.LPCB.EC0_.ECTP */ } Method (TPWR, 1, Serialized) { ^^^LPCB.EC0.ECTP = Arg0 } } }
Hi, On Mon, Jan 06, 2025 at 03:00:53AM -0600, R Ha wrote: > On Sun, Jan 5, 2025 at 2:34 AM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > In general it is good to follow the existing changelogs but in this case I > > would prefer to add the details of the system in question (so we know what > > systems the quirk is applied to). > > Alright, I sent an updated patch with a commit message that specifies > the devices affected. > > On Sun, Jan 5, 2025 at 2:34 AM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > Okay thanks for sharing. I don't see the "SPED" beeing assigned in the > > below snipped though. > > I believe "SPED" is left unassigned. There are two reasons for this. > 1. I could not find a place where it was assigned in the ACPI table > (in the snippet, every line with the word "SPED" was already > included). > 2. In the file drivers/i2c/busses/i2c-designware-common.c, the code in > the function "i2c_dw_adjust_bus_speed" falls through to the "else" > case. > > For (2), here is the relevant function where the control flow falls to > the "else" case. I found this by adding a print-debugging statement > after the last "else" statement. > static void i2c_dw_adjust_bus_speed(struct dw_i2c_dev *dev) > { > u32 acpi_speed = i2c_dw_acpi_round_bus_speed(dev->dev); > struct i2c_timings *t = &dev->timings; > > /* > * Find bus speed from the "clock-frequency" device property, ACPI > * or by using fast mode if neither is set. > */ > if (acpi_speed && t->bus_freq_hz) > t->bus_freq_hz = min(t->bus_freq_hz, acpi_speed); > else if (acpi_speed || t->bus_freq_hz) > t->bus_freq_hz = max(t->bus_freq_hz, acpi_speed); > else > t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ; > } > > Actually, after some further investigation, I found that I missed a > few lines in my previous snippet. Specifically the line concerning the > method "I2CSerialBusV2". > Here is the full snippet pasted below since I don't want to miss > anything else, I'm sorry for the length but want to make sure > everything is included. Thanks! Okay the speed set in the I2CSerialBusV2 resource is 400kHZ but there is one more variable in this equation: \\_SB.PC00.I2C1 that's the I2C controller itself. DW I2C has some timing related methods (HCNT/LCNT) that may affect this so I wonder if you can share that one too? > Scope (_SB.PC00.I2C1) > { > Name (I2CN, Zero) > Name (I2CX, Zero) > Name (I2CI, One) > Method (_INI, 0, NotSerialized) // _INI: Initialize > { > I2CN = SDS1 /* \SDS1 */ > I2CX = One > } > > Device (TPD0) > { > Name (HID2, Zero) > Name (SBFB, ResourceTemplate () > { > I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.PC00.I2C1", > 0x00, ResourceConsumer, _Y53, Exclusive, > ) > })
Hi, On Tue, Jan 07, 2025 at 06:16:09AM -0600, R Ha wrote: > Hi, > > Thanks for clarifying the speed. Seems like this bug is different than > I thought. > > In my ACPI table there were no references to HCNT or LCNT > specifically. I'm not sure if this is normal. > > In addition, I noticed that there were debug messages in dmesg > relating to the HCNT and LCNT. > I'm not sure if they'll be useful, but here they are (taken from an > unmodified kernel): > [ 3.543648] i2c i2c-14: Successfully instantiated SPD at 0x50 > [ 3.543790] Standard Mode HCNT:LCNT = 552:652 > [ 3.543794] Fast Mode HCNT:LCNT = 100:200 I'm adding Jarkko who is expert in the designware I2C maybe he has some ideas. IIRC we expect the HCNT/LCNT values to be passed from ACPI tables to the driver. @Jarkko, it seems that standard I2C HID touchpad does not work properly with the 400 kHz as passed in I2CSerialBusV2() resource but it works with either 100 kHz and 1 MHz. It also works fine in Windows. To me it sounds like that we may have wrong/missing HCNT/LCNT values? > Here's what I have found with the string "\\_SB.PC00.I2C1" in my ACPI table: > #1 There should be Device() node for that too. The ones you listed are just child devices connected to that bus. > Scope (_SB) > { > Device (PEPD) > { > Name (_HID, "INT33A1" /* Intel Power Engine */) // _HID: Hardware ID > Name (_CID, EisaId ("PNP0D80") /* Windows-compatible System > Power Management Controller */) // _CID: Compatible ID > Name (_UID, One) // _UID: Unique ID > Name (LBUF, Buffer (0xC0){}) > Name (PPD0, Package (0x03) > { > "\\_SB.PC00.SAT0", > Zero, > Package (0x02) > { > Zero, > Package (0x03) > { > 0xFF, > Zero, > 0x81 > } > } > }) > Name (PPD3, Package (0x03) > { > "\\_SB.PC00.SAT0", > Zero, > Package (0x02) > { > Zero, > Package (0x02) > { > 0xFF, > 0x03 > } > } > }) > Name (WWD3, Package (0x03) > { > "\\_SB.PC00.RP04", > Zero, > Package (0x02) > { > Zero, > Package (0x02) > { > 0xFF, > 0x03 > } > } > }) > Name (PKD0, Package (0x02) > { > Zero, > Package (0x03) > { > 0xFF, > Zero, > 0x81 > } > }) > Name (PKD3, Package (0x02) > { > Zero, > Package (0x02) > { > 0xFF, > 0x03 > } > }) > Name (DEVY, Package (0x77) > { > [...] > Package (0x03) > { > "\\_SB.PC00.I2C0", > One, > Package (0x02) > { > Zero, > Package (0x02) > { > 0xFF, > 0x03 > } > } > }, > > Package (0x03) > { > "\\_SB.PC00.I2C1", > One, > Package (0x02) > { > Zero, > Package (0x02) > { > 0xFF, > 0x03 > } > } > }, > > Package (0x03) > { > "\\_SB.PC00.XHCI", > One, > Package (0x02) > { > Zero, > Package (0x02) > { > 0xFF, > 0x03 > } > } > }, > > Package (0x03) > { > "\\_SB.PC00.HDAS", > One, > Package (0x02) > { > Zero, > Package (0x03) > { > 0xFF, > Zero, > 0x81 > } > } > }, > [...The rest are similar, only changinng the strings] > }) > } > } > > #2 (seems related to another device) > Scope (_SB.PC00.I2C1) > { > Device (PA06) > { > Name (_HID, "MCHP1930") // _HID: Hardware ID > Name (_UID, "I2C1&PA06") // _UID: Unique ID > Name (_S0W, 0x03) // _S0W: S0 Device Wake State > Method (_STA, 0, Serialized) // _STA: Status > { > If (POME) > { > Switch (ToInteger (PLID)) > { > Case (Package (0x01) > { > 0x0C > } > > ) > { > Return (0x0F) > } > Default > { > Return (Zero) > } > > } > } > > Return (Zero) > } > > Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings > { > Name (RBUF, ResourceTemplate () > { > I2cSerialBusV2 (0x0000, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.PC00.I2C1", > 0x00, ResourceConsumer, _Y3A, Exclusive, > ) > }) > CreateWordField (RBUF, \_SB.PC00.I2C1.PA06._CRS._Y3A._ADR, > BADR) // _ADR: Address > Switch (ToInteger (PLID)) > { > Case (Package (0x01) > { > 0x0C > } > > ) > { > BADR = 0x17 > } > Default > { > BADR = Zero > } > > } > > Return (RBUF) /* \_SB_.PC00.I2C1.PA06._CRS.RBUF */ > } > > Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method > { > If ((Arg0 != ToUUID > ("033771e0-1705-47b4-9535-d1bbe14d9a09") /* Unknown UUID */)) > { > Return (Buffer (One) > { > 0x00 // . > }) > } > > Switch (ToInteger (Arg2)) > { > Case (Zero) > { > Switch (ToInteger (Arg1)) > { > Case (Zero) > { > Return (Buffer (One) > { > 0x03 > // . > }) > } > Case (One) > { > Return (Buffer (One) > { > 0x7F > // . > }) > } > > } > > Break > } > Case (One) > { > Name (PKG1, Package (0x02) > { > Package (0x08) > { > "", > Zero, > "", > Zero, > "", > Zero, > "", > Zero > }, > > Package (0x08) > { > "", > Zero, > "VBAT_IN_ELPMIC", > 0x32, > "V3P3DX_EDP", > 0x0A, > "VCC_EDP_BKLT", > 0x32 > } > }) > Switch (ToInteger (PLID)) > { > Case (Package (0x01) > { > 0x0C > } > > ) > { > Return (DerefOf (PKG1 [One])) > } > Default > { > Return (DerefOf (PKG1 [Zero])) > } > > } > } > Case (0x02) > { > If ((Arg1 < One)) > { > Break > } > > Name (PKG2, Package (0x02) > { > Package (0x04) > { > Zero, > Zero, > Zero, > Zero > }, > > Package (0x04) > { > Zero, > 0xC350, > 0x2710, > 0xC350 > } > }) > Switch (ToInteger (PLID)) > { > Case (Package (0x01) > { > 0x0C > } > > ) > { > Return (DerefOf (PKG2 [One])) > } > Default > { > Return (DerefOf (PKG2 [Zero])) > } > > } > } > Case (0x03) > { > If ((Arg1 < One)) > { > Break > } > > Name (BUF3, Package (0x01) > { > 0x0F > }) > Return (BUF3) /* \_SB_.PC00.I2C1.PA06._DSM.BUF3 */ > } > Case (0x04) > { > If ((Arg1 < One)) > { > Break > } > > Name (BUF4, Package (0x01) > { > Zero > }) > Return (BUF4) /* \_SB_.PC00.I2C1.PA06._DSM.BUF4 */ > } > Case (0x05) > { > If ((Arg1 < One)) > { > Break > } > > Name (BUF5, Package (0x02) > { > 0x0400, > 0x08 > }) > Return (BUF5) /* \_SB_.PC00.I2C1.PA06._DSM.BUF5 */ > } > Case (0x06) > { > If ((Arg1 < One)) > { > Break > } > > Name (BUF6, Package (0x01) > { > 0x0384 > }) > Return (BUF6) /* \_SB_.PC00.I2C1.PA06._DSM.BUF6 */ > } > > } > > Return (Buffer (One) > { > 0x00 // . > }) > } > } > } > > #3 (also seems related to another device) > ElseIf ((I2SB == One)) > { > Scope (_SB.PC00.I2C1) > { > Device (HDAC) > { > Name (_HID, "INT00000") // _HID: Hardware ID > Name (_DDN, "Intel(R) Smart Sound Technology Audio Codec") > // _DDN: DOS Device Name > Name (_UID, One) // _UID: Unique ID > Name (CADR, Zero) > Name (CDIS, Zero) > Method (_INI, 0, NotSerialized) // _INI: Initialize > { > If ((I2SC == One)) > { > _HID = "INT34C2" > _CID = "INT34C2" > CADR = 0x1C > } > ElseIf ((I2SC == 0x02)) > { > _HID = "10EC1308" > _CID = "10EC1308" > CADR = 0x10 > } > ElseIf ((I2SC == 0x03)) > { > _HID = "ESSX8326" > _CID = "ESSX8326" > _DDN = "ESSX Codec Controller 8326 " > } > } > > Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings > { > If ((I2SC == 0x03)) > { > Name (SBFB, ResourceTemplate () > { > I2cSerialBusV2 (0x0018, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.PC00.I2C0", > 0x00, ResourceConsumer, , Exclusive, > ) > I2cSerialBusV2 (0x0009, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.PC00.I2C0", > 0x00, ResourceConsumer, , Exclusive, > ) > }) > Name (PBUF, ResourceTemplate () > { > GpioIo (Exclusive, PullDefault, 0x0000, > 0x0000, IoRestrictionOutputOnly, > "\\_SB.GPI0", 0x00, ResourceConsumer, , > ) > { // Pin list > 0x0000 > } > }) > Name (SBFG, ResourceTemplate () > { > GpioInt (Edge, ActiveBoth, ExclusiveAndWake, > PullNone, 0x0000, > "\\_SB.GPI0", 0x00, ResourceConsumer, , > ) > { // Pin list > 0x0000 > } > }) > CreateWordField (PBUF, 0x17, PWRP) > PWRP = GNUM (0x09030006) > CreateWordField (SBFG, 0x17, INTP) > INTP = GNUM (0x09030007) > Return (ConcatenateResTemplate (SBFB, > ConcatenateResTemplate (PBUF, SBFG))) > } > Else > { > Return (ConcatenateResTemplate (IICB (CADR, I2SB), > INTB (I2SI, Zero, Zero))) > } > } > > Method (_STA, 0, NotSerialized) // _STA: Status > { > If (((I2SC != Zero) && (CDIS != One))) > { > Return (0x0F) > } > > If ((CDIS == One)) > { > Return (0x0D) > } > > Return (Zero) > } > > Method (_SRS, 1, Serialized) // _SRS: Set Resource Settings > { > CDIS = Zero > } > > Method (_DIS, 0, NotSerialized) // _DIS: Disable Device > { > CDIS = One > } > > Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method > { > If ((Arg0 == Buffer (0x10) > { > /* 0000 */ 0x04, 0x0C, 0x80, 0xA9, 0x16, > 0xE0, 0x3E, 0x34, // ......>4 > /* 0008 */ 0x41, 0xF4, 0x6B, 0xCC, 0xE7, > 0x0F, 0x43, 0x32 // A.k...C2 > })) > { > If ((Arg2 == Zero)) > { > Return (0x55) > } > > [...Rest are similar to above, for values of Arg2 > from 0 to DF] > } > > Return (0xFF) > } > } > } > }
Hello, On Tue, Jan 7, 2025 at 11:51 PM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > There should be Device() node for that too. The ones you listed are just > child devices connected to that bus. You're right, after searching for Device nodes I was able to find this snippet. But it doesn't seem to have anything related to HCNT/LCNT values either, but maybe they're hidden somewhere. Thanks for the tip again. Scope (_SB.PC00) { Scope (\_SB.PC00) { Method (SOD3, 3, Serialized) { OperationRegion (ICB1, SystemMemory, (GPCB () + Arg0), 0x88) If (Arg1) { Field (ICB1, ByteAcc, NoLock, Preserve) { Offset (0x84), PMEC, 8 } PMEC = 0x03 PMEC |= Zero } If ((Arg1 && Arg2)) { Field (ICB1, AnyAcc, NoLock, Preserve) { Offset (0x10), BAR0, 64 } BAR0 = Zero } } } Method (I2CH, 1, Serialized) { OperationRegion (ICB1, SystemMemory, Arg0, 0x20) Field (ICB1, AnyAcc, NoLock, Preserve) { Offset (0x10), BAR0, 64, BAR1, 64 } Name (BUF0, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00001000, // Address Length _Y2B) }) Name (BUF1, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00001000, // Address Length _Y2C) }) CreateDWordField (BUF0, \_SB.PC00.I2CH._Y2B._BAS, ADR0) // _BAS: Base Address CreateDWordField (BUF1, \_SB.PC00.I2CH._Y2C._BAS, ADR1) // _BAS: Base Address ADR0 = (BAR0 & 0xFFFFFFFFFFFFF000) ADR1 = (BAR1 & 0xFFFFFFFFFFFFF000) ConcatenateResTemplate (BUF0, BUF1, Local0) Return (Local0) } Device (I2C0) { If ((IM00 == 0x02)) { Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { Return (I2CH (IC00)) } Name (_STA, 0x08) // _STA: Status } If ((IM00 == One)) { Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { If (PCIC (Arg0)) { Return (PCID (Arg0, Arg1, Arg2, Arg3)) } Return (Buffer (One) { 0x00 // . }) } Method (_PS3, 0, NotSerialized) // _PS3: Power State 3 { SOD3 (IC00, One, One) } Method (_PS0, 0, NotSerialized) // _PS0: Power State 0 { } } If (((IM00 == One) || (IM00 == Zero))) { Method (_ADR, 0, NotSerialized) // _ADR: Address { Return (0x00150000) } } } [...I2C1-7 nodes removed] Method (SPIH, 1, Serialized) { OperationRegion (ICB1, SystemMemory, Arg0, 0x20) Field (ICB1, AnyAcc, NoLock, Preserve) { Offset (0x10), BAR0, 64, BAR1, 64 } Name (BUF0, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00001000, // Address Length _Y2D) }) Name (BUF1, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00001000, // Address Length _Y2E) }) CreateDWordField (BUF0, \_SB.PC00.SPIH._Y2D._BAS, ADR0) // _BAS: Base Address CreateDWordField (BUF1, \_SB.PC00.SPIH._Y2E._BAS, ADR1) // _BAS: Base Address ADR0 = (BAR0 & 0xFFFFFFFFFFFFF000) ADR1 = (BAR1 & 0xFFFFFFFFFFFFF000) ConcatenateResTemplate (BUF0, BUF1, Local0) Return (Local0) } [...SPI nodes removed] }
Hi, On Wed, Jan 08, 2025 at 03:29:34AM -0600, R Ha wrote: > On Tue, Jan 7, 2025 at 11:51 PM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > There should be Device() node for that too. The ones you listed are just > > child devices connected to that bus. > > You're right, after searching for Device nodes I was able to find this > snippet. But it doesn't seem to have anything related to HCNT/LCNT > values either, but maybe they're hidden somewhere. > Thanks for the tip again. Can you share the whole acpidump? It is easier for us to check the necessary descriptions directly from that.
Hi, Sounds like a good idea. I'm a little worried I'm missing something, so I think being able to check my earlier answers will help as well. I'm sending the entire output as attachments, but let me know if it's better to upload them somewhere and paste the link instead. Some of the ssdt* files are missing, but they're empty files so Gmail won't let me attach them. On Thu, Jan 9, 2025 at 5:19 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Wed, Jan 08, 2025 at 03:29:34AM -0600, R Ha wrote: > > On Tue, Jan 7, 2025 at 11:51 PM Mika Westerberg > > <mika.westerberg@linux.intel.com> wrote: > > > There should be Device() node for that too. The ones you listed are just > > > child devices connected to that bus. > > > > You're right, after searching for Device nodes I was able to find this > > snippet. But it doesn't seem to have anything related to HCNT/LCNT > > values either, but maybe they're hidden somewhere. > > Thanks for the tip again. > > Can you share the whole acpidump? It is easier for us to check the > necessary descriptions directly from that.
On 1/10/25 1:26 PM, Mika Westerberg wrote: > Hi, > > On Fri, Jan 10, 2025 at 02:31:26AM -0600, R Ha wrote: >> Hi, >> >> Sounds like a good idea. I'm a little worried I'm missing something, >> so I think being able to check my earlier answers will help as well. >> I'm sending the entire output as attachments, but let me know if it's >> better to upload them somewhere and paste the link instead. Some of >> the ssdt* files are missing, but they're empty files so Gmail won't >> let me attach them. > > Thanks for sharing! Okay checked now dsdt.dsl (the other files are not > relevant here) and what I can tell the device is supposed to be run at 400 > kHz. I suspect this is what Windows is doing as well, there is nothing that > indicates otherwise. > > And since this is a standard I2C HID device it should just work (as it does > not require any vendor specific driver even in Windows). > > Only thing I can think of that affects this is the LCNT/HCNT and SDA hold > values of the I2C designware controller (and maybe the input clock) but > there is nothing in the ACPI tables that set these so it could be that the > Windows driver uses different values for those and that explains why it > works better there. > > @Jarkko, do you have any input here? If we cannot figure a better way then > I don't see other option than to add this quirk. Unfortunately I don't have idea.
In my device (Lenovo V15 G4 IRU) it appears that, even with the latest BIOS update, the ACPI tables do not specify that the ELAN06FA touchpad only works properly at a 100KHz I2C frequency, with other frequencies causing excessive smoothing to be applied intermittently. On some busses (designware-i2c), the default frequency is 400KHz. This smoothing causes the touchpad to be unusable under Linux, and the support request I filed with Lenovo was ignored. After adding a device ID check to force the frequency to 100KHz for this touchpad, the excessive smoothing disappears. As a similar patch was previously accepted for the Silead MSSL1680 touchscreen in [1], I hope this patch can be added into the kernel as well. Additional Notes: 1. I speculate that this issue was caused by the touchpad firmware interpreting the higher clock frequency as noise, since the smoothing effect is similar to that caused by using a noisy third-party charger. 2. Based on the coil whine emitted by my laptop, it appears as though the driver is ran at 100KHz under Windows by default as well, so I believe this is the fix, not a workaround. 3. This fix should also apply to the Lenovo V15 G4 AMN and the Ideapad Slim 3 15IAH8, but I do not have the devices to confirm. [1]: Commit 7574c0db2e68c ("i2c: acpi: Force bus speed to 400KHz if a Silead touchscreen is present") Randolph Ha (1): Force ELAN06FA touchpad I2C bus freq to 100KHz drivers/i2c/i2c-core-acpi.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)