diff mbox series

Input: synaptics-rmi4: Use %pe for error codes.

Message ID 1ec25bf991f576a98bd8fdc58264a92ee268eba9.1682793592.git.mirq-linux@rere.qmqm.pl (mailing list archive)
State New, archived
Headers show
Series Input: synaptics-rmi4: Use %pe for error codes. | expand

Commit Message

Michał Mirosław April 29, 2023, 6:41 p.m. UTC 
Make the error messages a bit easier to understand by showing
error names where that's enabled.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/input/rmi4/rmi_driver.c | 66 +++++++++++++++++----------------
 1 file changed, 34 insertions(+), 32 deletions(-)

Comments

Dmitry Torokhov May 2, 2023, 12:40 a.m. UTC | #1 
On Sat, Apr 29, 2023 at 08:41:19PM +0200, Michał Mirosław wrote:
> Make the error messages a bit easier to understand by showing
> error names where that's enabled.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/input/rmi4/rmi_driver.c | 66 +++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 258d5fe3d395..82d85c02a873 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -69,8 +69,8 @@ static int reset_one_function(struct rmi_function *fn)
>  	if (fh->reset) {
>  		retval = fh->reset(fn);
>  		if (retval < 0)
> -			dev_err(&fn->dev, "Reset failed with code %d.\n",
> -				retval);
> +			dev_err(&fn->dev, "Reset failed: %pe\n",
> +				ERR_PTR(retval));

If this is desired we should have a format option for
non-err-ptr-encoded errors.

Thanks.
Michał Mirosław May 5, 2023, 8:48 p.m. UTC | #2 
On Mon, May 01, 2023 at 05:40:55PM -0700, Dmitry Torokhov wrote:
> On Sat, Apr 29, 2023 at 08:41:19PM +0200, Michał Mirosław wrote:
> > Make the error messages a bit easier to understand by showing
> > error names where that's enabled.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> >  drivers/input/rmi4/rmi_driver.c | 66 +++++++++++++++++----------------
> >  1 file changed, 34 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> > index 258d5fe3d395..82d85c02a873 100644
> > --- a/drivers/input/rmi4/rmi_driver.c
> > +++ b/drivers/input/rmi4/rmi_driver.c
> > @@ -69,8 +69,8 @@ static int reset_one_function(struct rmi_function *fn)
> >  	if (fh->reset) {
> >  		retval = fh->reset(fn);
> >  		if (retval < 0)
> > -			dev_err(&fn->dev, "Reset failed with code %d.\n",
> > -				retval);
> > +			dev_err(&fn->dev, "Reset failed: %pe\n",
> > +				ERR_PTR(retval));
> 
> If this is desired we should have a format option for
> non-err-ptr-encoded errors.

This is a common case in the kernel source to use "%pe" with ERR_PTR().

The code in `lib/vsprintf.c` has only '%p' with extended treatment and
I'm not sure how much work would be needed to extend other format
specifier (I would consider confusing if "%p" would possibly take
something that's not a pointer).

Best Regards
Michał Mirosław
diff mbox series

Patch

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 258d5fe3d395..82d85c02a873 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -69,8 +69,8 @@  static int reset_one_function(struct rmi_function *fn)
 	if (fh->reset) {
 		retval = fh->reset(fn);
 		if (retval < 0)
-			dev_err(&fn->dev, "Reset failed with code %d.\n",
-				retval);
+			dev_err(&fn->dev, "Reset failed: %pe\n",
+				ERR_PTR(retval));
 	}
 
 	return retval;
@@ -88,8 +88,8 @@  static int configure_one_function(struct rmi_function *fn)
 	if (fh->config) {
 		retval = fh->config(fn);
 		if (retval < 0)
-			dev_err(&fn->dev, "Config failed with code %d.\n",
-				retval);
+			dev_err(&fn->dev, "Config failed: %pe\n",
+				ERR_PTR(retval));
 	}
 
 	return retval;
@@ -140,7 +140,7 @@  static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 				data->f01_container->fd.data_base_addr + 1,
 				data->irq_status, data->num_of_irq_regs);
 		if (error < 0) {
-			dev_err(dev, "Failed to read irqs, code=%d\n", error);
+			dev_err(dev, "Failed to read irqs: %pe\n", ERR_PTR(error));
 			return error;
 		}
 	}
@@ -201,7 +201,7 @@  static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
 	ret = rmi_process_interrupt_requests(rmi_dev);
 	if (ret)
 		rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
-			"Failed to process interrupt request: %d\n", ret);
+			"Failed to process interrupt request: %pe\n", ERR_PTR(ret));
 
 	if (count) {
 		kfree(attn_data.data);
@@ -229,8 +229,8 @@  static int rmi_irq_init(struct rmi_device *rmi_dev)
 					dev_driver_string(rmi_dev->xport->dev),
 					rmi_dev);
 	if (ret < 0) {
-		dev_err(&rmi_dev->dev, "Failed to register interrupt %d\n",
-			pdata->irq);
+		dev_err(&rmi_dev->dev, "Failed to register interrupt %d: %pe\n",
+			pdata->irq, ERR_PTR(ret));
 
 		return ret;
 	}
@@ -265,8 +265,8 @@  static int suspend_one_function(struct rmi_function *fn)
 	if (fh->suspend) {
 		retval = fh->suspend(fn);
 		if (retval < 0)
-			dev_err(&fn->dev, "Suspend failed with code %d.\n",
-				retval);
+			dev_err(&fn->dev, "Suspend failed: %pe\n",
+				ERR_PTR(retval));
 	}
 
 	return retval;
