Skip to contentSkip to conversation

mfd: intel-lpss: Fix Intel Cannon Lake LPSS I2C input clock

PATCH Applied Jarkko Nikula 5 participants 5 messages May 18, 2018 – Jun 4, 2018

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mfd: intel-lpss: Fix Intel Cannon Lake LPSS I2C input clock

Intel Cannon Lake PCH has much higher 216 MHz input clock to LPSS I2C than Sunrisepoint which uses 120 MHz. Preliminary information was that both share the same clock rate but actual silicon implements elevated rate for better support for 3.4 MHz high-speed I2C.

This incorrect input clock rate results too high I2C bus clock in case ACPI doesn't provide tuned I2C timing parameters since I2C host controller driver calculates them from input clock rate.

Fix this by using the correct rate. We still share the same 230 ns SDA hold time value than Sunrisepoint.

Fixes b418bbff36dd ("mfd: intel-lpss: Add Intel Cannonlake PCI IDs")

---
Hi Jian-Hong, Chris and Daniel. Could you test does this fix your touchpad issue?

---
 drivers/mfd/intel-lpss-pci.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
drivers/mfd/intel-lpss-pci.c +15 -10
index d1c46de89eb4..d9ae983095c5 100644
@@ -124,6 +124,11 @@ static const struct intel_lpss_platform_info apl_i2c_info = {
 	.properties = apl_i2c_properties,
 };
 
+static const struct intel_lpss_platform_info cnl_i2c_info = {
+	.clk_rate = 216000000,
+	.properties = spt_i2c_properties,
+};
+
 static const struct pci_device_id intel_lpss_pci_ids[] = {
 	/* BXT A-Step */
 	{ PCI_VDEVICE(INTEL, 0x0aac), (kernel_ulong_t)&bxt_i2c_info },
@@ -207,13 +212,13 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x9daa), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0x9dab), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0x9dfb), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9dc5), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0x9dc6), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9dc5), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9dc6), (kernel_ulong_t)&cnl_i2c_info },
 	{ PCI_VDEVICE(INTEL, 0x9dc7), (kernel_ulong_t)&spt_uart_info },
-	{ PCI_VDEVICE(INTEL, 0x9de8), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0x9de9), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0x9dea), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0x9deb), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9de8), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9de9), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9dea), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9deb), (kernel_ulong_t)&cnl_i2c_info },
 	/* SPT-H */
 	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
 	{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
@@ -240,10 +245,10 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0xa32b), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0xa37b), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0xa347), (kernel_ulong_t)&spt_uart_info },
-	{ PCI_VDEVICE(INTEL, 0xa368), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0xa369), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0xa36a), (kernel_ulong_t)&spt_i2c_info },
-	{ PCI_VDEVICE(INTEL, 0xa36b), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0xa368), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0xa369), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0xa36a), (kernel_ulong_t)&cnl_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0xa36b), (kernel_ulong_t)&cnl_i2c_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
Signature · 1 line
2.17.0
Acked-by on v1 1/1
#1 lore raw
Quoted 12 lines · Jarkko Nikula

Intel Cannon Lake PCH has much higher 216 MHz input clock to LPSS I2C than Sunrisepoint which uses 120 MHz. Preliminary information was that both share the same clock rate but actual silicon implements elevated rate for better support for 3.4 MHz high-speed I2C.

This incorrect input clock rate results too high I2C bus clock in case ACPI doesn't provide tuned I2C timing parameters since I2C host controller driver calculates them from input clock rate.

Fix this by using the correct rate. We still share the same 230 ns SDA hold time value than Sunrisepoint.

Quoted 6 lines
Fixes b418bbff36dd ("mfd: intel-lpss: Add Intel Cannonlake PCI IDs")
Tested-by
#2 lore raw

Hi Jarkko,

We have tried this patch on the two laptops we have now:

ASUS X580GD

