| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 28 Jan 2021 14:50:58 +0000
From: Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To: Mark Jonas <mark.jonas@...bosch.com>,
Support Opensource <Support.Opensource@...semi.com>,
Lee Jones <lee.jones@...aro.org>,
Rob Herring <robh+dt@...nel.org>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Adam Thomson <Adam.Thomson.Opensource@...semi.com>,
Steve Twiss <stwiss.opensource@...semi.com>,
"marek.vasut@...il.com" <marek.vasut@...il.com>,
"tingquan.ruan@...bosch.com" <tingquan.ruan@...bosch.com>,
"hubert.streidl@...bosch.com" <hubert.streidl@...bosch.com>
Subject: RE: [PATCH 1/1] mfd: da9063: Support SMBus and I2C mode
On 25 January 2021 12:55, Mark Jonas wrote:
> From: Hubert Streidl <hubert.streidl@...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?
>
> Signed-off-by: Hubert Streidl <hubert.streidl@...bosch.com>
> Signed-off-by: Mark Jonas <mark.jonas@...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
Powered by blists - more mailing lists