* Re: [PATCH] [media] s5p-mfc: Change MFC clock reference w.r.t Common Clock Framework
@ 2013-03-27 2:01 ` Mike Turquette
0 siblings, 0 replies; 7+ messages in thread
From: Mike Turquette @ 2013-03-27 2:01 UTC (permalink / raw)
To: Prasanna Kumar, linux-samsung-soc, kgene.kim, kyungmin.park,
linux-media, linux-arm-kernel
Cc: Prasanna Kumar
Quoting Prasanna Kumar (2013-03-25 22:20:51)
> From: Prasanna Kumar <prasanna.ps@samsung.com>
>
> According to Common Clock framework , modified the method of getting
> clock for MFC Block.
>
> Signed-off-by: Prasanna Kumar <prasanna.ps@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> index 6aa38a5..b8ac8f6 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> @@ -50,7 +50,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
> goto err_p_ip_clk;
> }
>
> - pm->clock = clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
> + pm->clock = clk_get_parent(pm->clock_gate);
Ok, I'll bite. Why make this change? Was there an issue using
clkdev/clk_get to get the clock you needed?
Regards,
Mike
> if (IS_ERR(pm->clock)) {
> mfc_err("Failed to get MFC clock\n");
> ret = PTR_ERR(pm->clock);
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] [media] s5p-mfc: Change MFC clock reference w.r.t Common Clock Framework
@ 2013-03-27 2:01 ` Mike Turquette
0 siblings, 0 replies; 7+ messages in thread
From: Mike Turquette @ 2013-03-27 2:01 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Prasanna Kumar (2013-03-25 22:20:51)
> From: Prasanna Kumar <prasanna.ps@samsung.com>
>
> According to Common Clock framework , modified the method of getting
> clock for MFC Block.
>
> Signed-off-by: Prasanna Kumar <prasanna.ps@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> index 6aa38a5..b8ac8f6 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> @@ -50,7 +50,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
> goto err_p_ip_clk;
> }
>
> - pm->clock = clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
> + pm->clock = clk_get_parent(pm->clock_gate);
Ok, I'll bite. Why make this change? Was there an issue using
clkdev/clk_get to get the clock you needed?
Regards,
Mike
> if (IS_ERR(pm->clock)) {
> mfc_err("Failed to get MFC clock\n");
> ret = PTR_ERR(pm->clock);
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] [media] s5p-mfc: Change MFC clock reference w.r.t Common Clock Framework
2013-03-27 2:01 ` Mike Turquette
@ 2013-03-27 17:51 ` Sylwester Nawrocki
-1 siblings, 0 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2013-03-27 17:51 UTC (permalink / raw)
To: Prasanna Kumar
Cc: Mike Turquette, linux-samsung-soc, kgene.kim, kyungmin.park,
linux-media, linux-arm-kernel
On 03/27/2013 03:01 AM, Mike Turquette wrote:
> Quoting Prasanna Kumar (2013-03-25 22:20:51)
>> From: Prasanna Kumar <prasanna.ps@samsung.com>
>>
>> According to Common Clock framework , modified the method of getting
Huh ? Could you explain in detail what exactly in this patch is related
to the Common Clock Framework ? I guess you mean the new clocks driver ?
>> clock for MFC Block.
>>
>> Signed-off-by: Prasanna Kumar <prasanna.ps@samsung.com>
>> ---
>> drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
>> index 6aa38a5..b8ac8f6 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
>> @@ -50,7 +50,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
>> goto err_p_ip_clk;
>> }
>>
>> - pm->clock = clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
>> + pm->clock = clk_get_parent(pm->clock_gate);
>
> Ok, I'll bite. Why make this change? Was there an issue using
> clkdev/clk_get to get the clock you needed?
Yes, this appears a pretty mysterious change to me... I wonder, have
you altered anything at the clock tree definition to make it work ?
What platforms has this change been tested on ? For Exynos4, I've
changed the sclk_mfc clock definition, so it is possible to assign it
to the MFC IP by listing it in the codec DT node clocks property.
I suppose something similar is needed for Exynos5, if this is the
platform you needed that change for.
BTW, I think the mclk_name field should be removed from struct
s5p_mfc_variant, an the driver should use fixed name for the
second clock. Now there is "mfc", "sclk_mfc" used for Exynos4 and
"mfc", "aclk_133" for Exynos5. Drivers are not really supposed to
care about differences like this. Such differences could be easily
handled in the device tree. The DT binding documentation just
needs to specify the meaning of each clock name.
[1] http://www.spinics.net/lists/arm-kernel/msg233521.html
Regards,
--
Sylwester Nawrocki
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] [media] s5p-mfc: Change MFC clock reference w.r.t Common Clock Framework
@ 2013-03-27 17:51 ` Sylwester Nawrocki
0 siblings, 0 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2013-03-27 17:51 UTC (permalink / raw)
To: linux-arm-kernel
On 03/27/2013 03:01 AM, Mike Turquette wrote:
> Quoting Prasanna Kumar (2013-03-25 22:20:51)
>> From: Prasanna Kumar <prasanna.ps@samsung.com>
>>
>> According to Common Clock framework , modified the method of getting
Huh ? Could you explain in detail what exactly in this patch is related
to the Common Clock Framework ? I guess you mean the new clocks driver ?
>> clock for MFC Block.
>>
>> Signed-off-by: Prasanna Kumar <prasanna.ps@samsung.com>
>> ---
>> drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
>> index 6aa38a5..b8ac8f6 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
>> @@ -50,7 +50,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
>> goto err_p_ip_clk;
>> }
>>
>> - pm->clock = clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
>> + pm->clock = clk_get_parent(pm->clock_gate);
>
> Ok, I'll bite. Why make this change? Was there an issue using
> clkdev/clk_get to get the clock you needed?
Yes, this appears a pretty mysterious change to me... I wonder, have
you altered anything at the clock tree definition to make it work ?
What platforms has this change been tested on ? For Exynos4, I've
changed the sclk_mfc clock definition, so it is possible to assign it
to the MFC IP by listing it in the codec DT node clocks property.
I suppose something similar is needed for Exynos5, if this is the
platform you needed that change for.
BTW, I think the mclk_name field should be removed from struct
s5p_mfc_variant, an the driver should use fixed name for the
second clock. Now there is "mfc", "sclk_mfc" used for Exynos4 and
"mfc", "aclk_133" for Exynos5. Drivers are not really supposed to
care about differences like this. Such differences could be easily
handled in the device tree. The DT binding documentation just
needs to specify the meaning of each clock name.
[1] http://www.spinics.net/lists/arm-kernel/msg233521.html
Regards,
--
Sylwester Nawrocki
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 7+ messages in thread