Re: [PATCH] clk: samsung: Add CLKOUT driver support for Exynos3250 SoC.

2015-03-01 Thread Tomasz Figa 
2015-02-25 19:13 GMT+09:00 Inha Song :
> Hi, Tomasz,
>
> Thanks for you comment :)
>
> On Wed, 25 Feb 2015 09:54:02 +0900
> Tomasz Figa  wrote:
>
>> Hi Inha,
>>
>> Thanks for the patch. Please see my comments inline.
>>
>> 2015-02-24 18:22 GMT+09:00 Inha Song :
>> > This patch add CLKOUT driver support for Exynos3250 SoC.
>>
>> Could you please add a little more information? I know that it might
>> be pretty obvious to people familiar with this driver and/or hardware,
>> but it might be a good idea to explicitly say that the CLKOUT
>> controller is compatible with Exynos4, so only a new compatible string
>> is added.
>>
>> On the other hand, do you really need to add a new compatible string
>> if an existing one can be reused? The reason why the DT property is
>> called "compatible" is to be able to use the same compatible strings
>> on different devices, because they are compatible, even though the
>> string might have its name after only one of them. If there is some
>> additional reason to add a new compatible string, please write this in
>> commit message.
>
> In in PMU document(Document/devicetree/bindgins/arm/samsung/pmu.txt),
> "samsung,exynos3250-pmu" compatible string had been added.
>
> So I add this compatible in driver.
>

Ah, right, the whole PMU block is not compatible with Exynos4, so a
new compatible string is needed indeed. It should be said in patch
description, though.

>>
>> >
>> > Signed-off-by: Inha Song 
>> > ---
>> >  drivers/clk/samsung/clk-exynos-clkout.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/drivers/clk/samsung/clk-exynos-clkout.c 
>> > b/drivers/clk/samsung/clk-exynos-clkout.c
>> > index 3a7cb25..1c02e73 100644
>> > --- a/drivers/clk/samsung/clk-exynos-clkout.c
>> > +++ b/drivers/clk/samsung/clk-exynos-clkout.c
>> > @@ -142,6 +142,8 @@ CLK_OF_DECLARE(exynos4212_clkout, 
>> > "samsung,exynos4212-pmu",
>> > exynos4_clkout_init);
>> >  CLK_OF_DECLARE(exynos4412_clkout, "samsung,exynos4412-pmu",
>> > exynos4_clkout_init);
>> > +CLK_OF_DECLARE(exynos3250_clkout, "samsung,exynos3250-pmu",
>> > +   exynos4_clkout_init);
>>
>> Are you sure that the PMU DEBUG register on Exynos3250 is indeed
>> compatible with Exynos4 and not with newer SoCs? AFAIR, the only
>> difference was the number of bits (4 on Exynos4 and 5 on Exynos5?) of
>> the main mux.
>
> Exynos3250 PMU_DEBUG register is same with Exynos4. So I just use 
> exynos4_clkout_init function.
> But, on second thought, It looks good to add exynos3_clkout_init function for 
> Exynos3 SoC even though it's the same with exynos4_clkout_init.
>
> what is your opinion?

Well, if there is no need then I don't see any reason why having a
separate function doing the same things could have any benefits. If
some differences are discovered in future it can be always modified in
a separate patch.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: samsung: Add CLKOUT driver support for Exynos3250 SoC.

2015-02-25 Thread Inha Song 
Hi, Tomasz,

Thanks for you comment :)

On Wed, 25 Feb 2015 09:54:02 +0900
Tomasz Figa  wrote:

> Hi Inha,
> 
> Thanks for the patch. Please see my comments inline.
> 
> 2015-02-24 18:22 GMT+09:00 Inha Song :
> > This patch add CLKOUT driver support for Exynos3250 SoC.
> 
> Could you please add a little more information? I know that it might
> be pretty obvious to people familiar with this driver and/or hardware,
> but it might be a good idea to explicitly say that the CLKOUT
> controller is compatible with Exynos4, so only a new compatible string
> is added.
> 
> On the other hand, do you really need to add a new compatible string
> if an existing one can be reused? The reason why the DT property is
> called "compatible" is to be able to use the same compatible strings
> on different devices, because they are compatible, even though the
> string might have its name after only one of them. If there is some
> additional reason to add a new compatible string, please write this in
> commit message.

In in PMU document(Document/devicetree/bindgins/arm/samsung/pmu.txt),
"samsung,exynos3250-pmu" compatible string had been added.

So I add this compatible in driver.

