From: "Jonas Mark (BT-FIR/ENG1-Grb)" <Mark.Jonas@de.bosch.com>
To: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
Support Opensource <Support.Opensource@diasemi.com>,
Lee Jones <lee.jones@linaro.org>,
"Rob Herring" <robh+dt@kernel.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
Steve Twiss <stwiss.opensource@diasemi.com>,
"marek.vasut@gmail.com" <marek.vasut@gmail.com>,
"RUAN Tingquan (BT-FIR/ENG1-Zhu)" <Tingquan.Ruan@cn.bosch.com>,
"Streidl Hubert (BT-FIR/ENG1-Grb)" <Hubert.Streidl@de.bosch.com>,
"Jonas Mark (BT-FIR/ENG1-Grb)" <Mark.Jonas@de.bosch.com>,
"festevam@gmail.com" <festevam@gmail.com>,
"o.rempel@pengutronix.de" <o.rempel@pengutronix.de>,
"anson.huang@nxp.com" <anson.huang@nxp.com>
Subject: AW: [PATCH 1/1] mfd: da9063: Support SMBus and I2C mode
Date: Thu, 28 Jan 2021 16:09:56 +0000 [thread overview]
Message-ID: <df52d5e7069541e78b8b01ddc54a6777@de.bosch.com> (raw)
In-Reply-To: <PR3PR10MB41420E24A1303DA91175593E80BA9@PR3PR10MB4142.EURPRD10.PROD.OUTLOOK.COM>
Hi Adam,
> > 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 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?
Thank you for the proposal. We were not aware of this possibility.
We experienced the SMBus timeout problem on a i.MX6 Solo. When looking
at the i2c-imx driver we see that it defines a list of functionality.
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
| I2C_FUNC_SMBUS_READ_BLOCK_DATA;
https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-imx.c#L1133
None of that seems to model the inability to perform atomic transactions
within the SMBus timeout. This is either a bug of this specific driver
or maybe the expressiveness of I2C_FUNC_* is not sufficient.
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/i2c.h#L88
What flag do you think we could check to find out whether the bus is
able to obey the SMBus timeout or not?
> > Signed-off-by: Hubert Streidl <hubert.streidl@de.bosch.com>
> > Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
> > ---
> > 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
Cheers,
Mark
next parent reply other threads:[~2021-01-28 16:19 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210125125458.1302525-1-mark.jonas@de.bosch.com>
[not found] ` <20210125125458.1302525-2-mark.jonas@de.bosch.com>
[not found] ` <PR3PR10MB41420E24A1303DA91175593E80BA9@PR3PR10MB4142.EURPRD10.PROD.OUTLOOK.COM>
2021-01-28 16:09 ` Jonas Mark (BT-FIR/ENG1-Grb) [this message]
2021-01-28 20:53 ` [PATCH 1/1] mfd: da9063: Support SMBus and I2C mode Wolfram Sang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=df52d5e7069541e78b8b01ddc54a6777@de.bosch.com \
--to=mark.jonas@de.bosch.com \
--cc=Adam.Thomson.Opensource@diasemi.com \
--cc=Hubert.Streidl@de.bosch.com \
--cc=Support.Opensource@diasemi.com \
--cc=Tingquan.Ruan@cn.bosch.com \
--cc=anson.huang@nxp.com \
--cc=devicetree@vger.kernel.org \
--cc=festevam@gmail.com \
--cc=lee.jones@linaro.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marek.vasut@gmail.com \
--cc=o.rempel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=stwiss.opensource@diasemi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).