[1/2,media] V4L: atmel-isi: add code to enable/disable ISI_MCK clock
Commit Message
This patch
- add ISI_MCK clock enable/disable code.
- change field name in isi_platform_data structure
Signed-off-by: Josh Wu <josh.wu@atmel.com>
[g.liakhovetski@gmx.de: fix label names]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
Hi, Guennadi
I rebase this patch to current media tree: staging/for_v3.3.
The second patch added the clk_prepare()/clk_unprepare() base on Russell King's suggestion.
Best Regards,
Josh Wu
drivers/media/video/atmel-isi.c | 31 +++++++++++++++++++++++++++++--
include/media/atmel-isi.h | 4 +++-
2 files changed, 32 insertions(+), 3 deletions(-)
Comments
On Wed, Nov 30, 2011 at 06:06:43PM +0800, Josh Wu wrote:
> + /* Get ISI_MCK, provided by programmable clock or external clock */
> + isi->mck = clk_get(dev, "isi_mck");
> + if (IS_ERR_OR_NULL(isi->mck)) {
This should be IS_ERR()
> + dev_err(dev, "Failed to get isi_mck\n");
> + ret = isi->mck ? PTR_ERR(isi->mck) : -EINVAL;
ret = PTR_ERR(isi->mck);
--
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
Hi, Russell King
On Wed, Dec 07, 2011 at 4:50 PM, Russell King wrote:
> On Wed, Nov 30, 2011 at 06:06:43PM +0800, Josh Wu wrote:
>> + /* Get ISI_MCK, provided by programmable clock or external clock
*/
>> + isi->mck = clk_get(dev, "isi_mck");
>> + if (IS_ERR_OR_NULL(isi->mck)) {
> This should be IS_ERR()
So it means the clk_get() will never return NULL even when clk structure
is NULL in clk lookup entry. Right?
>> + dev_err(dev, "Failed to get isi_mck\n");
>> + ret = isi->mck ? PTR_ERR(isi->mck) : -EINVAL;
> ret = PTR_ERR(isi->mck);
Best Regards,
Josh Wu
--
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
On Wed, Dec 07, 2011 at 06:12:52PM +0800, Wu, Josh wrote:
> Hi, Russell King
>
> On Wed, Dec 07, 2011 at 4:50 PM, Russell King wrote:
>
> > On Wed, Nov 30, 2011 at 06:06:43PM +0800, Josh Wu wrote:
> >> + /* Get ISI_MCK, provided by programmable clock or external clock
> */
> >> + isi->mck = clk_get(dev, "isi_mck");
> >> + if (IS_ERR_OR_NULL(isi->mck)) {
>
> > This should be IS_ERR()
>
> So it means the clk_get() will never return NULL even when clk structure
> is NULL in clk lookup entry. Right?
It is not the drivers business to know whether NULL is valid or not.
clk_get() is defined to either return an error pointer, or a cookie
which the rest of the clk API must accept.
If an implementation decides that clk_get() can return NULL and deals
with that in the rest of the API (eg, to mean 'there is no clock but
don't fail for this') then drivers must not reject that.
If a driver rejects NULL then it is performing checks outside of the
definition of the clk API, and making assumptions about the nature of
valid cookies.
--
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
On Thursday, December 08, 2011 6:40AM, Russell King wrote:
> On Wed, Dec 07, 2011 at 06:12:52PM +0800, Wu, Josh wrote:
>> Hi, Russell King
>>
>> On Wed, Dec 07, 2011 at 4:50 PM, Russell King wrote:
>>
>> > On Wed, Nov 30, 2011 at 06:06:43PM +0800, Josh Wu wrote:
>> >> + /* Get ISI_MCK, provided by programmable clock or external clock
>> */
>> >> + isi->mck = clk_get(dev, "isi_mck");
>> >> + if (IS_ERR_OR_NULL(isi->mck)) {
>>
>> > This should be IS_ERR()
>>
>> So it means the clk_get() will never return NULL even when clk
structure
>> is NULL in clk lookup entry. Right?
> It is not the drivers business to know whether NULL is valid or not.
> clk_get() is defined to either return an error pointer, or a cookie
> which the rest of the clk API must accept.
> If an implementation decides that clk_get() can return NULL and deals
> with that in the rest of the API (eg, to mean 'there is no clock but
> don't fail for this') then drivers must not reject that.
> If a driver rejects NULL then it is performing checks outside of the
> definition of the clk API, and making assumptions about the nature of
> valid cookies.
Thanks for the feedback. I will send v3 patch which will not check the
null return value.
Best Regards,
Josh Wu
--
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
@@ -90,7 +90,10 @@ struct atmel_isi {
struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
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;
@@ -766,6 +769,12 @@ static int isi_camera_add_device(struct soc_camera_device *icd)
if (ret)
return ret;
+ ret = clk_enable(isi->mck);
+ if (ret) {
+ clk_disable(isi->pclk);
+ return ret;
+ }
+
isi->icd = icd;
dev_dbg(icd->parent, "Atmel ISI Camera driver attached to camera %d\n",
icd->devnum);
@@ -779,6 +788,7 @@ static void isi_camera_remove_device(struct soc_camera_device *icd)
BUG_ON(icd != isi->icd);
+ clk_disable(isi->mck);
clk_disable(isi->pclk);
isi->icd = NULL;
@@ -874,7 +884,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd, u32 pixfmt)
if (isi->pdata->has_emb_sync)
cfg1 |= ISI_CFG1_EMB_SYNC;
- if (isi->pdata->isi_full_mode)
+ if (isi->pdata->full_mode)
cfg1 |= ISI_CFG1_FULL_MODE;
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -912,6 +922,7 @@ static int __devexit atmel_isi_remove(struct platform_device *pdev)
isi->fb_descriptors_phys);
iounmap(isi->regs);
+ clk_put(isi->mck);
clk_put(isi->pclk);
kfree(isi);
@@ -930,7 +941,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
struct isi_platform_data *pdata;
pdata = dev->platform_data;
- if (!pdata || !pdata->data_width_flags) {
+ if (!pdata || !pdata->data_width_flags || !pdata->mck_hz) {
dev_err(&pdev->dev,
"No config available for Atmel ISI\n");
return -EINVAL;
@@ -959,6 +970,19 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&isi->video_buffer_list);
INIT_LIST_HEAD(&isi->dma_desc_head);
+ /* Get ISI_MCK, provided by programmable clock or external clock */
+ isi->mck = clk_get(dev, "isi_mck");
+ if (IS_ERR_OR_NULL(isi->mck)) {
+ dev_err(dev, "Failed to get isi_mck\n");
+ ret = isi->mck ? PTR_ERR(isi->mck) : -EINVAL;
+ goto err_clk_get;
+ }
+
+ /* Set ISI_MCK's frequency, it should be faster than pixel clock */
+ ret = clk_set_rate(isi->mck, pdata->mck_hz);
+ if (ret < 0)
+ goto err_set_mck_rate;
+
isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev,
sizeof(struct fbd) * MAX_BUFFER_NUM,
&isi->fb_descriptors_phys,
@@ -1034,6 +1058,9 @@ err_alloc_ctx:
isi->p_fb_descriptors,
isi->fb_descriptors_phys);
err_alloc_descriptors:
+err_set_mck_rate:
+ clk_put(isi->mck);
+err_clk_get:
kfree(isi);
err_alloc_isi:
clk_put(pclk);
@@ -110,10 +110,12 @@ struct isi_platform_data {
u8 hsync_act_low;
u8 vsync_act_low;
u8 pclk_act_falling;
- u8 isi_full_mode;
+ u8 full_mode;
u32 data_width_flags;
/* Using for ISI_CFG1 */
u32 frate;
+ /* Using for ISI_MCK */
+ u32 mck_hz;
};
#endif /* __ATMEL_ISI_H__ */