> 
> >
> > Signed-off-by: Inha Song 
> > ---
> >  drivers/clk/samsung/clk-exynos-clkout.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos-clkout.c 
> > b/drivers/clk/samsung/clk-exynos-clkout.c
> > index 3a7cb25..1c02e73 100644
> > --- a/drivers/clk/samsung/clk-exynos-clkout.c
> > +++ b/drivers/clk/samsung/clk-exynos-clkout.c
> > @@ -142,6 +142,8 @@ CLK_OF_DECLARE(exynos4212_clkout, 
> > "samsung,exynos4212-pmu",
> > exynos4_clkout_init);
> >  CLK_OF_DECLARE(exynos4412_clkout, "samsung,exynos4412-pmu",
> > exynos4_clkout_init);
> > +CLK_OF_DECLARE(exynos3250_clkout, "samsung,exynos3250-pmu",
> > +   exynos4_clkout_init);
> 
> Are you sure that the PMU DEBUG register on Exynos3250 is indeed
> compatible with Exynos4 and not with newer SoCs? AFAIR, the only
> difference was the number of bits (4 on Exynos4 and 5 on Exynos5?) of
> the main mux.

Exynos3250 PMU_DEBUG register is same with Exynos4. So I just use 
exynos4_clkout_init function.
But, on second thought, It looks good to add exynos3_clkout_init function for 
Exynos3 SoC even though it's the same with exynos4_clkout_init.

what is your opinion?

Best Regards,
Inha Song.

> 
> Best regards,
> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: samsung: Add CLKOUT driver support for Exynos3250 SoC.

2015-02-24 Thread Tomasz Figa 
Hi Inha,

Thanks for the patch. Please see my comments inline.

2015-02-24 18:22 GMT+09:00 Inha Song :
> This patch add CLKOUT driver support for Exynos3250 SoC.

Could you please add a little more information? I know that it might
be pretty obvious to people familiar with this driver and/or hardware,
but it might be a good idea to explicitly say that the CLKOUT
controller is compatible with Exynos4, so only a new compatible string
is added.

On the other hand, do you really need to add a new compatible string
if an existing one can be reused? The reason why the DT property is
called "compatible" is to be able to use the same compatible strings
on different devices, because they are compatible, even though the
string might have its name after only one of them. If there is some
additional reason to add a new compatible string, please write this in
commit message.

>
> Signed-off-by: Inha Song 
> ---
>  drivers/clk/samsung/clk-exynos-clkout.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c 
> b/drivers/clk/samsung/clk-exynos-clkout.c
> index 3a7cb25..1c02e73 100644
> --- a/drivers/clk/samsung/clk-exynos-clkout.c
> +++ b/drivers/clk/samsung/clk-exynos-clkout.c
> @@ -142,6 +142,8 @@ CLK_OF_DECLARE(exynos4212_clkout, 
> "samsung,exynos4212-pmu",
> exynos4_clkout_init);
>  CLK_OF_DECLARE(exynos4412_clkout, "samsung,exynos4412-pmu",
> exynos4_clkout_init);
> +CLK_OF_DECLARE(exynos3250_clkout, "samsung,exynos3250-pmu",
> +   exynos4_clkout_init);

Are you sure that the PMU DEBUG register on Exynos3250 is indeed
compatible with Exynos4 and not with newer SoCs? AFAIR, the only
difference was the number of bits (4 on Exynos4 and 5 on Exynos5?) of
the main mux.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] clk: samsung: Add CLKOUT driver support for Exynos3250 SoC.

2015-02-24 Thread Inha Song 
This patch add CLKOUT driver support for Exynos3250 SoC.

Signed-off-by: Inha Song 
---
 drivers/clk/samsung/clk-exynos-clkout.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/samsung/clk-exynos-clkout.c 
b/drivers/clk/samsung/clk-exynos-clkout.c
index 3a7cb25..1c02e73 100644
--- a/drivers/clk/samsung/clk-exynos-clkout.c
+++ b/drivers/clk/samsung/clk-exynos-clkout.c
@@ -142,6 +142,8 @@ CLK_OF_DECLARE(exynos4212_clkout, "samsung,exynos4212-pmu",
exynos4_clkout_init);
 CLK_OF_DECLARE(exynos4412_clkout, "samsung,exynos4412-pmu",
exynos4_clkout_init);
+CLK_OF_DECLARE(exynos3250_clkout, "samsung,exynos3250-pmu",
+   exynos4_clkout_init);
 
 static void __init exynos5_clkout_init(struct device_node *node)
 {
-- 
2.0.0.390.gcb682f8

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