dev@endless:~$ lscpu | grep "Model name"
Model name:          Intel(R) Core(TM) i5-8300H CPU @ 2.30GHz
dev@endless:~$ lspci | grep -E '(a36[89ab]|97c[56])'
00:15.0 Serial bus controller [0c80]: Intel Corporation Device a368 (rev 10)
00:15.1 Serial bus controller [0c80]: Intel Corporation Device a369 (rev 10)
dev@endless:~$ dmesg | grep -E 'lpss|i2c'
[    9.692511] intel-lpss 0000:00:15.0: enabling device (0000 -> 0002)
[    9.697702] intel-lpss 0000:00:15.1: enabling device (0000 -> 0002)
[    9.920034] input: ELAN1200:00 04F3:303E Touchpad as
/devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-8/i2c-ELAN1200:00/0018:04F3:303E.0001/input/input18
[    9.920204] hid-multitouch 0018:04F3:303E.0001: input,hidraw0: I2C
HID v1.00 Mouse [ELAN1200:00 04F3:303E] on i2c-ELAN1200:00
[    9.923873] intel-lpss 0000:00:1e.0: enabling device (0000 -> 0002)
[    9.924806] intel-lpss 0000:00:1e.2: enabling device (0000 -> 0002)

ASUS UX550GE

dev@endless:~$ lscpu | grep "Model name"
Model name:          Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
dev@endless:~$ lspci | grep -E '(a36[89ab]|97c[56])'
00:15.0 Serial bus controller [0c80]: Intel Corporation Device a368 (rev 10)
00:15.1 Serial bus controller [0c80]: Intel Corporation Device a369 (rev 10)
dev@endless:~$ dmesg | grep -E 'lpss|i2c'
[    6.926801] intel-lpss 0000:00:15.0: enabling device (0000 -> 0002)
[    6.940907] intel-lpss 0000:00:15.1: enabling device (0000 -> 0002)
[    6.971915] input: FTE1200:00 0B05:0201 Touchpad as
/devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-9/i2c-FTE1200:00/0018:0B05:0201.0002/input/input21
[    6.971961] hid-multitouch 0018:0B05:0201.0002: input,hidraw1: I2C
HID v1.00 Mouse [FTE1200:00 0B05:0201] on i2c-FTE1200:00
[    6.973930] intel-lpss 0000:00:1e.0: enabling device (0000 -> 0002)
[    6.974700] intel-lpss 0000:00:1e.2: enabling device (0000 -> 0002)

The patch works on both of the laptops with the touchpads.

2018-05-18 18:46 GMT+08:00 Mika Westerberg <[email protected]>:

Quoted 13 lines · Jarkko Nikula
Quoted 12 lines

Intel Cannon Lake PCH has much higher 216 MHz input clock to LPSS I2C than Sunrisepoint which uses 120 MHz. Preliminary information was that both share the same clock rate but actual silicon implements elevated rate for better support for 3.4 MHz high-speed I2C.

This incorrect input clock rate results too high I2C bus clock in case ACPI doesn't provide tuned I2C timing parameters since I2C host controller driver calculates them from input clock rate.

Fix this by using the correct rate. We still share the same 230 ns SDA hold time value than Sunrisepoint.

Quoted 8 lines
Quoted 6 lines
Fixes b418bbff36dd ("mfd: intel-lpss: Add Intel Cannonlake PCI IDs")
reviewer Reviewed-by on v1 1/1
#3 lore raw
In reply to Jarkko Nikula
Quoted 5 lines · Jarkko Nikula

Intel Cannon Lake PCH has much higher 216 MHz input clock to LPSS I2C than Sunrisepoint which uses 120 MHz. Preliminary information was that both share the same clock rate but actual silicon implements elevated rate for better support for 3.4 MHz high-speed I2C.

Quoted 7 lines

This incorrect input clock rate results too high I2C bus clock in case ACPI doesn't provide tuned I2C timing parameters since I2C host controller driver calculates them from input clock rate.

Fix this by using the correct rate. We still share the same 230 ns SDA hold time value than Sunrisepoint.

P.S. Documentation we have is not perfect, that's why previously I though about Broxton/Cannonlake case as single, which is wrong, they are different in terms of i2c clock organization.

Quoted 93 lines
Fixes b418bbff36dd ("mfd: intel-lpss: Add Intel Cannonlake PCI IDs")

---
Hi Jian-Hong, Chris and Daniel. Could you test does this fix your touchpad issue?

---
 drivers/mfd/intel-lpss-pci.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
drivers/mfd/intel-lpss- renamed from drivers/mfd/intel-lpss-pci.c +0 -0

pci.c
index d1c46de89eb4..d9ae983095c5 100644

