lkml.org 
[lkml]   [2022]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] watchdog: da9062: da9063: use unlocked xfer function in restart
From
On 12/16/22 00:36, Primoz Fiser wrote:
> Machine resets via da9062/da9063 PMICs are challenging since one needs
> to use special i2c atomic transfers due to the fact interrupts are
> disabled in such late system stages. This is the reason both PMICs don't
> use regmap and have instead opted for i2c_smbus_write_byte_data() in
> restart handlers.
>
> However extensive testing revealed that even using atomic safe function
> is not enough and occasional resets fail with error message "Failed to
> shutdown (err = -11)". This is due to the fact that function
> i2c_smbus_write_byte_data() in turn calls __i2c_lock_bus_helper()
> which might fail with -EAGAIN when bus lock is already taken and cannot
> be released anymore.
>
> Thus replace i2c_smbus_write_byte_data() with unlocked flavor of
> i2c_smbus_xfer() function to avoid above dead-lock scenario. At this
> system stage we don't care about proper locking anymore and only want
> proper machine reset to be carried out.
>
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> drivers/watchdog/da9062_wdt.c | 15 ++++++++++++---
> drivers/watchdog/da9063_wdt.c | 15 ++++++++++++---
> 2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
> index f02cbd530538..1ec33b4bd2f2 100644
> --- a/drivers/watchdog/da9062_wdt.c
> +++ b/drivers/watchdog/da9062_wdt.c
> @@ -155,11 +155,20 @@ static int da9062_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> {
> struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
> struct i2c_client *client = to_i2c_client(wdt->hw->dev);
> + union i2c_smbus_data msg;
> int ret;
>
> - /* Don't use regmap because it is not atomic safe */
> - ret = i2c_smbus_write_byte_data(client, DA9062AA_CONTROL_F,
> - DA9062AA_SHUTDOWN_MASK);
> + /*
> + * Don't use regmap because it is not atomic safe. Additionally, use
> + * unlocked flavor of i2c_smbus_xfer to avoid scenario where i2c bus
> + * might be previously locked by some process unable to release the
> + * lock due to interrupts already being disabled at this late stage.
> + */
> + msg.byte = DA9062AA_SHUTDOWN_MASK;
> + ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> + I2C_SMBUS_WRITE, DA9062AA_CONTROL_F,
> + I2C_SMBUS_BYTE_DATA, &msg);
> +
> if (ret < 0)
> dev_alert(wdt->hw->dev, "Failed to shutdown (err = %d)\n",
> ret);
> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
> index 09a4af4c58fc..684667469b10 100644
> --- a/drivers/watchdog/da9063_wdt.c
> +++ b/drivers/watchdog/da9063_wdt.c
> @@ -174,11 +174,20 @@ static int da9063_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> {
> struct da9063 *da9063 = watchdog_get_drvdata(wdd);
> struct i2c_client *client = to_i2c_client(da9063->dev);
> + union i2c_smbus_data msg;
> int ret;
>
> - /* Don't use regmap because it is not atomic safe */
> - ret = i2c_smbus_write_byte_data(client, DA9063_REG_CONTROL_F,
> - DA9063_SHUTDOWN);
> + /*
> + * Don't use regmap because it is not atomic safe. Additionally, use
> + * unlocked flavor of i2c_smbus_xfer to avoid scenario where i2c bus
> + * might previously be locked by some process unable to release the
> + * lock due to interrupts already being disabled at this late stage.
> + */
> + msg.byte = DA9063_SHUTDOWN;
> + ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> + I2C_SMBUS_WRITE, DA9063_REG_CONTROL_F,
> + I2C_SMBUS_BYTE_DATA, &msg);
> +
> if (ret < 0)
> dev_alert(da9063->dev, "Failed to shutdown (err = %d)\n",
> ret);

\
 
 \ /
  Last update: 2022-12-16 11:47    [W:0.029 / U:0.528 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site