Re: [PATCH 2/4] soc-camera: tw9910: Add power control
Guennadi Liakhovetski <[email protected]>
| Newsgroups | gmane.comp.video.video4linux |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 2 Nov 2009, Kuninori Morimoto wrote: > > Signed-off-by: Kuninori Morimoto <[email protected]> > --- > drivers/media/video/tw9910.c | 31 +++++++++++++++++++++++++++---- > 1 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c > index a0b5bbe..8debcd6 100644 > --- a/drivers/media/video/tw9910.c > +++ b/drivers/media/video/tw9910.c > @@ -152,7 +152,9 @@ > /* 1 : non-auto */ > #define VSCTL 0x08 /* 1 : Vertical out ctrl by DVALID */ > /* 0 : Vertical out ctrl by HACTIVE and DVALID */ > -#define OEN 0x04 /* Output Enable together with TRI_SEL. */ > +#define OEN_TRI_SEL_ALL_ON 0x00 /* Enable output */ > +#define OEN_TRI_SEL_CLK_ON 0x04 /* TRI_SEL output */ You don't seem to use the OEN_TRI_SEL_CLK_ON macro. Didn't you want to tristate pins when inactive? > + Why an extra empty line? > > /* OUTCTR1 */ > #define VSP_LO 0x00 /* 0 : VS pin output polarity is active low */ > @@ -178,6 +180,12 @@ > * but all register content remain unchanged. > * This bit is self-resetting. > */ > +#define CLK_PDN 0x08 /* clock power down */ Hopefully, switching clock off doesn't destroy register content? > +#define Y_PDN 0x04 /* Luma ADC power down */ > +#define C_PDN 0x02 /* Chroma ADC power down */ > + > +/* ACNTL2 */ > +#define PLL_PDN 0x40 /* PLL power down */ > > /* VBICNTL */ > > @@ -236,7 +244,6 @@ struct tw9910_priv { > > static const struct regval_list tw9910_default_regs[] = > { > - { OPFORM, 0x00 }, > { OUTCTR1, VSP_LO | VSSL_VVALID | HSP_HI | HSSL_HSYNC }, > ENDMARKER, > }; > @@ -471,10 +478,24 @@ static int tw9910_mask_set(struct i2c_client *client, u8 command, > > static void tw9910_reset(struct i2c_client *client) > { > - i2c_smbus_write_byte_data(client, ACNTL1, SRESET); > + tw9910_mask_set(client, ACNTL1, SRESET, SRESET); > msleep(1); > } > > +static void tw9910_power(struct i2c_client *client, int enable) > +{ > + u8 acntl1 = 0; > + u8 acntl2 = 0; > + > + if (!enable) { > + acntl1 = CLK_PDN | Y_PDN | C_PDN; > + acntl2 = PLL_PDN; > + } > + > + tw9910_mask_set(client, ACNTL1, 0x0e, acntl1); > + tw9910_mask_set(client, ACNTL2, 0x40, acntl2); > +} > + > static const struct tw9910_scale_ctrl* > tw9910_select_norm(struct soc_camera_device *icd, u32 width, u32 height) > { > @@ -514,6 +535,8 @@ static int tw9910_s_stream(struct v4l2_subdev *sd, int enable) > struct i2c_client *client = sd->priv; > struct tw9910_priv *priv = to_tw9910(client); > > + tw9910_power(client, enable); > + I think you should check the return code here, at least for the stream-on case. You shouldn't return success if register accesses failed. > if (!enable) > return 0; > > @@ -527,7 +550,7 @@ static int tw9910_s_stream(struct v4l2_subdev *sd, int enable) > priv->scale->width, > priv->scale->height); > > - return 0; > + return tw9910_mask_set(client, OPFORM, 0x7, OEN_TRI_SEL_ALL_ON); > } > > static int tw9910_set_bus_param(struct soc_camera_device *icd, > -- > 1.6.0.4 > Thanks Guennadi --- Guennadi Liakhovetski -- video4linux-list mailing list Unsubscribe mailto:[email protected]?subject=unsubscribe https://www.redhat.com/mailman/listinfo/video4linux-list