Re: [PATCH v3 07/10] arm: omap4430sdp: Add support for omap4iss camera

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

It should be possible to have s_power() callback NULL, too.

> +static int sdp4430_ov_cam2_power(struct v4l2_subdev *subdev, int on)
> +{
> +	struct device *dev = subdev->v4l2_dev->dev;
> +	int ret;
> +
> +	if (on) {
> +		u8 gpoctl = 0;
> +
> +		ret = regulator_enable(sdp4430_cam2pwr_reg);
> +		if (ret) {
> +			dev_err(dev,
> +				"Error in enabling sensor power regulator 'cam2pwr'\n");
> +			return ret;
> +		}
> +
> +		msleep(50);
> +
> +		if (twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &gpoctl,
> +				    TWL6040_REG_GPOCTL))
> +			return -ENODEV;
> +		if (twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE,
> +				     gpoctl | TWL6040_GPO3,
> +				     TWL6040_REG_GPOCTL))
> +			return -ENODEV;

The above piece of code looks quite interesting. What does it do?

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@xxxxxx	jabber/XMPP/Gmail: sailus@xxxxxxxxxxxxxx
--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux