[PATCH v2 00/10] i2c: busses: Introduce and use i2c_10bit_addr_*_from_msg()

[PATCH v2 00/10] i2c: busses: Introduce and use i2c_10bit_addr_*_from_msg()

[Andy Shevchenko]

For 8-bit addresses we have a helper function, define similar ones
for 10-bit addresses and use it in the drivers. It allows to remove
some boilerplate code.

Fabrizio, I haven't collected your tags as code changed more than 50%, however
semantically the parts you reviewed stay the same.

In v2:
- introduced hi/lo helpers (Geert)
- rewrote series based on the above
- added a couple of new converted drivers

v1: https://lore.kernel.org/r/20250212163359.2407327-1-andriy.shevchenko@linux.intel.com

Andy Shevchenko (10):
  i2c: Introduce i2c_10bit_addr_*_from_msg() helpers
  i2c: axxia: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: bcm-kona: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: brcmstb: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: eg20t: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: kempld: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: mt7621: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: rzv2m: Use i2c_10bit_addr_*_from_msg() helpers
  i2c: ibm_iic: Use i2c_*bit_addr*_from_msg() helpers
  i2c: mv64xxx: Use i2c_*bit_addr*_from_msg() helpers

 drivers/i2c/busses/i2c-axxia.c    | 21 +++------------------
 drivers/i2c/busses/i2c-bcm-kona.c |  6 +++---
 drivers/i2c/busses/i2c-brcmstb.c  | 11 +++++------
 drivers/i2c/busses/i2c-eg20t.c    | 28 +++++-----------------------
 drivers/i2c/busses/i2c-ibm_iic.c  | 14 ++++++--------
 drivers/i2c/busses/i2c-kempld.c   | 10 +++++-----
 drivers/i2c/busses/i2c-mt7621.c   | 20 ++++++++------------
 drivers/i2c/busses/i2c-mv64xxx.c  | 12 +++---------
 drivers/i2c/busses/i2c-rzv2m.c    | 15 +++++----------
 include/linux/i2c.h               | 14 ++++++++++++++
 10 files changed, 57 insertions(+), 94 deletions(-)

-- 
2.45.1.3035.g276e886db78b

[PATCH v2 01/10] i2c: Introduce i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

There are already a lot of drivers that have been using
i2c_8bit_addr_from_msg() for 7-bit addresses, now it's time
to have the similar for 10-bit addresses.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/i2c.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 997e80649889..e23b373b6b57 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -952,6 +952,20 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
 	return (msg->addr << 1) | (msg->flags & I2C_M_RD);
 }
 
+/*
+ * 10-bit address
+ *   addr_1: 5'b11110 | addr[9:8] | (R/nW)
+ *   addr_2: addr[7:0]
+ */
+static inline u8 i2c_10bit_addr_hi_from_msg(const struct i2c_msg *msg)
+{
+	return 0xf0 | ((msg->addr & GENMASK(9, 8)) >> 7) | (msg->flags & I2C_M_RD);
+}
+static inline u8 i2c_10bit_addr_lo_from_msg(const struct i2c_msg *msg)
+{
+	return msg->addr & GENMASK(7, 0);
+}
+
 u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
 void i2c_put_dma_safe_msg_buf(u8 *buf, struct i2c_msg *msg, bool xferred);
 
-- 
2.45.1.3035.g276e886db78b

Re: [PATCH v2 01/10] i2c: Introduce i2c_10bit_addr_*_from_msg() helpers

[AngeloGioacchino Del Regno]

Il 13/02/25 15:07, Andy Shevchenko ha scritto:
> There are already a lot of drivers that have been using
> i2c_8bit_addr_from_msg() for 7-bit addresses, now it's time
> to have the similar for 10-bit addresses.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v2 02/10] i2c: axxia: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-axxia.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-axxia.c b/drivers/i2c/busses/i2c-axxia.c
index 48916cf45ff7..50030256cd85 100644
--- a/drivers/i2c/busses/i2c-axxia.c
+++ b/drivers/i2c/busses/i2c-axxia.c
@@ -255,11 +255,6 @@ static int i2c_m_rd(const struct i2c_msg *msg)
 	return (msg->flags & I2C_M_RD) != 0;
 }
 
