| Message ID | 1756723.mDdT6UkUyR@avalon (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Laurent On Mon, 22 Apr 2013, Laurent Pinchart wrote: > Hi Guennadi, > > Thanks for the patch. > > On Thursday 18 April 2013 23:35:39 Guennadi Liakhovetski wrote: > > The mt9p031 driver first accesses the I2C device in its .registered() > > method. While doing that it furst powers the device up, but if probing > > s/furst/first/ > > > fails, it doesn't power the chip back down. This patch fixes that bug. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > > > --- > > drivers/media/i2c/mt9p031.c | 10 ++++++---- > > 1 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c > > index eb2de22..70f4525 100644 > > --- a/drivers/media/i2c/mt9p031.c > > +++ b/drivers/media/i2c/mt9p031.c > > @@ -844,7 +844,7 @@ static int mt9p031_registered(struct v4l2_subdev > > *subdev) ret = mt9p031_power_on(mt9p031); > > if (ret < 0) { > > dev_err(&client->dev, "MT9P031 power up failed\n"); > > - return ret; > > + goto done; > > Not here. If power on fails, there's no need to power off. Oops, right. > > } > > > > /* Read out the chip version register */ > > @@ -852,13 +852,15 @@ static int mt9p031_registered(struct v4l2_subdev > > *subdev) if (data != MT9P031_CHIP_VERSION_VALUE) { > > dev_err(&client->dev, "MT9P031 not detected, wrong version " > > "0x%04x\n", data); > > - return -ENODEV; > > + ret = -ENODEV; > > } > > > > +done: > > mt9p031_power_off(mt9p031); > > > > - dev_info(&client->dev, "MT9P031 detected at address 0x%02x\n", > > - client->addr); > > + if (!ret) > > + dev_info(&client->dev, "MT9P031 detected at address 0x%02x\n", > > + client->addr); > > > > return ret; > > } > > It would be easier to just move the power off line right after the > mt9p031_read() call and leave the rest unchanged. Sure, please, do. Thanks Guennadi > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c > index 28cf95b..8de84c0 100644 > --- a/drivers/media/i2c/mt9p031.c > +++ b/drivers/media/i2c/mt9p031.c > @@ -849,18 +849,18 @@ static int mt9p031_registered(struct v4l2_subdev > *subdev) > > /* Read out the chip version register */ > data = mt9p031_read(client, MT9P031_CHIP_VERSION); > + mt9p031_power_off(mt9p031); > + > if (data != MT9P031_CHIP_VERSION_VALUE) { > dev_err(&client->dev, "MT9P031 not detected, wrong version " > "0x%04x\n", data); > return -ENODEV; > } > > - mt9p031_power_off(mt9p031); > - > dev_info(&client->dev, "MT9P031 detected at address 0x%02x\n", > client->addr); > > - return ret; > + return 0; > } > > static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh > *fh) > > If you're happy with that there's no need to resubmit, I'll apply the patch to > my tree for v3.11. > > -- > Regards, > > Laurent Pinchart > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 28cf95b..8de84c0 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -849,18 +849,18 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) /* Read out the chip version register */ data = mt9p031_read(client, MT9P031_CHIP_VERSION); + mt9p031_power_off(mt9p031); + if (data != MT9P031_CHIP_VERSION_VALUE) { dev_err(&client->dev, "MT9P031 not detected, wrong version " "0x%04x\n", data); return -ENODEV; } - mt9p031_power_off(mt9p031); - dev_info(&client->dev, "MT9P031 detected at address 0x%02x\n", client->addr); - return ret; + return 0; }