hello
On 28/03/24 4:39 pm, Laurent Pinchart wrote:
Sakari, there's a question for you below.
On Thu, Mar 28, 2024 at 04:29:41PM +0530, Umang Jain wrote:
On 28/03/24 4:23 pm, Laurent Pinchart wrote:
On Fri, Mar 15, 2024 at 12:13:15PM +0530, Umang Jain wrote:
On 14/03/24 8:51 pm, Laurent Pinchart wrote:
On Thu, Mar 14, 2024 at 06:51:04PM +0530, Umang Jain wrote:
On 11/03/24 5:05 pm, Tommaso Merciai wrote:
On Mon, Mar 11, 2024 at 02:30:42PM +0530, Umang Jain wrote:
Drop dev_err() and use the dev_err_probe() helper on probe path.
No functional changes intended.
Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
---
drivers/media/i2c/imx219.c | 64 +++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index e17ef2e9d9d0..acd27e2ef849 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -551,8 +551,9 @@ static int imx219_init_controls(struct imx219 *imx219)
if (ctrl_hdlr->error) {
ret = ctrl_hdlr->error;
- dev_err(&client->dev, "%s control init failed (%d)\n",
- __func__, ret);
+ dev_err_probe(&client->dev, ret,
+ "%s control init failed\n",
+ __func__);
ctrl_hdlr->error can never be -EPROBE_DEFER, is dev_err_probe() really
useful here ?
is dev_err_probe() really /only/ about -EPROBE_DEFER ? or all probe()
errors?
The documentation is explicitly stating for dev_Err_probe()
```
* Note that it is deemed acceptable to use this function for error
* prints during probe even if the @err is known to never be -EPROBE_DEFER.
* The benefit compared to a normal dev_err() is the standardized format
* of the error code and the fact that the error code is returned.
*
```
As in so many cases, it's partly a matter of taste :-) When it comes to
changes such as
- dev_err(dev, "xclk frequency not supported: %d Hz\n",
- imx219->xclk_freq);
- return -EINVAL;
+ return dev_err_probe(dev, -EINVAL,
+ "xclk frequency not supported: %d Hz\n",
+ imx219->xclk_freq);
I find the resulting less readable. The indentation is higher, you have
to look at the parameters to dev_err_probe() to see what is returned
(compared to reading "return -EINVAL"), and adding the error code to the
printed message also makes the kernel log less as the error code really
doesn't need to be printed in this case.
Indeed, a matter of taste ...
On IMX283 driver, I got the feedback that all probe errors should go via
dev_err_probe()
"Make all messages in ->probe() stage to use the same pattern." [1]
For IMX219, (since it's the most appropriate reference driver from
framework PoV), I saw that it is not logging through dev_err_probe(),
and in such cases hence tried to align with [1]
I'd say we should have common guidelines for all sensor drivers. As
Sakari is the maintainer here, he can be the judge too :-)
There is also a v2 in case someone has missed it:
https://patchwork.kernel.org/project/linux-media/patch/20240320070027.77194-1-umang.jain@xxxxxxxxxxxxxxxx/
[1] https://lore.kernel.org/all/CAHp75Vcvvadd6EeTWk2ZDrmtCQzWBV8rOoxNCotzYRRPwecaEA@xxxxxxxxxxxxxx/
goto error;
}
[snip]