-static int i2c_m_ten(const struct i2c_msg *msg)
-{
-	return (msg->flags & I2C_M_TEN) != 0;
-}
-
 static int i2c_m_recv_len(const struct i2c_msg *msg)
 {
 	return (msg->flags & I2C_M_RECV_LEN) != 0;
@@ -439,20 +434,10 @@ static void axxia_i2c_set_addr(struct axxia_i2c_dev *idev, struct i2c_msg *msg)
 {
 	u32 addr_1, addr_2;
 
-	if (i2c_m_ten(msg)) {
-		/* 10-bit address
-		 *   addr_1: 5'b11110 | addr[9:8] | (R/nW)
-		 *   addr_2: addr[7:0]
-		 */
-		addr_1 = 0xF0 | ((msg->addr >> 7) & 0x06);
-		if (i2c_m_rd(msg))
-			addr_1 |= 1;	/* Set the R/nW bit of the address */
-		addr_2 = msg->addr & 0xFF;
+	if (msg->flags & I2C_M_TEN) {
+		addr_1 = i2c_10bit_addr_hi_from_msg(msg);
+		addr_2 = i2c_10bit_addr_lo_from_msg(msg);
 	} else {
-		/* 7-bit address
-		 *   addr_1: addr[6:0] | (R/nW)
-		 *   addr_2: dont care
-		 */
 		addr_1 = i2c_8bit_addr_from_msg(msg);
 		addr_2 = 0;
 	}
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 03/10] i2c: bcm-kona: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-bcm-kona.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
index 340fe1305dd9..9d8838bbd938 100644
--- a/drivers/i2c/busses/i2c-bcm-kona.c
+++ b/drivers/i2c/busses/i2c-bcm-kona.c
@@ -471,12 +471,12 @@ static int bcm_kona_i2c_do_addr(struct bcm_kona_i2c_dev *dev,
 
 	if (msg->flags & I2C_M_TEN) {
 		/* First byte is 11110XX0 where XX is upper 2 bits */
-		addr = 0xF0 | ((msg->addr & 0x300) >> 7);
+		addr = i2c_10bit_addr_hi_from_msg(msg) & ~I2C_M_RD;
 		if (bcm_kona_i2c_write_byte(dev, addr, 0) < 0)
 			return -EREMOTEIO;
 
 		/* Second byte is the remaining 8 bits */
-		addr = msg->addr & 0xFF;
+		addr = i2c_10bit_addr_lo_from_msg(msg);
 		if (bcm_kona_i2c_write_byte(dev, addr, 0) < 0)
 			return -EREMOTEIO;
 
@@ -486,7 +486,7 @@ static int bcm_kona_i2c_do_addr(struct bcm_kona_i2c_dev *dev,
 				return -EREMOTEIO;
 
 			/* Then re-send the first byte with the read bit set */
-			addr = 0xF0 | ((msg->addr & 0x300) >> 7) | 0x01;
+			addr = i2c_10bit_addr_hi_from_msg(msg);
 			if (bcm_kona_i2c_write_byte(dev, addr, 0) < 0)
 				return -EREMOTEIO;
 		}
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 04/10] i2c: brcmstb: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-brcmstb.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c
index 00f1a046e985..5fa30e8926c5 100644
--- a/drivers/i2c/busses/i2c-brcmstb.c
+++ b/drivers/i2c/busses/i2c-brcmstb.c
@@ -414,23 +414,22 @@ static int brcmstb_i2c_do_addr(struct brcmstb_i2c_dev *dev,
 
 	if (msg->flags & I2C_M_TEN) {
 		/* First byte is 11110XX0 where XX is upper 2 bits */
-		addr = 0xF0 | ((msg->addr & 0x300) >> 7);
+		addr = i2c_10bit_addr_hi_from_msg(msg) & ~I2C_M_RD;
 		bsc_writel(dev, addr, chip_address);
 
 		/* Second byte is the remaining 8 bits */
-		addr = msg->addr & 0xFF;
+		addr = i2c_10bit_addr_lo_from_msg(msg);
 		if (brcmstb_i2c_write_data_byte(dev, &addr, 0) < 0)
 			return -EREMOTEIO;
 
 		if (msg->flags & I2C_M_RD) {
 			/* For read, send restart without stop condition */
-			brcmstb_set_i2c_start_stop(dev, COND_RESTART
-						   | COND_NOSTOP);
+			brcmstb_set_i2c_start_stop(dev, COND_RESTART | COND_NOSTOP);
+
 			/* Then re-send the first byte with the read bit set */
-			addr = 0xF0 | ((msg->addr & 0x300) >> 7) | 0x01;
+			addr = i2c_10bit_addr_hi_from_msg(msg);
 			if (brcmstb_i2c_write_data_byte(dev, &addr, 0) < 0)
 				return -EREMOTEIO;
-
 		}
 	} else {
 		addr = i2c_8bit_addr_from_msg(msg);
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 05/10] i2c: eg20t: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-eg20t.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index 4914bfbee2a9..efdaddf99f9e 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -48,8 +48,6 @@
 
 #define BUS_IDLE_TIMEOUT	20
 #define PCH_I2CCTL_I2CMEN	0x0080
-#define TEN_BIT_ADDR_DEFAULT	0xF000
-#define TEN_BIT_ADDR_MASK	0xF0
 #define PCH_START		0x0020
 #define PCH_RESTART		0x0004
 #define PCH_ESR_START		0x0001
@@ -58,7 +56,6 @@
 #define PCH_ACK			0x0008
 #define PCH_GETACK		0x0001
 #define CLR_REG			0x0
-#define I2C_RD			0x1
 #define I2CMCF_BIT		0x0080
 #define I2CMIF_BIT		0x0002
 #define I2CMAL_BIT		0x0010
@@ -76,8 +73,6 @@
 #define I2CMBB_BIT		0x0020
 #define BUFFER_MODE_MASK	(I2CBMFI_BIT | I2CBMAL_BIT | I2CBMNA_BIT | \
 				I2CBMTO_BIT | I2CBMIS_BIT)
-#define I2C_ADDR_MSK		0xFF
-#define I2C_MSB_2B_MSK		0x300
 #define FAST_MODE_CLK		400
 #define FAST_MODE_EN		0x0001
 #define SUB_ADDR_LEN_MAX	4
@@ -371,16 +366,12 @@ static s32 pch_i2c_writebytes(struct i2c_adapter *i2c_adap,
 	struct i2c_algo_pch_data *adap = i2c_adap->algo_data;
 	u8 *buf;
 	u32 length;
-	u32 addr;
-	u32 addr_2_msb;
-	u32 addr_8_lsb;
 	s32 wrcount;
 	s32 rtn;
 	void __iomem *p = adap->pch_base_address;
 
 	length = msgs->len;
 	buf = msgs->buf;
-	addr = msgs->addr;
 
 	/* enable master tx */
 	pch_setbit(adap->pch_base_address, PCH_I2CCTL, I2C_TX_MODE);
@@ -394,8 +385,7 @@ static s32 pch_i2c_writebytes(struct i2c_adapter *i2c_adap,
 	}
 
 	if (msgs->flags & I2C_M_TEN) {
-		addr_2_msb = ((addr & I2C_MSB_2B_MSK) >> 7) & 0x06;
-		iowrite32(addr_2_msb | TEN_BIT_ADDR_MASK, p + PCH_I2CDR);
+		iowrite32(i2c_10bit_addr_hi_from_msg(msgs), p + PCH_I2CDR);
 		if (first)
 			pch_i2c_start(adap);
 
@@ -403,8 +393,7 @@ static s32 pch_i2c_writebytes(struct i2c_adapter *i2c_adap,
 		if (rtn)
 			return rtn;
 
-		addr_8_lsb = (addr & I2C_ADDR_MSK);
-		iowrite32(addr_8_lsb, p + PCH_I2CDR);
+		iowrite32(i2c_10bit_addr_lo_from_msg(msgs), p + PCH_I2CDR);
 	} else {
 		/* set 7 bit slave address and R/W bit as 0 */
 		iowrite32(i2c_8bit_addr_from_msg(msgs), p + PCH_I2CDR);
@@ -490,15 +479,11 @@ static s32 pch_i2c_readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs,
 	u8 *buf;
 	u32 count;
 	u32 length;
-	u32 addr;
-	u32 addr_2_msb;
-	u32 addr_8_lsb;
 	void __iomem *p = adap->pch_base_address;
 	s32 rtn;
 
 	length = msgs->len;
 	buf = msgs->buf;
-	addr = msgs->addr;
 
 	/* enable master reception */
 	pch_clrbit(adap->pch_base_address, PCH_I2CCTL, I2C_TX_MODE);
@@ -509,8 +494,7 @@ static s32 pch_i2c_readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs,
 	}
 
 	if (msgs->flags & I2C_M_TEN) {
-		addr_2_msb = ((addr & I2C_MSB_2B_MSK) >> 7);
-		iowrite32(addr_2_msb | TEN_BIT_ADDR_MASK, p + PCH_I2CDR);
+		iowrite32(i2c_10bit_addr_hi_from_msg(msgs) & ~I2C_M_RD, p + PCH_I2CDR);
 		if (first)
 			pch_i2c_start(adap);
 
@@ -518,8 +502,7 @@ static s32 pch_i2c_readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs,
 		if (rtn)
 			return rtn;
 
-		addr_8_lsb = (addr & I2C_ADDR_MSK);
-		iowrite32(addr_8_lsb, p + PCH_I2CDR);
+		iowrite32(i2c_10bit_addr_lo_from_msg(msgs), p + PCH_I2CDR);
 
 		pch_i2c_restart(adap);
 
@@ -527,8 +510,7 @@ static s32 pch_i2c_readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs,
 		if (rtn)
 			return rtn;
 
-		addr_2_msb |= I2C_RD;
-		iowrite32(addr_2_msb | TEN_BIT_ADDR_MASK, p + PCH_I2CDR);
+		iowrite32(i2c_10bit_addr_hi_from_msg(msgs), p + PCH_I2CDR);
 	} else {
 		/* 7 address bits + R/W bit */
 		iowrite32(i2c_8bit_addr_from_msg(msgs), p + PCH_I2CDR);
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 06/10] i2c: kempld: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-kempld.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-kempld.c b/drivers/i2c/busses/i2c-kempld.c
index 212196af68ba..9b4c7cba62b6 100644
--- a/drivers/i2c/busses/i2c-kempld.c
+++ b/drivers/i2c/busses/i2c-kempld.c
@@ -115,9 +115,7 @@ static int kempld_i2c_process(struct kempld_i2c_data *i2c)
 	if (i2c->state == STATE_ADDR) {
 		/* 10 bit address? */
 		if (i2c->msg->flags & I2C_M_TEN) {
-			addr = 0xf0 | ((i2c->msg->addr >> 7) & 0x6);
-			/* Set read bit if necessary */
-			addr |= (i2c->msg->flags & I2C_M_RD) ? 1 : 0;
+			addr = i2c_10bit_addr_hi_from_msg(msg);
 			i2c->state = STATE_ADDR10;
 		} else {
 			addr = i2c_8bit_addr_from_msg(i2c->msg);
@@ -132,10 +130,12 @@ static int kempld_i2c_process(struct kempld_i2c_data *i2c)
 
 	/* Second part of 10 bit addressing */
 	if (i2c->state == STATE_ADDR10) {
-		kempld_write8(pld, KEMPLD_I2C_DATA, i2c->msg->addr & 0xff);
+		addr = i2c_10bit_addr_lo_from_msg(msg);
+		i2c->state = STATE_START;
+
+		kempld_write8(pld, KEMPLD_I2C_DATA, addr);
 		kempld_write8(pld, KEMPLD_I2C_CMD, I2C_CMD_WRITE);
 
-		i2c->state = STATE_START;
 		return 0;
 	}
 
-- 
2.45.1.3035.g276e886db78b

Re: [PATCH v2 06/10] i2c: kempld: Use i2c_10bit_addr_*_from_msg() helpers

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 565 bytes --]


> @@ -132,10 +130,12 @@ static int kempld_i2c_process(struct kempld_i2c_data *i2c)
>  
>  	/* Second part of 10 bit addressing */
>  	if (i2c->state == STATE_ADDR10) {
> -		kempld_write8(pld, KEMPLD_I2C_DATA, i2c->msg->addr & 0xff);
> +		addr = i2c_10bit_addr_lo_from_msg(msg);
> +		i2c->state = STATE_START;

Any reason you moved this?

> +
> +		kempld_write8(pld, KEMPLD_I2C_DATA, addr);

Maybe we could skip using 'addr' here?

>  		kempld_write8(pld, KEMPLD_I2C_CMD, I2C_CMD_WRITE);
>  
> -		i2c->state = STATE_START;
>  		return 0;
>  	}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 06/10] i2c: kempld: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

On Thu, Feb 13, 2025 at 05:15:09PM +0100, Wolfram Sang wrote:

...

> >  	/* Second part of 10 bit addressing */
> >  	if (i2c->state == STATE_ADDR10) {
> > -		kempld_write8(pld, KEMPLD_I2C_DATA, i2c->msg->addr & 0xff);
> > +		addr = i2c_10bit_addr_lo_from_msg(msg);
> > +		i2c->state = STATE_START;
> 
> Any reason you moved this?

Yes, I would like to be in sync in the above state machine case, just upper in
the code which is not visible in this patch.

> > +		kempld_write8(pld, KEMPLD_I2C_DATA, addr);
> 
> Maybe we could skip using 'addr' here?

Same reason as above.

> >  		kempld_write8(pld, KEMPLD_I2C_CMD, I2C_CMD_WRITE);
> >  
> > -		i2c->state = STATE_START;
> >  		return 0;
> >  	}

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 07/10] i2c: mt7621: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-mt7621.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
index 2103f21f9ddd..0a288c998419 100644
--- a/drivers/i2c/busses/i2c-mt7621.c
+++ b/drivers/i2c/busses/i2c-mt7621.c
@@ -164,22 +164,18 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		/* write address */
 		if (pmsg->flags & I2C_M_TEN) {
 			/* 10 bits address */
-			addr = 0xf0 | ((pmsg->addr >> 7) & 0x06);
-			addr |= (pmsg->addr & 0xff) << 8;
-			if (pmsg->flags & I2C_M_RD)
-				addr |= 1;
-			iowrite32(addr, i2c->base + REG_SM0D0_REG);
-			ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, 2);
-			if (ret)
-				goto err_timeout;
+			addr = i2c_10bit_addr_hi_from_msg(pmsg);
+			addr |= i2c_10bit_addr_lo_from_msg(pmsg) << 8;
+			len = 2;
 		} else {
 			/* 7 bits address */
 			addr = i2c_8bit_addr_from_msg(pmsg);
-			iowrite32(addr, i2c->base + REG_SM0D0_REG);
-			ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, 1);
-			if (ret)
-				goto err_timeout;
+			len = 1;
 		}
+		iowrite32(addr, i2c->base + REG_SM0D0_REG);
+		ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, len);
+		if (ret)
+			goto err_timeout;
 
 		/* check address ACK */
 		if (!(pmsg->flags & I2C_M_IGNORE_NAK)) {
-- 
2.45.1.3035.g276e886db78b

Re: [PATCH v2 07/10] i2c: mt7621: Use i2c_10bit_addr_*_from_msg() helpers

[AngeloGioacchino Del Regno]

Il 13/02/25 15:07, Andy Shevchenko ha scritto:
> Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Can we please do the helper conversion as one commit and the (much needed)
cleanup of assigning len and de-duplicating the call to mtk_i2c_cmd() as
two commits?

One with just the conversion, one with the cleanup (or in inverse order,
as you wish).

Thanks,
Angelo

> ---
>   drivers/i2c/busses/i2c-mt7621.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index 2103f21f9ddd..0a288c998419 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -164,22 +164,18 @@ static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>   		/* write address */
>   		if (pmsg->flags & I2C_M_TEN) {
>   			/* 10 bits address */
> -			addr = 0xf0 | ((pmsg->addr >> 7) & 0x06);
> -			addr |= (pmsg->addr & 0xff) << 8;
> -			if (pmsg->flags & I2C_M_RD)
> -				addr |= 1;
> -			iowrite32(addr, i2c->base + REG_SM0D0_REG);
> -			ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, 2);
> -			if (ret)
> -				goto err_timeout;
> +			addr = i2c_10bit_addr_hi_from_msg(pmsg);
> +			addr |= i2c_10bit_addr_lo_from_msg(pmsg) << 8;
> +			len = 2;
>   		} else {
>   			/* 7 bits address */
>   			addr = i2c_8bit_addr_from_msg(pmsg);
> -			iowrite32(addr, i2c->base + REG_SM0D0_REG);
> -			ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, 1);
> -			if (ret)
> -				goto err_timeout;
> +			len = 1;
>   		}
> +		iowrite32(addr, i2c->base + REG_SM0D0_REG);
> +		ret = mtk_i2c_cmd(i2c, SM0CTL1_WRITE, len);
> +		if (ret)
> +			goto err_timeout;
>   
>   		/* check address ACK */
>   		if (!(pmsg->flags & I2C_M_IGNORE_NAK)) {

Re: [PATCH v2 07/10] i2c: mt7621: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

On Thu, Feb 13, 2025 at 05:01:20PM +0100, AngeloGioacchino Del Regno wrote:
> Il 13/02/25 15:07, Andy Shevchenko ha scritto:
> > Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
> > No functional change intended.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Can we please do the helper conversion as one commit and the (much needed)
> cleanup of assigning len and de-duplicating the call to mtk_i2c_cmd() as
> two commits?
> 
> One with just the conversion, one with the cleanup (or in inverse order,
> as you wish).

Yes we can.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 08/10] i2c: rzv2m: Use i2c_10bit_addr_*_from_msg() helpers