drivers/mfd/intel-lpss-pci.c +5 -0
@@ -124,6 +124,11 @@ static const struct intel_lpss_platform_info
 apl_i2c_info = {
 	.properties = apl_i2c_properties,
 };
 
+static const struct intel_lpss_platform_info cnl_i2c_info = {
+	.clk_rate = 216000000,
+	.properties = spt_i2c_properties,
+};
+
 static const struct pci_device_id intel_lpss_pci_ids[] = {
 	/* BXT A-Step */

{ PCI_VDEVICE(INTEL, 0x0aac), (kernel_ulong_t)&bxt_i2c_info
},

drivers/mfd/intel-lpss-pci.c +2 -3
@@ -207,13 +212,13 @@ static const struct pci_device_id
 intel_lpss_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x9daa), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0x9dab), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0x9dfb), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9dc5), (kernel_ulong_t)&spt_i2c_info
 },
-	{ PCI_VDEVICE(INTEL, 0x9dc6), (kernel_ulong_t)&spt_i2c_info
 },
+	{ PCI_VDEVICE(INTEL, 0x9dc5), (kernel_ulong_t)&cnl_i2c_info
 },
+	{ PCI_VDEVICE(INTEL, 0x9dc6), (kernel_ulong_t)&cnl_i2c_info
 },
 	{ PCI_VDEVICE(INTEL, 0x9dc7), (kernel_ulong_t)&spt_uart_info
 },
-	{ PCI_VDEVICE(INTEL, 0x9de8), (kernel_ulong_t)&spt_i2c_info

},

drivers/mfd/intel-lpss-pci.c +0 -1
-	{ PCI_VDEVICE(INTEL, 0x9de9), (kernel_ulong_t)&spt_i2c_info

},

drivers/mfd/intel-lpss-pci.c +0 -1
-	{ PCI_VDEVICE(INTEL, 0x9dea), (kernel_ulong_t)&spt_i2c_info

},

drivers/mfd/intel-lpss-pci.c +0 -1
-	{ PCI_VDEVICE(INTEL, 0x9deb), (kernel_ulong_t)&spt_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0x9de8), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0x9de9), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0x9dea), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0x9deb), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +0 -0
 	/* SPT-H */
 	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info

},
{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info
},

drivers/mfd/intel-lpss-pci.c +0 -3
@@ -240,10 +245,10 @@ static const struct pci_device_id
 intel_lpss_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0xa32b), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0xa37b), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0xa347), (kernel_ulong_t)&spt_uart_info
 },
-	{ PCI_VDEVICE(INTEL, 0xa368), (kernel_ulong_t)&spt_i2c_info
 },
-	{ PCI_VDEVICE(INTEL, 0xa369), (kernel_ulong_t)&spt_i2c_info
 },
-	{ PCI_VDEVICE(INTEL, 0xa36a), (kernel_ulong_t)&spt_i2c_info

},

drivers/mfd/intel-lpss-pci.c +0 -1
-	{ PCI_VDEVICE(INTEL, 0xa36b), (kernel_ulong_t)&spt_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0xa368), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0xa369), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0xa36a), (kernel_ulong_t)&cnl_i2c_info

},

drivers/mfd/intel-lpss-pci.c +1 -0
+	{ PCI_VDEVICE(INTEL, 0xa36b), (kernel_ulong_t)&cnl_i2c_info

},
{ }
};
MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
Applied
#4 lore raw
In reply to Jarkko Nikula
Quoted 18 lines · Jarkko Nikula

Intel Cannon Lake PCH has much higher 216 MHz input clock to LPSS I2C than Sunrisepoint which uses 120 MHz. Preliminary information was that both share the same clock rate but actual silicon implements elevated rate for better support for 3.4 MHz high-speed I2C.

This incorrect input clock rate results too high I2C bus clock in case ACPI doesn't provide tuned I2C timing parameters since I2C host controller driver calculates them from input clock rate.

Fix this by using the correct rate. We still share the same 230 ns SDA hold time value than Sunrisepoint.

Fixes b418bbff36dd ("mfd: intel-lpss: Add Intel Cannonlake PCI IDs")
Quoted 6 lines

---
Hi Jian-Hong, Chris and Daniel. Could you test does this fix your touchpad issue?

---
 drivers/mfd/intel-lpss-pci.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

Applied, thanks.

Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog