Hi Sergio, On Thu, May 03, 2012 at 10:20:47PM -0500, Aguirre, Sergio wrote: > Hi Sakari, > > On Thu, May 3, 2012 at 7:03 AM, Aguirre, Sergio <saagui...@ti.com> wrote: > > Hi Sakari, > > > > Thanks for reviewing. > > > > On Wed, May 2, 2012 at 2:47 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote: > >> > >> Hi Sergio, > >> > >> Thanks for the patches!! > >> > >> On Wed, May 02, 2012 at 10:15:46AM -0500, Sergio Aguirre wrote: > >> ... > >>> +static int sdp4430_ov_cam1_power(struct v4l2_subdev *subdev, int on) > >>> +{ > >>> + struct device *dev = subdev->v4l2_dev->dev; > >>> + int ret; > >>> + > >>> + if (on) { > >>> + if (!regulator_is_enabled(sdp4430_cam2pwr_reg)) { > >>> + ret = regulator_enable(sdp4430_cam2pwr_reg); > >>> + if (ret) { > >>> + dev_err(dev, > >>> + "Error in enabling sensor power > >>> regulator 'cam2pwr'\n"); > >>> + return ret; > >>> + } > >>> + > >>> + msleep(50); > >>> + } > >>> + > >>> + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 1); > >>> + msleep(10); > >>> + ret = clk_enable(sdp4430_cam1_aux_clk); /* Enable XCLK */ > >>> + if (ret) { > >>> + dev_err(dev, > >>> + "Error in clk_enable() in %s(%d)\n", > >>> + __func__, on); > >>> + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 0); > >>> + return ret; > >>> + } > >>> + msleep(10); > >>> + } else { > >>> + clk_disable(sdp4430_cam1_aux_clk); > >>> + msleep(1); > >>> + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 0); > >>> + if (regulator_is_enabled(sdp4430_cam2pwr_reg)) { > >>> + ret = regulator_disable(sdp4430_cam2pwr_reg); > >>> + if (ret) { > >>> + dev_err(dev, > >>> + "Error in disabling sensor power > >>> regulator 'cam2pwr'\n"); > >>> + return ret; > >>> + } > >>> + } > >>> + } > >>> + > >>> + return 0; > >>> +} > >> > >> Isn't this something that should be part of the sensor driver? There's > >> nothing in the above code that would be board specific, except the names of > >> the clocks, regulators and GPIOs. The sensor driver could hold the names > >> instead; this would be also compatible with the device tree. > > > > Agreed. I see what you mean... > > > > I'll take care of that. > > Can you please check out these patches? > > 1. > http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/cb6c10d58053180364461e6bc8d30d1ec87e6e22
Ideally we should really get rid of the board code callbacks. What do you need to do there? > 2. > http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/6732e0db25c6647b34ef8f01c244a49a1fd6b45d Isn't reset voltage level (high or low) a property of the sensor rather than the board? Well, I know sometimes the people who typically design the hardware can be quite inventive. ;) > 3. > http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/d61c4e3142dc9cae972f9128fe73d986838c0ca1 > 4. > http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/e83f36001c7f7cbe184ad094d9b0c95c39e5028f Cheers, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html