media: atmel-isi: rework on the clock part and add runtime pm support
Subsystems (2)
cover media: atmel-isi: rework on the clock part and add runtime pm support
This patch series fix the peripheral clock code and enable runtime pm support.
Aslo clean up the code which is for the compatiblity of mck.
Josh Wu (3):
media: atmel-isi: move the peripheral clock to start/stop_stream()
function
media: atmel-isi: add runtime pm support
media: atmel-isi: remove mck back compatiable code as we don't need it
drivers/media/platform/soc_camera/atmel-isi.c | 72 +++++++++++++-------------- 1 file changed, 34 insertions(+), 38 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/3 media: atmel-isi: move the peripheral clock to start/stop_stream() function
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.
---
drivers/media/platform/soc_camera/atmel-isi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/media/platform/soc_camera/atmel-isi.c +6 -6
@@ -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)
Signature · 1 line
1.9.1
PATCH 1/3 media: atmel-isi: move the peripheral clock to start/stop_stream() function
Hi Josh,
Thank you for the patch.
Quoted 3 lines · 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 ?
Quoted 55 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 1208818..eb179e7 100644drivers/media/platform/soc_camera/atmel-isi.c +6
-6@@ -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)
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 1/3 media: atmel-isi: move the peripheral clock to start/stop_stream() function
Quoted 5 lines · Laurent Pinchart
Hi Josh,
Thank you for the patch.
On Thursday 05 March 2015 13:00:59 Josh Wu wrote:
Quoted 8 lines
Quoted 3 lines
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()(callingconfigure_geometry),isi_camera_set_bus_param()oratmel_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
Quoted 56 lines
Quoted 56 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 1208818..eb179e7 100644drivers/media/platform/soc_camera/atmel-isi.c +6
-6@@ -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)
PATCH 2/3 media: atmel-isi: add runtime pm support
In reply to Josh WuThe runtime pm resume/suspend will enable/disable pclk (ISI peripheral clock).
And we need to call rumtime_pm_get/put_sync function to make pm resume/suspend.
---
drivers/media/platform/soc_camera/atmel-isi.c | 36 ++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-)
drivers/media/platform/soc_camera/atmel-isi.c +32 -4
@@ -20,6 +20,7 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #include <media/atmel-isi.h>
@@ -386,9 +387,7 @@ 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; +pm_runtime_get_sync (ici->v4l2_dev.dev);/* Reset ISI */ ret =atmel_isi_wait_status (isi,WAIT_ISI_RESET );
@@ -450,7 +449,7 @@ static void stop_streaming(struct vb2_queue *vq)if (ret <0 )dev_err (icd->parent,"Disable ISI timed out\n" ); -clk_disable_unprepare (isi->pclk); +pm_runtime_put_sync (ici->v4l2_dev.dev); }static struct vb2_ops isi_video_qops = {
@@ -1042,6 +1041,9 @@ static int atmel_isi_probe(struct platform_device *pdev) soc_host->v4l2_dev.dev = &pdev->dev; soc_host->nr = pdev->id; +pm_suspend_ignore_children (&pdev->dev, true); +pm_runtime_enable (&pdev->dev); +if (isi->pdata.asd_sizes) { soc_host->asd = isi->pdata.asd; soc_host->asd_sizes = isi->pdata.asd_sizes;
@@ -1055,6 +1057,7 @@ static int atmel_isi_probe(struct platform_device *pdev)return 0 ; err_register_soc_camera_host: +pm_runtime_disable (&pdev->dev); err_req_irq: err_ioremap:vb2_dma_contig_cleanup_ctx (isi->alloc_ctx);
@@ -1067,6 +1070,30 @@ err_alloc_ctx:return ret; } +static int atmel_isi_runtime_suspend (struct device *dev) +{ +struct soc_camera_host *soc_host =to_soc_camera_host (dev); +struct atmel_isi *isi =container_of (soc_host, + struct atmel_isi, soc_host); + +clk_disable_unprepare (isi->pclk); + +return 0 ; +} +static int atmel_isi_runtime_resume (struct device *dev) +{ +struct soc_camera_host *soc_host =to_soc_camera_host (dev); +struct atmel_isi *isi =container_of (soc_host, + struct atmel_isi, soc_host); + +return clk_prepare_enable (isi->pclk); +} + +static const struct dev_pm_ops atmel_isi_dev_pm_ops = { +SET_RUNTIME_PM_OPS (atmel_isi_runtime_suspend, + atmel_isi_runtime_resume,NULL ) +}; +static const struct of_device_id atmel_isi_of_match[] = { { .compatible ="atmel,at91sam9g45-isi" }, { }
@@ -1079,6 +1106,7 @@ static struct platform_driver atmel_isi_driver = { .name = "atmel_isi", .owner = THIS_MODULE, .of_match_table = of_match_ptr(atmel_isi_of_match), + .pm = &atmel_isi_dev_pm_ops, }, };
Signature · 1 line
1.9.1
PATCH 2/3 media: atmel-isi: add runtime pm support
Hi Josh,
Thank you for the patch.
Quoted 36 lines · On Thursday 05 March 2015 13:01:00 Josh Wu wrote:
The runtime pm resume/suspend will enable/disable pclk (ISI peripheral clock).
And we need to call rumtime_pm_get/put_sync function to make pm resume/suspend.Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 36 +++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex eb179e7..4a384f1 100644drivers/media/platform/soc_camera/atmel-isi.c +2
-3@@ -20,6 +20,7 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #include <media/atmel-isi.h> @@ -386,9 +387,7 @@ 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; +pm_runtime_get_sync (ici->v4l2_dev.dev);/* Reset ISI */ ret =atmel_isi_wait_status (isi,WAIT_ISI_RESET );
drivers/media/platform/soc_camera/atmel-isi.c static void stop_streaming(struct vb2_queue *vq) · line 452
Quoted 5 lines
drivers/media/platform/soc_camera/atmel-isi.c +1
-1@@ -450,7 +449,7 @@ static void stop_streaming(struct vb2_queue *vq)if (ret <0 )dev_err (icd->parent,"Disable ISI timed out\n" ); -clk_disable_unprepare (isi->pclk); +pm_runtime_put_sync (ici->v4l2_dev.dev);
Is there a reason to use pm_runtime_put_sync() and not just pm_runtime_put() ?
Quoted 59 lines
drivers/media/platform/soc_camera/atmel-isi.c +29
-0}static struct vb2_ops isi_video_qops = {@@ -1042,6 +1041,9 @@ static int atmel_isi_probe(struct platform_device *pdev) soc_host->v4l2_dev.dev = &pdev->dev; soc_host->nr = pdev->id; +pm_suspend_ignore_children (&pdev->dev, true); +pm_runtime_enable (&pdev->dev); +if (isi->pdata.asd_sizes) { soc_host->asd = isi->pdata.asd; soc_host->asd_sizes = isi->pdata.asd_sizes;@@ -1055,6 +1057,7 @@ static int atmel_isi_probe(struct platform_device *pdev) return 0; err_register_soc_camera_host: +pm_runtime_disable (&pdev->dev); err_req_irq: err_ioremap:vb2_dma_contig_cleanup_ctx (isi->alloc_ctx);@@ -1067,6 +1070,30 @@ err_alloc_ctx:return ret; } +static int atmel_isi_runtime_suspend (struct device *dev) +{ +struct soc_camera_host *soc_host =to_soc_camera_host (dev); +struct atmel_isi *isi =container_of (soc_host, + struct atmel_isi, soc_host); + +clk_disable_unprepare (isi->pclk); + +return 0 ; +} +static int atmel_isi_runtime_resume (struct device *dev) +{ +struct soc_camera_host *soc_host =to_soc_camera_host (dev); +struct atmel_isi *isi =container_of (soc_host, + struct atmel_isi, soc_host); + +return clk_prepare_enable (isi->pclk); +} + +static const struct dev_pm_ops atmel_isi_dev_pm_ops = { +SET_RUNTIME_PM_OPS (atmel_isi_runtime_suspend, + atmel_isi_runtime_resume,NULL ) +}; +static const struct of_device_id atmel_isi_of_match[] = { { .compatible ="atmel,at91sam9g45-isi" }, { }@@ -1079,6 +1106,7 @@ static struct platform_driver atmel_isi_driver = { .name = "atmel_isi", .owner = THIS_MODULE, .of_match_table = of_match_ptr(atmel_isi_of_match), + .pm = &atmel_isi_dev_pm_ops, }, };
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 2/3 media: atmel-isi: add runtime pm support
Quoted 44 lines · Laurent Pinchart
Hi Josh,
Thank you for the patch.
Quoted 39 lines · On Thursday 05 March 2015 13:01:00 Josh Wu wrote:
The runtime pm resume/suspend will enable/disable pclk (ISI peripheral clock).
And we need to call rumtime_pm_get/put_sync function to make pm resume/suspend.Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 36 +++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex eb179e7..4a384f1 100644drivers/media/platform/soc_camera/atmel-isi.c +2
-3@@ -20,6 +20,7 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #include <media/atmel-isi.h> @@ -386,9 +387,7 @@ 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; +pm_runtime_get_sync (ici->v4l2_dev.dev);/* Reset ISI */ ret =atmel_isi_wait_status (isi,WAIT_ISI_RESET );@@ -450,7 +449,7 @@ static void stop_streaming(struct vb2_queue *vq)if (ret <0 )dev_err (icd->parent,"Disable ISI timed out\n" );
Quoted 3 lines
drivers/media/platform/soc_camera/atmel-isi.c static void stop_streaming(struct vb2_queue *vq) · line 452
Quoted 2 lines
drivers/media/platform/soc_camera/atmel-isi.c +1
-1-clk_disable_unprepare (isi->pclk); +pm_runtime_put_sync (ici->v4l2_dev.dev);Is there a reason to use
pm_runtime_put_sync()and not justpm_runtime_put()?
right, for sure we can use pm_runtime_put() here.
Best Regards,
Josh Wu
Quoted 60 lines
Quoted 60 lines
drivers/media/platform/soc_camera/atmel-isi.c +29
-0}static struct vb2_ops isi_video_qops = {@@ -1042,6 +1041,9 @@ static int atmel_isi_probe(struct platform_device *pdev) soc_host->v4l2_dev.dev = &pdev->dev; soc_host->nr = pdev->id; +pm_suspend_ignore_children (&pdev->dev, true); +pm_runtime_enable (&pdev->dev); +if (isi->pdata.asd_sizes) { soc_host->asd = isi->pdata.asd; soc_host->asd_sizes = isi->pdata.asd_sizes;@@ -1055,6 +1057,7 @@ static int atmel_isi_probe(struct platform_device *pdev) return 0; err_register_soc_camera_host: +pm_runtime_disable (&pdev->dev); err_req_irq: err_ioremap:vb2_dma_contig_cleanup_ctx (isi->alloc_ctx);@@ -1067,6 +1070,30 @@ err_alloc_ctx:return ret; } +static int atmel_isi_runtime_suspend (struct device *dev) +{ +struct soc_camera_host *soc_host =to_soc_camera_host (dev); +struct atmel_isi *isi =container_of (soc_host, + struct atmel_isi, soc_host); + +clk_disable_unprepare (isi->pclk); + +return 0 ; +} +static int atmel_isi_runtime_resume (struct device *dev) +{ +struct soc_camera_host *soc_host =to_soc_camera_host (dev); +struct atmel_isi *isi =container_of (soc_host, + struct atmel_isi, soc_host); + +return clk_prepare_enable (isi->pclk); +} + +static const struct dev_pm_ops atmel_isi_dev_pm_ops = { +SET_RUNTIME_PM_OPS (atmel_isi_runtime_suspend, + atmel_isi_runtime_resume,NULL ) +}; +static const struct of_device_id atmel_isi_of_match[] = { { .compatible ="atmel,at91sam9g45-isi" }, { }@@ -1079,6 +1106,7 @@ static struct platform_driver atmel_isi_driver = { .name = "atmel_isi", .owner = THIS_MODULE, .of_match_table = of_match_ptr(atmel_isi_of_match), + .pm = &atmel_isi_dev_pm_ops, }, };
PATCH 3/3 media: atmel-isi: remove mck back compatiable code as we don't need it
In reply to Josh WuThe master clock should handled by sensor itself.
---
drivers/media/platform/soc_camera/atmel-isi.c | 32 --------------------------- 1 file changed, 32 deletions(-)
drivers/media/platform/soc_camera/atmel-isi.c +0 -32
@@ -83,8 +83,6 @@ struct atmel_isi {struct completion complete;/* ISI peripherial clock */ struct clk *pclk; -/* ISI_MCK, feed to camera sensor to generate pixel clock */ -struct clk *mck;unsigned int irq;struct isi_platform_data pdata;
@@ -725,26 +723,12 @@ static void isi_camera_remove_device(struct soc_camera_device *icd)/* Called with .host_lock held */ static int isi_camera_clock_start (struct soc_camera_host *ici) { -struct atmel_isi *isi = ici->priv; -int ret; - -if (!IS_ERR (isi->mck)) { - ret =clk_prepare_enable (isi->mck); -if (ret) { -return ret; - } - } -return 0 ; }/* Called with .host_lock held */ static void isi_camera_clock_stop (struct soc_camera_host *ici) { -struct atmel_isi *isi = ici->priv; - -if (!IS_ERR (isi->mck)) -clk_disable_unprepare (isi->mck); }static unsigned int isi_camera_poll (struct file *file,poll_table *pt)
@@ -894,7 +878,6 @@ static int atmel_isi_probe_dt(struct atmel_isi *isi,/* Default settings for ISI */ isi->pdata.full_mode =1 ; - isi->pdata.mck_hz =ISI_DEFAULT_MCLK_FREQ ; isi->pdata.frate =ISI_CFG1_FRATE_CAPTURE_ALL ; np =of_graph_get_next_endpoint (np,NULL );
@@ -970,21 +953,6 @@ static int atmel_isi_probe(struct platform_device *pdev)INIT_LIST_HEAD (&isi->video_buffer_list);INIT_LIST_HEAD (&isi->dma_desc_head); -/* ISI_MCK is the sensor master clock. It should be handled by the -* sensor driver directly, as the ISI has no use for that clock. Make -* the clock optional here while platforms transition to the correct -* model. -*/ - isi->mck =devm_clk_get (dev,"isi_mck" ); -if (!IS_ERR (isi->mck)) { -/* Set ISI_MCK's frequency, it should be faster than pixel -* clock. -*/ - ret =clk_set_rate (isi->mck, isi->pdata.mck_hz); -if (ret <0 ) -return ret; - } - isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev, sizeof(struct fbd) * MAX_BUFFER_NUM, &isi->fb_descriptors_phys,
Signature · 1 line
1.9.1
PATCH 3/3 media: atmel-isi: remove mck back compatiable code as we don't need it
Hi Josh,
Thank you for the patch.
Quoted 1 line · On Thursday 05 March 2015 13:01:01 Josh Wu wrote:
The master clock should handled by sensor itself.
I like that :-)
Quoted 25 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 32 ------------------------ 1 file changed, 32 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 4a384f1..50375ce 100644drivers/media/platform/soc_camera/atmel-isi.c +0
-2@@ -83,8 +83,6 @@ struct atmel_isi {struct completion complete;/* ISI peripherial clock */ struct clk *pclk; -/* ISI_MCK, feed to camera sensor to generate pixel clock */ -struct clk *mck;unsigned int irq;struct isi_platform_data pdata;@@ -725,26 +723,12 @@ static void isi_camera_remove_device(struct
soc_camera_device*icd) /* Called with .host_lock held */
static intisi_camera_clock_start(struct soc_camera_host *ici){drivers/media/platform/soc_camera/atmel-isi.c +0
-1-struct atmel_isi *isi = ici->priv;
drivers/media/platform/soc_camera/atmel-isi.c static void isi_camera_remove_device(struct soc_camera_device *icd) · line 726
Quoted 10 lines
drivers/media/platform/soc_camera/atmel-isi.c +0
-9-int ret; - -if (!IS_ERR (isi->mck)) { - ret =clk_prepare_enable (isi->mck); -if (ret) { -return ret; - } - } -return 0 ;
Would it make sense to make the clock_start and clock_stop operations optional in the soc-camera core ?
Quoted 42 lines
}
/* Called with .host_lock held */
static voidisi_camera_clock_stop(struct soc_camera_host *ici){drivers/media/platform/soc_camera/atmel-isi.c +0
-20-struct atmel_isi *isi = ici->priv; - -if (!IS_ERR (isi->mck)) -clk_disable_unprepare (isi->mck); }static unsigned int isi_camera_poll (struct file *file,poll_table *pt)@@ -894,7 +878,6 @@ static int atmel_isi_probe_dt(struct atmel_isi *isi,/* Default settings for ISI */ isi->pdata.full_mode =1 ; - isi->pdata.mck_hz =ISI_DEFAULT_MCLK_FREQ ; isi->pdata.frate =ISI_CFG1_FRATE_CAPTURE_ALL ; np =of_graph_get_next_endpoint (np,NULL );@@ -970,21 +953,6 @@ static int atmel_isi_probe(struct platform_device *pdev) INIT_LIST_HEAD(&isi->video_buffer_list);INIT_LIST_HEAD (&isi->dma_desc_head); -/* ISI_MCK is the sensor master clock. It should be handled by the -* sensor driver directly, as the ISI has no use for that clock. Make -* the clock optional here while platforms transition to the correct -* model. -*/ - isi->mck =devm_clk_get (dev,"isi_mck" ); -if (!IS_ERR (isi->mck)) { -/* Set ISI_MCK's frequency, it should be faster than pixel -* clock. -*/ - ret =clk_set_rate (isi->mck, isi->pdata.mck_hz); -if (ret <0 ) -return ret; - } - isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev, sizeof(struct fbd) * MAX_BUFFER_NUM, &isi->fb_descriptors_phys,
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 3/3 media: atmel-isi: remove mck back compatiable code as we don't need it
Quoted 37 lines · Laurent Pinchart
Hi Josh,
Thank you for the patch.
Quoted 1 line · On Thursday 05 March 2015 13:01:01 Josh Wu wrote:
The master clock should handled by sensor itself.
I like that :-)
Quoted 29 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 32 ------------------------ 1 file changed, 32 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 4a384f1..50375ce 100644drivers/media/platform/soc_camera/atmel-isi.c +0
-2@@ -83,8 +83,6 @@ struct atmel_isi {struct completion complete;/* ISI peripherial clock */ struct clk *pclk; -/* ISI_MCK, feed to camera sensor to generate pixel clock */ -struct clk *mck;unsigned int irq;struct isi_platform_data pdata;@@ -725,26 +723,12 @@ static void isi_camera_remove_device(struct
soc_camera_device*icd) /* Called with .host_lock held */
static intisi_camera_clock_start(struct soc_camera_host *ici){drivers/media/platform/soc_camera/atmel-isi.c +0
-5-struct atmel_isi *isi = ici->priv; -int ret; - -if (!IS_ERR (isi->mck)) { - ret =clk_prepare_enable (isi->mck);
Quoted 8 lines
drivers/media/platform/soc_camera/atmel-isi.c static void isi_camera_remove_device(struct soc_camera_device *icd) · line 726
Quoted 6 lines
drivers/media/platform/soc_camera/atmel-isi.c +0
-5-if (ret) { -return ret; - } - } -return 0 ;Would it make sense to make the
clock_startandclock_stopoperations optional in the soc-camera core ?
I agree. For those camera host which don't provide master clock for sensor, clock_start and clock_stop should be optional.
Hi, Guennadi
Do you agree with this?
Best Regards,
Josh Wu
Quoted 43 lines
Quoted 43 lines
} /* Called with .host_lock held */ static void isi_camera_clock_stop(struct soc_camera_host *ici) {drivers/media/platform/soc_camera/atmel-isi.c +0
-20-struct atmel_isi *isi = ici->priv; - -if (!IS_ERR (isi->mck)) -clk_disable_unprepare (isi->mck); }static unsigned int isi_camera_poll (struct file *file,poll_table *pt)@@ -894,7 +878,6 @@ static int atmel_isi_probe_dt(struct atmel_isi *isi,/* Default settings for ISI */ isi->pdata.full_mode =1 ; - isi->pdata.mck_hz =ISI_DEFAULT_MCLK_FREQ ; isi->pdata.frate =ISI_CFG1_FRATE_CAPTURE_ALL ; np =of_graph_get_next_endpoint (np,NULL );@@ -970,21 +953,6 @@ static int atmel_isi_probe(struct platform_device *pdev) INIT_LIST_HEAD(&isi->video_buffer_list);INIT_LIST_HEAD (&isi->dma_desc_head); -/* ISI_MCK is the sensor master clock. It should be handled by the -* sensor driver directly, as the ISI has no use for that clock. Make -* the clock optional here while platforms transition to the correct -* model. -*/ - isi->mck =devm_clk_get (dev,"isi_mck" ); -if (!IS_ERR (isi->mck)) { -/* Set ISI_MCK's frequency, it should be faster than pixel -* clock. -*/ - ret =clk_set_rate (isi->mck, isi->pdata.mck_hz); -if (ret <0 ) -return ret; - } - isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev, sizeof(struct fbd) * MAX_BUFFER_NUM, &isi->fb_descriptors_phys,
PATCH 3/3 media: atmel-isi: remove mck back compatiable code as we don't need it
Hi Josh, Laurent,
Quoted 48 lines · Josh Wu
Quoted 39 lines · Laurent Pinchart
Hi Josh,
Thank you for the patch.
Quoted 1 line · On Thursday 05 March 2015 13:01:01 Josh Wu wrote:
The master clock should handled by sensor itself.
I like that :-)
Quoted 31 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 32 ------------------------ 1 file changed, 32 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 4a384f1..50375ce 100644drivers/media/platform/soc_camera/atmel-isi.c +0
-2@@ -83,8 +83,6 @@ struct atmel_isi {struct completion complete;/* ISI peripherial clock */ struct clk *pclk; -/* ISI_MCK, feed to camera sensor to generate pixel clock */ -struct clk *mck;unsigned int irq;struct isi_platform_data pdata;@@ -725,26 +723,12 @@ static void isi_camera_remove_device(struct
soc_camera_device*icd) /* Called with .host_lock held */
static intisi_camera_clock_start(struct soc_camera_host *ici){drivers/media/platform/soc_camera/atmel-isi.c +0
-6-struct atmel_isi *isi = ici->priv; -int ret; - -if (!IS_ERR (isi->mck)) { - ret =clk_prepare_enable (isi->mck); -if (ret) {Quoted 8 lines
drivers/media/platform/soc_camera/atmel-isi.c static void isi_camera_remove_device(struct soc_camera_device *icd) · line 726
Quoted 5 lines
drivers/media/platform/soc_camera/atmel-isi.c +0
-4-return ret; - } - } -return 0 ;Would it make sense to make the
clock_startandclock_stopoperations optional
in the soc-camera core ?
Quoted 6 lines
I agree. For those camera host which don't provide master clock for sensor,
clock_startandclock_stopshould be optional.Hi, Guennadi
Do you agree with this?
Yes, sure, we can do this. Would anyone like to prepare a patch?
Thanks
Guennadi
PATCH 3/3 media: atmel-isi: remove mck back compatiable code as we don't need it
Quoted 53 lines · On Friday 06 March 2015 21:25:36 Guennadi Liakhovetski wrote:
Quoted 52 lines · Josh Wu
Quoted 50 lines · Laurent Pinchart
Quoted 1 line · On Thursday 05 March 2015 13:01:01 Josh Wu wrote:
The master clock should handled by sensor itself.
I like that :-)
Quoted 32 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 32 ------------------- 1 file changed, 32 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 4a384f1..50375ce 100644drivers/media/platform/soc_camera/atmel-isi.c +0
-2@@ -83,8 +83,6 @@ struct atmel_isi {struct completion complete;/* ISI peripherial clock */ struct clk *pclk; -/* ISI_MCK, feed to camera sensor to generate pixel clock */ -struct clk *mck;unsigned int irq;
struct
isi_platform_datapdata;drivers/media/platform/soc_camera/atmel-isi.c +0
-0@@ -725,26 +723,12 @@ static void isi_camera_remove_device(struct
soc_camera_device*icd) /* Called with .host_lock held */static int isi_camera_clock_start(struct soc_camera_host *ici) {drivers/media/platform/soc_camera/atmel-isi.c +0
-2-struct atmel_isi *isi = ici->priv; -int ret;drivers/media/platform/soc_camera/atmel-isi.c static void isi_camera_remove_device(struct soc_camera_device *icd)
Quoted 10 lines
drivers/media/platform/soc_camera/atmel-isi.c +0
-8- -if (!IS_ERR (isi->mck)) { - ret =clk_prepare_enable (isi->mck); -if (ret) { -return ret; - } - } -return 0 ;Would it make sense to make the
clock_startandclock_stopoperations optional in the soc-camera core ?
Quoted 8 lines
Quoted 6 lines
I agree. For those camera host which don't provide master clock for sensor,
clock_startandclock_stopshould be optional.Hi, Guennadi
Do you agree with this?
Yes, sure, we can do this. Would anyone like to prepare a patch?
Josh, would you like to do that, or should I give it a go ?
Signature · 3 lines
Regards, Laurent Pinchart
PATCH 3/3 media: atmel-isi: remove mck back compatiable code as we don't need it
Hi, Laurent
Quoted 51 lines · Laurent Pinchart
Quoted 50 lines · On Friday 06 March 2015 21:25:36 Guennadi Liakhovetski wrote:
Quoted 49 lines · Josh Wu
Quoted 48 lines · Laurent Pinchart
Quoted 1 line · On Thursday 05 March 2015 13:01:01 Josh Wu wrote:
The master clock should handled by sensor itself.
I like that :-)
Quoted 32 lines
Signed-off-by Josh Wu <[email protected]>---
drivers/media/platform/soc_camera/atmel-isi.c | 32 ------------------- 1 file changed, 32 deletions(-)(quoted diff) +0
-0
b/drivers/media/platform/soc_camera/atmel-isi.cindex 4a384f1..50375ce 100644drivers/media/platform/soc_camera/atmel-isi.c +0
-2@@ -83,8 +83,6 @@ struct atmel_isi {struct completion complete;/* ISI peripherial clock */ struct clk *pclk; -/* ISI_MCK, feed to camera sensor to generate pixel clock */ -struct clk *mck;unsigned int irq; struct isi_platform_data pdata;drivers/media/platform/soc_camera/atmel-isi.c +0
-0@@ -725,26 +723,12 @@ static void isi_camera_remove_device(struct
soc_camera_device*icd) /* Called with .host_lock held */static int isi_camera_clock_start(struct soc_camera_host *ici) {drivers/media/platform/soc_camera/atmel-isi.c +0
-2-struct atmel_isi *isi = ici->priv; -int ret;drivers/media/platform/soc_camera/atmel-isi.c static void isi_camera_remove_device(struct soc_camera_device *icd)
Quoted 10 lines
drivers/media/platform/soc_camera/atmel-isi.c +0
-8- -if (!IS_ERR (isi->mck)) { - ret =clk_prepare_enable (isi->mck); -if (ret) { -return ret; - } - } -return 0 ;Would it make sense to make the
clock_startandclock_stopoperations optional in the soc-camera core ?
Quoted 9 lines
Quoted 7 lines
Quoted 6 lines
I agree. For those camera host which don't provide master clock for sensor,
clock_startandclock_stopshould be optional.Hi, Guennadi
Do you agree with this?
Yes, sure, we can do this. Would anyone like to prepare a patch?
Josh, would you like to do that, or should I give it a go ?
Yes, you can do that if you have time. ;-)
Best Regards,
Josh Wu