diff mbox series

[v3] mfd: da9063: Support SMBus and I2C mode

Message ID 20210205155006.130458-1-mark.jonas@de.bosch.com
State Superseded
Headers show
Series [v3] mfd: da9063: Support SMBus and I2C mode | expand

Commit Message

Jonas Mark (BT-FIR/ENG1-Grb) Feb. 5, 2021, 3:50 p.m. UTC 
From: Hubert Streidl <hubert.streidl@de.bosch.com>

By default the PMIC DA9063 2-wire interface is SMBus compliant. This
means the PMIC will automatically reset the interface when the clock
signal ceases for more than the SMBus timeout of 35 ms.

If the I2C driver / device is not capable of creating atomic I2C
transactions, a context change can cause a ceasing of the clock signal.
This can happen if for example a real-time thread is scheduled. Then
the DA9063 in SMBus mode will reset the 2-wire interface. Subsequently
a write message could end up in the wrong register. This could cause
unpredictable system behavior.

The DA9063 PMIC also supports an I2C compliant mode for the 2-wire
interface. This mode does not reset the interface when the clock
signal ceases. Thus the problem depicted above does not occur.

This patch tests for the bus functionality "I2C_FUNC_I2C". It can
reasonably be assumed that the bus cannot obey SMBus timings if
this functionality is set. SMBus commands most probably are emulated
in this case which is prone to the latency issue described above.

This patch enables the I2C bus mode if I2C_FUNC_I2C is set or
otherwise enables the SMBus mode for a native SMBus controller
which doesn't have I2C_FUNC_I2C set.

Signed-off-by: Hubert Streidl <hubert.streidl@de.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
---
Changes in v3:
  - busmode now contains the correct bit DA9063_TWOWIRE_TO

Changes in v2:
  - Implement proposal by Adam Thomson and Wolfram Sang to check for
    functionality I2C_FUNC_I2C instead of introducing a new DT property.

 drivers/mfd/da9063-i2c.c             | 15 +++++++++++++++
 include/linux/mfd/da9063/registers.h |  3 +++
 2 files changed, 18 insertions(+)

Comments

Wolfram Sang Feb. 6, 2021, 1:16 a.m. UTC | #1 
> +	if (i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C)) {

> +		dev_info(da9063->dev, "I2C mode");

> +		busmode = 0;

> +	} else {

> +		dev_info(da9063->dev, "SMBus mode");

> +		busmode = DA9063_TWOWIRE_TO;

> +	}


In principle, this looks good and is sane to do. I'd just suggest to
leave out the dev_info calls and maybe just do:

	busmode = i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C) ?
		  0 : DA9063_TWOWIRE_TO;
diff mbox series

Patch

diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index 3781d0bb7786..4c5e0f69942f 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -355,6 +355,7 @@  static int da9063_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
 	struct da9063 *da9063;
+	unsigned int busmode;
 	int ret;
 
 	da9063 = devm_kzalloc(&i2c->dev, sizeof(struct da9063), GFP_KERNEL);
@@ -442,6 +443,20 @@  static int da9063_i2c_probe(struct i2c_client *i2c,
 		return ret;
 	}
 
+	if (i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C)) {
+		dev_info(da9063->dev, "I2C mode");
+		busmode = 0;
+	} else {
+		dev_info(da9063->dev, "SMBus mode");
+		busmode = DA9063_TWOWIRE_TO;
+	}
+	ret = regmap_update_bits(da9063->regmap, DA9063_REG_CONFIG_J,
+	      DA9063_TWOWIRE_TO, busmode);
+	if (ret < 0) {
+		dev_err(da9063->dev, "Failed to set 2-wire bus mode.\n");
+		return -EIO;
+	}
+
 	return da9063_device_init(da9063, i2c->irq);
 }
 
diff --git a/include/linux/mfd/da9063/registers.h b/include/linux/mfd/da9063/registers.h
index 1dbabf1b3cb8..6e0f66a2e727 100644
--- a/include/linux/mfd/da9063/registers.h
+++ b/include/linux/mfd/da9063/registers.h
@@ -1037,6 +1037,9 @@ 
 #define		DA9063_NONKEY_PIN_AUTODOWN	0x02
 #define		DA9063_NONKEY_PIN_AUTOFLPRT	0x03
 
+/* DA9063_REG_CONFIG_J (addr=0x10F) */
+#define DA9063_TWOWIRE_TO			0x40
+
 /* DA9063_REG_MON_REG_5 (addr=0x116) */
 #define DA9063_MON_A8_IDX_MASK			0x07
 #define		DA9063_MON_A8_IDX_NONE		0x00