[Andy Shevchenko]

Use i2c_10bit_addr_*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-rzv2m.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rzv2m.c b/drivers/i2c/busses/i2c-rzv2m.c
index 02b76e24a476..53762cc56d28 100644
--- a/drivers/i2c/busses/i2c-rzv2m.c
+++ b/drivers/i2c/busses/i2c-rzv2m.c
@@ -287,20 +287,15 @@ static int rzv2m_i2c_send_address(struct rzv2m_i2c_priv *priv,
 	int ret;
 
 	if (msg->flags & I2C_M_TEN) {
-		/*
-		 * 10-bit address
-		 *   addr_1: 5'b11110 | addr[9:8] | (R/nW)
-		 *   addr_2: addr[7:0]
-		 */
-		addr = 0xf0 | ((msg->addr & GENMASK(9, 8)) >> 7);
-		addr |= !!(msg->flags & I2C_M_RD);
-		/* Send 1st address(extend code) */
+		/* 10-bit address: Send 1st address(extend code) */
+		addr = i2c_10bit_addr_hi_from_msg(msg);
 		ret = rzv2m_i2c_write_with_ack(priv, addr);
 		if (ret)
 			return ret;
 
-		/* Send 2nd address */
-		ret = rzv2m_i2c_write_with_ack(priv, msg->addr & 0xff);
+		/* 10-bit address: Send 2nd address */
+		addr = i2c_10bit_addr_lo_from_msg(msg);
+		ret = rzv2m_i2c_write_with_ack(priv, addr);
 	} else {
 		/* 7-bit address */
 		addr = i2c_8bit_addr_from_msg(msg);
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 09/10] i2c: ibm_iic: Use i2c_*bit_addr*_from_msg() helpers

[Andy Shevchenko]

Use i2c_*bit_addr*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-ibm_iic.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index c76c4116ddc7..6bf45d752ff9 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -512,19 +512,17 @@ static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
 static inline void iic_address(struct ibm_iic_private* dev, struct i2c_msg* msg)
 {
 	volatile struct iic_regs __iomem *iic = dev->vaddr;
-	u16 addr = msg->addr;
 
 	DBG2("%d: iic_address, 0x%03x (%d-bit)\n", dev->idx,
-		addr, msg->flags & I2C_M_TEN ? 10 : 7);
+		msg->addr, msg->flags & I2C_M_TEN ? 10 : 7);
 
-	if (msg->flags & I2C_M_TEN){
+	if (msg->flags & I2C_M_TEN) {
 	    out_8(&iic->cntl, CNTL_AMD);
-	    out_8(&iic->lmadr, addr);
-	    out_8(&iic->hmadr, 0xf0 | ((addr >> 7) & 0x06));
-	}
-	else {
+	    out_8(&iic->lmadr, i2c_10bit_addr_lo_from_msg(msg));
+	    out_8(&iic->hmadr, i2c_10bit_addr_hi_from_msg(msg) & ~I2C_M_RD);
+	} else {
 	    out_8(&iic->cntl, 0);
-	    out_8(&iic->lmadr, addr << 1);
+	    out_8(&iic->lmadr, i2c_8bit_addr_from_msg(msg) & ~I2C_M_RD);
 	}
 }
 
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 10/10] i2c: mv64xxx: Use i2c_*bit_addr*_from_msg() helpers

