| Message ID | YCJsUCngklBkJMgw@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx334: Fix an error message | expand |
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?
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
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.
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 */
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(-)