From 4a74e79b543c115bf2b5b7a4b29db139da20b90d Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 4 Oct 2022 20:27:15 +0100 Subject: [PATCH 1/6] i2c: microchip: pci1xxxx: Fix comparison of -EPERM against an unsigned variable The comparison of variable ret with -EPERM is always false because ret is an u8 type. Fix this by making ret an int. Cleans up clang warning: drivers/i2c/busses/i2c-mchp-pci1xxxx.c:714:10: warning: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare] Fixes: 361693697249 ("i2c: microchip: pci1xxxx: Add driver for I2C host controller in multifunction endpoint of pci1xxxx switch") Signed-off-by: Colin Ian King Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-mchp-pci1xxxx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-mchp-pci1xxxx.c b/drivers/i2c/busses/i2c-mchp-pci1xxxx.c index f5342201eb6b..09af75921147 100644 --- a/drivers/i2c/busses/i2c-mchp-pci1xxxx.c +++ b/drivers/i2c/busses/i2c-mchp-pci1xxxx.c @@ -708,7 +708,7 @@ static void pci1xxxx_i2c_init(struct pci1xxxx_i2c *i2c) void __iomem *p2 = i2c->i2c_base + SMBUS_STATUS_REG_OFF; void __iomem *p1 = i2c->i2c_base + SMB_GPR_REG; u8 regval; - u8 ret; + int ret; ret = set_sys_lock(i2c); if (ret == -EPERM) { From 8673b6d97a314c2e73352f4a34c1aa9b2730d7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matti=20Lehtim=C3=A4ki?= Date: Sun, 2 Oct 2022 15:28:54 +0300 Subject: [PATCH 2/6] dt-bindings: i2c: qcom,i2c-cci: Document MSM8226 compatible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MSM8226's Camera Control Interface has one master and 3 clocks. Signed-off-by: Matti Lehtimäki Reviewed-by: Krzysztof Kozlowski Signed-off-by: Wolfram Sang --- .../devicetree/bindings/i2c/qcom,i2c-cci.yaml | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml index e51a85848d6e..c0f9537a4bb1 100644 --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml @@ -13,6 +13,7 @@ maintainers: properties: compatible: enum: + - qcom,msm8226-cci - qcom,msm8916-cci - qcom,msm8974-cci - qcom,msm8996-cci @@ -27,11 +28,11 @@ properties: const: 0 clocks: - minItems: 4 + minItems: 3 maxItems: 6 clock-names: - minItems: 4 + minItems: 3 maxItems: 6 interrupts: @@ -78,11 +79,28 @@ allOf: compatible: contains: enum: + - qcom,msm8226-cci - qcom,msm8916-cci then: properties: i2c-bus@1: false + - if: + properties: + compatible: + contains: + enum: + - qcom,msm8226-cci + then: + properties: + clocks: + maxItems: 3 + clock-names: + items: + - const: camss_top_ahb + - const: cci_ahb + - const: cci + - if: properties: compatible: From 9ad16f9639646762455bf3ed1e6dfcc6ccc2c099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matti=20Lehtim=C3=A4ki?= Date: Sun, 2 Oct 2022 15:28:55 +0300 Subject: [PATCH 3/6] dt-bindings: i2c: qcom,i2c-cci: Document clocks for MSM8974 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses same clocks as MSM8226. Signed-off-by: Matti Lehtimäki Reviewed-by: Krzysztof Kozlowski Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml index c0f9537a4bb1..cf9f8fda595f 100644 --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml @@ -91,6 +91,7 @@ allOf: contains: enum: - qcom,msm8226-cci + - qcom,msm8974-cci then: properties: clocks: From d046bd1372a5c5448c7c7ba3383a4316fdb32a60 Mon Sep 17 00:00:00 2001 From: Rayyan Ansari Date: Sun, 2 Oct 2022 15:28:56 +0300 Subject: [PATCH 4/6] i2c: qcom-cci: Add MSM8226 compatible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a compatible for MSM8226's Camera Control Interface, which is similar to the one used on MSM8916. Signed-off-by: Rayyan Ansari Signed-off-by: Matti Lehtimäki Reviewed-by: Loic Poulain Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qcom-cci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c index ea48e6a9cfca..87739fb4388b 100644 --- a/drivers/i2c/busses/i2c-qcom-cci.c +++ b/drivers/i2c/busses/i2c-qcom-cci.c @@ -807,6 +807,7 @@ static const struct cci_data cci_v2_data = { }; static const struct of_device_id cci_dt_match[] = { + { .compatible = "qcom,msm8226-cci", .data = &cci_v1_data}, { .compatible = "qcom,msm8916-cci", .data = &cci_v1_data}, { .compatible = "qcom,msm8974-cci", .data = &cci_v1_5_data}, { .compatible = "qcom,msm8996-cci", .data = &cci_v2_data}, From 301c8f5c32c8fb79c67539bc23972dc3ef48024c Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Tue, 27 Sep 2022 16:56:44 +0300 Subject: [PATCH 5/6] i2c: designware: Fix handling of real but unexpected device interrupts Commit c7b79a752871 ("mfd: intel-lpss: Add Intel Alder Lake PCH-S PCI IDs") caused a regression on certain Gigabyte motherboards for Intel Alder Lake-S where system crashes to NULL pointer dereference in i2c_dw_xfer_msg() when system resumes from S3 sleep state ("deep"). I was able to debug the issue on Gigabyte Z690 AORUS ELITE and made following notes: - Issue happens when resuming from S3 but not when resuming from "s2idle" - PCI device 00:15.0 == i2c_designware.0 is already in D0 state when system enters into pci_pm_resume_noirq() while all other i2c_designware PCI devices are in D3. Devices were runtime suspended and in D3 prior entering into suspend - Interrupt comes after pci_pm_resume_noirq() when device interrupts are re-enabled - According to register dump the interrupt really comes from the i2c_designware.0. Controller is enabled, I2C target address register points to a one detectable I2C device address 0x60 and the DW_IC_RAW_INTR_STAT register START_DET, STOP_DET, ACTIVITY and TX_EMPTY bits are set indicating completed I2C transaction. My guess is that the firmware uses this controller to communicate with an on-board I2C device during resume but does not disable the controller before giving control to an operating system. I was told the UEFI update fixes this but never the less it revealed the driver is not ready to handle TX_EMPTY (or RX_FULL) interrupt when device is supposed to be idle and state variables are not set (especially the dev->msgs pointer which may point to NULL or stale old data). Introduce a new software status flag STATUS_ACTIVE indicating when the controller is active in driver point of view. Now treat all interrupts that occur when is not set as unexpected and mask all interrupts from the controller. Fixes: c7b79a752871 ("mfd: intel-lpss: Add Intel Alder Lake PCH-S PCI IDs") Reported-by: Samuel Clark Link: https://bugzilla.kernel.org/show_bug.cgi?id=215907 Cc: stable@vger.kernel.org # v5.12+ Signed-off-by: Jarkko Nikula Reviewed-by: Andy Shevchenko Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-core.h | 7 +++++-- drivers/i2c/busses/i2c-designware-master.c | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index 70b80e710990..4d3a3b464ecd 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -126,8 +126,9 @@ * status codes */ #define STATUS_IDLE 0x0 -#define STATUS_WRITE_IN_PROGRESS 0x1 -#define STATUS_READ_IN_PROGRESS 0x2 +#define STATUS_ACTIVE 0x1 +#define STATUS_WRITE_IN_PROGRESS 0x2 +#define STATUS_READ_IN_PROGRESS 0x4 /* * operation modes @@ -334,12 +335,14 @@ void i2c_dw_disable_int(struct dw_i2c_dev *dev); static inline void __i2c_dw_enable(struct dw_i2c_dev *dev) { + dev->status |= STATUS_ACTIVE; regmap_write(dev->map, DW_IC_ENABLE, 1); } static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev) { regmap_write(dev->map, DW_IC_ENABLE, 0); + dev->status &= ~STATUS_ACTIVE; } void __i2c_dw_disable(struct dw_i2c_dev *dev); diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index 44a94b225ed8..dc3c5a15a95b 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -716,6 +716,19 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev) u32 stat; stat = i2c_dw_read_clear_intrbits(dev); + + if (!(dev->status & STATUS_ACTIVE)) { + /* + * Unexpected interrupt in driver point of view. State + * variables are either unset or stale so acknowledge and + * disable interrupts for suppressing further interrupts if + * interrupt really came from this HW (E.g. firmware has left + * the HW active). + */ + regmap_write(dev->map, DW_IC_INTR_MASK, 0); + return 0; + } + if (stat & DW_IC_INTR_TX_ABRT) { dev->cmd_err |= DW_IC_ERR_TX_ABRT; dev->status = STATUS_IDLE; From fd66bd74afe880de4f008f96a795fedee887ff44 Mon Sep 17 00:00:00 2001 From: Quan Nguyen Date: Tue, 4 Oct 2022 16:31:06 +0700 Subject: [PATCH 6/6] i2c: aspeed: Assert NAK when slave is busy On I2C_SLAVE_WRITE_REQUESTED event, Slave already ACK'ed on the address phase. But as the backend driver is busy and unable to process any request from Master, issue RxCmdLast for Slave to auto send NACK on next incoming byte. Signed-off-by: Quan Nguyen Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-aspeed.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index 185dedfebbac..c64c381b69b7 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -244,6 +244,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) u32 command, irq_handled = 0; struct i2c_client *slave = bus->slave; u8 value; + int ret; if (!slave) return 0; @@ -311,7 +312,13 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) break; case ASPEED_I2C_SLAVE_WRITE_REQUESTED: bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED; - i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value); + ret = i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value); + /* + * Slave ACK's on this address phase already but as the backend driver + * returns an errno, the bus driver should nack the next incoming byte. + */ + if (ret < 0) + writel(ASPEED_I2CD_M_S_RX_CMD_LAST, bus->base + ASPEED_I2C_CMD_REG); break; case ASPEED_I2C_SLAVE_WRITE_RECEIVED: i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);