linux-stable.git/drivers/i2c, branch v6.7 Linux kernel stable tree i2c: core: Fix atomic xfer check for non-preempt config 2024-01-06T13:10:10+00:00 Benjamin Bara benjamin.bara@skidata.com 2024-01-04T08:17:08+00:00 a3368e1186e3ce8e38f78cbca019622095b1f331 Since commit aa49c90894d0 ("i2c: core: Run atomic i2c xfer when !preemptible"), the whole reboot/power off sequence on non-preempt kernels is using atomic i2c xfer, as !preemptible() always results to 1. During device_shutdown(), the i2c might be used a lot and not all busses have implemented an atomic xfer handler. This results in a lot of avoidable noise, like: [ 12.687169] No atomic I2C transfer handler for 'i2c-0' [ 12.692313] WARNING: CPU: 6 PID: 275 at drivers/i2c/i2c-core.h:40 i2c_smbus_xfer+0x100/0x118 ... Fix this by allowing non-atomic xfer when the interrupts are enabled, as it was before. Link: https://lore.kernel.org/r/20231222230106.73f030a5@yea Link: https://lore.kernel.org/r/20240102150350.3180741-1-mwalle@kernel.org Link: https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailbox.org/ Fixes: aa49c90894d0 ("i2c: core: Run atomic i2c xfer when !preemptible") Cc: stable@vger.kernel.org # v5.2+ Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> Tested-by: Michael Walle <mwalle@kernel.org> Tested-by: Tor Vic <torvic9@mailbox.org> [wsa: removed a comment which needs more work, code is ok] Signed-off-by: Wolfram Sang <wsa@kernel.org> 
Since commit aa49c90894d0 ("i2c: core: Run atomic i2c xfer when
!preemptible"), the whole reboot/power off sequence on non-preempt kernels
is using atomic i2c xfer, as !preemptible() always results to 1.

During device_shutdown(), the i2c might be used a lot and not all busses
have implemented an atomic xfer handler. This results in a lot of
avoidable noise, like:

[   12.687169] No atomic I2C transfer handler for 'i2c-0'
[   12.692313] WARNING: CPU: 6 PID: 275 at drivers/i2c/i2c-core.h:40 i2c_smbus_xfer+0x100/0x118
...

Fix this by allowing non-atomic xfer when the interrupts are enabled, as
it was before.

Link: https://lore.kernel.org/r/20231222230106.73f030a5@yea
Link: https://lore.kernel.org/r/20240102150350.3180741-1-mwalle@kernel.org
Link: https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailbox.org/
Fixes: aa49c90894d0 ("i2c: core: Run atomic i2c xfer when !preemptible")
Cc: stable@vger.kernel.org # v5.2+
Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
Tested-by: Michael Walle <mwalle@kernel.org>
Tested-by: Tor Vic <torvic9@mailbox.org>
[wsa: removed a comment which needs more work, code is ok]
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: aspeed: Handle the coalesced stop conditions with the start conditions. 2023-12-19T20:54:57+00:00 Quan Nguyen quan@os.amperecomputing.com 2023-12-11T10:22:16+00:00 b4cc1cbba5195a4dd497cf2f8f09e7807977d543 Some masters may drive the transfers with low enough latency between the nak/stop phase of the current command and the start/address phase of the following command that the interrupts are coalesced by the time we process them. Handle the stop conditions before processing SLAVE_MATCH to fix the complaints that sometimes occur below. "aspeed-i2c-bus 1e78a040.i2c-bus: irq handled != irq. Expected 0x00000086, but was 0x00000084" Fixes: f9eb91350bb2 ("i2c: aspeed: added slave support for Aspeed I2C driver") Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
Some masters may drive the transfers with low enough latency between
the nak/stop phase of the current command and the start/address phase
of the following command that the interrupts are coalesced by the
time we process them.
Handle the stop conditions before processing SLAVE_MATCH to fix the
complaints that sometimes occur below.

"aspeed-i2c-bus 1e78a040.i2c-bus: irq handled != irq. Expected
0x00000086, but was 0x00000084"

Fixes: f9eb91350bb2 ("i2c: aspeed: added slave support for Aspeed I2C driver")
Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: rk3x: fix potential spinlock recursion on poll 2023-12-19T17:23:01+00:00 Jensen Huang jensenhuang@friendlyarm.com 2023-12-07T08:21:59+00:00 19cde9c92b8d3b7ee555d0da3bcb0232d3a784f4 Possible deadlock scenario (on reboot): rk3x_i2c_xfer_common(polling) -> rk3x_i2c_wait_xfer_poll() -> rk3x_i2c_irq(0, i2c); --> spin_lock(&i2c->lock); ... <rk3x i2c interrupt> -> rk3x_i2c_irq(0, i2c); --> spin_lock(&i2c->lock); (deadlock here) Store the IRQ number and disable/enable it around the polling transfer. This patch has been tested on NanoPC-T4. Signed-off-by: Jensen Huang <jensenhuang@friendlyarm.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
Possible deadlock scenario (on reboot):
rk3x_i2c_xfer_common(polling)
    -> rk3x_i2c_wait_xfer_poll()
        -> rk3x_i2c_irq(0, i2c);
            --> spin_lock(&i2c->lock);
            ...
        <rk3x i2c interrupt>
        -> rk3x_i2c_irq(0, i2c);
            --> spin_lock(&i2c->lock); (deadlock here)

Store the IRQ number and disable/enable it around the polling transfer.
This patch has been tested on NanoPC-T4.

Signed-off-by: Jensen Huang <jensenhuang@friendlyarm.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: qcom-geni: fix missing clk_disable_unprepare() and geni_se_resources_off() 2023-12-19T17:14:17+00:00 Yang Yingliang yangyingliang@huawei.com 2023-11-30T01:43:24+00:00 043465b66506e8c647cdd38a2db1f2ee0f369a1b Add missing clk_disable_unprepare() and geni_se_resources_off() in the error path in geni_i2c_probe(). Fixes: 14d02fbadb5d ("i2c: qcom-geni: add desc struct to prepare support for I2C Master Hub variant") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
Add missing clk_disable_unprepare() and geni_se_resources_off() in the error
path in geni_i2c_probe().

Fixes: 14d02fbadb5d ("i2c: qcom-geni: add desc struct to prepare support for I2C Master Hub variant")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: ocores: Move system PM hooks to the NOIRQ phase 2023-11-13T17:43:42+00:00 Samuel Holland samuel.holland@sifive.com 2023-11-13T02:32:45+00:00 382561d16854a747e6df71034da08d20d6013dfe When an I2C device contains a wake IRQ subordinate to a regmap-irq chip, the regmap-irq code must be able to perform I2C transactions during suspend_device_irqs() and resume_device_irqs(). Therefore, the bus must be suspended/resumed during the NOIRQ phase. Signed-off-by: Samuel Holland <samuel.holland@sifive.com> Acked-by: Peter Korsgaard <peter@korsgaard.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
When an I2C device contains a wake IRQ subordinate to a regmap-irq chip,
the regmap-irq code must be able to perform I2C transactions during
suspend_device_irqs() and resume_device_irqs(). Therefore, the bus must
be suspended/resumed during the NOIRQ phase.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Acked-by: Peter Korsgaard <peter@korsgaard.com>
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: designware: Fix corrupted memory seen in the ISR 2023-11-13T09:51:14+00:00 Jan Bottorff janb@os.amperecomputing.com 2023-11-09T03:19:27+00:00 f726eaa787e9f9bc858c902d18a09af6bcbfcdaf When running on a many core ARM64 server, errors were happening in the ISR that looked like corrupted memory. These corruptions would fix themselves if small delays were inserted in the ISR. Errors reported by the driver included "i2c_designware APMC0D0F:00: i2c_dw_xfer_msg: invalid target address" and "i2c_designware APMC0D0F:00:controller timed out" during in-band IPMI SSIF stress tests. The problem was determined to be memory writes in the driver were not becoming visible to all cores when execution rapidly shifted between cores, like when a register write immediately triggers an ISR. Processors with weak memory ordering, like ARM64, make no guarantees about the order normal memory writes become globally visible, unless barrier instructions are used to control ordering. To solve this, regmap accessor functions configured by this driver were changed to use non-relaxed forms of the low-level register access functions, which include a barrier on platforms that require it. This assures memory writes before a controller register access are visible to all cores. The community concluded defaulting to correct operation outweighed defaulting to the small performance gains from using relaxed access functions. Being a low speed device added weight to this choice of default register access behavior. Signed-off-by: Jan Bottorff <janb@os.amperecomputing.com> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Tested-by: Serge Semin <fancer.lancer@gmail.com> Reviewed-by: Serge Semin <fancer.lancer@gmail.com> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
When running on a many core ARM64 server, errors were
happening in the ISR that looked like corrupted memory. These
corruptions would fix themselves if small delays were inserted
in the ISR. Errors reported by the driver included "i2c_designware
APMC0D0F:00: i2c_dw_xfer_msg: invalid target address" and
"i2c_designware APMC0D0F:00:controller timed out" during
in-band IPMI SSIF stress tests.

The problem was determined to be memory writes in the driver were not
becoming visible to all cores when execution rapidly shifted between
cores, like when a register write immediately triggers an ISR.
Processors with weak memory ordering, like ARM64, make no
guarantees about the order normal memory writes become globally
visible, unless barrier instructions are used to control ordering.

To solve this, regmap accessor functions configured by this driver
were changed to use non-relaxed forms of the low-level register
access functions, which include a barrier on platforms that require
it. This assures memory writes before a controller register access are
visible to all cores. The community concluded defaulting to correct
operation outweighed defaulting to the small performance gains from
using relaxed access functions. Being a low speed device added weight to
this choice of default register access behavior.

Signed-off-by: Jan Bottorff <janb@os.amperecomputing.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
Revert "i2c: pxa: move to generic GPIO recovery" 2023-11-13T01:52:29+00:00 Robert Marko robert.marko@sartura.hr 2023-11-10T09:30:11+00:00 7b211c7671212cad0b83603c674838c7e824d845 This reverts commit 0b01392c18b9993a584f36ace1d61118772ad0ca. Conversion of PXA to generic I2C recovery, makes the I2C bus completely lock up if recovery pinctrl is present in the DT and I2C recovery is enabled. So, until the generic I2C recovery can also work with PXA lets revert to have working I2C and I2C recovery again. Signed-off-by: Robert Marko <robert.marko@sartura.hr> Cc: stable@vger.kernel.org # 5.11+ Acked-by: Andi Shyti <andi.shyti@kernel.org> Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Acked-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
This reverts commit 0b01392c18b9993a584f36ace1d61118772ad0ca.

Conversion of PXA to generic I2C recovery, makes the I2C bus completely
lock up if recovery pinctrl is present in the DT and I2C recovery is
enabled.

So, until the generic I2C recovery can also work with PXA lets revert
to have working I2C and I2C recovery again.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Cc: stable@vger.kernel.org # 5.11+
Acked-by: Andi Shyti <andi.shyti@kernel.org>
Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: cp2615: Fix 'assignment to __be16' warning 2023-11-08T09:27:17+00:00 Bence Csókás bence98@sch.bme.hu 2023-10-30T17:19:10+00:00 bdba49cbba41f0bea54fc93529bbef4d6ceaa3cd While the preamble field _is_ technically big-endian, its value is always 0x2A2A, which is the same in either endianness. However, to avoid generating a warning, we should still call `htons()` explicitly. Signed-off-by: Bence Csókás <bence98@sch.bme.hu> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
While the preamble field _is_ technically big-endian, its value is always 0x2A2A,
which is the same in either endianness. However, to avoid generating a warning,
we should still call `htons()` explicitly.

Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: dev: copy userspace array safely 2023-11-08T09:26:27+00:00 Philipp Stanner pstanner@redhat.com 2023-11-02T19:26:13+00:00 cc9c54232f04aef3a5d7f64a0ece7df00f1aaa3d i2c-dev.c utilizes memdup_user() to copy a userspace array. This is done without an overflow check. Use the new wrapper memdup_array_user() to copy the array more safely. Suggested-by: Dave Airlie <airlied@redhat.com> Signed-off-by: Philipp Stanner <pstanner@redhat.com> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
i2c-dev.c utilizes memdup_user() to copy a userspace array. This is done
without an overflow check.

Use the new wrapper memdup_array_user() to copy the array more safely.

Suggested-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Philipp Stanner <pstanner@redhat.com>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
i2c: designware: Disable TX_EMPTY irq while waiting for block length byte 2023-11-08T09:19:48+00:00 Tam Nguyen tamnguyenchi@os.amperecomputing.com 2023-11-02T03:30:08+00:00 e8183fa10c25c7b3c20670bf2b430ddcc1ee03c0 During SMBus block data read process, we have seen high interrupt rate because of TX_EMPTY irq status while waiting for block length byte (the first data byte after the address phase). The interrupt handler does not do anything because the internal state is kept as STATUS_WRITE_IN_PROGRESS. Hence, we should disable TX_EMPTY IRQ until I2C DesignWare receives first data byte from I2C device, then re-enable it to resume SMBus transaction. It takes 0.789 ms for host to receive data length from slave. Without the patch, i2c_dw_isr() is called 99 times by TX_EMPTY interrupt. And it is none after applying the patch. Cc: stable@vger.kernel.org Co-developed-by: Chuong Tran <chuong@os.amperecomputing.com> Signed-off-by: Chuong Tran <chuong@os.amperecomputing.com> Signed-off-by: Tam Nguyen <tamnguyenchi@os.amperecomputing.com> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Reviewed-by: Serge Semin <fancer.lancer@gmail.com> Signed-off-by: Wolfram Sang <wsa@kernel.org> 
During SMBus block data read process, we have seen high interrupt rate
because of TX_EMPTY irq status while waiting for block length byte (the
first data byte after the address phase). The interrupt handler does not
do anything because the internal state is kept as STATUS_WRITE_IN_PROGRESS.
Hence, we should disable TX_EMPTY IRQ until I2C DesignWare receives
first data byte from I2C device, then re-enable it to resume SMBus
transaction.

It takes 0.789 ms for host to receive data length from slave.
Without the patch, i2c_dw_isr() is called 99 times by TX_EMPTY interrupt.
And it is none after applying the patch.

Cc: stable@vger.kernel.org
Co-developed-by: Chuong Tran <chuong@os.amperecomputing.com>
Signed-off-by: Chuong Tran <chuong@os.amperecomputing.com>
Signed-off-by: Tam Nguyen <tamnguyenchi@os.amperecomputing.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Wolfram Sang <wsa@kernel.org>