[PATCH] media: i2c: imx334: Fix an error message

[PATCH] media: i2c: imx334: Fix an error message

[Dan Carpenter]

The "ret" variable is uninitialized in this error message.

Fixes: 9746b11715c3 ("media: i2c: Add imx334 camera sensor driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
When new drivers are merged into the kernel, then could we use the
driver prefix?  In other words something like this:

media: i2c/imx334: Add imx334 camera sensor driver

 drivers/media/i2c/imx334.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 07e31bc2ef18..7fbea7caef42 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -790,8 +790,9 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
 	imx334->reset_gpio = devm_gpiod_get_optional(imx334->dev, "reset",
 						     GPIOD_OUT_LOW);
 	if (IS_ERR(imx334->reset_gpio)) {
+		ret = PTR_ERR(imx334->reset_gpio);
 		dev_err(imx334->dev, "failed to get reset gpio %d", ret);
-		return PTR_ERR(imx334->reset_gpio);
+		return ret;
 	}
 
 	/* Get sensor input clock */
-- 
2.30.0

Re: [PATCH] media: i2c: imx334: Fix an error message

[Sakari Ailus]

Hi Dan,

On Tue, Feb 09, 2021 at 02:04:48PM +0300, Dan Carpenter wrote:
> The "ret" variable is uninitialized in this error message.
> 
> Fixes: 9746b11715c3 ("media: i2c: Add imx334 camera sensor driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks for the patch. This has been already addressed by a patch from Hans
(but not merged yet):

<URL:https://patchwork.linuxtv.org/project/linux-media/patch/917ccfef-b93e-4d90-0b5a-4974145ab187@xs4all.nl/>

> ---
> When new drivers are merged into the kernel, then could we use the
> driver prefix?  In other words something like this:
> 
> media: i2c/imx334: Add imx334 camera sensor driver

We've usually had driver's name and Mauro's scripts add media: prefix ---
unless it's already there. Are you suggesting also the bus should be part
of it?

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] media: i2c: imx334: Fix an error message

[Dan Carpenter]

On Tue, Feb 09, 2021 at 02:08:04PM +0200, Sakari Ailus wrote:
> Hi Dan,
> 
> On Tue, Feb 09, 2021 at 02:04:48PM +0300, Dan Carpenter wrote:
> > The "ret" variable is uninitialized in this error message.
> > 
> > Fixes: 9746b11715c3 ("media: i2c: Add imx334 camera sensor driver")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Thanks for the patch. This has been already addressed by a patch from Hans
> (but not merged yet):
> 
> <URL:https://patchwork.linuxtv.org/project/linux-media/patch/917ccfef-b93e-4d90-0b5a-4974145ab187@xs4all.nl/>
> 
> > ---
> > When new drivers are merged into the kernel, then could we use the
> > driver prefix?  In other words something like this:
> > 
> > media: i2c/imx334: Add imx334 camera sensor driver
> 
> We've usually had driver's name and Mauro's scripts add media: prefix ---
> unless it's already there. Are you suggesting also the bus should be part
> of it?

No, what I'm saying is when people add a new driver they do:

[PATCH] subsystem: Add new driver for foo345

But it would be better if they added "foo345" to the prefix.

[PATCH] subsystem: foo345: Add new driver for foo345

Doing it the way that I'm suggesting has become more common for the past
four years.  Four years ago was when I started complaining that I can't
guess the correct prefix for new drivers.  That was also the last time
that someone complained to me that I had used the incorrect patch prefix.
I would argue that Hans used the wrong patch prefix for his patch so
maybe we have just become more mellow these days.

And also I'm surprised that Mauro adds the media: prefix for you instead
of making everyone do it themselves...  He's the only person who does
this that I know of.

regards,
dan carpenter

Re: [PATCH] media: i2c: imx334: Fix an error message

[Sakari Ailus]

On Tue, Feb 09, 2021 at 04:49:39PM +0300, Dan Carpenter wrote:
> On Tue, Feb 09, 2021 at 02:08:04PM +0200, Sakari Ailus wrote:
> > Hi Dan,
> > 
> > On Tue, Feb 09, 2021 at 02:04:48PM +0300, Dan Carpenter wrote:
> > > The "ret" variable is uninitialized in this error message.
> > > 
> > > Fixes: 9746b11715c3 ("media: i2c: Add imx334 camera sensor driver")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > Thanks for the patch. This has been already addressed by a patch from Hans
> > (but not merged yet):
> > 
> > <URL:https://patchwork.linuxtv.org/project/linux-media/patch/917ccfef-b93e-4d90-0b5a-4974145ab187@xs4all.nl/>
> > 
> > > ---
> > > When new drivers are merged into the kernel, then could we use the
> > > driver prefix?  In other words something like this:
> > > 
> > > media: i2c/imx334: Add imx334 camera sensor driver
> > 
> > We've usually had driver's name and Mauro's scripts add media: prefix ---
> > unless it's already there. Are you suggesting also the bus should be part
> > of it?
> 
> No, what I'm saying is when people add a new driver they do:
> 
> [PATCH] subsystem: Add new driver for foo345
> 
> But it would be better if they added "foo345" to the prefix.
> 
> [PATCH] subsystem: foo345: Add new driver for foo345

Ah, yes. I also prefer this but I guess this hasn't been consistently
enforced nor documented (or is it?) and can be easily missed in review.
Then there are patches that touch a number of drivers at the same time...

> 
> Doing it the way that I'm suggesting has become more common for the past
> four years.  Four years ago was when I started complaining that I can't
> guess the correct prefix for new drivers.  That was also the last time
> that someone complained to me that I had used the incorrect patch prefix.
> I would argue that Hans used the wrong patch prefix for his patch so
> maybe we have just become more mellow these days.
> 
> And also I'm surprised that Mauro adds the media: prefix for you instead
> of making everyone do it themselves...  He's the only person who does
> this that I know of.

It's possible since Mauro picks the patches from the pull request instead
of pulling it.

-- 
Sakari Ailus