Hi, Tomasz, Thanks for you comment :) On Wed, 25 Feb 2015 09:54:02 +0900 Tomasz Figa <tomasz.figa@xxxxxxxxx> wrote: > Hi Inha, > > Thanks for the patch. Please see my comments inline. > > 2015-02-24 18:22 GMT+09:00 Inha Song <ideal.song@xxxxxxxxxxx>: > > 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 <ideal.song@xxxxxxxxxxx> > > --- > > 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-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html