[Andy Shevchenko]

Use i2c_*bit_addr*_from_msg() helpers instead of local copy.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 874309580c33..8fc26a511320 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -27,7 +27,6 @@
 #include <linux/err.h>
 #include <linux/delay.h>
 
-#define MV64XXX_I2C_ADDR_ADDR(val)			((val & 0x7f) << 1)
 #define MV64XXX_I2C_BAUD_DIV_N(val)			(val & 0x7)
 #define MV64XXX_I2C_BAUD_DIV_M(val)			((val & 0xf) << 3)
 
@@ -176,22 +175,17 @@ static void
 mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data *drv_data,
 	struct i2c_msg *msg)
 {
-	u32	dir = 0;
-
 	drv_data->cntl_bits = MV64XXX_I2C_REG_CONTROL_ACK |
 			      MV64XXX_I2C_REG_CONTROL_TWSIEN;
 
 	if (!drv_data->atomic)
 		drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_INTEN;
 
-	if (msg->flags & I2C_M_RD)
-		dir = 1;
-
 	if (msg->flags & I2C_M_TEN) {
-		drv_data->addr1 = 0xf0 | (((u32)msg->addr & 0x300) >> 7) | dir;
-		drv_data->addr2 = (u32)msg->addr & 0xff;
+		drv_data->addr1 = i2c_10bit_addr_hi_from_msg(msg);
+		drv_data->addr2 = i2c_10bit_addr_lo_from_msg(msg);
 	} else {
-		drv_data->addr1 = MV64XXX_I2C_ADDR_ADDR((u32)msg->addr) | dir;
+		drv_data->addr1 = i2c_8bit_addr_from_msg(msg);
 		drv_data->addr2 = 0;
 	}
 }
-- 
2.45.1.3035.g276e886db78b

Re: [PATCH v2 00/10] i2c: busses: Introduce and use i2c_10bit_addr_*_from_msg()

[Andi Shyti]

Hi Andy,

> Andy Shevchenko (10):
>   i2c: Introduce i2c_10bit_addr_*_from_msg() helpers
>   i2c: axxia: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: bcm-kona: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: brcmstb: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: eg20t: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: kempld: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: mt7621: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: rzv2m: Use i2c_10bit_addr_*_from_msg() helpers
>   i2c: ibm_iic: Use i2c_*bit_addr*_from_msg() helpers
>   i2c: mv64xxx: Use i2c_*bit_addr*_from_msg() helpers

merged to i2c/i2c-host.

Thanks,
Andi

Re: [PATCH v2 00/10] i2c: busses: Introduce and use i2c_10bit_addr_*_from_msg()

[Andy Shevchenko]

On Tue, Mar 11, 2025 at 02:12:03AM +0100, Andi Shyti wrote:
> Hi Andy,
> 
> > Andy Shevchenko (10):
> >   i2c: Introduce i2c_10bit_addr_*_from_msg() helpers
> >   i2c: axxia: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: bcm-kona: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: brcmstb: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: eg20t: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: kempld: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: mt7621: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: rzv2m: Use i2c_10bit_addr_*_from_msg() helpers
> >   i2c: ibm_iic: Use i2c_*bit_addr*_from_msg() helpers
> >   i2c: mv64xxx: Use i2c_*bit_addr*_from_msg() helpers
> 
> merged to i2c/i2c-host.

Thank you!

-- 
With Best Regards,
Andy Shevchenko