[1/3] media: atmel-isi: move the peripheral clock to start/stop_stream() function
Commit Message
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.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Comments
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.wu@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)
On 3/5/2015 6:39 PM, Laurent Pinchart wrote:
> 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 ?
So far, I don't see any problem yet. But I'd like to be sure of that.
I'll give you the feedback after more test.
Thanks for make me head up.
Best Regards,
Josh Wu
>
>> Signed-off-by: Josh Wu <josh.wu@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)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -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)