Skip to contentSkip to conversation

Introduce Airoha EN7581 i2c support

PATCH Superseded Lorenzo Bianconi 2 participants 6 messages Jul 4, 2024 – Jul 5, 2024

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Introduce Airoha EN7581 i2c support

Introduce i2c support to Airoha EN7581 SoC

Lorenzo Bianconi (2):
i2c: mt7621: Add Airoha EN7581 i2c support
i2c: mt7621: make device_reset optional

 drivers/i2c/busses/Kconfig      | 2 +-
 drivers/i2c/busses/i2c-mt7621.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Signature · 1 line
2.45.2
#1 lore raw

PATCH 1/2 i2c: mt7621: Add Airoha EN7581 i2c support

Introduce i2c support to Airoha EN7581 SoC through the i2c-mt7621 driver.

---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +1 -1
index fe6e8a1bb607..4d93c00264fc 100644
@@ -857,7 +857,7 @@ config I2C_MT65XX
 
 config I2C_MT7621
 	tristate "MT7621/MT7628 I2C Controller"
-	depends on (RALINK && (SOC_MT7620 || SOC_MT7621)) || COMPILE_TEST
+	depends on (RALINK && (SOC_MT7620 || SOC_MT7621)) || ARCH_AIROHA || COMPILE_TEST
 	help
 	  Say Y here to include support for I2C controller in the
 	  MediaTek MT7621/MT7628 SoCs.
Signature · 1 line
2.45.2
#3 lore raw

PATCH 2/2 i2c: mt7621: make device_reset optional

In reply to Lorenzo Bianconi

Rely on device_reset_optional() instead of device_reset() in mtk_i2c_reset routine since Airoha EN7581 reset controller does not implement the reset callback and device reset is not mandatory on this SoC. This will remove the following log:

i2c-mt7621 1fbf8000.i2c0: I2C reset failed!

---
 drivers/i2c/busses/i2c-mt7621.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-mt7621.c +1 -1
index 81d46169bc1f..8cd89a2f59df 100644
@@ -85,7 +85,7 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
 {
 	int ret;
 
-	ret = device_reset(i2c->adap.dev.parent);
+	ret = device_reset_optional(i2c->adap.dev.parent);
 	if (ret)
 		dev_err(i2c->dev, "I2C reset failed!\n");
 
Signature · 1 line
2.45.2
#4 lore raw

Re: [PATCH 2/2] i2c: mt7621: make device_reset optional

Il 05/07/24 00:16, Lorenzo Bianconi ha scritto:

Quoted 3 lines

Rely on device_reset_optional() instead of device_reset() in mtk_i2c_reset routine since Airoha EN7581 reset controller does not implement the reset callback and device reset is not mandatory on this

Not mandatory means that the reset line for I2C exists, but you don't want to use it? If so, that's not right.

On the other hand, if there is an issue with using that reset line and you want to avoid using it for a good reason, please describe it into the commit description and - in that case, you'll have to add a EN7581 compatible and code to avoid calling device_reset() on that SoC.

What you're doing here is removing an error log on SoCs that do *need* that reset line to be in place, so, hiding a problem that could be causing other issues on peripherals connected to the I2C bus of those other SoCs.

Cheers,
Angelo

Quoted 23 lines

SoC. This will remove the following log:

i2c-mt7621 1fbf8000.i2c0: I2C reset failed!

---
  drivers/i2c/busses/i2c-mt7621.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-mt7621.c +1 -1
index 81d46169bc1f..8cd89a2f59df 100644
@@ -85,7 +85,7 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
  {
  	int ret;
 
-	ret = device_reset(i2c->adap.dev.parent);
+	ret = device_reset_optional(i2c->adap.dev.parent);
  	if (ret)
  		dev_err(i2c->dev, "I2C reset failed!\n");
 
#5 lore raw

Re: [PATCH 2/2] i2c: mt7621: make device_reset optional

Quoted 7 lines

Il 05/07/24 00:16, Lorenzo Bianconi ha scritto:

Quoted 3 lines

Rely on device_reset_optional() instead of device_reset() in mtk_i2c_reset routine since Airoha EN7581 reset controller does not implement the reset callback and device reset is not mandatory on this

Not mandatory means that the reset line for I2C exists, but you don't want to use it? If so, that's not right.

ack, it was not so clear, sorry for that. What I mean with "not mandatory" is the mt7621 i2c controller works on EN7581 SoC even w/o this reset configuration.

Quoted 5 lines

On the other hand, if there is an issue with using that reset line and you want to avoid using it for a good reason, please describe it into the commit description and - in that case, you'll have to add a EN7581 compatible and code to avoid calling device_reset() on that SoC.

what I mean is device_reset() will call .reset() callback of reset APIs. EN7581 reset driver does not implemetn this callback, just .assert() and .deassert() (I think the hw does not support the reset line automatic re-arm) so the call to device_reset() will always fails. Maybe a possible approach would be adding in the reset driver something like:

en7581_reset() {
    en7581_assert();
    msleep(x);
    en7581_deassert();
    msleep(x);
}

What do you think? It seems a bit hacky to me and I am not sure what would be the right delay in this case.

Regards,
Lorenzo

Quoted 31 lines

What you're doing here is removing an error log on SoCs that do *need* that reset line to be in place, so, hiding a problem that could be causing other issues on peripherals connected to the I2C bus of those other SoCs.

Cheers,
Angelo

Quoted 21 lines

SoC. This will remove the following log:

i2c-mt7621 1fbf8000.i2c0: I2C reset failed!

---
  drivers/i2c/busses/i2c-mt7621.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/i2c-mt7621.c +1 -1
index 81d46169bc1f..8cd89a2f59df 100644
@@ -85,7 +85,7 @@ static void mtk_i2c_reset(struct mtk_i2c *i2c)
  {
  	int ret;
-	ret = device_reset(i2c->adap.dev.parent);
+	ret = device_reset_optional(i2c->adap.dev.parent);
  	if (ret)
  		dev_err(i2c->dev, "I2C reset failed!\n");