watchdog: da9062: da9063: use unlocked xfer function in restart

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>
---
 drivers/watchdog/da9062_wdt.c | 15 ++++++++++++---
 drivers/watchdog/da9063_wdt.c | 15 ++++++++++++---
 2 files changed, 24 insertions(+), 6 deletions(-)

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);