[2/5] watchdog: ziirave_wdt: Be verbose about errors in probe()
diff mbox series

Message ID 20190731174252.18041-3-andrew.smirnov@gmail.com
State Accepted
Headers show
Series
  • Ziirave_wdt driver fixes
Related show

Commit Message

Andrey Smirnov July 31, 2019, 5:42 p.m. UTC
The driver is quite silent in case of probe failure, which makes it
more difficult to diagnose problem from the kernel log. Add logging to
all of the silent error paths ziirave_wdt_probe() to improve that.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Rick Ramstetter <rick@anteaterllc.com>
Cc: linux-watchdog@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

Comments

Guenter Roeck July 31, 2019, 9:42 p.m. UTC | #1
On Wed, Jul 31, 2019 at 10:42:49AM -0700, Andrey Smirnov wrote:
> The driver is quite silent in case of probe failure, which makes it
> more difficult to diagnose problem from the kernel log. Add logging to
> all of the silent error paths ziirave_wdt_probe() to improve that.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Rick Ramstetter <rick@anteaterllc.com>
> Cc: linux-watchdog@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

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

> ---
>  drivers/watchdog/ziirave_wdt.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
> index 6ec028fb2635..8c71341a9c1c 100644
> --- a/drivers/watchdog/ziirave_wdt.c
> +++ b/drivers/watchdog/ziirave_wdt.c
> @@ -658,8 +658,10 @@ static int ziirave_wdt_probe(struct i2c_client *client,
>  	 */
>  	if (w_priv->wdd.timeout == 0) {
>  		val = i2c_smbus_read_byte_data(client, ZIIRAVE_WDT_TIMEOUT);
> -		if (val < 0)
> +		if (val < 0) {
> +			dev_err(&client->dev, "Failed to read timeout\n");
>  			return val;
> +		}
>  
>  		if (val < ZIIRAVE_TIMEOUT_MIN)
>  			return -ENODEV;
> @@ -668,8 +670,10 @@ static int ziirave_wdt_probe(struct i2c_client *client,
>  	} else {
>  		ret = ziirave_wdt_set_timeout(&w_priv->wdd,
>  					      w_priv->wdd.timeout);
> -		if (ret)
> +		if (ret) {
> +			dev_err(&client->dev, "Failed to set timeout\n");
>  			return ret;
> +		}
>  
>  		dev_info(&client->dev, "Timeout set to %ds\n",
>  			 w_priv->wdd.timeout);
> @@ -681,34 +685,46 @@ static int ziirave_wdt_probe(struct i2c_client *client,
>  
>  	/* If in unconfigured state, set to stopped */
>  	val = i2c_smbus_read_byte_data(client, ZIIRAVE_WDT_STATE);
> -	if (val < 0)
> +	if (val < 0) {
> +		dev_err(&client->dev, "Failed to read state\n");
>  		return val;
> +	}
>  
>  	if (val == ZIIRAVE_STATE_INITIAL)
>  		ziirave_wdt_stop(&w_priv->wdd);
>  
>  	ret = ziirave_wdt_init_duration(client);
> -	if (ret)
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to init duration\n");
>  		return ret;
> +	}
>  
>  	ret = ziirave_wdt_revision(client, &w_priv->firmware_rev,
>  				   ZIIRAVE_WDT_FIRM_VER_MAJOR);
> -	if (ret)
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to read firmware version\n");
>  		return ret;
> +	}
>  
>  	ret = ziirave_wdt_revision(client, &w_priv->bootloader_rev,
>  				   ZIIRAVE_WDT_BOOT_VER_MAJOR);
> -	if (ret)
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to read bootloader version\n");
>  		return ret;
> +	}
>  
>  	w_priv->reset_reason = i2c_smbus_read_byte_data(client,
>  						ZIIRAVE_WDT_RESET_REASON);
> -	if (w_priv->reset_reason < 0)
> +	if (w_priv->reset_reason < 0) {
> +		dev_err(&client->dev, "Failed to read reset reason\n");
>  		return w_priv->reset_reason;
> +	}
>  
>  	if (w_priv->reset_reason >= ARRAY_SIZE(ziirave_reasons) ||
> -	    !ziirave_reasons[w_priv->reset_reason])
> +	    !ziirave_reasons[w_priv->reset_reason]) {
> +		dev_err(&client->dev, "Invalid reset reason\n");
>  		return -ENODEV;
> +	}
>  
>  	ret = watchdog_register_device(&w_priv->wdd);
>  
> -- 
> 2.21.0
>

Patch
diff mbox series

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index 6ec028fb2635..8c71341a9c1c 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -658,8 +658,10 @@  static int ziirave_wdt_probe(struct i2c_client *client,
 	 */
 	if (w_priv->wdd.timeout == 0) {
 		val = i2c_smbus_read_byte_data(client, ZIIRAVE_WDT_TIMEOUT);
-		if (val < 0)
+		if (val < 0) {
+			dev_err(&client->dev, "Failed to read timeout\n");
 			return val;
+		}
 
 		if (val < ZIIRAVE_TIMEOUT_MIN)
 			return -ENODEV;
@@ -668,8 +670,10 @@  static int ziirave_wdt_probe(struct i2c_client *client,
 	} else {
 		ret = ziirave_wdt_set_timeout(&w_priv->wdd,
 					      w_priv->wdd.timeout);
-		if (ret)
+		if (ret) {
+			dev_err(&client->dev, "Failed to set timeout\n");
 			return ret;
+		}
 
 		dev_info(&client->dev, "Timeout set to %ds\n",
 			 w_priv->wdd.timeout);
@@ -681,34 +685,46 @@  static int ziirave_wdt_probe(struct i2c_client *client,
 
 	/* If in unconfigured state, set to stopped */
 	val = i2c_smbus_read_byte_data(client, ZIIRAVE_WDT_STATE);
-	if (val < 0)
+	if (val < 0) {
+		dev_err(&client->dev, "Failed to read state\n");
 		return val;
+	}
 
 	if (val == ZIIRAVE_STATE_INITIAL)
 		ziirave_wdt_stop(&w_priv->wdd);
 
 	ret = ziirave_wdt_init_duration(client);
-	if (ret)
+	if (ret) {
+		dev_err(&client->dev, "Failed to init duration\n");
 		return ret;
+	}
 
 	ret = ziirave_wdt_revision(client, &w_priv->firmware_rev,
 				   ZIIRAVE_WDT_FIRM_VER_MAJOR);
-	if (ret)
+	if (ret) {
+		dev_err(&client->dev, "Failed to read firmware version\n");
 		return ret;
+	}
 
 	ret = ziirave_wdt_revision(client, &w_priv->bootloader_rev,
 				   ZIIRAVE_WDT_BOOT_VER_MAJOR);
-	if (ret)
+	if (ret) {
+		dev_err(&client->dev, "Failed to read bootloader version\n");
 		return ret;
+	}
 
 	w_priv->reset_reason = i2c_smbus_read_byte_data(client,
 						ZIIRAVE_WDT_RESET_REASON);
-	if (w_priv->reset_reason < 0)
+	if (w_priv->reset_reason < 0) {
+		dev_err(&client->dev, "Failed to read reset reason\n");
 		return w_priv->reset_reason;
+	}
 
 	if (w_priv->reset_reason >= ARRAY_SIZE(ziirave_reasons) ||
-	    !ziirave_reasons[w_priv->reset_reason])
+	    !ziirave_reasons[w_priv->reset_reason]) {
+		dev_err(&client->dev, "Invalid reset reason\n");
 		return -ENODEV;
+	}
 
 	ret = watchdog_register_device(&w_priv->wdd);