diff mbox series

[v4,1/6] i2c: npcm: correct the read/write operation procedure

Message ID 20240920101820.44850-2-kfting@nuvoton.com
State New
Headers show
Series i2c: npcm: read/write operation, checkpatch | expand

Commit Message

Tyrone Ting Sept. 20, 2024, 10:18 a.m. UTC 
From: Tyrone Ting <kfting@nuvoton.com>

From: Tyrone Ting <kfting@nuvoton.com>

Originally the driver uses the XMIT bit in SMBnST register to decide
the upcoming i2c transaction. If XMIT bit is 1, then it will be an i2c
write operation. If it's 0, then a read operation will be executed.

In slave mode the XMIT bit can simply be used directly to set the state.
XMIT bit can be used as an indication to the current state of the state
machine during slave operation. (meaning XMIT = 1 during writing and
XMIT = 0 during reading).

In master operation XMIT is valid only if there are no bus errors.
For example: in a multi master where the same module is switching from
master to slave at runtime, and there are collisions, the XMIT bit
cannot be trusted.

However the maser already "knows" what the bus state is, so this bit
is not needed and the driver can just track what it is currently doing.

Signed-off-by: Tyrone Ting <kfting@nuvoton.com>
Reviewed-by: Tali Perry <tali.perry1@gmail.com>
---
 drivers/i2c/busses/i2c-npcm7xx.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Andy Shevchenko Sept. 20, 2024, 2:30 p.m. UTC | #1 
On Fri, Sep 20, 2024 at 06:18:15PM +0800, warp5tw@gmail.com wrote:
> From: Tyrone Ting <kfting@nuvoton.com>
> 
> From: Tyrone Ting <kfting@nuvoton.com>

Something wrong with your email setup. This patch may not be applied in this form.

But before (re-)sending, wait for others to comment (up to 1 week).
Tyrone Ting Sept. 23, 2024, 1:51 a.m. UTC | #2 
Hi Andy:

Thank you for your feedback.

Andy Shevchenko <andriy.shevchenko@linux.intel.com> 於 2024年9月20日 週五 下午10:30寫道：
>
> On Fri, Sep 20, 2024 at 06:18:15PM +0800, warp5tw@gmail.com wrote:
> > From: Tyrone Ting <kfting@nuvoton.com>
> >
> > From: Tyrone Ting <kfting@nuvoton.com>
>
> Something wrong with your email setup. This patch may not be applied in this form.
>
> But before (re-)sending, wait for others to comment (up to 1 week).
>

I'll remove the "--from" parameter to git send-email command in next patch set.

> --
> With Best Regards,
> Andy Shevchenko
>
>

Thank you.

Regards,
Tyrone
Andi Shyti Sept. 25, 2024, 8:19 p.m. UTC | #3 
Hi Tyrone,

On Fri, Sep 20, 2024 at 06:18:15PM GMT, warp5tw@gmail.com wrote:
> From: Tyrone Ting <kfting@nuvoton.com>
> 
> From: Tyrone Ting <kfting@nuvoton.com>

no worries, I can take care of this.

> Originally the driver uses the XMIT bit in SMBnST register to decide
> the upcoming i2c transaction. If XMIT bit is 1, then it will be an i2c
> write operation. If it's 0, then a read operation will be executed.
> 
> In slave mode the XMIT bit can simply be used directly to set the state.
> XMIT bit can be used as an indication to the current state of the state
> machine during slave operation. (meaning XMIT = 1 during writing and
> XMIT = 0 during reading).
> 
> In master operation XMIT is valid only if there are no bus errors.
> For example: in a multi master where the same module is switching from
> master to slave at runtime, and there are collisions, the XMIT bit
> cannot be trusted.
> 
> However the maser already "knows" what the bus state is, so this bit
> is not needed and the driver can just track what it is currently doing.
> 
> Signed-off-by: Tyrone Ting <kfting@nuvoton.com>
> Reviewed-by: Tali Perry <tali.perry1@gmail.com>

This patch is independent from the rest of the series, can I
start takin this in and unburden you from this?

Andi
Tyrone Ting Sept. 27, 2024, 1:59 a.m. UTC | #4 
Hi Andi:

Thank you for your help.

Andi Shyti <andi.shyti@kernel.org> 於 2024年9月26日 週四 上午4:19寫道：
>
> Hi Tyrone,
>
> On Fri, Sep 20, 2024 at 06:18:15PM GMT, warp5tw@gmail.com wrote:
> > From: Tyrone Ting <kfting@nuvoton.com>
> >
> > From: Tyrone Ting <kfting@nuvoton.com>
>
> no worries, I can take care of this.
>
> > Originally the driver uses the XMIT bit in SMBnST register to decide
> > the upcoming i2c transaction. If XMIT bit is 1, then it will be an i2c
> > write operation. If it's 0, then a read operation will be executed.
> >
> > In slave mode the XMIT bit can simply be used directly to set the state.
> > XMIT bit can be used as an indication to the current state of the state
> > machine during slave operation. (meaning XMIT = 1 during writing and
> > XMIT = 0 during reading).
> >
> > In master operation XMIT is valid only if there are no bus errors.
> > For example: in a multi master where the same module is switching from
> > master to slave at runtime, and there are collisions, the XMIT bit
> > cannot be trusted.
> >
> > However the maser already "knows" what the bus state is, so this bit
> > is not needed and the driver can just track what it is currently doing.
> >
> > Signed-off-by: Tyrone Ting <kfting@nuvoton.com>
> > Reviewed-by: Tali Perry <tali.perry1@gmail.com>
>
> This patch is independent from the rest of the series, can I
> start takin this in and unburden you from this?
>

Sure, thank you again.

> Andi

Regards,
Tyrone
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c
index bbcb4d6668ce..2b76dbfba438 100644
--- a/drivers/i2c/busses/i2c-npcm7xx.c
+++ b/drivers/i2c/busses/i2c-npcm7xx.c
@@ -1628,13 +1628,10 @@  static void npcm_i2c_irq_handle_sda(struct npcm_i2c *bus, u8 i2cst)
 			npcm_i2c_wr_byte(bus, bus->dest_addr | BIT(0));
 	/* SDA interrupt, after start\restart */
 	} else {
-		if (NPCM_I2CST_XMIT & i2cst) {
-			bus->operation = I2C_WRITE_OPER;
+		if (bus->operation == I2C_WRITE_OPER)
 			npcm_i2c_irq_master_handler_write(bus);
-		} else {
-			bus->operation = I2C_READ_OPER;
+		else if (bus->operation == I2C_READ_OPER)
 			npcm_i2c_irq_master_handler_read(bus);
-		}
 	}
 }