From 1732c22abca8f4e2528a4927baf6ffb2e3be58b0 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Fri, 31 Aug 2018 17:11:08 +0200 Subject: [PATCH 01/35] i2c: designware: use generic table matching Switch to device_get_match_data in probe to match the device specific data instead of using the acpi specific function. Suggested-by: Andy Shevchenko Acked-by: Andy Shevchenko Tested-by: Jarkko Nikula Acked-by: Jarkko Nikula Signed-off-by: Alexandre Belloni Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-platdrv.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index b5750fd85125..99012f23fe85 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -86,7 +86,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) struct i2c_timings *t = &dev->timings; u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0; acpi_handle handle = ACPI_HANDLE(&pdev->dev); - const struct acpi_device_id *id; struct acpi_device *adev; const char *uid; @@ -119,10 +118,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) break; } - id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); - if (id && id->driver_data) - dev->flags |= (u32)id->driver_data; - if (acpi_bus_get_device(handle, &adev)) return -ENODEV; @@ -291,6 +286,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) else t->bus_freq_hz = 400000; + dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev); + if (has_acpi_companion(&pdev->dev)) dw_i2c_acpi_configure(pdev); From 96742775a3c0649b88140803737ffa1915eb4509 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Fri, 31 Aug 2018 17:11:09 +0200 Subject: [PATCH 02/35] i2c: designware: move #ifdef CONFIG_OF to the top Move the #ifdef CONFIG_OF section to the top of the file, after the ACPI section so functions defined there can be used in dw_i2c_plat_probe. Tested-by: Jarkko Nikula Acked-by: Jarkko Nikula Signed-off-by: Alexandre Belloni Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index 99012f23fe85..723892820e2b 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -156,6 +156,14 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev) } #endif +#ifdef CONFIG_OF +static const struct of_device_id dw_i2c_of_match[] = { + { .compatible = "snps,designware-i2c", }, + {}, +}; +MODULE_DEVICE_TABLE(of, dw_i2c_of_match); +#endif + static void i2c_dw_configure_master(struct dw_i2c_dev *dev) { struct i2c_timings *t = &dev->timings; @@ -390,14 +398,6 @@ static int dw_i2c_plat_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_OF -static const struct of_device_id dw_i2c_of_match[] = { - { .compatible = "snps,designware-i2c", }, - {}, -}; -MODULE_DEVICE_TABLE(of, dw_i2c_of_match); -#endif - #ifdef CONFIG_PM_SLEEP static int dw_i2c_plat_prepare(struct device *dev) { From c7fa7aeff85cbd71c724e7a7b167c24356de9c5f Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Fri, 31 Aug 2018 17:11:10 +0200 Subject: [PATCH 03/35] i2c: designware: allow IP specific sda_hold_time Because some old designware IPs were not supporting setting an SDA hold time, vendors developed their own solution. Add a way for the final driver to provide its own SDA hold time handling. Reviewed-by: Andy Shevchenko Tested-by: Jarkko Nikula Acked-by: Jarkko Nikula Signed-off-by: Alexandre Belloni Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-common.c | 2 ++ drivers/i2c/busses/i2c-designware-core.h | 1 + 2 files changed, 3 insertions(+) diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c index 69ec4a791f23..36271cd75342 100644 --- a/drivers/i2c/busses/i2c-designware-common.c +++ b/drivers/i2c/busses/i2c-designware-common.c @@ -201,6 +201,8 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev) dev_dbg(dev->dev, "SDA Hold Time TX:RX = %d:%d\n", dev->sda_hold_time & ~(u32)DW_IC_SDA_HOLD_RX_MASK, dev->sda_hold_time >> DW_IC_SDA_HOLD_RX_SHIFT); + } else if (dev->set_sda_hold_time) { + dev->set_sda_hold_time(dev); } else if (dev->sda_hold_time) { dev_warn(dev->dev, "Hardware too old to adjust SDA hold time.\n"); diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index e367b1af4ab2..f6cad20a86ff 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -269,6 +269,7 @@ struct dw_i2c_dev { void (*disable)(struct dw_i2c_dev *dev); void (*disable_int)(struct dw_i2c_dev *dev); int (*init)(struct dw_i2c_dev *dev); + int (*set_sda_hold_time)(struct dw_i2c_dev *dev); int mode; struct i2c_bus_recovery_info rinfo; }; From ffbc01bff2ef98430e4ab486b878f04fe7bb7c29 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Fri, 31 Aug 2018 17:11:11 +0200 Subject: [PATCH 04/35] dt-bindings: i2c: designware: document MSCC Ocelot bindings Document bindings for the Microsemi Ocelot integration of the Designware I2C controller. Reviewed-by: Rob Herring Signed-off-by: Alexandre Belloni Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/i2c-designware.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt index fbb0a6d8b964..3e4bcc2fb6f7 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt @@ -3,6 +3,7 @@ Required properties : - compatible : should be "snps,designware-i2c" + or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback - reg : Offset and length of the register set for the device - interrupts : where IRQ is the interrupt number. @@ -11,8 +12,12 @@ Recommended properties : - clock-frequency : desired I2C bus clock frequency in Hz. Optional properties : + - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold + time, named ICPU_CFG:TWI_DELAY in the datasheet. + - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds. - This option is only supported in hardware blocks version 1.11a or newer. + This option is only supported in hardware blocks version 1.11a or newer and + on Microsemi SoCs ("mscc,ocelot-i2c" compatible). - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds. This value which is by default 300ns is used to compute the tLOW period. From 1bb39959623b438d6b7705abfd0538e8ef4f5f0f Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Fri, 31 Aug 2018 17:11:12 +0200 Subject: [PATCH 05/35] i2c: designware: add MSCC Ocelot support The Microsemi Ocelot I2C controller is a designware IP. It also has a second set of registers to allow tweaking SDA hold time and spike filtering. Reviewed-by: Andy Shevchenko Tested-by: Jarkko Nikula Acked-by: Jarkko Nikula Signed-off-by: Alexandre Belloni [wsa: made one function static] Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-core.h | 3 ++ drivers/i2c/busses/i2c-designware-platdrv.c | 40 +++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index f6cad20a86ff..5b550ab9a009 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -225,6 +225,7 @@ struct dw_i2c_dev { struct device *dev; void __iomem *base; + void __iomem *ext; struct completion cmd_complete; struct clk *clk; struct reset_control *rst; @@ -279,6 +280,8 @@ struct dw_i2c_dev { #define ACCESS_INTR_MASK 0x00000004 #define MODEL_CHERRYTRAIL 0x00000100 +#define MODEL_MSCC_OCELOT 0x00000200 +#define MODEL_MASK 0x00000f00 u32 dw_readl(struct dw_i2c_dev *dev, int offset); void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset); diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index 723892820e2b..a56af235e9a6 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -157,11 +157,48 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev) #endif #ifdef CONFIG_OF +#define MSCC_ICPU_CFG_TWI_DELAY 0x0 +#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE BIT(0) +#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER 0x4 + +static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev) +{ + writel((dev->sda_hold_time << 1) | MSCC_ICPU_CFG_TWI_DELAY_ENABLE, + dev->ext + MSCC_ICPU_CFG_TWI_DELAY); + + return 0; +} + +static int dw_i2c_of_configure(struct platform_device *pdev) +{ + struct dw_i2c_dev *dev = platform_get_drvdata(pdev); + struct resource *mem; + + switch (dev->flags & MODEL_MASK) { + case MODEL_MSCC_OCELOT: + mem = platform_get_resource(pdev, IORESOURCE_MEM, 1); + dev->ext = devm_ioremap_resource(&pdev->dev, mem); + if (!IS_ERR(dev->ext)) + dev->set_sda_hold_time = mscc_twi_set_sda_hold_time; + break; + default: + break; + } + + return 0; +} + static const struct of_device_id dw_i2c_of_match[] = { { .compatible = "snps,designware-i2c", }, + { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT }, {}, }; MODULE_DEVICE_TABLE(of, dw_i2c_of_match); +#else +static inline int dw_i2c_of_configure(struct platform_device *pdev) +{ + return -ENODEV; +} #endif static void i2c_dw_configure_master(struct dw_i2c_dev *dev) @@ -296,6 +333,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev); + if (pdev->dev.of_node) + dw_i2c_of_configure(pdev); + if (has_acpi_companion(&pdev->dev)) dw_i2c_acpi_configure(pdev); From 9cbeeca05049b1109e7e445369898b8a88d5ea7b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 5 Sep 2018 21:51:31 +0200 Subject: [PATCH 06/35] i2c: designware: Remove Cherry Trail PMIC I2C bus pm_disabled workaround Commit a3d411fb38c0 ("i2c: designware: Disable pm for PMIC i2c-bus even if there is no _SEM method"), always set the pm_disabled flag on the I2C7 controller, even if its bus was not shared with the PUNIT. This was a workaround for various suspend/resume issues, after the following 2 commits this workaround is no longer necessary: Commit 541527728341 ("PM: i2c-designware-platdrv: Suspend/resume at the late/early stages") Commit e6ce0ce34f65 ("ACPI / LPSS: Add device link for CHT SD card dependency on I2C") Therefor this commit removes this workaround. After this commit the pm_disabled flag is only used to indicate that the bus is shared with the PUNIT and after other recent changes we no longer call dev_pm_syscore_device(dev, true), so we are no longer actually disabling (non-runtime) pm, so this commit also renames the flag to shared_with_punit to better reflect what it is for. Reviewed-by: Andy Shevchenko Acked-by: Jarkko Nikula Signed-off-by: Hans de Goede Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-baytrail.c | 2 +- drivers/i2c/busses/i2c-designware-core.h | 4 ++-- drivers/i2c/busses/i2c-designware-master.c | 2 +- drivers/i2c/busses/i2c-designware-platdrv.c | 23 ++++---------------- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c index a2a275cfc1f6..9ca1feaba98f 100644 --- a/drivers/i2c/busses/i2c-designware-baytrail.c +++ b/drivers/i2c/busses/i2c-designware-baytrail.c @@ -164,7 +164,7 @@ int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) dev_info(dev->dev, "I2C bus managed by PUNIT\n"); dev->acquire_lock = baytrail_i2c_acquire; dev->release_lock = baytrail_i2c_release; - dev->pm_disabled = true; + dev->shared_with_punit = true; pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE); diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index 5b550ab9a009..fb40d76639da 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -212,7 +212,7 @@ * @pm_qos: pm_qos_request used while holding a hardware lock on the bus * @acquire_lock: function to acquire a hardware lock on the bus * @release_lock: function to release a hardware lock on the bus - * @pm_disabled: true if power-management should be disabled for this i2c-bus + * @shared_with_punit: true if this bus is shared with the SoCs PUNIT * @disable: function to disable the controller * @disable_int: function to disable all interrupts * @init: function to initialize the I2C hardware @@ -266,7 +266,7 @@ struct dw_i2c_dev { struct pm_qos_request pm_qos; int (*acquire_lock)(struct dw_i2c_dev *dev); void (*release_lock)(struct dw_i2c_dev *dev); - bool pm_disabled; + bool shared_with_punit; void (*disable)(struct dw_i2c_dev *dev); void (*disable_int)(struct dw_i2c_dev *dev); int (*init)(struct dw_i2c_dev *dev); diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index 94d94b4a9a0d..2ccb527735f9 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -707,7 +707,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) adap->dev.parent = dev->dev; i2c_set_adapdata(adap, dev); - if (dev->pm_disabled) { + if (dev->shared_with_punit) { irq_flags = IRQF_NO_SUSPEND; } else { irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND; diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index a56af235e9a6..51cb17287c47 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -85,9 +85,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) struct dw_i2c_dev *dev = platform_get_drvdata(pdev); struct i2c_timings *t = &dev->timings; u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0; - acpi_handle handle = ACPI_HANDLE(&pdev->dev); - struct acpi_device *adev; - const char *uid; dev->adapter.nr = -1; dev->tx_fifo_depth = 32; @@ -118,18 +115,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) break; } - if (acpi_bus_get_device(handle, &adev)) - return -ENODEV; - - /* - * Cherrytrail I2C7 gets used for the PMIC which gets accessed - * through ACPI opregions during late suspend / early resume - * disable pm for it. - */ - uid = adev->pnp.unique_id; - if ((dev->flags & MODEL_CHERRYTRAIL) && !strcmp(uid, "7")) - dev->pm_disabled = true; - return 0; } @@ -261,7 +246,7 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev) { pm_runtime_disable(dev->dev); - if (dev->pm_disabled) + if (dev->shared_with_punit) pm_runtime_put_noidle(dev->dev); } @@ -393,7 +378,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_set_active(&pdev->dev); - if (dev->pm_disabled) + if (dev->shared_with_punit) pm_runtime_get_noresume(&pdev->dev); pm_runtime_enable(&pdev->dev); @@ -471,7 +456,7 @@ static int dw_i2c_plat_suspend(struct device *dev) { struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); - if (i_dev->pm_disabled) + if (i_dev->shared_with_punit) return 0; i_dev->disable(i_dev); @@ -484,7 +469,7 @@ static int dw_i2c_plat_resume(struct device *dev) { struct dw_i2c_dev *i_dev = dev_get_drvdata(dev); - if (!i_dev->pm_disabled) + if (!i_dev->shared_with_punit) i2c_dw_prepare_clk(i_dev, true); i_dev->init(i_dev); From fc66b39fe36acfd06f716e338de7cd8f9550fad2 Mon Sep 17 00:00:00 2001 From: Jun Gao Date: Thu, 6 Sep 2018 21:15:29 +0800 Subject: [PATCH 07/35] i2c: mediatek: Use DMA safe buffers for i2c transactions DMA mode will always be used in i2c transactions, try to allocate a DMA safe buffer if the buf of struct i2c_msg used is not DMA safe. Signed-off-by: Jun Gao Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-mt65xx.c | 68 ++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c index 1e57f58fcb00..a74ef76705e0 100644 --- a/drivers/i2c/busses/i2c-mt65xx.c +++ b/drivers/i2c/busses/i2c-mt65xx.c @@ -441,6 +441,8 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, u16 control_reg; u16 restart_flag = 0; u32 reg_4g_mode; + u8 *dma_rd_buf = NULL; + u8 *dma_wr_buf = NULL; dma_addr_t rpaddr = 0; dma_addr_t wpaddr = 0; int ret; @@ -500,11 +502,19 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, if (i2c->op == I2C_MASTER_RD) { writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG); writel(I2C_DMA_CON_RX, i2c->pdmabase + OFFSET_CON); - rpaddr = dma_map_single(i2c->dev, msgs->buf, - msgs->len, DMA_FROM_DEVICE); - if (dma_mapping_error(i2c->dev, rpaddr)) + + dma_rd_buf = i2c_get_dma_safe_msg_buf(msgs, 0); + if (!dma_rd_buf) return -ENOMEM; + rpaddr = dma_map_single(i2c->dev, dma_rd_buf, + msgs->len, DMA_FROM_DEVICE); + if (dma_mapping_error(i2c->dev, rpaddr)) { + i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, false); + + return -ENOMEM; + } + if (i2c->dev_comp->support_33bits) { reg_4g_mode = mtk_i2c_set_4g_mode(rpaddr); writel(reg_4g_mode, i2c->pdmabase + OFFSET_RX_4G_MODE); @@ -515,11 +525,19 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, } else if (i2c->op == I2C_MASTER_WR) { writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG); writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON); - wpaddr = dma_map_single(i2c->dev, msgs->buf, - msgs->len, DMA_TO_DEVICE); - if (dma_mapping_error(i2c->dev, wpaddr)) + + dma_wr_buf = i2c_get_dma_safe_msg_buf(msgs, 0); + if (!dma_wr_buf) return -ENOMEM; + wpaddr = dma_map_single(i2c->dev, dma_wr_buf, + msgs->len, DMA_TO_DEVICE); + if (dma_mapping_error(i2c->dev, wpaddr)) { + i2c_put_dma_safe_msg_buf(dma_wr_buf, msgs, false); + + return -ENOMEM; + } + if (i2c->dev_comp->support_33bits) { reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr); writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE); @@ -530,16 +548,39 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, } else { writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_INT_FLAG); writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_CON); - wpaddr = dma_map_single(i2c->dev, msgs->buf, - msgs->len, DMA_TO_DEVICE); - if (dma_mapping_error(i2c->dev, wpaddr)) + + dma_wr_buf = i2c_get_dma_safe_msg_buf(msgs, 0); + if (!dma_wr_buf) return -ENOMEM; - rpaddr = dma_map_single(i2c->dev, (msgs + 1)->buf, + + wpaddr = dma_map_single(i2c->dev, dma_wr_buf, + msgs->len, DMA_TO_DEVICE); + if (dma_mapping_error(i2c->dev, wpaddr)) { + i2c_put_dma_safe_msg_buf(dma_wr_buf, msgs, false); + + return -ENOMEM; + } + + dma_rd_buf = i2c_get_dma_safe_msg_buf((msgs + 1), 0); + if (!dma_rd_buf) { + dma_unmap_single(i2c->dev, wpaddr, + msgs->len, DMA_TO_DEVICE); + + i2c_put_dma_safe_msg_buf(dma_wr_buf, msgs, false); + + return -ENOMEM; + } + + rpaddr = dma_map_single(i2c->dev, dma_rd_buf, (msgs + 1)->len, DMA_FROM_DEVICE); if (dma_mapping_error(i2c->dev, rpaddr)) { dma_unmap_single(i2c->dev, wpaddr, msgs->len, DMA_TO_DEVICE); + + i2c_put_dma_safe_msg_buf(dma_wr_buf, msgs, false); + i2c_put_dma_safe_msg_buf(dma_rd_buf, (msgs + 1), false); + return -ENOMEM; } @@ -578,14 +619,21 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, if (i2c->op == I2C_MASTER_WR) { dma_unmap_single(i2c->dev, wpaddr, msgs->len, DMA_TO_DEVICE); + + i2c_put_dma_safe_msg_buf(dma_wr_buf, msgs, true); } else if (i2c->op == I2C_MASTER_RD) { dma_unmap_single(i2c->dev, rpaddr, msgs->len, DMA_FROM_DEVICE); + + i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, true); } else { dma_unmap_single(i2c->dev, wpaddr, msgs->len, DMA_TO_DEVICE); dma_unmap_single(i2c->dev, rpaddr, (msgs + 1)->len, DMA_FROM_DEVICE); + + i2c_put_dma_safe_msg_buf(dma_wr_buf, msgs, true); + i2c_put_dma_safe_msg_buf(dma_rd_buf, (msgs + 1), true); } if (ret == 0) { From 3e9efc3299dd78a0fa96515f0a453fab1ed4a1bd Mon Sep 17 00:00:00 2001 From: Jae Hyun Yoo Date: Thu, 23 Aug 2018 15:57:31 -0700 Subject: [PATCH 08/35] i2c: aspeed: Handle master/slave combined irq events properly In most of cases, interrupt bits are set one by one but there are also a lot of other cases that Aspeed I2C IP sends multiple interrupt bits with combining master and slave events using a single interrupt call. It happens much more in multi-master environment than single-master. For an example, when master is waiting for a NORMAL_STOP interrupt in its MASTER_STOP state, SLAVE_MATCH and RX_DONE interrupts could come along with the NORMAL_STOP in case of an another master immediately sends data just after acquiring the bus. In this case, the NORMAL_STOP interrupt should be handled by master_irq and the SLAVE_MATCH and RX_DONE interrupts should be handled by slave_irq. This commit modifies irq hadling logic to handle the master/slave combined events properly. Signed-off-by: Jae Hyun Yoo Reviewed-by: Brendan Higgins Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-aspeed.c | 131 ++++++++++++++++++-------------- 1 file changed, 76 insertions(+), 55 deletions(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index a4f956c6d567..c258c4d9a4c0 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -82,6 +82,11 @@ #define ASPEED_I2CD_INTR_RX_DONE BIT(2) #define ASPEED_I2CD_INTR_TX_NAK BIT(1) #define ASPEED_I2CD_INTR_TX_ACK BIT(0) +#define ASPEED_I2CD_INTR_MASTER_ERRORS \ + (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \ + ASPEED_I2CD_INTR_SCL_TIMEOUT | \ + ASPEED_I2CD_INTR_ABNORMAL | \ + ASPEED_I2CD_INTR_ARBIT_LOSS) #define ASPEED_I2CD_INTR_ALL \ (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \ ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \ @@ -227,32 +232,26 @@ reset_out: } #if IS_ENABLED(CONFIG_I2C_SLAVE) -static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus) +static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) { - u32 command, irq_status, status_ack = 0; + u32 command, irq_handled = 0; struct i2c_client *slave = bus->slave; - bool irq_handled = true; u8 value; - if (!slave) { - irq_handled = false; - goto out; - } + if (!slave) + return 0; command = readl(bus->base + ASPEED_I2C_CMD_REG); - irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG); /* Slave was requested, restart state machine. */ if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) { - status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH; + irq_handled |= ASPEED_I2CD_INTR_SLAVE_MATCH; bus->slave_state = ASPEED_I2C_SLAVE_START; } /* Slave is not currently active, irq was for someone else. */ - if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) { - irq_handled = false; - goto out; - } + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) + return irq_handled; dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n", irq_status, command); @@ -269,31 +268,31 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus) bus->slave_state = ASPEED_I2C_SLAVE_WRITE_REQUESTED; } - status_ack |= ASPEED_I2CD_INTR_RX_DONE; + irq_handled |= ASPEED_I2CD_INTR_RX_DONE; } /* Slave was asked to stop. */ if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) { - status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP; + irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP; bus->slave_state = ASPEED_I2C_SLAVE_STOP; } if (irq_status & ASPEED_I2CD_INTR_TX_NAK) { - status_ack |= ASPEED_I2CD_INTR_TX_NAK; + irq_handled |= ASPEED_I2CD_INTR_TX_NAK; bus->slave_state = ASPEED_I2C_SLAVE_STOP; } + if (irq_status & ASPEED_I2CD_INTR_TX_ACK) + irq_handled |= ASPEED_I2CD_INTR_TX_ACK; switch (bus->slave_state) { case ASPEED_I2C_SLAVE_READ_REQUESTED: if (irq_status & ASPEED_I2CD_INTR_TX_ACK) dev_err(bus->dev, "Unexpected ACK on read request.\n"); bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED; - i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value); writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG); writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG); break; case ASPEED_I2C_SLAVE_READ_PROCESSED: - status_ack |= ASPEED_I2CD_INTR_TX_ACK; if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK)) dev_err(bus->dev, "Expected ACK after processed read.\n"); @@ -317,13 +316,6 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus) break; } - if (status_ack != irq_status) - dev_err(bus->dev, - "irq handled != irq. expected %x, but was %x\n", - irq_status, status_ack); - writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG); - -out: return irq_handled; } #endif /* CONFIG_I2C_SLAVE */ @@ -380,21 +372,21 @@ static int aspeed_i2c_is_irq_error(u32 irq_status) return 0; } -static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus) +static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status) { - u32 irq_status, status_ack = 0, command = 0; + u32 irq_handled = 0, command = 0; struct i2c_msg *msg; u8 recv_byte; int ret; - irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG); - /* Ack all interrupt bits. */ - writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG); - if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) { bus->master_state = ASPEED_I2C_MASTER_INACTIVE; - status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE; + irq_handled |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE; goto out_complete; + } else { + /* Master is not currently active, irq was for someone else. */ + if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE) + goto out_no_complete; } /* @@ -403,19 +395,22 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus) * INACTIVE state. */ ret = aspeed_i2c_is_irq_error(irq_status); - if (ret < 0) { + if (ret) { dev_dbg(bus->dev, "received error interrupt: 0x%08x\n", irq_status); bus->cmd_err = ret; bus->master_state = ASPEED_I2C_MASTER_INACTIVE; + irq_handled |= (irq_status & ASPEED_I2CD_INTR_MASTER_ERRORS); goto out_complete; } /* We are in an invalid state; reset bus to a known state. */ if (!bus->msgs) { - dev_err(bus->dev, "bus in unknown state\n"); + dev_err(bus->dev, "bus in unknown state. irq_status: 0x%x\n", + irq_status); bus->cmd_err = -EIO; - if (bus->master_state != ASPEED_I2C_MASTER_STOP) + if (bus->master_state != ASPEED_I2C_MASTER_STOP && + bus->master_state != ASPEED_I2C_MASTER_INACTIVE) aspeed_i2c_do_stop(bus); goto out_no_complete; } @@ -428,13 +423,18 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus) */ if (bus->master_state == ASPEED_I2C_MASTER_START) { if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) { + if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_NAK))) { + bus->cmd_err = -ENXIO; + bus->master_state = ASPEED_I2C_MASTER_INACTIVE; + goto out_complete; + } pr_devel("no slave present at %02x\n", msg->addr); - status_ack |= ASPEED_I2CD_INTR_TX_NAK; + irq_handled |= ASPEED_I2CD_INTR_TX_NAK; bus->cmd_err = -ENXIO; aspeed_i2c_do_stop(bus); goto out_no_complete; } - status_ack |= ASPEED_I2CD_INTR_TX_ACK; + irq_handled |= ASPEED_I2CD_INTR_TX_ACK; if (msg->len == 0) { /* SMBUS_QUICK */ aspeed_i2c_do_stop(bus); goto out_no_complete; @@ -449,13 +449,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus) case ASPEED_I2C_MASTER_TX: if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) { dev_dbg(bus->dev, "slave NACKed TX\n"); - status_ack |= ASPEED_I2CD_INTR_TX_NAK; + irq_handled |= ASPEED_I2CD_INTR_TX_NAK; goto error_and_stop; } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) { dev_err(bus->dev, "slave failed to ACK TX\n"); goto error_and_stop; } - status_ack |= ASPEED_I2CD_INTR_TX_ACK; + irq_handled |= ASPEED_I2CD_INTR_TX_ACK; /* fallthrough intended */ case ASPEED_I2C_MASTER_TX_FIRST: if (bus->buf_index < msg->len) { @@ -478,7 +478,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus) dev_err(bus->dev, "master failed to RX\n"); goto error_and_stop; } - status_ack |= ASPEED_I2CD_INTR_RX_DONE; + irq_handled |= ASPEED_I2CD_INTR_RX_DONE; recv_byte = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8; msg->buf[bus->buf_index++] = recv_byte; @@ -506,11 +506,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus) goto out_no_complete; case ASPEED_I2C_MASTER_STOP: if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) { - dev_err(bus->dev, "master failed to STOP\n"); + dev_err(bus->dev, + "master failed to STOP. irq_status:0x%x\n", + irq_status); bus->cmd_err = -EIO; /* Do not STOP as we have already tried. */ } else { - status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP; + irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP; } bus->master_state = ASPEED_I2C_MASTER_INACTIVE; @@ -540,33 +542,52 @@ out_complete: bus->master_xfer_result = bus->msgs_index + 1; complete(&bus->cmd_complete); out_no_complete: - if (irq_status != status_ack) - dev_err(bus->dev, - "irq handled != irq. expected 0x%08x, but was 0x%08x\n", - irq_status, status_ack); - return !!irq_status; + return irq_handled; } static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id) { struct aspeed_i2c_bus *bus = dev_id; - bool ret; + u32 irq_received, irq_remaining, irq_handled; spin_lock(&bus->lock); + irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG); + irq_remaining = irq_received; #if IS_ENABLED(CONFIG_I2C_SLAVE) - if (aspeed_i2c_slave_irq(bus)) { - dev_dbg(bus->dev, "irq handled by slave.\n"); - ret = true; - goto out; + /* + * In most cases, interrupt bits will be set one by one, although + * multiple interrupt bits could be set at the same time. It's also + * possible that master interrupt bits could be set along with slave + * interrupt bits. Each case needs to be handled using corresponding + * handlers depending on the current state. + */ + if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) { + irq_handled = aspeed_i2c_master_irq(bus, irq_remaining); + irq_remaining &= ~irq_handled; + if (irq_remaining) + irq_handled |= aspeed_i2c_slave_irq(bus, irq_remaining); + } else { + irq_handled = aspeed_i2c_slave_irq(bus, irq_remaining); + irq_remaining &= ~irq_handled; + if (irq_remaining) + irq_handled |= aspeed_i2c_master_irq(bus, + irq_remaining); } +#else + irq_handled = aspeed_i2c_master_irq(bus, irq_remaining); #endif /* CONFIG_I2C_SLAVE */ - ret = aspeed_i2c_master_irq(bus); + irq_remaining &= ~irq_handled; + if (irq_remaining) + dev_err(bus->dev, + "irq handled != irq. expected 0x%08x, but was 0x%08x\n", + irq_received, irq_handled); -out: + /* Ack all interrupt bits. */ + writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG); spin_unlock(&bus->lock); - return ret ? IRQ_HANDLED : IRQ_NONE; + return irq_remaining ? IRQ_NONE : IRQ_HANDLED; } static int aspeed_i2c_master_xfer(struct i2c_adapter *adap, From 2be6b47211e17e6c90ead40d24d2a5cc815f2d5c Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Thu, 13 Sep 2018 20:30:10 -0700 Subject: [PATCH 09/35] i2c: aspeed: Acknowledge most interrupts early in interrupt handler Commit 3e9efc3299dd ("i2c: aspeed: Handle master/slave combined irq events properly") moved interrupt acknowledgment to the end of the interrupt handler. In part this was done because the AST2500 datasheet says: I2CD10 Interrupt Status Register bit 2 Receive Done Interrupt status S/W needs to clear this status bit to allow next data receiving. Acknowledging Receive Done before receive data was handled resulted in receive errors on high speed I2C busses. However, interrupt acknowledgment was not only moved to the end of the interrupt handler for Receive Done Interrupt status, but for all interrupt status bits. This could result in race conditions if a second interrupt was received during interrupt handling and not handled but still acknowledged at the end of the interrupt handler. Acknowledge only "Receive Done Interrupt status" late in the interrupt handler to solve the problem. Fixes: 3e9efc3299dd ("i2c: aspeed: Handle master/slave combined irq events properly") Cc: Jae Hyun Yoo Cc: Joel Stanley Signed-off-by: Guenter Roeck Acked-by: Jae Hyun Yoo Tested-by: Joel Stanley Acked-by: Brendan Higgins Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-aspeed.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index c258c4d9a4c0..3d518e09369f 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -552,6 +552,9 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id) spin_lock(&bus->lock); irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG); + /* Ack all interrupts except for Rx done */ + writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE, + bus->base + ASPEED_I2C_INTR_STS_REG); irq_remaining = irq_received; #if IS_ENABLED(CONFIG_I2C_SLAVE) @@ -584,8 +587,10 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id) "irq handled != irq. expected 0x%08x, but was 0x%08x\n", irq_received, irq_handled); - /* Ack all interrupt bits. */ - writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG); + /* Ack Rx done */ + if (irq_received & ASPEED_I2CD_INTR_RX_DONE) + writel(ASPEED_I2CD_INTR_RX_DONE, + bus->base + ASPEED_I2C_INTR_STS_REG); spin_unlock(&bus->lock); return irq_remaining ? IRQ_NONE : IRQ_HANDLED; } From f8878fadba1e8cfcaadf98d437040d3fe9e2d12c Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 6 Sep 2018 20:44:24 +0200 Subject: [PATCH 10/35] i2c: aspeed: use proper annotation for "fall through" Use a better annotation, so GCC won't complain anymore: drivers/i2c/busses/i2c-aspeed.c:458:15: warning: this statement may fall through [-Wimplicit-fallthrough=] Signed-off-by: Wolfram Sang Tested-by: Jae Hyun Yoo Reviewed-by: Brendan Higgins Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-aspeed.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index 3d518e09369f..9d6f02265a1b 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -456,7 +456,7 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status) goto error_and_stop; } irq_handled |= ASPEED_I2CD_INTR_TX_ACK; - /* fallthrough intended */ + /* fall through */ case ASPEED_I2C_MASTER_TX_FIRST: if (bus->buf_index < msg->len) { bus->master_state = ASPEED_I2C_MASTER_TX; @@ -472,7 +472,7 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status) /* RX may not have completed yet (only address cycle) */ if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) goto out_no_complete; - /* fallthrough intended */ + /* fall through */ case ASPEED_I2C_MASTER_RX: if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) { dev_err(bus->dev, "master failed to RX\n"); From 17ccba67109cd0631f206cf49e17986218b47854 Mon Sep 17 00:00:00 2001 From: Brendan Higgins Date: Fri, 21 Sep 2018 16:30:50 -0700 Subject: [PATCH 11/35] i2c: aspeed: fix invalid clock parameters for very large divisors The function that computes clock parameters from divisors did not respect the maximum size of the bitfields that the parameters were written to. This fixes the bug. This bug can be reproduced with (and this fix verified with) the test at: https://kunit-review.googlesource.com/c/linux/+/1035/ Discovered-by-KUnit: https://kunit-review.googlesource.com/c/linux/+/1035/ Signed-off-by: Brendan Higgins Reviewed-by: Jae Hyun Yoo Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-aspeed.c | 65 +++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index 9d6f02265a1b..8dc9161ced38 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -142,7 +142,8 @@ struct aspeed_i2c_bus { /* Synchronizes I/O mem access to base. */ spinlock_t lock; struct completion cmd_complete; - u32 (*get_clk_reg_val)(u32 divisor); + u32 (*get_clk_reg_val)(struct device *dev, + u32 divisor); unsigned long parent_clk_frequency; u32 bus_frequency; /* Transaction state. */ @@ -710,16 +711,27 @@ static const struct i2c_algorithm aspeed_i2c_algo = { #endif /* CONFIG_I2C_SLAVE */ }; -static u32 aspeed_i2c_get_clk_reg_val(u32 clk_high_low_max, u32 divisor) +static u32 aspeed_i2c_get_clk_reg_val(struct device *dev, + u32 clk_high_low_mask, + u32 divisor) { - u32 base_clk, clk_high, clk_low, tmp; + u32 base_clk_divisor, clk_high_low_max, clk_high, clk_low, tmp; + + /* + * SCL_high and SCL_low represent a value 1 greater than what is stored + * since a zero divider is meaningless. Thus, the max value each can + * store is every bit set + 1. Since SCL_high and SCL_low are added + * together (see below), the max value of both is the max value of one + * them times two. + */ + clk_high_low_max = (clk_high_low_mask + 1) * 2; /* * The actual clock frequency of SCL is: * SCL_freq = APB_freq / (base_freq * (SCL_high + SCL_low)) * = APB_freq / divisor * where base_freq is a programmable clock divider; its value is - * base_freq = 1 << base_clk + * base_freq = 1 << base_clk_divisor * SCL_high is the number of base_freq clock cycles that SCL stays high * and SCL_low is the number of base_freq clock cycles that SCL stays * low for a period of SCL. @@ -729,47 +741,59 @@ static u32 aspeed_i2c_get_clk_reg_val(u32 clk_high_low_max, u32 divisor) * SCL_low = clk_low + 1 * Thus, * SCL_freq = APB_freq / - * ((1 << base_clk) * (clk_high + 1 + clk_low + 1)) + * ((1 << base_clk_divisor) * (clk_high + 1 + clk_low + 1)) * The documentation recommends clk_high >= clk_high_max / 2 and * clk_low >= clk_low_max / 2 - 1 when possible; this last constraint * gives us the following solution: */ - base_clk = divisor > clk_high_low_max ? + base_clk_divisor = divisor > clk_high_low_max ? ilog2((divisor - 1) / clk_high_low_max) + 1 : 0; - tmp = (divisor + (1 << base_clk) - 1) >> base_clk; - clk_low = tmp / 2; - clk_high = tmp - clk_low; - if (clk_high) - clk_high--; + if (base_clk_divisor > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK) { + base_clk_divisor = ASPEED_I2CD_TIME_BASE_DIVISOR_MASK; + clk_low = clk_high_low_mask; + clk_high = clk_high_low_mask; + dev_err(dev, + "clamping clock divider: divider requested, %u, is greater than largest possible divider, %u.\n", + divisor, (1 << base_clk_divisor) * clk_high_low_max); + } else { + tmp = (divisor + (1 << base_clk_divisor) - 1) + >> base_clk_divisor; + clk_low = tmp / 2; + clk_high = tmp - clk_low; - if (clk_low) - clk_low--; + if (clk_high) + clk_high--; + + if (clk_low) + clk_low--; + } return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT) & ASPEED_I2CD_TIME_SCL_HIGH_MASK) | ((clk_low << ASPEED_I2CD_TIME_SCL_LOW_SHIFT) & ASPEED_I2CD_TIME_SCL_LOW_MASK) - | (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK); + | (base_clk_divisor + & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK); } -static u32 aspeed_i2c_24xx_get_clk_reg_val(u32 divisor) +static u32 aspeed_i2c_24xx_get_clk_reg_val(struct device *dev, u32 divisor) { /* * clk_high and clk_low are each 3 bits wide, so each can hold a max * value of 8 giving a clk_high_low_max of 16. */ - return aspeed_i2c_get_clk_reg_val(16, divisor); + return aspeed_i2c_get_clk_reg_val(dev, GENMASK(2, 0), divisor); } -static u32 aspeed_i2c_25xx_get_clk_reg_val(u32 divisor) +static u32 aspeed_i2c_25xx_get_clk_reg_val(struct device *dev, u32 divisor) { /* * clk_high and clk_low are each 4 bits wide, so each can hold a max * value of 16 giving a clk_high_low_max of 32. */ - return aspeed_i2c_get_clk_reg_val(32, divisor); + return aspeed_i2c_get_clk_reg_val(dev, GENMASK(3, 0), divisor); } /* precondition: bus.lock has been acquired. */ @@ -782,7 +806,7 @@ static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus) clk_reg_val &= (ASPEED_I2CD_TIME_TBUF_MASK | ASPEED_I2CD_TIME_THDSTA_MASK | ASPEED_I2CD_TIME_TACST_MASK); - clk_reg_val |= bus->get_clk_reg_val(divisor); + clk_reg_val |= bus->get_clk_reg_val(bus->dev, divisor); writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1); writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2); @@ -898,7 +922,8 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev) if (!match) bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val; else - bus->get_clk_reg_val = (u32 (*)(u32))match->data; + bus->get_clk_reg_val = (u32 (*)(struct device *, u32)) + match->data; /* Initialize the I2C adapter */ spin_lock_init(&bus->lock); From 4310834412199ac9ab02350e5c360a210478e015 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 13 Sep 2018 13:59:13 -0500 Subject: [PATCH 12/35] i2c: synquacer: fix fall-through annotation Replace "fallthru" with a proper "fall through" annotation. This fix is part of the ongoing efforts to enabling -Wimplicit-fallthrough Signed-off-by: Gustavo A. R. Silva Acked-by: Ard Biesheuvel Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-synquacer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c index 915f5edbab33..2184b7c3580e 100644 --- a/drivers/i2c/busses/i2c-synquacer.c +++ b/drivers/i2c/busses/i2c-synquacer.c @@ -404,7 +404,7 @@ static irqreturn_t synquacer_i2c_isr(int irq, void *dev_id) if (i2c->state == STATE_READ) goto prepare_read; - /* fallthru */ + /* fall through */ case STATE_WRITE: if (bsr & SYNQUACER_I2C_BSR_LRB) { From f4089fcfdb3d31421bbe0b1f3c3d6c11cd0dcb4d Mon Sep 17 00:00:00 2001 From: Fabrizio Castro Date: Fri, 21 Sep 2018 18:36:58 +0100 Subject: [PATCH 13/35] dt-bindings: i2c: rcar: Add r8a77470 support Although the I2C IP found in the RZ/G1C is not exactly the same as the one found in the R-Car Gen2 family or R-Car Gen3 family, it can still be considered as compatible with R-Car Gen2 from a software perpective. This patch therefore documents the SoC specific compatible string, and the compatibility with Gen2 fallback is retained. Signed-off-by: Fabrizio Castro Reviewed-by: Biju Das Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/i2c-rcar.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt index 39cd21d95810..671e2a14ddbb 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt @@ -4,6 +4,7 @@ Required properties: - compatible: "renesas,i2c-r8a7743" if the device is a part of a R8A7743 SoC. "renesas,i2c-r8a7745" if the device is a part of a R8A7745 SoC. + "renesas,i2c-r8a77470" if the device is a part of a R8A77470 SoC. "renesas,i2c-r8a774a1" if the device is a part of a R8A774A1 SoC. "renesas,i2c-r8a7778" if the device is a part of a R8A7778 SoC. "renesas,i2c-r8a7779" if the device is a part of a R8A7779 SoC. From 8bca7089af56be53ecd87091b1f5bf81030e571b Mon Sep 17 00:00:00 2001 From: Biju Das Date: Fri, 21 Sep 2018 18:36:59 +0100 Subject: [PATCH 14/35] dt-bindings: i2c: rcar: Document r8a7744 support Document i2c Device Tree support for RZ/G1N (R8A7744) SoC, which is compatible with R-Car Gen2 SoC family. Signed-off-by: Biju Das Reviewed-by: Fabrizio Castro Reviewed-by: Simon Horman Reviewed-by: Geert Uytterhoeven Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/i2c-rcar.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt index 671e2a14ddbb..30c0485b167b 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt @@ -3,6 +3,7 @@ I2C for R-Car platforms Required properties: - compatible: "renesas,i2c-r8a7743" if the device is a part of a R8A7743 SoC. + "renesas,i2c-r8a7744" if the device is a part of a R8A7744 SoC. "renesas,i2c-r8a7745" if the device is a part of a R8A7745 SoC. "renesas,i2c-r8a77470" if the device is a part of a R8A77470 SoC. "renesas,i2c-r8a774a1" if the device is a part of a R8A774A1 SoC. From f88c18ddb0903c49a6b999cd7fe05f06246d4a62 Mon Sep 17 00:00:00 2001 From: Biju Das Date: Fri, 21 Sep 2018 18:37:00 +0100 Subject: [PATCH 15/35] dt-bindings: i2c: sh_mobile: Document r8a7744 support Document i2c Device Tree support for RZ/G1N (R8A7744) SoC, which is compatible with R-Car Gen2 SoC family. Signed-off-by: Biju Das Reviewed-by: Fabrizio Castro Reviewed-by: Simon Horman Reviewed-by: Geert Uytterhoeven Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt index 872673adff5a..d81b62643655 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt @@ -5,6 +5,7 @@ Required properties: - "renesas,iic-r8a73a4" (R-Mobile APE6) - "renesas,iic-r8a7740" (R-Mobile A1) - "renesas,iic-r8a7743" (RZ/G1M) + - "renesas,iic-r8a7744" (RZ/G1N) - "renesas,iic-r8a7745" (RZ/G1E) - "renesas,iic-r8a774a1" (RZ/G2M) - "renesas,iic-r8a7790" (R-Car H2) From 9a9e295e7c5c0409c020088b0ae017e6c2b7df6e Mon Sep 17 00:00:00 2001 From: Wang Xin Date: Thu, 16 Aug 2018 19:45:34 +0200 Subject: [PATCH 16/35] eeprom: at24: fix unexpected timeout under high load Within at24_loop_until_timeout the timestamp used for timeout checking is recorded after the I2C transfer and sleep_range(). Under high CPU load either the execution time for I2C transfer or sleep_range() could actually be larger than the timeout value. Worst case the I2C transfer is only tried once because the loop will exit due to the timeout although the EEPROM is now ready. To fix this issue the timestamp is recorded at the beginning of each iteration. That is, before I2C transfer and sleep. Then the timeout is actually checked against the timestamp of the previous iteration. This makes sure that even if the timeout is reached, there is still one more chance to try the I2C transfer in case the EEPROM is ready. Example: If you have a system which combines high CPU load with repeated EEPROM writes you will run into the following scenario. - System makes a successful regmap_bulk_write() to EEPROM. - System wants to perform another write to EEPROM but EEPROM is still busy with the last write. - Because of high CPU load the usleep_range() will sleep more than 25 ms (at24_write_timeout). - Within the over-long sleeping the EEPROM finished the previous write operation and is ready again. - at24_loop_until_timeout() will detect timeout and won't try to write. Signed-off-by: Wang Xin Signed-off-by: Mark Jonas Signed-off-by: Bartosz Golaszewski --- drivers/misc/eeprom/at24.c | 43 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 7e50e1d6f58c..636ed7149793 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -106,23 +106,6 @@ static unsigned int at24_write_timeout = 25; module_param_named(write_timeout, at24_write_timeout, uint, 0); MODULE_PARM_DESC(at24_write_timeout, "Time (in ms) to try writes (default 25)"); -/* - * Both reads and writes fail if the previous write didn't complete yet. This - * macro loops a few times waiting at least long enough for one entire page - * write to work while making sure that at least one iteration is run before - * checking the break condition. - * - * It takes two parameters: a variable in which the future timeout in jiffies - * will be stored and a temporary variable holding the time of the last - * iteration of processing the request. Both should be unsigned integers - * holding at least 32 bits. - */ -#define at24_loop_until_timeout(tout, op_time) \ - for (tout = jiffies + msecs_to_jiffies(at24_write_timeout), \ - op_time = 0; \ - op_time ? time_before(op_time, tout) : true; \ - usleep_range(1000, 1500), op_time = jiffies) - struct at24_chip_data { /* * these fields mirror their equivalents in @@ -308,13 +291,22 @@ static ssize_t at24_regmap_read(struct at24_data *at24, char *buf, /* adjust offset for mac and serial read ops */ offset += at24->offset_adj; - at24_loop_until_timeout(timeout, read_time) { + timeout = jiffies + msecs_to_jiffies(at24_write_timeout); + do { + /* + * The timestamp shall be taken before the actual operation + * to avoid a premature timeout in case of high CPU load. + */ + read_time = jiffies; + ret = regmap_bulk_read(regmap, offset, buf, count); dev_dbg(&client->dev, "read %zu@%d --> %d (%ld)\n", count, offset, ret, jiffies); if (!ret) return count; - } + + usleep_range(1000, 1500); + } while (time_before(read_time, timeout)); return -ETIMEDOUT; } @@ -358,14 +350,23 @@ static ssize_t at24_regmap_write(struct at24_data *at24, const char *buf, regmap = at24_client->regmap; client = at24_client->client; count = at24_adjust_write_count(at24, offset, count); + timeout = jiffies + msecs_to_jiffies(at24_write_timeout); + + do { + /* + * The timestamp shall be taken before the actual operation + * to avoid a premature timeout in case of high CPU load. + */ + write_time = jiffies; - at24_loop_until_timeout(timeout, write_time) { ret = regmap_bulk_write(regmap, offset, buf, count); dev_dbg(&client->dev, "write %zu@%d --> %d (%ld)\n", count, offset, ret, jiffies); if (!ret) return count; - } + + usleep_range(1000, 1500); + } while (time_before(write_time, timeout)); return -ETIMEDOUT; } From e1eba2ea54a2de0e4c58d87270d25706bb77b844 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Mon, 1 Oct 2018 10:43:47 -0700 Subject: [PATCH 17/35] i2c: brcmstb: Allow enabling the driver on DSL SoCs ARCH_BCM_63XX which is used by ARM-based DSL SoCs from Broadcom uses the same controller, make it possible to select the STB driver and update the Kconfig and help text a bit. Signed-off-by: Florian Fainelli Signed-off-by: Wolfram Sang --- drivers/i2c/busses/Kconfig | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 451d4ae50e66..56ccb1ea7da5 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -432,12 +432,13 @@ config I2C_BCM_KONA If you do not need KONA I2C interface, say N. config I2C_BRCMSTB - tristate "BRCM Settop I2C controller" - depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST + tristate "BRCM Settop/DSL I2C controller" + depends on ARCH_BRCMSTB || BMIPS_GENERIC || ARCH_BCM_63XX || \ + COMPILE_TEST default y help If you say yes to this option, support will be included for the - I2C interface on the Broadcom Settop SoCs. + I2C interface on the Broadcom Settop/DSL SoCs. If you do not need I2C interface, say N. From 49d54abee9d1507e117a4218dc5baa3ebc5b93f1 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 28 Sep 2018 17:49:47 -0500 Subject: [PATCH 18/35] i2c: Convert to using %pOFn instead of device_node.name In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier. Reviewed-by: Peter Rosin Signed-off-by: Rob Herring Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-powermac.c | 17 +++++++++-------- drivers/i2c/muxes/i2c-mux-gpmux.c | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c index f2a2067525ef..f6f4ed8afc93 100644 --- a/drivers/i2c/busses/i2c-powermac.c +++ b/drivers/i2c/busses/i2c-powermac.c @@ -388,9 +388,8 @@ static void i2c_powermac_register_devices(struct i2c_adapter *adap, static int i2c_powermac_probe(struct platform_device *dev) { struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev); - struct device_node *parent = NULL; + struct device_node *parent; struct i2c_adapter *adapter; - const char *basename; int rc; if (bus == NULL) @@ -407,23 +406,25 @@ static int i2c_powermac_probe(struct platform_device *dev) parent = of_get_parent(pmac_i2c_get_controller(bus)); if (parent == NULL) return -EINVAL; - basename = parent->name; + snprintf(adapter->name, sizeof(adapter->name), "%pOFn %d", + parent, + pmac_i2c_get_channel(bus)); + of_node_put(parent); break; case pmac_i2c_bus_pmu: - basename = "pmu"; + snprintf(adapter->name, sizeof(adapter->name), "pmu %d", + pmac_i2c_get_channel(bus)); break; case pmac_i2c_bus_smu: /* This is not what we used to do but I'm fixing drivers at * the same time as this change */ - basename = "smu"; + snprintf(adapter->name, sizeof(adapter->name), "smu %d", + pmac_i2c_get_channel(bus)); break; default: return -EINVAL; } - snprintf(adapter->name, sizeof(adapter->name), "%s %d", basename, - pmac_i2c_get_channel(bus)); - of_node_put(parent); platform_set_drvdata(dev, adapter); adapter->algo = &i2c_powermac_algorithm; diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c index 92cf5f48afe6..f60b670deff7 100644 --- a/drivers/i2c/muxes/i2c-mux-gpmux.c +++ b/drivers/i2c/muxes/i2c-mux-gpmux.c @@ -120,8 +120,8 @@ static int i2c_mux_probe(struct platform_device *pdev) ret = of_property_read_u32(child, "reg", &chan); if (ret < 0) { - dev_err(dev, "no reg property for node '%s'\n", - child->name); + dev_err(dev, "no reg property for node '%pOFn'\n", + child); goto err_children; } From f37b2bb6ac3e6ebf855d9d4f05cc6932a8e5b463 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Mon, 23 Jul 2018 22:26:08 +0200 Subject: [PATCH 19/35] i2c: omap: use core to detect 'no zero length' quirk And don't reimplement in the driver. Signed-off-by: Wolfram Sang Reviewed-by: Grygorii Strashko Acked-by: Tony Lindgren Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-omap.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 65d06a819307..b1086bfb0465 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -661,9 +661,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, dev_dbg(omap->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n", msg->addr, msg->len, msg->flags, stop); - if (msg->len == 0) - return -EINVAL; - omap->receiver = !!(msg->flags & I2C_M_RD); omap_i2c_resize_fifo(omap, msg->len, omap->receiver); @@ -1179,6 +1176,10 @@ static const struct i2c_algorithm omap_i2c_algo = { .functionality = omap_i2c_func, }; +static const struct i2c_adapter_quirks omap_i2c_quirks = { + .flags = I2C_AQ_NO_ZERO_LEN, +}; + #ifdef CONFIG_OF static struct omap_i2c_bus_platform_data omap2420_pdata = { .rev = OMAP_I2C_IP_VERSION_1, @@ -1453,6 +1454,7 @@ omap_i2c_probe(struct platform_device *pdev) adap->class = I2C_CLASS_DEPRECATED; strlcpy(adap->name, "OMAP I2C adapter", sizeof(adap->name)); adap->algo = &omap_i2c_algo; + adap->quirks = &omap_i2c_quirks; adap->dev.parent = &pdev->dev; adap->dev.of_node = pdev->dev.of_node; adap->bus_recovery_info = &omap_i2c_bus_recovery_info; From de82bb431855580ad659bfed3e858bd9dd12efd0 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Mon, 23 Jul 2018 22:26:10 +0200 Subject: [PATCH 20/35] i2c: qup: use core to detect 'no zero length' quirk And don't reimplement in the driver. Signed-off-by: Wolfram Sang Reviewed-by: Andy Gross Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qup.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index c86c3ae1318f..e09cd0775ae9 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -1088,11 +1088,6 @@ static int qup_i2c_xfer(struct i2c_adapter *adap, writel(I2C_MINI_CORE | I2C_N_VAL, qup->base + QUP_CONFIG); for (idx = 0; idx < num; idx++) { - if (msgs[idx].len == 0) { - ret = -EINVAL; - goto out; - } - if (qup_i2c_poll_state_i2c_master(qup)) { ret = -EIO; goto out; @@ -1520,9 +1515,6 @@ qup_i2c_determine_mode_v2(struct qup_i2c_dev *qup, /* All i2c_msgs should be transferred using either dma or cpu */ for (idx = 0; idx < num; idx++) { - if (msgs[idx].len == 0) - return -EINVAL; - if (msgs[idx].flags & I2C_M_RD) max_rx_len = max_t(unsigned int, max_rx_len, msgs[idx].len); @@ -1636,9 +1628,14 @@ static const struct i2c_algorithm qup_i2c_algo_v2 = { * which limits the possible read to 256 (QUP_READ_LIMIT) bytes. */ static const struct i2c_adapter_quirks qup_i2c_quirks = { + .flags = I2C_AQ_NO_ZERO_LEN, .max_read_len = QUP_READ_LIMIT, }; +static const struct i2c_adapter_quirks qup_i2c_quirks_v2 = { + .flags = I2C_AQ_NO_ZERO_LEN, +}; + static void qup_i2c_enable_clocks(struct qup_i2c_dev *qup) { clk_prepare_enable(qup->clk); @@ -1701,6 +1698,7 @@ static int qup_i2c_probe(struct platform_device *pdev) is_qup_v1 = true; } else { qup->adap.algo = &qup_i2c_algo_v2; + qup->adap.quirks = &qup_i2c_quirks_v2; is_qup_v1 = false; if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) goto nodma; From c96c0f2683804b710531e7b754dcd02b5ded6d4a Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Mon, 23 Jul 2018 22:26:12 +0200 Subject: [PATCH 21/35] i2c: tegra: use core to detect 'no zero length' quirk And don't reimplement in the driver. Signed-off-by: Wolfram Sang Acked-by: Jon Hunter Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-tegra.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 60c8561fbe65..437294ea2f0a 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -684,9 +684,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, tegra_i2c_flush_fifos(i2c_dev); - if (msg->len == 0) - return -EINVAL; - i2c_dev->msg_buf = msg->buf; i2c_dev->msg_buf_remaining = msg->len; i2c_dev->msg_err = I2C_ERR_NONE; @@ -831,6 +828,7 @@ static const struct i2c_algorithm tegra_i2c_algo = { /* payload size is only 12 bit */ static const struct i2c_adapter_quirks tegra_i2c_quirks = { + .flags = I2C_AQ_NO_ZERO_LEN, .max_read_len = 4096, .max_write_len = 4096, }; From e2115ace4196bcd2126446fb874bcfc90cba79be Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Mon, 23 Jul 2018 22:26:13 +0200 Subject: [PATCH 22/35] i2c: zx2967: use core to detect 'no zero length' quirk And don't reimplement in the driver. Signed-off-by: Wolfram Sang Acked-by: Shawn Guo Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-zx2967.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-zx2967.c b/drivers/i2c/busses/i2c-zx2967.c index 48281c1b30c6..b8f9e020d80e 100644 --- a/drivers/i2c/busses/i2c-zx2967.c +++ b/drivers/i2c/busses/i2c-zx2967.c @@ -281,9 +281,6 @@ static int zx2967_i2c_xfer_msg(struct zx2967_i2c *i2c, int ret; int i; - if (msg->len == 0) - return -EINVAL; - zx2967_i2c_flush_fifos(i2c); i2c->cur_trans = msg->buf; @@ -498,6 +495,10 @@ static const struct i2c_algorithm zx2967_i2c_algo = { .functionality = zx2967_i2c_func, }; +static const struct i2c_adapter_quirks zx2967_i2c_quirks = { + .flags = I2C_AQ_NO_ZERO_LEN, +}; + static const struct of_device_id zx2967_i2c_of_match[] = { { .compatible = "zte,zx296718-i2c", }, { }, @@ -568,6 +569,7 @@ static int zx2967_i2c_probe(struct platform_device *pdev) strlcpy(i2c->adap.name, "zx2967 i2c adapter", sizeof(i2c->adap.name)); i2c->adap.algo = &zx2967_i2c_algo; + i2c->adap.quirks = &zx2967_i2c_quirks; i2c->adap.nr = pdev->id; i2c->adap.dev.parent = &pdev->dev; i2c->adap.dev.of_node = pdev->dev.of_node; From a7163dc2138d96b3593b7d4430d35e506696a62f Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 20 Sep 2018 18:14:20 +0200 Subject: [PATCH 23/35] i2c: core: remove outdated DEBUG output The usefulness of this debug output is questionable. It covers only direct i2c_transfer calls and no SMBUS calls, neither direct nor emulated ones. And the latter one is largely used in the kernel, so a lot of stuff is missed. False positives are also generated in case the locking later fails. Also, we have a proper tracing mechanism for all these kinds of transfers in place for years now. Remove this old one. Signed-off-by: Wolfram Sang Acked-by: Peter Rosin Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-base.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 9ee9a15e7134..c2b352c46fae 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1940,16 +1940,6 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) */ if (adap->algo->master_xfer) { -#ifdef DEBUG - for (ret = 0; ret < num; ret++) { - dev_dbg(&adap->dev, - "master_xfer[%d] %c, addr=0x%02x, len=%d%s\n", - ret, (msgs[ret].flags & I2C_M_RD) ? 'R' : 'W', - msgs[ret].addr, msgs[ret].len, - (msgs[ret].flags & I2C_M_RECV_LEN) ? "+" : ""); - } -#endif - if (in_atomic() || irqs_disabled()) { ret = i2c_trylock_bus(adap, I2C_LOCK_SEGMENT); if (!ret) From cc52612ec0f3b80c19126a36b8c1e12a8f5a8e78 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 20 Sep 2018 18:14:21 +0200 Subject: [PATCH 24/35] i2c: core: remove level of indentation in i2c_transfer Using the common kernel pattern to bail out at the beginning if some conditions are not met, we can save a level of indentation. No functional change. Signed-off-by: Wolfram Sang Acked-by: Peter Rosin Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-base.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index c2b352c46fae..799776c6d421 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1922,6 +1922,11 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) { int ret; + if (!adap->algo->master_xfer) { + dev_dbg(&adap->dev, "I2C level transfers not supported\n"); + return -EOPNOTSUPP; + } + /* REVISIT the fault reporting model here is weak: * * - When we get an error after receiving N bytes from a slave, @@ -1938,25 +1943,19 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) * one (discarding status on the second message) or errno * (discarding status on the first one). */ - - if (adap->algo->master_xfer) { - if (in_atomic() || irqs_disabled()) { - ret = i2c_trylock_bus(adap, I2C_LOCK_SEGMENT); - if (!ret) - /* I2C activity is ongoing. */ - return -EAGAIN; - } else { - i2c_lock_bus(adap, I2C_LOCK_SEGMENT); - } - - ret = __i2c_transfer(adap, msgs, num); - i2c_unlock_bus(adap, I2C_LOCK_SEGMENT); - - return ret; + if (in_atomic() || irqs_disabled()) { + ret = i2c_trylock_bus(adap, I2C_LOCK_SEGMENT); + if (!ret) + /* I2C activity is ongoing. */ + return -EAGAIN; } else { - dev_dbg(&adap->dev, "I2C level transfers not supported\n"); - return -EOPNOTSUPP; + i2c_lock_bus(adap, I2C_LOCK_SEGMENT); } + + ret = __i2c_transfer(adap, msgs, num); + i2c_unlock_bus(adap, I2C_LOCK_SEGMENT); + + return ret; } EXPORT_SYMBOL(i2c_transfer); From f2e0821377a37a74fe219d8c38f886d88425794b Mon Sep 17 00:00:00 2001 From: Luca Ceresoli Date: Wed, 3 Oct 2018 17:50:22 +0200 Subject: [PATCH 25/35] i2c: mux: pca954x: simplify code to reach the adapter struct i2c_client has a direct pointer to the adapter, no need to dig it out of the struct device tree. Signed-off-by: Luca Ceresoli Acked-by: Wolfram Sang Signed-off-by: Peter Rosin --- drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c index 24bd9275fde5..bfabf985e830 100644 --- a/drivers/i2c/muxes/i2c-mux-pca954x.c +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c @@ -347,7 +347,7 @@ static void pca954x_cleanup(struct i2c_mux_core *muxc) static int pca954x_probe(struct i2c_client *client, const struct i2c_device_id *id) { - struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent); + struct i2c_adapter *adap = client->adapter; struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev); struct device *dev = &client->dev; struct device_node *np = dev->of_node; From df6dd24f76641ba4216a6008a38956e245c6129b Mon Sep 17 00:00:00 2001 From: Luca Ceresoli Date: Wed, 3 Oct 2018 17:50:23 +0200 Subject: [PATCH 26/35] i2c: mux: ltc4306: simplify code to reach the adapter struct i2c_client has a direct pointer to the adapter, no need to dig it out of the struct device tree. Suggested-by: Peter Rosin Signed-off-by: Luca Ceresoli Acked-by: Wolfram Sang Signed-off-by: Peter Rosin --- drivers/i2c/muxes/i2c-mux-ltc4306.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c b/drivers/i2c/muxes/i2c-mux-ltc4306.c index a9af93259b19..83a714605cd6 100644 --- a/drivers/i2c/muxes/i2c-mux-ltc4306.c +++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c @@ -208,7 +208,7 @@ MODULE_DEVICE_TABLE(of, ltc4306_of_match); static int ltc4306_probe(struct i2c_client *client) { - struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent); + struct i2c_adapter *adap = client->adapter; const struct chip_desc *chip; struct i2c_mux_core *muxc; struct ltc4306 *data; From 7451dc608564f9f2077c6c895f9c94a815d401e5 Mon Sep 17 00:00:00 2001 From: Luca Ceresoli Date: Wed, 3 Oct 2018 17:50:24 +0200 Subject: [PATCH 27/35] i2c: mux: mlxcpld: simplify code to reach the adapter struct i2c_client has a direct pointer to the adapter, no need to dig it out of the struct device tree. Suggested-by: Peter Rosin Signed-off-by: Luca Ceresoli Acked-by: Wolfram Sang Signed-off-by: Peter Rosin --- drivers/i2c/muxes/i2c-mux-mlxcpld.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c index f2bf3e57ed67..5ed55ca4fe93 100644 --- a/drivers/i2c/muxes/i2c-mux-mlxcpld.c +++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c @@ -132,7 +132,7 @@ static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan) static int mlxcpld_mux_probe(struct i2c_client *client, const struct i2c_device_id *id) { - struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent); + struct i2c_adapter *adap = client->adapter; struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev); struct i2c_mux_core *muxc; int num, force; From b30f2f65568f840e5ca522d98ba2ad73b8f59cde Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 6 Oct 2018 10:25:39 +0200 Subject: [PATCH 28/35] i2c: designware: Set IRQF_NO_SUSPEND flag for all BYT and CHT controllers On some Cherry Trail systems the GPU ACPI fwnode has power-resources which point to the PMIC, which is connected over a LPSS I2C controller. The GPU is a PCI device and PCI devices are powered-on at the resume_noirq resume phase. Since the GPU power-resources need the I2C controller, recent acpi_lpss.c changes now also power-up the LPSS I2C controllers on BYT and CHT devices in the resume_noirq resume phase. But during this phase the IRQ of the controller is disabled leading to these errors: i2c_designware 808622C1:06: controller timed out ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR video LNXVIDEO:00: Failed to change power state to D0 This commit makes the i2c-designware controller set the IRQF_NO_SUSPEND flag when requesting the interrupt on BYT and CHT devices, so that the IRQ is left enabled during the noirq phase, fixing this. Tested-by: Jarkko Nikula Acked-by: Jarkko Nikula Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Reviewed-by: Mika Westerberg Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-core.h | 1 + drivers/i2c/busses/i2c-designware-master.c | 2 +- drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index fb40d76639da..9ec8394f4787 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -278,6 +278,7 @@ struct dw_i2c_dev { #define ACCESS_SWAP 0x00000001 #define ACCESS_16BIT 0x00000002 #define ACCESS_INTR_MASK 0x00000004 +#define ACCESS_NO_IRQ_SUSPEND 0x00000008 #define MODEL_CHERRYTRAIL 0x00000100 #define MODEL_MSCC_OCELOT 0x00000200 diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index 2ccb527735f9..2a630ac35ba2 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -707,7 +707,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) adap->dev.parent = dev->dev; i2c_set_adapdata(adap, dev); - if (dev->shared_with_punit) { + if (dev->flags & ACCESS_NO_IRQ_SUSPEND) { irq_flags = IRQF_NO_SUSPEND; } else { irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND; diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index 51cb17287c47..997bbb3d925f 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -123,8 +123,8 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = { { "INT33C3", 0 }, { "INT3432", 0 }, { "INT3433", 0 }, - { "80860F41", 0 }, - { "808622C1", MODEL_CHERRYTRAIL }, + { "80860F41", ACCESS_NO_IRQ_SUSPEND }, + { "808622C1", ACCESS_NO_IRQ_SUSPEND | MODEL_CHERRYTRAIL }, { "AMD0010", ACCESS_INTR_MASK }, { "AMDI0010", ACCESS_INTR_MASK }, { "AMDI0510", 0 }, From ef8d1639f2b71c683277cc2b27354541792f6ecf Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 24 Sep 2018 16:52:34 -0700 Subject: [PATCH 29/35] i2c: i2c-qcom-geni: Simplify tx/rx functions We never really look at the 'ret' local variable in these functions, so let's remove it to make way for shorter and simpler code. Furthermore, we can shorten some lines by adding two local variables for the SE and the message length so that everything fits in 80 columns and testing the 'dma_buf' local variable in lieu of the 'mode' local variable. And kernel style is to leave the return statement by itself, detached from the rest of the function. Signed-off-by: Stephen Boyd Reviewed-by: Douglas Anderson Reviewed-by: Alok Chauhan Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qcom-geni.c | 71 ++++++++++++++---------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 9f2eb02481d3..0b466835cf40 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -365,29 +365,24 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, u32 m_param) { dma_addr_t rx_dma; - enum geni_se_xfer_mode mode; - unsigned long time_left = XFER_TIMEOUT; + unsigned long time_left; void *dma_buf; + struct geni_se *se = &gi2c->se; + size_t len = msg->len; - gi2c->cur = msg; - mode = GENI_SE_FIFO; dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); if (dma_buf) - mode = GENI_SE_DMA; + geni_se_select_mode(se, GENI_SE_DMA); + else + geni_se_select_mode(se, GENI_SE_FIFO); - geni_se_select_mode(&gi2c->se, mode); - writel_relaxed(msg->len, gi2c->se.base + SE_I2C_RX_TRANS_LEN); - geni_se_setup_m_cmd(&gi2c->se, I2C_READ, m_param); - if (mode == GENI_SE_DMA) { - int ret; + writel_relaxed(len, se->base + SE_I2C_RX_TRANS_LEN); + geni_se_setup_m_cmd(se, I2C_READ, m_param); - ret = geni_se_rx_dma_prep(&gi2c->se, dma_buf, msg->len, - &rx_dma); - if (ret) { - mode = GENI_SE_FIFO; - geni_se_select_mode(&gi2c->se, mode); - i2c_put_dma_safe_msg_buf(dma_buf, msg, false); - } + if (dma_buf && geni_se_rx_dma_prep(se, dma_buf, len, &rx_dma)) { + geni_se_select_mode(se, GENI_SE_FIFO); + i2c_put_dma_safe_msg_buf(dma_buf, msg, false); + dma_buf = NULL; } time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); @@ -395,12 +390,13 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, geni_i2c_abort_xfer(gi2c); gi2c->cur_rd = 0; - if (mode == GENI_SE_DMA) { + if (dma_buf) { if (gi2c->err) geni_i2c_rx_fsm_rst(gi2c); - geni_se_rx_dma_unprep(&gi2c->se, rx_dma, msg->len); + geni_se_rx_dma_unprep(se, rx_dma, len); i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err); } + return gi2c->err; } @@ -408,45 +404,41 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, u32 m_param) { dma_addr_t tx_dma; - enum geni_se_xfer_mode mode; unsigned long time_left; void *dma_buf; + struct geni_se *se = &gi2c->se; + size_t len = msg->len; - gi2c->cur = msg; - mode = GENI_SE_FIFO; dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); if (dma_buf) - mode = GENI_SE_DMA; + geni_se_select_mode(se, GENI_SE_DMA); + else + geni_se_select_mode(se, GENI_SE_FIFO); - geni_se_select_mode(&gi2c->se, mode); - writel_relaxed(msg->len, gi2c->se.base + SE_I2C_TX_TRANS_LEN); - geni_se_setup_m_cmd(&gi2c->se, I2C_WRITE, m_param); - if (mode == GENI_SE_DMA) { - int ret; + writel_relaxed(len, se->base + SE_I2C_TX_TRANS_LEN); + geni_se_setup_m_cmd(se, I2C_WRITE, m_param); - ret = geni_se_tx_dma_prep(&gi2c->se, dma_buf, msg->len, - &tx_dma); - if (ret) { - mode = GENI_SE_FIFO; - geni_se_select_mode(&gi2c->se, mode); - i2c_put_dma_safe_msg_buf(dma_buf, msg, false); - } + if (dma_buf && geni_se_tx_dma_prep(se, dma_buf, len, &tx_dma)) { + geni_se_select_mode(se, GENI_SE_FIFO); + i2c_put_dma_safe_msg_buf(dma_buf, msg, false); + dma_buf = NULL; } - if (mode == GENI_SE_FIFO) /* Get FIFO IRQ */ - writel_relaxed(1, gi2c->se.base + SE_GENI_TX_WATERMARK_REG); + if (!dma_buf) /* Get FIFO IRQ */ + writel_relaxed(1, se->base + SE_GENI_TX_WATERMARK_REG); time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); if (!time_left) geni_i2c_abort_xfer(gi2c); gi2c->cur_wr = 0; - if (mode == GENI_SE_DMA) { + if (dma_buf) { if (gi2c->err) geni_i2c_tx_fsm_rst(gi2c); - geni_se_tx_dma_unprep(&gi2c->se, tx_dma, msg->len); + geni_se_tx_dma_unprep(se, tx_dma, len); i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err); } + return gi2c->err; } @@ -474,6 +466,7 @@ static int geni_i2c_xfer(struct i2c_adapter *adap, m_param |= ((msgs[i].addr << SLV_ADDR_SHFT) & SLV_ADDR_MSK); + gi2c->cur = &msgs[i]; if (msgs[i].flags & I2C_M_RD) ret = geni_i2c_rx_one_msg(gi2c, &msgs[i], m_param); else From a676973e4501991e2ede959ee69c6553caa29038 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 24 Sep 2018 16:52:35 -0700 Subject: [PATCH 30/35] i2c: i2c-qcom-geni: Simplify irq handler We don't need to use goto here, we can just collapse the if statement and goto chain into multiple branches and then combine some duplicate completion calls into one big if statement. Let's do it to clean up code some more. Signed-off-by: Stephen Boyd Reviewed-by: Douglas Anderson Reviewed-by: Alok Chauhan Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qcom-geni.c | 72 +++++++++++++----------------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 0b466835cf40..527f55c8c4c7 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -201,21 +201,23 @@ static void geni_i2c_err(struct geni_i2c_dev *gi2c, int err) static irqreturn_t geni_i2c_irq(int irq, void *dev) { struct geni_i2c_dev *gi2c = dev; - int j; + void __iomem *base = gi2c->se.base; + int j, p; u32 m_stat; u32 rx_st; u32 dm_tx_st; u32 dm_rx_st; u32 dma; + u32 val; struct i2c_msg *cur; unsigned long flags; spin_lock_irqsave(&gi2c->lock, flags); - m_stat = readl_relaxed(gi2c->se.base + SE_GENI_M_IRQ_STATUS); - rx_st = readl_relaxed(gi2c->se.base + SE_GENI_RX_FIFO_STATUS); - dm_tx_st = readl_relaxed(gi2c->se.base + SE_DMA_TX_IRQ_STAT); - dm_rx_st = readl_relaxed(gi2c->se.base + SE_DMA_RX_IRQ_STAT); - dma = readl_relaxed(gi2c->se.base + SE_GENI_DMA_MODE_EN); + m_stat = readl_relaxed(base + SE_GENI_M_IRQ_STATUS); + rx_st = readl_relaxed(base + SE_GENI_RX_FIFO_STATUS); + dm_tx_st = readl_relaxed(base + SE_DMA_TX_IRQ_STAT); + dm_rx_st = readl_relaxed(base + SE_DMA_RX_IRQ_STAT); + dma = readl_relaxed(base + SE_GENI_DMA_MODE_EN); cur = gi2c->cur; if (!cur || @@ -238,26 +240,17 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev) /* Disable the TX Watermark interrupt to stop TX */ if (!dma) - writel_relaxed(0, gi2c->se.base + - SE_GENI_TX_WATERMARK_REG); - goto irqret; - } - - if (dma) { + writel_relaxed(0, base + SE_GENI_TX_WATERMARK_REG); + } else if (dma) { dev_dbg(gi2c->se.dev, "i2c dma tx:0x%x, dma rx:0x%x\n", dm_tx_st, dm_rx_st); - goto irqret; - } - - if (cur->flags & I2C_M_RD && - m_stat & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) { + } else if (cur->flags & I2C_M_RD && + m_stat & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) { u32 rxcnt = rx_st & RX_FIFO_WC_MSK; for (j = 0; j < rxcnt; j++) { - u32 val; - int p = 0; - - val = readl_relaxed(gi2c->se.base + SE_GENI_RX_FIFOn); + p = 0; + val = readl_relaxed(base + SE_GENI_RX_FIFOn); while (gi2c->cur_rd < cur->len && p < sizeof(val)) { cur->buf[gi2c->cur_rd++] = val & 0xff; val >>= 8; @@ -270,44 +263,39 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev) m_stat & M_TX_FIFO_WATERMARK_EN) { for (j = 0; j < gi2c->tx_wm; j++) { u32 temp; - u32 val = 0; - int p = 0; + val = 0; + p = 0; while (gi2c->cur_wr < cur->len && p < sizeof(val)) { temp = cur->buf[gi2c->cur_wr++]; val |= temp << (p * 8); p++; } - writel_relaxed(val, gi2c->se.base + SE_GENI_TX_FIFOn); + writel_relaxed(val, base + SE_GENI_TX_FIFOn); /* TX Complete, Disable the TX Watermark interrupt */ if (gi2c->cur_wr == cur->len) { - writel_relaxed(0, gi2c->se.base + - SE_GENI_TX_WATERMARK_REG); + writel_relaxed(0, base + SE_GENI_TX_WATERMARK_REG); break; } } } -irqret: - if (m_stat) - writel_relaxed(m_stat, gi2c->se.base + SE_GENI_M_IRQ_CLEAR); - if (dma) { - if (dm_tx_st) - writel_relaxed(dm_tx_st, gi2c->se.base + - SE_DMA_TX_IRQ_CLR); - if (dm_rx_st) - writel_relaxed(dm_rx_st, gi2c->se.base + - SE_DMA_RX_IRQ_CLR); - } + if (m_stat) + writel_relaxed(m_stat, base + SE_GENI_M_IRQ_CLEAR); + + if (dma && dm_tx_st) + writel_relaxed(dm_tx_st, base + SE_DMA_TX_IRQ_CLR); + if (dma && dm_rx_st) + writel_relaxed(dm_rx_st, base + SE_DMA_RX_IRQ_CLR); + /* if this is err with done-bit not set, handle that through timeout. */ - if (m_stat & M_CMD_DONE_EN || m_stat & M_CMD_ABORT_EN) - complete(&gi2c->done); - else if (dm_tx_st & TX_DMA_DONE || dm_tx_st & TX_RESET_DONE) - complete(&gi2c->done); - else if (dm_rx_st & RX_DMA_DONE || dm_rx_st & RX_RESET_DONE) + if (m_stat & M_CMD_DONE_EN || m_stat & M_CMD_ABORT_EN || + dm_tx_st & TX_DMA_DONE || dm_tx_st & TX_RESET_DONE || + dm_rx_st & RX_DMA_DONE || dm_rx_st & RX_RESET_DONE) complete(&gi2c->done); spin_unlock_irqrestore(&gi2c->lock, flags); + return IRQ_HANDLED; } From f1fdcbbdf45d9609f3d4063b67e9ea941ba3a58f Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 16 Oct 2018 12:01:47 +0900 Subject: [PATCH 31/35] i2c: uniphier-f: make driver robust against concurrency This is unlikely to happen, but it is possible for a CPU to enter the interrupt handler just after wait_for_completion_timeout() has expired. If this happens, the hardware is accessed from multiple contexts concurrently. Disable the IRQ after wait_for_completion_timeout(), and do nothing from the handler when the IRQ is disabled. Fixes: 6a62974b667f ("i2c: uniphier_f: add UniPhier FIFO-builtin I2C driver") Signed-off-by: Masahiro Yamada Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-uniphier-f.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c index a403e8579b65..200dfd1771af 100644 --- a/drivers/i2c/busses/i2c-uniphier-f.c +++ b/drivers/i2c/busses/i2c-uniphier-f.c @@ -98,6 +98,7 @@ struct uniphier_fi2c_priv { unsigned int flags; unsigned int busy_cnt; unsigned int clk_cycle; + spinlock_t lock; /* IRQ synchronization */ }; static void uniphier_fi2c_fill_txfifo(struct uniphier_fi2c_priv *priv, @@ -162,7 +163,10 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void *dev_id) struct uniphier_fi2c_priv *priv = dev_id; u32 irq_status; + spin_lock(&priv->lock); + irq_status = readl(priv->membase + UNIPHIER_FI2C_INT); + irq_status &= priv->enabled_irqs; dev_dbg(&priv->adap.dev, "interrupt: enabled_irqs=%04x, irq_status=%04x\n", @@ -230,6 +234,8 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void *dev_id) goto handled; } + spin_unlock(&priv->lock); + return IRQ_NONE; data_done: @@ -246,6 +252,8 @@ complete: handled: uniphier_fi2c_clear_irqs(priv); + spin_unlock(&priv->lock); + return IRQ_HANDLED; } @@ -311,7 +319,7 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap, { struct uniphier_fi2c_priv *priv = i2c_get_adapdata(adap); bool is_read = msg->flags & I2C_M_RD; - unsigned long time_left; + unsigned long time_left, flags; dev_dbg(&adap->dev, "%s: addr=0x%02x, len=%d, stop=%d\n", is_read ? "receive" : "transmit", msg->addr, msg->len, stop); @@ -342,6 +350,12 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap, priv->membase + UNIPHIER_FI2C_CR); time_left = wait_for_completion_timeout(&priv->comp, adap->timeout); + + spin_lock_irqsave(&priv->lock, flags); + priv->enabled_irqs = 0; + uniphier_fi2c_set_irqs(priv); + spin_unlock_irqrestore(&priv->lock, flags); + if (!time_left) { dev_err(&adap->dev, "transaction timeout.\n"); uniphier_fi2c_recover(priv); @@ -529,6 +543,7 @@ static int uniphier_fi2c_probe(struct platform_device *pdev) priv->clk_cycle = clk_rate / bus_speed; init_completion(&priv->comp); + spin_lock_init(&priv->lock); priv->adap.owner = THIS_MODULE; priv->adap.algo = &uniphier_fi2c_algo; priv->adap.dev.parent = dev; From 39226aaa85f002d695e3cafade3309e12ffdaecd Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 16 Oct 2018 12:01:48 +0900 Subject: [PATCH 32/35] i2c: uniphier-f: fix occasional timeout error Currently, a timeout error could happen at a repeated START condition. For a (non-repeated) START condition, the controller starts sending data when the UNIPHIER_FI2C_CR_STA bit is set. However, for a repeated START condition, the hardware starts running when the slave address is written to the TX FIFO - the write to the UNIPHIER_FI2C_CR register is actually unneeded. Because the hardware is already running before the IRQ is enabled for a repeated START, the driver may miss the IRQ event. In most cases, this problem does not show up since modern CPUs are much faster than the I2C transfer. However, it is still possible that a context switch happens after the controller starts, but before the IRQ register is set up. To fix this, - Do not write UNIPHIER_FI2C_CR for repeated START conditions. - Enable IRQ *before* writing the slave address to the TX FIFO. - Disable IRQ for the current CPU while queuing up the TX FIFO; If the CPU is interrupted by some task, the interrupt handler might be invoked due to the empty TX FIFO before completing the setup. Fixes: 6a62974b667f ("i2c: uniphier_f: add UniPhier FIFO-builtin I2C driver") Signed-off-by: Masahiro Yamada Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-uniphier-f.c | 33 ++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c index 200dfd1771af..067715305976 100644 --- a/drivers/i2c/busses/i2c-uniphier-f.c +++ b/drivers/i2c/busses/i2c-uniphier-f.c @@ -260,6 +260,8 @@ handled: static void uniphier_fi2c_tx_init(struct uniphier_fi2c_priv *priv, u16 addr) { priv->enabled_irqs |= UNIPHIER_FI2C_INT_TE; + uniphier_fi2c_set_irqs(priv); + /* do not use TX byte counter */ writel(0, priv->membase + UNIPHIER_FI2C_TBC); /* set slave address */ @@ -292,6 +294,8 @@ static void uniphier_fi2c_rx_init(struct uniphier_fi2c_priv *priv, u16 addr) priv->enabled_irqs |= UNIPHIER_FI2C_INT_RF; } + uniphier_fi2c_set_irqs(priv); + /* set slave address with RD bit */ writel(UNIPHIER_FI2C_DTTX_CMD | UNIPHIER_FI2C_DTTX_RD | addr << 1, priv->membase + UNIPHIER_FI2C_DTTX); @@ -315,14 +319,16 @@ static void uniphier_fi2c_recover(struct uniphier_fi2c_priv *priv) } static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap, - struct i2c_msg *msg, bool stop) + struct i2c_msg *msg, bool repeat, + bool stop) { struct uniphier_fi2c_priv *priv = i2c_get_adapdata(adap); bool is_read = msg->flags & I2C_M_RD; unsigned long time_left, flags; - dev_dbg(&adap->dev, "%s: addr=0x%02x, len=%d, stop=%d\n", - is_read ? "receive" : "transmit", msg->addr, msg->len, stop); + dev_dbg(&adap->dev, "%s: addr=0x%02x, len=%d, repeat=%d, stop=%d\n", + is_read ? "receive" : "transmit", msg->addr, msg->len, + repeat, stop); priv->len = msg->len; priv->buf = msg->buf; @@ -338,16 +344,24 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap, writel(UNIPHIER_FI2C_RST_TBRST | UNIPHIER_FI2C_RST_RBRST, priv->membase + UNIPHIER_FI2C_RST); /* reset TX/RX FIFO */ + spin_lock_irqsave(&priv->lock, flags); + if (is_read) uniphier_fi2c_rx_init(priv, msg->addr); else uniphier_fi2c_tx_init(priv, msg->addr); - uniphier_fi2c_set_irqs(priv); - dev_dbg(&adap->dev, "start condition\n"); - writel(UNIPHIER_FI2C_CR_MST | UNIPHIER_FI2C_CR_STA, - priv->membase + UNIPHIER_FI2C_CR); + /* + * For a repeated START condition, writing a slave address to the FIFO + * kicks the controller. So, the UNIPHIER_FI2C_CR register should be + * written only for a non-repeated START condition. + */ + if (!repeat) + writel(UNIPHIER_FI2C_CR_MST | UNIPHIER_FI2C_CR_STA, + priv->membase + UNIPHIER_FI2C_CR); + + spin_unlock_irqrestore(&priv->lock, flags); time_left = wait_for_completion_timeout(&priv->comp, adap->timeout); @@ -408,6 +422,7 @@ static int uniphier_fi2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) { struct i2c_msg *msg, *emsg = msgs + num; + bool repeat = false; int ret; ret = uniphier_fi2c_check_bus_busy(adap); @@ -418,9 +433,11 @@ static int uniphier_fi2c_master_xfer(struct i2c_adapter *adap, /* Emit STOP if it is the last message or I2C_M_STOP is set. */ bool stop = (msg + 1 == emsg) || (msg->flags & I2C_M_STOP); - ret = uniphier_fi2c_master_xfer_one(adap, msg, stop); + ret = uniphier_fi2c_master_xfer_one(adap, msg, repeat, stop); if (ret) return ret; + + repeat = !stop; } return num; From eaba68785c2d24ebf1f0d46c24e11b79cc2f94c7 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 16 Oct 2018 12:01:49 +0900 Subject: [PATCH 33/35] i2c: uniphier-f: fix race condition when IRQ is cleared The current IRQ handler clears all the IRQ status bits when it bails out. This is dangerous because it might clear away the status bits that have just been set while processing the current handler. If this happens, the IRQ event for the latest transfer is lost forever. The IRQ status bits must be cleared *before* the next transfer is kicked. Fixes: 6a62974b667f ("i2c: uniphier_f: add UniPhier FIFO-builtin I2C driver") Signed-off-by: Masahiro Yamada Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-uniphier-f.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c index 067715305976..dd384743dbbd 100644 --- a/drivers/i2c/busses/i2c-uniphier-f.c +++ b/drivers/i2c/busses/i2c-uniphier-f.c @@ -143,9 +143,10 @@ static void uniphier_fi2c_set_irqs(struct uniphier_fi2c_priv *priv) writel(priv->enabled_irqs, priv->membase + UNIPHIER_FI2C_IE); } -static void uniphier_fi2c_clear_irqs(struct uniphier_fi2c_priv *priv) +static void uniphier_fi2c_clear_irqs(struct uniphier_fi2c_priv *priv, + u32 mask) { - writel(-1, priv->membase + UNIPHIER_FI2C_IC); + writel(mask, priv->membase + UNIPHIER_FI2C_IC); } static void uniphier_fi2c_stop(struct uniphier_fi2c_priv *priv) @@ -172,6 +173,8 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void *dev_id) "interrupt: enabled_irqs=%04x, irq_status=%04x\n", priv->enabled_irqs, irq_status); + uniphier_fi2c_clear_irqs(priv, irq_status); + if (irq_status & UNIPHIER_FI2C_INT_STOP) goto complete; @@ -250,8 +253,6 @@ complete: } handled: - uniphier_fi2c_clear_irqs(priv); - spin_unlock(&priv->lock); return IRQ_HANDLED; @@ -340,7 +341,7 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap, priv->flags |= UNIPHIER_FI2C_STOP; reinit_completion(&priv->comp); - uniphier_fi2c_clear_irqs(priv); + uniphier_fi2c_clear_irqs(priv, U32_MAX); writel(UNIPHIER_FI2C_RST_TBRST | UNIPHIER_FI2C_RST_RBRST, priv->membase + UNIPHIER_FI2C_RST); /* reset TX/RX FIFO */ From 7ce6c65b617e64d310fcf18b35e29fd4d28b9c4f Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Wed, 24 Oct 2018 16:51:32 +0300 Subject: [PATCH 34/35] i2c: sh_mobile: Remove dummy runtime PM callbacks Platform drivers don't need dummy runtime PM callbacks that just return success and non-NULL pm pointer in their struct device_driver in order to have runtime PM happening. This has changed since following commits: 05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly") Signed-off-by: Jarkko Nikula Reviewed-by: Wolfram Sang Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-sh_mobile.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c index 818cab14e87c..a7a7a9c3bc7c 100644 --- a/drivers/i2c/busses/i2c-sh_mobile.c +++ b/drivers/i2c/busses/i2c-sh_mobile.c @@ -947,27 +947,9 @@ static int sh_mobile_i2c_remove(struct platform_device *dev) return 0; } -static int sh_mobile_i2c_runtime_nop(struct device *dev) -{ - /* Runtime PM callback shared between ->runtime_suspend() - * and ->runtime_resume(). Simply returns success. - * - * This driver re-initializes all registers after - * pm_runtime_get_sync() anyway so there is no need - * to save and restore registers here. - */ - return 0; -} - -static const struct dev_pm_ops sh_mobile_i2c_dev_pm_ops = { - .runtime_suspend = sh_mobile_i2c_runtime_nop, - .runtime_resume = sh_mobile_i2c_runtime_nop, -}; - static struct platform_driver sh_mobile_i2c_driver = { .driver = { .name = "i2c-sh_mobile", - .pm = &sh_mobile_i2c_dev_pm_ops, .of_match_table = sh_mobile_i2c_dt_ids, }, .probe = sh_mobile_i2c_probe, From 84de6e96e036dcfb18dded04a506b1dd72ba0e5b Mon Sep 17 00:00:00 2001 From: Chris Brandt Date: Fri, 26 Oct 2018 14:57:10 -0500 Subject: [PATCH 35/35] MAINTAINERS: add maintainer for Renesas RIIC driver The RIIC I2C controller is used in Renesas RZ/A SoCs. Signed-off-by: Chris Brandt [wsa: added documentation file] Signed-off-by: Wolfram Sang --- MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 48a65c3a4189..289dc9cc28c6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12387,6 +12387,12 @@ S: Supported F: drivers/i2c/busses/i2c-rcar.c F: drivers/i2c/busses/i2c-sh_mobile.c +RENESAS RIIC DRIVER +M: Chris Brandt +S: Supported +F: Documentation/devicetree/bindings/i2c/i2c-riic.txt +F: drivers/i2c/busses/i2c-riic.c + RENESAS USB PHY DRIVER M: Yoshihiro Shimoda L: linux-renesas-soc@vger.kernel.org