Hi Josh,

Thank you for the patch.

On Thursday 05 March 2015 13:00:59 Josh Wu wrote:
> As the clock_start/stop() use to control the mclk for the sensor not the
> ISI peripheral clock.
> So we move them to start/stop_stream() function.

Then the driver will access registers with the peripheral clock disabled, for 
instance in isi_camera_set_fmt() (calling configure_geometry), 
isi_camera_set_bus_param() or atmel_isi_probe(). Isn't that a problem ? Or are 
all registers guaranteed to be accessible (and retained) when the clock is 
disabled ?

> Signed-off-by: Josh Wu <josh...@atmel.com>
> ---
> 
>  drivers/media/platform/soc_camera/atmel-isi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
> b/drivers/media/platform/soc_camera/atmel-isi.c index 1208818..eb179e7
> 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -386,6 +386,10 @@ static int start_streaming(struct vb2_queue *vq,
> unsigned int count) struct atmel_isi *isi = ici->priv;
>       int ret;
> 
> +     ret = clk_prepare_enable(isi->pclk);
> +     if (ret)
> +             return ret;
> +
>       /* Reset ISI */
>       ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
>       if (ret < 0) {
> @@ -445,6 +449,8 @@ static void stop_streaming(struct vb2_queue *vq)
>       ret = atmel_isi_wait_status(isi, WAIT_ISI_DISABLE);
>       if (ret < 0)
>               dev_err(icd->parent, "Disable ISI timed out\n");
> +
> +     clk_disable_unprepare(isi->pclk);
>  }
> 
>  static struct vb2_ops isi_video_qops = {
> @@ -723,14 +729,9 @@ static int isi_camera_clock_start(struct
> soc_camera_host *ici) struct atmel_isi *isi = ici->priv;
>       int ret;
> 
> -     ret = clk_prepare_enable(isi->pclk);
> -     if (ret)
> -             return ret;
> -
>       if (!IS_ERR(isi->mck)) {
>               ret = clk_prepare_enable(isi->mck);
>               if (ret) {
> -                     clk_disable_unprepare(isi->pclk);
>                       return ret;
>               }
>       }
> @@ -745,7 +746,6 @@ static void isi_camera_clock_stop(struct soc_camera_host
> *ici)
> 
>       if (!IS_ERR(isi->mck))
>               clk_disable_unprepare(isi->mck);
> -     clk_disable_unprepare(isi->pclk);
>  }
> 
>  static unsigned int isi_camera_poll(struct file *file, poll_table *pt)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to