* [PATCH 0/3] i2c: xiic: Support forcing single-master in DT @ 2020-08-27 14:48 ` ext-jaakko.laine 0 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: linux-i2c, michal.simek, linux-arm-kernel, Jaakko Laine From: Jaakko Laine <ext-jaakko.laine@vaisala.com> This patch series allows xiic driver to take advantage of single-master -tag in device tree to improve reliability. Changes from V2 review based on comments: - Improved xiic_i2c struct memory alignment - Simplify bus is busy checking code in single-master mode Link to V2 review https://www.spinics.net/lists/arm-kernel/msg831068.html Jaakko Laine (3): i2c: xiic: Change code alignment to 1 space only i2c: xiic: Improve struct memory alignment i2c: xiic: Support forcing single-master in DT drivers/i2c/busses/i2c-xiic.c | 62 +++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 24 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/3] i2c: xiic: Support forcing single-master in DT @ 2020-08-27 14:48 ` ext-jaakko.laine 0 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: Jaakko Laine, linux-i2c, linux-arm-kernel, michal.simek From: Jaakko Laine <ext-jaakko.laine@vaisala.com> This patch series allows xiic driver to take advantage of single-master -tag in device tree to improve reliability. Changes from V2 review based on comments: - Improved xiic_i2c struct memory alignment - Simplify bus is busy checking code in single-master mode Link to V2 review https://www.spinics.net/lists/arm-kernel/msg831068.html Jaakko Laine (3): i2c: xiic: Change code alignment to 1 space only i2c: xiic: Improve struct memory alignment i2c: xiic: Support forcing single-master in DT drivers/i2c/busses/i2c-xiic.c | 62 +++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 24 deletions(-) -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] i2c: xiic: Change code alignment to 1 space only 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-27 14:48 ` ext-jaakko.laine -1 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: linux-i2c, michal.simek, linux-arm-kernel, Jaakko Laine From: Jaakko Laine <ext-jaakko.laine@vaisala.com> Alignment removed and replaced with 1 space only to reduce need for future alignment changes affecting multiple lines, when new variables are added. Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> Acked-by: Michal Simek <michal.simek@xilinx.com> --- drivers/i2c/busses/i2c-xiic.c | 48 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 90c1c362394d..10380531d45c 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -46,33 +46,33 @@ enum xiic_endian { /** * struct xiic_i2c - Internal representation of the XIIC I2C bus - * @dev: Pointer to device structure - * @base: Memory base of the HW registers - * @wait: Wait queue for callers - * @adap: Kernel adapter representation - * @tx_msg: Messages from above to be sent - * @lock: Mutual exclusion - * @tx_pos: Current pos in TX message - * @nmsgs: Number of messages in tx_msg - * @state: See STATE_ - * @rx_msg: Current RX message - * @rx_pos: Position within current RX message + * @dev: Pointer to device structure + * @base: Memory base of the HW registers + * @wait: Wait queue for callers + * @adap: Kernel adapter representation + * @tx_msg: Messages from above to be sent + * @lock: Mutual exclusion + * @tx_pos: Current pos in TX message + * @nmsgs: Number of messages in tx_msg + * @state: See STATE_ + * @rx_msg: Current RX message + * @rx_pos: Position within current RX message * @endianness: big/little-endian byte order - * @clk: Pointer to AXI4-lite input clock + * @clk: Pointer to AXI4-lite input clock */ struct xiic_i2c { - struct device *dev; - void __iomem *base; - wait_queue_head_t wait; - struct i2c_adapter adap; - struct i2c_msg *tx_msg; - struct mutex lock; - unsigned int tx_pos; - unsigned int nmsgs; - enum xilinx_i2c_state state; - struct i2c_msg *rx_msg; - int rx_pos; - enum xiic_endian endianness; + struct device *dev; + void __iomem *base; + wait_queue_head_t wait; + struct i2c_adapter adap; + struct i2c_msg *tx_msg; + struct mutex lock; + unsigned int tx_pos; + unsigned int nmsgs; + enum xilinx_i2c_state state; + struct i2c_msg *rx_msg; + int rx_pos; + enum xiic_endian endianness; struct clk *clk; }; -- 2.19.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/3] i2c: xiic: Change code alignment to 1 space only @ 2020-08-27 14:48 ` ext-jaakko.laine 0 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: Jaakko Laine, linux-i2c, linux-arm-kernel, michal.simek From: Jaakko Laine <ext-jaakko.laine@vaisala.com> Alignment removed and replaced with 1 space only to reduce need for future alignment changes affecting multiple lines, when new variables are added. Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> Acked-by: Michal Simek <michal.simek@xilinx.com> --- drivers/i2c/busses/i2c-xiic.c | 48 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 90c1c362394d..10380531d45c 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -46,33 +46,33 @@ enum xiic_endian { /** * struct xiic_i2c - Internal representation of the XIIC I2C bus - * @dev: Pointer to device structure - * @base: Memory base of the HW registers - * @wait: Wait queue for callers - * @adap: Kernel adapter representation - * @tx_msg: Messages from above to be sent - * @lock: Mutual exclusion - * @tx_pos: Current pos in TX message - * @nmsgs: Number of messages in tx_msg - * @state: See STATE_ - * @rx_msg: Current RX message - * @rx_pos: Position within current RX message + * @dev: Pointer to device structure + * @base: Memory base of the HW registers + * @wait: Wait queue for callers + * @adap: Kernel adapter representation + * @tx_msg: Messages from above to be sent + * @lock: Mutual exclusion + * @tx_pos: Current pos in TX message + * @nmsgs: Number of messages in tx_msg + * @state: See STATE_ + * @rx_msg: Current RX message + * @rx_pos: Position within current RX message * @endianness: big/little-endian byte order - * @clk: Pointer to AXI4-lite input clock + * @clk: Pointer to AXI4-lite input clock */ struct xiic_i2c { - struct device *dev; - void __iomem *base; - wait_queue_head_t wait; - struct i2c_adapter adap; - struct i2c_msg *tx_msg; - struct mutex lock; - unsigned int tx_pos; - unsigned int nmsgs; - enum xilinx_i2c_state state; - struct i2c_msg *rx_msg; - int rx_pos; - enum xiic_endian endianness; + struct device *dev; + void __iomem *base; + wait_queue_head_t wait; + struct i2c_adapter adap; + struct i2c_msg *tx_msg; + struct mutex lock; + unsigned int tx_pos; + unsigned int nmsgs; + enum xilinx_i2c_state state; + struct i2c_msg *rx_msg; + int rx_pos; + enum xiic_endian endianness; struct clk *clk; }; -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] i2c: xiic: Change code alignment to 1 space only 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-28 7:02 ` Wolfram Sang -1 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2020-08-28 7:02 UTC (permalink / raw) To: ext-jaakko.laine; +Cc: linux-i2c, michal.simek, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 446 bytes --] On Thu, Aug 27, 2020 at 05:48:46PM +0300, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > Alignment removed and replaced with 1 space only to > reduce need for future alignment changes affecting multiple > lines, when new variables are added. > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] i2c: xiic: Change code alignment to 1 space only @ 2020-08-28 7:02 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2020-08-28 7:02 UTC (permalink / raw) To: ext-jaakko.laine; +Cc: linux-i2c, linux-arm-kernel, michal.simek [-- Attachment #1.1: Type: text/plain, Size: 446 bytes --] On Thu, Aug 27, 2020 at 05:48:46PM +0300, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > Alignment removed and replaced with 1 space only to > reduce need for future alignment changes affecting multiple > lines, when new variables are added. > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> Applied to for-next, thanks! [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] i2c: xiic: Improve struct memory alignment 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-27 14:48 ` ext-jaakko.laine -1 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: linux-i2c, michal.simek, linux-arm-kernel, Jaakko Laine From: Jaakko Laine <ext-jaakko.laine@vaisala.com> xiic_i2c struct alignment causes the struct to take more space in memory than strictly required. Move state -member to end of struct to get less padding. Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> Suggested-by: Michal Simek <michal.simek@xilinx.com> --- drivers/i2c/busses/i2c-xiic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 10380531d45c..1453d82bb664 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -54,11 +54,11 @@ enum xiic_endian { * @lock: Mutual exclusion * @tx_pos: Current pos in TX message * @nmsgs: Number of messages in tx_msg - * @state: See STATE_ * @rx_msg: Current RX message * @rx_pos: Position within current RX message * @endianness: big/little-endian byte order * @clk: Pointer to AXI4-lite input clock + * @state: See STATE_ */ struct xiic_i2c { struct device *dev; @@ -69,11 +69,11 @@ struct xiic_i2c { struct mutex lock; unsigned int tx_pos; unsigned int nmsgs; - enum xilinx_i2c_state state; struct i2c_msg *rx_msg; int rx_pos; enum xiic_endian endianness; struct clk *clk; + enum xilinx_i2c_state state; }; -- 2.19.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] i2c: xiic: Improve struct memory alignment @ 2020-08-27 14:48 ` ext-jaakko.laine 0 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: Jaakko Laine, linux-i2c, linux-arm-kernel, michal.simek From: Jaakko Laine <ext-jaakko.laine@vaisala.com> xiic_i2c struct alignment causes the struct to take more space in memory than strictly required. Move state -member to end of struct to get less padding. Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> Suggested-by: Michal Simek <michal.simek@xilinx.com> --- drivers/i2c/busses/i2c-xiic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 10380531d45c..1453d82bb664 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -54,11 +54,11 @@ enum xiic_endian { * @lock: Mutual exclusion * @tx_pos: Current pos in TX message * @nmsgs: Number of messages in tx_msg - * @state: See STATE_ * @rx_msg: Current RX message * @rx_pos: Position within current RX message * @endianness: big/little-endian byte order * @clk: Pointer to AXI4-lite input clock + * @state: See STATE_ */ struct xiic_i2c { struct device *dev; @@ -69,11 +69,11 @@ struct xiic_i2c { struct mutex lock; unsigned int tx_pos; unsigned int nmsgs; - enum xilinx_i2c_state state; struct i2c_msg *rx_msg; int rx_pos; enum xiic_endian endianness; struct clk *clk; + enum xilinx_i2c_state state; }; -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] i2c: xiic: Improve struct memory alignment 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-27 15:56 ` Michal Simek -1 siblings, 0 replies; 18+ messages in thread From: Michal Simek @ 2020-08-27 15:56 UTC (permalink / raw) To: ext-jaakko.laine, wsa; +Cc: linux-i2c, michal.simek, linux-arm-kernel On 27. 08. 20 16:48, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > xiic_i2c struct alignment causes the struct to > take more space in memory than strictly required. > Move state -member to end of struct to get less > padding. > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > Suggested-by: Michal Simek <michal.simek@xilinx.com> > --- > drivers/i2c/busses/i2c-xiic.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index 10380531d45c..1453d82bb664 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -54,11 +54,11 @@ enum xiic_endian { > * @lock: Mutual exclusion > * @tx_pos: Current pos in TX message > * @nmsgs: Number of messages in tx_msg > - * @state: See STATE_ > * @rx_msg: Current RX message > * @rx_pos: Position within current RX message > * @endianness: big/little-endian byte order > * @clk: Pointer to AXI4-lite input clock > + * @state: See STATE_ > */ > struct xiic_i2c { > struct device *dev; > @@ -69,11 +69,11 @@ struct xiic_i2c { > struct mutex lock; > unsigned int tx_pos; > unsigned int nmsgs; > - enum xilinx_i2c_state state; > struct i2c_msg *rx_msg; > int rx_pos; > enum xiic_endian endianness; > struct clk *clk; > + enum xilinx_i2c_state state; > }; > > > Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] i2c: xiic: Improve struct memory alignment @ 2020-08-27 15:56 ` Michal Simek 0 siblings, 0 replies; 18+ messages in thread From: Michal Simek @ 2020-08-27 15:56 UTC (permalink / raw) To: ext-jaakko.laine, wsa; +Cc: linux-i2c, linux-arm-kernel, michal.simek On 27. 08. 20 16:48, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > xiic_i2c struct alignment causes the struct to > take more space in memory than strictly required. > Move state -member to end of struct to get less > padding. > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > Suggested-by: Michal Simek <michal.simek@xilinx.com> > --- > drivers/i2c/busses/i2c-xiic.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index 10380531d45c..1453d82bb664 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -54,11 +54,11 @@ enum xiic_endian { > * @lock: Mutual exclusion > * @tx_pos: Current pos in TX message > * @nmsgs: Number of messages in tx_msg > - * @state: See STATE_ > * @rx_msg: Current RX message > * @rx_pos: Position within current RX message > * @endianness: big/little-endian byte order > * @clk: Pointer to AXI4-lite input clock > + * @state: See STATE_ > */ > struct xiic_i2c { > struct device *dev; > @@ -69,11 +69,11 @@ struct xiic_i2c { > struct mutex lock; > unsigned int tx_pos; > unsigned int nmsgs; > - enum xilinx_i2c_state state; > struct i2c_msg *rx_msg; > int rx_pos; > enum xiic_endian endianness; > struct clk *clk; > + enum xilinx_i2c_state state; > }; > > > Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] i2c: xiic: Improve struct memory alignment 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-28 7:02 ` Wolfram Sang -1 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2020-08-28 7:02 UTC (permalink / raw) To: ext-jaakko.laine; +Cc: linux-i2c, michal.simek, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 458 bytes --] On Thu, Aug 27, 2020 at 05:48:47PM +0300, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > xiic_i2c struct alignment causes the struct to > take more space in memory than strictly required. > Move state -member to end of struct to get less > padding. > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > Suggested-by: Michal Simek <michal.simek@xilinx.com> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] i2c: xiic: Improve struct memory alignment @ 2020-08-28 7:02 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2020-08-28 7:02 UTC (permalink / raw) To: ext-jaakko.laine; +Cc: linux-i2c, linux-arm-kernel, michal.simek [-- Attachment #1.1: Type: text/plain, Size: 458 bytes --] On Thu, Aug 27, 2020 at 05:48:47PM +0300, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > xiic_i2c struct alignment causes the struct to > take more space in memory than strictly required. > Move state -member to end of struct to get less > padding. > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > Suggested-by: Michal Simek <michal.simek@xilinx.com> Applied to for-next, thanks! [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] i2c: xiic: Support forcing single-master in DT 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-27 14:48 ` ext-jaakko.laine -1 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: linux-i2c, michal.simek, linux-arm-kernel, Jaakko Laine From: Jaakko Laine <ext-jaakko.laine@vaisala.com> I2C master operating in multimaster mode can get stuck indefinitely if I2C start is detected on bus, but no master has a transaction going. This is a weakness in I2C standard, which defines no way to recover, since all masters are indefinitely disallowed from interrupting the currently operating master. A start condition can be created for example by an electromagnetic discharge applied near physical I2C lines. Or a already operating master could get reset immediately after sending a start. If it is known during device tree creation that only a single I2C master will be present on the bus, this deadlock of the I2C bus could be avoided in the driver by ignoring the bus_is_busy register of the xiic, since bus can never be reserved by any other master. This patch adds this support for detecting single-master flag in device tree and when provided, improves I2C reliability by ignoring the therefore unnecessary xiic bus_is_busy register. Error can be reproduced by pulling I2C SDA -line temporarily low by shorting it to ground, while linux I2C master is operating on it using the xiic driver. The application using the bus will start receiving linux error code 16: "Device or resource busy" indefinitely: kernel: pca953x 0-0020: failed writing register app: Error writing file, error: 16 With multi-master disabled device will instead receive error code 5: "I/O error" while SDA is grounded, but recover normal operation once short is removed. kernel: pca953x 0-0020: failed reading register app: Error reading file, error: 5 Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> --- drivers/i2c/busses/i2c-xiic.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 1453d82bb664..087b2951942e 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -59,6 +59,7 @@ enum xiic_endian { * @endianness: big/little-endian byte order * @clk: Pointer to AXI4-lite input clock * @state: See STATE_ + * @singlemaster: Indicates bus is single master */ struct xiic_i2c { struct device *dev; @@ -74,6 +75,7 @@ struct xiic_i2c { enum xiic_endian endianness; struct clk *clk; enum xilinx_i2c_state state; + bool singlemaster; }; @@ -526,6 +528,15 @@ static int xiic_busy(struct xiic_i2c *i2c) if (i2c->tx_msg) return -EBUSY; + /* In single master mode bus can only be busy, when in use by this + * driver. If the register indicates bus being busy for some reason we + * should ignore it, since bus will never be released and i2c will be + * stuck forever. + */ + if (i2c->singlemaster) { + return 0; + } + /* for instance if previous transfer was terminated due to TX error * it might be that the bus is on it's way to become available * give it at most 3 ms to wake @@ -811,6 +822,9 @@ static int xiic_i2c_probe(struct platform_device *pdev) goto err_clk_dis; } + i2c->singlemaster = + of_property_read_bool(pdev->dev.of_node, "single-master"); + /* * Detect endianness * Try to reset the TX FIFO. Then check the EMPTY flag. If it is not -- 2.19.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] i2c: xiic: Support forcing single-master in DT @ 2020-08-27 14:48 ` ext-jaakko.laine 0 siblings, 0 replies; 18+ messages in thread From: ext-jaakko.laine @ 2020-08-27 14:48 UTC (permalink / raw) To: wsa; +Cc: Jaakko Laine, linux-i2c, linux-arm-kernel, michal.simek From: Jaakko Laine <ext-jaakko.laine@vaisala.com> I2C master operating in multimaster mode can get stuck indefinitely if I2C start is detected on bus, but no master has a transaction going. This is a weakness in I2C standard, which defines no way to recover, since all masters are indefinitely disallowed from interrupting the currently operating master. A start condition can be created for example by an electromagnetic discharge applied near physical I2C lines. Or a already operating master could get reset immediately after sending a start. If it is known during device tree creation that only a single I2C master will be present on the bus, this deadlock of the I2C bus could be avoided in the driver by ignoring the bus_is_busy register of the xiic, since bus can never be reserved by any other master. This patch adds this support for detecting single-master flag in device tree and when provided, improves I2C reliability by ignoring the therefore unnecessary xiic bus_is_busy register. Error can be reproduced by pulling I2C SDA -line temporarily low by shorting it to ground, while linux I2C master is operating on it using the xiic driver. The application using the bus will start receiving linux error code 16: "Device or resource busy" indefinitely: kernel: pca953x 0-0020: failed writing register app: Error writing file, error: 16 With multi-master disabled device will instead receive error code 5: "I/O error" while SDA is grounded, but recover normal operation once short is removed. kernel: pca953x 0-0020: failed reading register app: Error reading file, error: 5 Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> --- drivers/i2c/busses/i2c-xiic.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 1453d82bb664..087b2951942e 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -59,6 +59,7 @@ enum xiic_endian { * @endianness: big/little-endian byte order * @clk: Pointer to AXI4-lite input clock * @state: See STATE_ + * @singlemaster: Indicates bus is single master */ struct xiic_i2c { struct device *dev; @@ -74,6 +75,7 @@ struct xiic_i2c { enum xiic_endian endianness; struct clk *clk; enum xilinx_i2c_state state; + bool singlemaster; }; @@ -526,6 +528,15 @@ static int xiic_busy(struct xiic_i2c *i2c) if (i2c->tx_msg) return -EBUSY; + /* In single master mode bus can only be busy, when in use by this + * driver. If the register indicates bus being busy for some reason we + * should ignore it, since bus will never be released and i2c will be + * stuck forever. + */ + if (i2c->singlemaster) { + return 0; + } + /* for instance if previous transfer was terminated due to TX error * it might be that the bus is on it's way to become available * give it at most 3 ms to wake @@ -811,6 +822,9 @@ static int xiic_i2c_probe(struct platform_device *pdev) goto err_clk_dis; } + i2c->singlemaster = + of_property_read_bool(pdev->dev.of_node, "single-master"); + /* * Detect endianness * Try to reset the TX FIFO. Then check the EMPTY flag. If it is not -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] i2c: xiic: Support forcing single-master in DT 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-27 15:57 ` Michal Simek -1 siblings, 0 replies; 18+ messages in thread From: Michal Simek @ 2020-08-27 15:57 UTC (permalink / raw) To: ext-jaakko.laine, wsa; +Cc: linux-i2c, michal.simek, linux-arm-kernel On 27. 08. 20 16:48, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > I2C master operating in multimaster mode can get stuck > indefinitely if I2C start is detected on bus, but no master > has a transaction going. > > This is a weakness in I2C standard, which defines no way > to recover, since all masters are indefinitely disallowed > from interrupting the currently operating master. A start > condition can be created for example by an electromagnetic > discharge applied near physical I2C lines. Or a already > operating master could get reset immediately after sending > a start. > > If it is known during device tree creation that only a single > I2C master will be present on the bus, this deadlock of the > I2C bus could be avoided in the driver by ignoring the > bus_is_busy register of the xiic, since bus can never be > reserved by any other master. > > This patch adds this support for detecting single-master flag > in device tree and when provided, improves I2C reliability by > ignoring the therefore unnecessary xiic bus_is_busy register. > > Error can be reproduced by pulling I2C SDA -line temporarily low > by shorting it to ground, while linux I2C master is operating on > it using the xiic driver. The application using the bus will > start receiving linux error code 16: "Device or resource busy" > indefinitely: > > kernel: pca953x 0-0020: failed writing register > app: Error writing file, error: 16 > > With multi-master disabled device will instead receive error > code 5: "I/O error" while SDA is grounded, but recover normal > operation once short is removed. > > kernel: pca953x 0-0020: failed reading register > app: Error reading file, error: 5 > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > --- > drivers/i2c/busses/i2c-xiic.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index 1453d82bb664..087b2951942e 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -59,6 +59,7 @@ enum xiic_endian { > * @endianness: big/little-endian byte order > * @clk: Pointer to AXI4-lite input clock > * @state: See STATE_ > + * @singlemaster: Indicates bus is single master > */ > struct xiic_i2c { > struct device *dev; > @@ -74,6 +75,7 @@ struct xiic_i2c { > enum xiic_endian endianness; > struct clk *clk; > enum xilinx_i2c_state state; > + bool singlemaster; > }; > > > @@ -526,6 +528,15 @@ static int xiic_busy(struct xiic_i2c *i2c) > if (i2c->tx_msg) > return -EBUSY; > > + /* In single master mode bus can only be busy, when in use by this > + * driver. If the register indicates bus being busy for some reason we > + * should ignore it, since bus will never be released and i2c will be > + * stuck forever. > + */ > + if (i2c->singlemaster) { > + return 0; > + } > + > /* for instance if previous transfer was terminated due to TX error > * it might be that the bus is on it's way to become available > * give it at most 3 ms to wake > @@ -811,6 +822,9 @@ static int xiic_i2c_probe(struct platform_device *pdev) > goto err_clk_dis; > } > > + i2c->singlemaster = > + of_property_read_bool(pdev->dev.of_node, "single-master"); > + > /* > * Detect endianness > * Try to reset the TX FIFO. Then check the EMPTY flag. If it is not > Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] i2c: xiic: Support forcing single-master in DT @ 2020-08-27 15:57 ` Michal Simek 0 siblings, 0 replies; 18+ messages in thread From: Michal Simek @ 2020-08-27 15:57 UTC (permalink / raw) To: ext-jaakko.laine, wsa; +Cc: linux-i2c, linux-arm-kernel, michal.simek On 27. 08. 20 16:48, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > I2C master operating in multimaster mode can get stuck > indefinitely if I2C start is detected on bus, but no master > has a transaction going. > > This is a weakness in I2C standard, which defines no way > to recover, since all masters are indefinitely disallowed > from interrupting the currently operating master. A start > condition can be created for example by an electromagnetic > discharge applied near physical I2C lines. Or a already > operating master could get reset immediately after sending > a start. > > If it is known during device tree creation that only a single > I2C master will be present on the bus, this deadlock of the > I2C bus could be avoided in the driver by ignoring the > bus_is_busy register of the xiic, since bus can never be > reserved by any other master. > > This patch adds this support for detecting single-master flag > in device tree and when provided, improves I2C reliability by > ignoring the therefore unnecessary xiic bus_is_busy register. > > Error can be reproduced by pulling I2C SDA -line temporarily low > by shorting it to ground, while linux I2C master is operating on > it using the xiic driver. The application using the bus will > start receiving linux error code 16: "Device or resource busy" > indefinitely: > > kernel: pca953x 0-0020: failed writing register > app: Error writing file, error: 16 > > With multi-master disabled device will instead receive error > code 5: "I/O error" while SDA is grounded, but recover normal > operation once short is removed. > > kernel: pca953x 0-0020: failed reading register > app: Error reading file, error: 5 > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> > --- > drivers/i2c/busses/i2c-xiic.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index 1453d82bb664..087b2951942e 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -59,6 +59,7 @@ enum xiic_endian { > * @endianness: big/little-endian byte order > * @clk: Pointer to AXI4-lite input clock > * @state: See STATE_ > + * @singlemaster: Indicates bus is single master > */ > struct xiic_i2c { > struct device *dev; > @@ -74,6 +75,7 @@ struct xiic_i2c { > enum xiic_endian endianness; > struct clk *clk; > enum xilinx_i2c_state state; > + bool singlemaster; > }; > > > @@ -526,6 +528,15 @@ static int xiic_busy(struct xiic_i2c *i2c) > if (i2c->tx_msg) > return -EBUSY; > > + /* In single master mode bus can only be busy, when in use by this > + * driver. If the register indicates bus being busy for some reason we > + * should ignore it, since bus will never be released and i2c will be > + * stuck forever. > + */ > + if (i2c->singlemaster) { > + return 0; > + } > + > /* for instance if previous transfer was terminated due to TX error > * it might be that the bus is on it's way to become available > * give it at most 3 ms to wake > @@ -811,6 +822,9 @@ static int xiic_i2c_probe(struct platform_device *pdev) > goto err_clk_dis; > } > > + i2c->singlemaster = > + of_property_read_bool(pdev->dev.of_node, "single-master"); > + > /* > * Detect endianness > * Try to reset the TX FIFO. Then check the EMPTY flag. If it is not > Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] i2c: xiic: Support forcing single-master in DT 2020-08-27 14:48 ` ext-jaakko.laine @ 2020-08-28 7:02 ` Wolfram Sang -1 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2020-08-28 7:02 UTC (permalink / raw) To: ext-jaakko.laine; +Cc: linux-i2c, michal.simek, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 1889 bytes --] On Thu, Aug 27, 2020 at 05:48:48PM +0300, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > I2C master operating in multimaster mode can get stuck > indefinitely if I2C start is detected on bus, but no master > has a transaction going. > > This is a weakness in I2C standard, which defines no way > to recover, since all masters are indefinitely disallowed > from interrupting the currently operating master. A start > condition can be created for example by an electromagnetic > discharge applied near physical I2C lines. Or a already > operating master could get reset immediately after sending > a start. > > If it is known during device tree creation that only a single > I2C master will be present on the bus, this deadlock of the > I2C bus could be avoided in the driver by ignoring the > bus_is_busy register of the xiic, since bus can never be > reserved by any other master. > > This patch adds this support for detecting single-master flag > in device tree and when provided, improves I2C reliability by > ignoring the therefore unnecessary xiic bus_is_busy register. > > Error can be reproduced by pulling I2C SDA -line temporarily low > by shorting it to ground, while linux I2C master is operating on > it using the xiic driver. The application using the bus will > start receiving linux error code 16: "Device or resource busy" > indefinitely: > > kernel: pca953x 0-0020: failed writing register > app: Error writing file, error: 16 > > With multi-master disabled device will instead receive error > code 5: "I/O error" while SDA is grounded, but recover normal > operation once short is removed. > > kernel: pca953x 0-0020: failed reading register > app: Error reading file, error: 5 > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] i2c: xiic: Support forcing single-master in DT @ 2020-08-28 7:02 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2020-08-28 7:02 UTC (permalink / raw) To: ext-jaakko.laine; +Cc: linux-i2c, linux-arm-kernel, michal.simek [-- Attachment #1.1: Type: text/plain, Size: 1889 bytes --] On Thu, Aug 27, 2020 at 05:48:48PM +0300, ext-jaakko.laine@vaisala.com wrote: > From: Jaakko Laine <ext-jaakko.laine@vaisala.com> > > I2C master operating in multimaster mode can get stuck > indefinitely if I2C start is detected on bus, but no master > has a transaction going. > > This is a weakness in I2C standard, which defines no way > to recover, since all masters are indefinitely disallowed > from interrupting the currently operating master. A start > condition can be created for example by an electromagnetic > discharge applied near physical I2C lines. Or a already > operating master could get reset immediately after sending > a start. > > If it is known during device tree creation that only a single > I2C master will be present on the bus, this deadlock of the > I2C bus could be avoided in the driver by ignoring the > bus_is_busy register of the xiic, since bus can never be > reserved by any other master. > > This patch adds this support for detecting single-master flag > in device tree and when provided, improves I2C reliability by > ignoring the therefore unnecessary xiic bus_is_busy register. > > Error can be reproduced by pulling I2C SDA -line temporarily low > by shorting it to ground, while linux I2C master is operating on > it using the xiic driver. The application using the bus will > start receiving linux error code 16: "Device or resource busy" > indefinitely: > > kernel: pca953x 0-0020: failed writing register > app: Error writing file, error: 16 > > With multi-master disabled device will instead receive error > code 5: "I/O error" while SDA is grounded, but recover normal > operation once short is removed. > > kernel: pca953x 0-0020: failed reading register > app: Error reading file, error: 5 > > Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com> Applied to for-next, thanks! [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-08-28 7:04 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-27 14:48 [PATCH 0/3] i2c: xiic: Support forcing single-master in DT ext-jaakko.laine 2020-08-27 14:48 ` ext-jaakko.laine 2020-08-27 14:48 ` [PATCH 1/3] i2c: xiic: Change code alignment to 1 space only ext-jaakko.laine 2020-08-27 14:48 ` ext-jaakko.laine 2020-08-28 7:02 ` Wolfram Sang 2020-08-28 7:02 ` Wolfram Sang 2020-08-27 14:48 ` [PATCH 2/3] i2c: xiic: Improve struct memory alignment ext-jaakko.laine 2020-08-27 14:48 ` ext-jaakko.laine 2020-08-27 15:56 ` Michal Simek 2020-08-27 15:56 ` Michal Simek 2020-08-28 7:02 ` Wolfram Sang 2020-08-28 7:02 ` Wolfram Sang 2020-08-27 14:48 ` [PATCH 3/3] i2c: xiic: Support forcing single-master in DT ext-jaakko.laine 2020-08-27 14:48 ` ext-jaakko.laine 2020-08-27 15:57 ` Michal Simek 2020-08-27 15:57 ` Michal Simek 2020-08-28 7:02 ` Wolfram Sang 2020-08-28 7:02 ` Wolfram Sang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.