On 25 January 2021 12:55, Mark Jonas wrote: > From: Hubert Streidl <hubert.streidl@xxxxxxxxxxxx> > > 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 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 makes the I2C mode configurable by device tree. The SMBus > compliant mode is kept as the default. Could we not just check the bus' functionality flags and set this accordingly? Something like this is already done in regmap-i2c to determine how to access the device: https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap-i2c.c#L309 This seems cleaner than a new DT property, or will this not work in this situation? > > Signed-off-by: Hubert Streidl <hubert.streidl@xxxxxxxxxxxx> > Signed-off-by: Mark Jonas <mark.jonas@xxxxxxxxxxxx> > --- > Documentation/devicetree/bindings/mfd/da9063.txt | 7 +++++++ > drivers/mfd/da9063-core.c | 9 +++++++++ > drivers/mfd/da9063-i2c.c | 13 +++++++++++++ > include/linux/mfd/da9063/core.h | 1 + > include/linux/mfd/da9063/registers.h | 3 +++ > 5 files changed, 33 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt > b/Documentation/devicetree/bindings/mfd/da9063.txt > index 8da879935c59..256f2a25fe0a 100644 > --- a/Documentation/devicetree/bindings/mfd/da9063.txt > +++ b/Documentation/devicetree/bindings/mfd/da9063.txt > @@ -19,6 +19,12 @@ Required properties: > - interrupts : IRQ line information. > - interrupt-controller > > +Optional properties: > + > +- i2c-mode : Switch serial 2-wire interface into I2C mode. Without this > + property the PMIC uses the SMBus mode (resets the interface if the clock > + ceases for a longer time than the SMBus timeout). > + > Sub-nodes: > > - regulators : This node defines the settings for the LDOs and BUCKs. > @@ -77,6 +83,7 @@ Example: > interrupt-parent = <&gpio6>; > interrupts = <11 IRQ_TYPE_LEVEL_LOW>; > interrupt-controller; > + i2c-mode; > > rtc { > compatible = "dlg,da9063-rtc"; > diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c > index df407c3afce3..baa1e4310c8c 100644 > --- a/drivers/mfd/da9063-core.c > +++ b/drivers/mfd/da9063-core.c > @@ -162,6 +162,15 @@ int da9063_device_init(struct da9063 *da9063, unsigned > int irq) > { > int ret; > > + if (da9063->i2cmode) { > + ret = regmap_update_bits(da9063->regmap, > DA9063_REG_CONFIG_J, > + DA9063_TWOWIRE_TO, 0); > + if (ret < 0) { > + dev_err(da9063->dev, "Cannot enable I2C mode.\n"); > + return -EIO; > + } > + } > + > ret = da9063_clear_fault_log(da9063); > if (ret < 0) > dev_err(da9063->dev, "Cannot clear fault log\n"); > diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c > index 3781d0bb7786..af0bf13ab43e 100644 > --- a/drivers/mfd/da9063-i2c.c > +++ b/drivers/mfd/da9063-i2c.c > @@ -351,6 +351,17 @@ static const struct of_device_id da9063_dt_ids[] = { > { } > }; > MODULE_DEVICE_TABLE(of, da9063_dt_ids); > + > +static void da9063_i2c_parse_dt(struct i2c_client *client, struct da9063 *da9063) > +{ > + struct device_node *np = client->dev.of_node; > + > + if (of_property_read_bool(np, "i2c-mode")) > + da9063->i2cmode = true; > + else > + da9063->i2cmode = false; > +} > + > static int da9063_i2c_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > @@ -366,6 +377,8 @@ static int da9063_i2c_probe(struct i2c_client *i2c, > da9063->chip_irq = i2c->irq; > da9063->type = id->driver_data; > > + da9063_i2c_parse_dt(i2c, da9063); > + > ret = da9063_get_device_type(i2c, da9063); > if (ret) > return ret; > diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h > index fa7a43f02f27..866864c50f78 100644 > --- a/include/linux/mfd/da9063/core.h > +++ b/include/linux/mfd/da9063/core.h > @@ -77,6 +77,7 @@ struct da9063 { > enum da9063_type type; > unsigned char variant_code; > unsigned int flags; > + bool i2cmode; > > /* Control interface */ > struct regmap *regmap; > 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 > -- > 2.25.1