@@ -299,8 +299,8 @@  static int resume_one_function(struct rmi_function *fn)
 	if (fh->resume) {
 		retval = fh->resume(fn);
 		if (retval < 0)
-			dev_err(&fn->dev, "Resume failed with code %d.\n",
-				retval);
+			dev_err(&fn->dev, "Resume failed: %pe\n",
+				ERR_PTR(retval));
 	}
 
 	return retval;
@@ -464,8 +464,8 @@  static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
 
 	error = rmi_read_block(rmi_dev, pdt_address, buf, RMI_PDT_ENTRY_SIZE);
 	if (error) {
-		dev_err(&rmi_dev->dev, "Read PDT entry at %#06x failed, code: %d.\n",
-				pdt_address, error);
+		dev_err(&rmi_dev->dev, "Read PDT entry at %#06x failed: %pe\n",
+				pdt_address, ERR_PTR(error));
 		return error;
 	}
 
@@ -752,7 +752,8 @@  static int rmi_check_bootloader_mode(struct rmi_device *rmi_dev,
 		ret = rmi_read(rmi_dev, pdt->data_base_addr, &status);
 		if (ret) {
 			dev_err(&rmi_dev->dev,
-				"Failed to read F34 status: %d.\n", ret);
+				"Failed to read F34 status: %pe\n",
+				ERR_PTR(ret));
 			return ret;
 		}
 
@@ -762,7 +763,8 @@  static int rmi_check_bootloader_mode(struct rmi_device *rmi_dev,
 		ret = rmi_read(rmi_dev, pdt->data_base_addr, &status);
 		if (ret) {
 			dev_err(&rmi_dev->dev,
-				"Failed to read F01 status: %d.\n", ret);
+				"Failed to read F01 status: %pe\n",
+				ERR_PTR(ret));
 			return ret;
 		}
 
@@ -812,7 +814,7 @@  int rmi_initial_reset(struct rmi_device *rmi_dev, void *ctx,
 		error = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
 		if (error) {
 			dev_err(&rmi_dev->dev,
-				"Initial reset failed. Code = %d.\n", error);
+				"Initial reset failed: %pe\n", ERR_PTR(error));
 			return error;
 		}
 
@@ -892,8 +894,8 @@  void rmi_enable_irq(struct rmi_device *rmi_dev, bool clear_wake)
 		retval = disable_irq_wake(irq);
 		if (retval)
 			dev_warn(&rmi_dev->dev,
-				 "Failed to disable irq for wake: %d\n",
-				 retval);
+				 "Failed to disable irq for wake: %pe\n",
+				 ERR_PTR(retval));
 	}
 
 	/*
@@ -927,8 +929,8 @@  void rmi_disable_irq(struct rmi_device *rmi_dev, bool enable_wake)
 		retval = enable_irq_wake(irq);
 		if (retval)
 			dev_warn(&rmi_dev->dev,
-				 "Failed to enable irq for wake: %d\n",
-				 retval);
+				 "Failed to enable irq for wake: %pe\n",
+				 ERR_PTR(retval));
 	}
 
 	/* make sure the fifo is clean */
@@ -948,8 +950,8 @@  int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
 
 	retval = rmi_suspend_functions(rmi_dev);
 	if (retval)
-		dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n",
-			retval);
+		dev_warn(&rmi_dev->dev, "Failed to suspend functions: %pe\n",
+			ERR_PTR(retval));
 
 	rmi_disable_irq(rmi_dev, enable_wake);
 	return retval;
@@ -964,8 +966,8 @@  int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
 
 	retval = rmi_resume_functions(rmi_dev);
 	if (retval)
-		dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n",
-			retval);
+		dev_warn(&rmi_dev->dev, "Failed to resume functions: %pe\n",
+			ERR_PTR(retval));
 
 	return retval;
 }
@@ -1028,7 +1030,7 @@  int rmi_probe_interrupts(struct rmi_driver_data *data)
 
 	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_count_irqs);
 	if (retval < 0) {
-		dev_err(dev, "IRQ counting failed with code %d.\n", retval);
+		dev_err(dev, "IRQ counting failed: %pe\n", ERR_PTR(retval));
 		return retval;
 	}
 
@@ -1072,8 +1074,8 @@  int rmi_init_functions(struct rmi_driver_data *data)
 	rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Creating functions.\n", __func__);
 	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
 	if (retval < 0) {
-		dev_err(dev, "Function creation failed with code %d.\n",
-			retval);
+		dev_err(dev, "Function creation failed: %pe\n",
+			ERR_PTR(retval));
 		goto err_destroy_functions;
 	}
 
@@ -1166,8 +1168,8 @@  static int rmi_driver_probe(struct device *dev)
 		 * we'll print out a warning and continue since
 		 * failure to get the PDT properties is not a cause to fail
 		 */
-		dev_warn(dev, "Could not read PDT properties from %#06x (code %d). Assuming 0x00.\n",
-			 PDT_PROPERTIES_LOCATION, retval);
+		dev_warn(dev, "Could not read PDT properties from %#06x (%pe). Assuming 0x00.\n",
+			 PDT_PROPERTIES_LOCATION, ERR_PTR(retval));
 	}
 
 	mutex_init(&data->irq_mutex);
@@ -1265,8 +1267,8 @@  int __init rmi_register_physical_driver(void)
 
 	error = driver_register(&rmi_physical_driver.driver);
 	if (error) {
-		pr_err("%s: driver register failed, code=%d.\n", __func__,
-		       error);
+		pr_err("%s: driver register failed: %pe\n", __func__,
+		       ERR_PTR(error));
 		return error;
 	}