On Thu, 2011-08-18 at 14:51 +0300, Sakari Ailus wrote: > On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: > > On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: > > > The ->power() could be absent or not used on some platforms. This patch makes > > > its presence optional. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > > Cc: Sakari Ailus <sakari.ailus@xxxxxx> > > > --- > > > drivers/media/video/adp1653.c | 5 +++++ > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c > > > index 0fd9579..f830313 100644 > > > --- a/drivers/media/video/adp1653.c > > > +++ b/drivers/media/video/adp1653.c > > > @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) > > > struct adp1653_flash *flash = to_adp1653_flash(subdev); > > > int ret = 0; > > > > > > + /* There is no need to switch power in case of absence ->power() > > > + * method. */ > > > + if (flash->platform_data->power == NULL) > > > + return 0; > > > + > > > mutex_lock(&flash->power_lock); > > > > > > /* If the power count is modified from 0 to != 0 or from != 0 to 0, > > > > He-h, I guess you are not going to apply this one. > > The patch breaks init logic of the device. If we have no ->power(), we > > still need to bring the device to the known state. I have no good idea > > how to do this. > > I don't think it breaks anything actually. Albeit in practice one is still > likely to put the adp1653 reset line to the board since that lowers its power > consumption significantly. Yeah, even in practice we might see various ways of a chip connection. > Instead of being in power-up state after opening the flash subdev, it will > reach this state already when the system is powered up. At subdev open all > the relevant registers are written to anyway, so I don't see an issue here. You mean at first writing to the V4L2 value, do you? Because ->open() uses set_power() which will be skipped in case of no ->power method defined. > I think either this one, or one should check in probe() that the power() > callback is non-NULL. > The board code is going away in the near future so this callback will > disappear eventually anyway. So, it's up to you to include or not my last patch. > The gpio code in the board file should likely > be moved to the driver itself. The line could be different, the hw could be used in environment w/o gpio, but with (for example) external gate, and so on. I think current generic driver is pretty okay. And what to do with limits? Pass them as the module parameters? > That assumes that there will be a gpio which > can be used to enable and disable the device and I'm not fully certain this > is generic enough. Hopefully it is, but I don't know where else the adp1653 > would be used than on the N900. Don't narrow a chip application to the one device. -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
