From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH v2 5/6] clk: samsung: add clock-driver for s3c2416, s3c2443 and s3c2450 Date: Thu, 11 Jul 2013 11:13:55 +0200 Message-ID: <2621812.eUdv97DGos@thinkpad> References: <201307100057.06061.heiko@sntech.de> <201307100100.00872.heiko@sntech.de> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-bk0-f45.google.com ([209.85.214.45]:40380 "EHLO mail-bk0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932597Ab3GKJOR convert rfc822-to-8bit (ORCPT ); Thu, 11 Jul 2013 05:14:17 -0400 Received: by mail-bk0-f45.google.com with SMTP id je9so3178984bkc.4 for ; Thu, 11 Jul 2013 02:14:16 -0700 (PDT) In-Reply-To: <201307100100.00872.heiko@sntech.de> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: Heiko =?ISO-8859-1?Q?St=FCbner?= Cc: Kukjin Kim , mturquette@linaro.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, Thomas Abraham , Russell King Hi Heiko, Looks mostly good, but please see my comments inline. On Wednesday 10 of July 2013 01:00:00 Heiko St=FCbner wrote: > The three SoCs share a common clock tree which only differs in the > existence of some special clocks. >=20 > As with all parts common to these three SoCs the driver is named > after the s3c2443, as it was the first SoC introducing this structure > and there exists no other label to describe this s3c24xx epoch. >=20 > The clock structure is built according to the manuals of the included > SoCs and might include changes in comparison to the previous clock > structure. As an example the sclk_uart gate was never handled previou= sly > and the div_uart was made to be the clock used by the serial driver. >=20 > Signed-off-by: Heiko Stuebner > --- > .../bindings/clock/samsung,s3c2443-clock.txt | 48 +++ > drivers/clk/Kconfig | 1 + > drivers/clk/samsung/Kconfig | 2 + > drivers/clk/samsung/Makefile | 1 + > drivers/clk/samsung/clk-s3c2443.c | 422 > ++++++++++++++++++++ include/dt-bindings/clock/samsung,s3c2443-clock.= h | =20 > 96 +++++ > 6 files changed, 570 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/clock/samsung,s3c2443-clock.txt cre= ate > mode 100644 drivers/clk/samsung/Kconfig > create mode 100644 drivers/clk/samsung/clk-s3c2443.c > create mode 100644 include/dt-bindings/clock/samsung,s3c2443-clock.h >=20 > diff --git > a/Documentation/devicetree/bindings/clock/samsung,s3c2443-clock.txt > b/Documentation/devicetree/bindings/clock/samsung,s3c2443-clock.txt n= ew > file mode 100644 > index 0000000..a61d8d1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/samsung,s3c2443-clock.t= xt > @@ -0,0 +1,48 @@ > +* Samsung S3C2443 Clock Controller > + > +The S3C2443 clock controller generates and supplies clock to various > controllers +within the SoC. The clock binding described here is appl= icable > to all SoCs in +the s3c24x family starting with the s3c2443. > + > +Required Properties: > + > +- comptible: should be one of the following. nit: compatible > + - "samsung,s3c2416-clock" - controller compatible with S3C2416 SoC= =2E > + - "samsung,s3c2443-clock" - controller compatible with S3C2443 SoC= =2E > + - "samsung,s3c2450-clock" - controller compatible with S3C2450 SoC= =2E > + > +- reg: physical base address of the controller and length of memory = mapped > + region. > + > +- #clock-cells: should be 1. > + > +Each clock is assigned an identifier and client nodes can use this > identifier +to specify the clock which they consume. Some of the cloc= ks are > available only +on a particular SoC. > + > +All available clocks are defined as preprocessor macros in > +dt-bindings/clock/samsung,s3c2443-clock.h header and can be used in = device > +tree sources. > + > +Example: Clock controller node: > + > + clocks: clock-controller@4c000000 { > + compatible =3D "samsung,s3c2416-clock"; > + reg =3D <0x4c000000 0x40>; > + #clock-cells =3D <1>; > + }; > + > +Example: UART controller node that consumes the clock generated by t= he > clock + controller (refer to the standard clock bindings for informa= tion > about + "clocks" and "clock-names" properties): > + > + serial@50004000 { > + compatible =3D "samsung,s3c2440-uart"; > + reg =3D <0x50004000 0x4000>; > + interrupts =3D <1 23 3 4>, <1 23 4 4>; > + clock-names =3D "uart", "clk_uart_baud2", > + "clk_uart_baud3"; > + clocks =3D <&clocks PCLK_UART0>, <&clocks PCLK_UART0>, > + <&clocks SCLK_UART>; > + status =3D "disabled"; > + }; > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 0357ac4..b2fdd68 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -84,3 +84,4 @@ config COMMON_CLK_AXI_CLKGEN > endmenu >=20 > source "drivers/clk/mvebu/Kconfig" > +source "drivers/clk/samsung/Kconfig" > diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfi= g > new file mode 100644 > index 0000000..0cfbb29 > --- /dev/null > +++ b/drivers/clk/samsung/Kconfig > @@ -0,0 +1,2 @@ > +config COMMON_CLK_S3C2443 > + bool Do you need to introduce a new Kconfig file for this? I guess it's just= a matter=20 of preference, but since it's here just temporarily, I would just put t= his=20 Kconfig entry into the top level clk Kconfig file or even in arch/arm/m= ach- s3c24xx/Kconfig. > diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makef= ile > index 5d4d432..1c7932c 100644 > --- a/drivers/clk/samsung/Makefile > +++ b/drivers/clk/samsung/Makefile > @@ -8,3 +8,4 @@ obj-$(CONFIG_SOC_EXYNOS5250) +=3D clk-exynos5250.o > obj-$(CONFIG_SOC_EXYNOS5420) +=3D clk-exynos5420.o > obj-$(CONFIG_SOC_EXYNOS5440) +=3D clk-exynos5440.o > obj-$(CONFIG_ARCH_EXYNOS) +=3D clk-exynos-audss.o > +obj-$(CONFIG_COMMON_CLK_S3C2443)+=3D clk-s3c2443.o > diff --git a/drivers/clk/samsung/clk-s3c2443.c > b/drivers/clk/samsung/clk-s3c2443.c new file mode 100644 > index 0000000..7d57b08 > --- /dev/null > +++ b/drivers/clk/samsung/clk-s3c2443.c > @@ -0,0 +1,422 @@ > +/* > + * Copyright (c) 2013 Heiko Stuebner > + * > + * This program is free software; you can redistribute it and/or mod= ify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Common Clock Framework support for S3C2443 and following SoCs. > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include "clk.h" > +#include "clk-pll.h" > + > +/* S3C2416 clock controller register offsets */ > +#define MPLLCON 0x10 > +#define EPLLCON 0x18 > +#define EPLLCON_K 0x1C > +#define CLKSRC 0x20 > +#define CLKDIV0 0x24 > +#define CLKDIV1 0x28 > +#define CLKDIV2 0x2C > +#define HCLKCON 0x30 > +#define PCLKCON 0x34 > +#define SCLKCON 0x38 > + > +/* the soc types */ > +enum supported_socs { > + S3C2416, > + S3C2443, > + S3C2450, > +}; > + > +/* > + * list of controller registers to be saved and restored during a > + * suspend/resume cycle. > + */ > +static __initdata unsigned long s3c2443_clk_regs[] =3D { > + MPLLCON, > + EPLLCON, > + EPLLCON_K, > + CLKSRC, > + CLKDIV0, > + CLKDIV1, > + CLKDIV2, > + PCLKCON, > + HCLKCON, > + SCLKCON, > +}; > + > +PNAME(epllref_p) =3D { "mpllref", "mpllref", "xti", "ext" }; > +PNAME(esysclk_p) =3D { "epllref", "epll" }; > +PNAME(msysclk_p) =3D { "mpllref, mdivclk", "mpll", "mpll" }; > +PNAME(armclk_p) =3D { "armdiv" , "hclk" }; > +PNAME(i2s0_p) =3D { "div_i2s0", "ext_i2s", "epllref", "epllref" }; > + > +/* fixed rate clocks generated outside the soc */ > +struct samsung_fixed_rate_clock s3c2443_common_frate_clks[] __initda= ta =3D { > + FRATE(XTI, "xti", NULL, CLK_IS_ROOT, 0), > + FRATE(EXT, "ext", NULL, CLK_IS_ROOT, 0), > + FRATE(EXT_I2S, "ext_i2s", NULL, CLK_IS_ROOT, 0), > + FRATE(EXT_UART, "ext_uart", NULL, CLK_IS_ROOT, 0), > +}; > + > +/* mpllref is a direct descendant of clk_xtal by default, but it is = not nit: The preferred style of multiline comments is: /* * mpllref is a... > + * elided as the EPLL can be either sourced by the XTAL or EXTCLK an= d as > + * such directly equating the two source clocks is impossible. > + */ > +struct samsung_fixed_factor_clock s3c2443_common_ffactor[] __initdat= a =3D { > + FFACTOR(0, "mpllref", "xti", 1, 1, 0), > +}; Hmm, I don't understand the reason for having this fixed factor clock. = Why=20 can't xti be used directly instead? > +struct samsung_mux_clock s3c2443_common_muxes[] __initdata =3D { > + MUX(0, "epllref", epllref_p, CLKSRC, 7, 2), > + MUX(ESYSCLK, "esysclk", esysclk_p, CLKSRC, 6, 1), > + MUX_A(MSYSCLK, "msysclk", msysclk_p, CLKSRC, 3, 2, "msysclk"), > + MUX_A(ARMCLK, "armclk", armclk_p, CLKDIV0, 13, 1, "armclk"), > + MUX(0, "mux_i2s0", i2s0_p, CLKSRC, 14, 2), > +}; > + > +static struct clk_div_table hclk_d[] =3D { > + { .val =3D 0, .div =3D 1 }, > + { .val =3D 1, .div =3D 2 }, > + { .val =3D 3, .div =3D 4 }, > + { .div =3D 0 }, > +}; Hmm, this makes me rethink some of the code I made for S3C64xx. There a= re few=20 dividers that have additional constraints, like only even divisors, or = so, but=20 they are just normal linear dividers, so can be registered as simple di= vider=20 clocks. Originally I registered them as simple dividers, but now I'm thinking i= f they=20 shouldn't be constrained by a table. > +static struct clk_div_table mdivclk_d[] =3D { > + { .val =3D 0, .div =3D 1 }, > + { .val =3D 1, .div =3D 3 }, > + { .val =3D 2, .div =3D 5 }, > + { .val =3D 3, .div =3D 7 }, > + { .val =3D 4, .div =3D 9 }, > + { .val =3D 5, .div =3D 11 }, > + { .val =3D 6, .div =3D 13 }, > + { .val =3D 7, .div =3D 15 }, > + { .div =3D 0 }, > +}; > + > +struct samsung_div_clock s3c2443_common_dividers[] __initdata =3D { > + DIV_T(0, "mdivclk", "mpllref", CLKDIV0, 6, 3, mdivclk_d), > + DIV(0, "prediv", "msysclk", CLKDIV0, 4, 2), > + DIV_T(HCLK, "hclk", "prediv", CLKDIV0, 0, 2, hclk_d), > + DIV(PCLK, "pclk", "hclk", CLKDIV0, 2, 1), > + DIV(0, "div_hsspi0_epll", "esysclk", CLKDIV1, 24, 2), > + DIV(0, "div_fimd", "esysclk", CLKDIV1, 16, 8), > + DIV(0, "div_i2s0", "esysclk", CLKDIV1, 12, 4), > + DIV(0, "div_uart", "esysclk", CLKDIV1, 8, 4), > + DIV(0, "div_hsmmc1", "esysclk", CLKDIV1, 6, 2), > + DIV(0, "div_usbhost", "esysclk", CLKDIV1, 4, 2), > +}; > + > +struct samsung_gate_clock s3c2443_common_gates[] __initdata =3D { > + GATE(SCLK_HSMMC_EXT, "sclk_hsmmcext", "ext", SCLKCON, 13, 0, 0), > + GATE(SCLK_HSMMC1, "sclk_hsmmc1", "div_hsmmc1", SCLKCON, 12, 0, 0), > + GATE(SCLK_FIMD, "sclk_fimd", "div_fimd", SCLKCON, 10, 0, 0), > + GATE(SCLK_I2S0, "sclk_i2s0", "mux_i2s0", SCLKCON, 9, 0, 0), > + GATE(SCLK_UART, "sclk_uart", "div_uart", SCLKCON, 8, 0, 0), > + GATE(SCLK_USBH, "sclk_usbhost", "div_usbhost", SCLKCON, 1, 0, 0), > + GATE(HCLK_DRAM, "dram", "hclk", HCLKCON, 19, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_SSMC, "ssmc", "hclk", HCLKCON, 18, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_HSMMC1, "hsmmc1", "hclk", HCLKCON, 16, 0, 0), > + GATE(HCLK_USBD, "usb-device", "hclk", HCLKCON, 12, 0, 0), > + GATE(HCLK_USBH, "usb-host", "hclk", HCLKCON, 11, 0, 0), > + GATE(HCLK_LCD, "lcd", "hclk", HCLKCON, 9, 0, 0), > + GATE(HCLK_DMA5, "dma5", "hclk", HCLKCON, 5, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_DMA4, "dma4", "hclk", HCLKCON, 4, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_DMA3, "dma3", "hclk", HCLKCON, 3, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_DMA2, "dma2", "hclk", HCLKCON, 2, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_DMA1, "dma1", "hclk", HCLKCON, 1, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_DMA0, "dma0", "hclk", HCLKCON, 0, CLK_IGNORE_UNUSED, 0), Hmm, I guess these CLK_IGNORE_UNUSED flags are needed because the old D= MA=20 driver doesn't handle clocks, right? By the way, do you have some plans to continue working on the DMA engin= e=20 driver for s3c24xx you posted some time ago? This would be really helpf= ul, as=20 we could then switch all the drivers using currently the Samsung-specif= ic DMA=20 API to the generic DMA engine API. (Actually I already have some patche= s for=20 Samsung sound drivers. Will probably post an RFC soon.) > + GATE(PCLK_GPIO, "gpio", "pclk", PCLKCON, 13, CLK_IGNORE_UNUSED, 0), > + GATE(PCLK_RTC, "rtc", "pclk", PCLKCON, 12, 0, 0), > + GATE(PCLK_WDT, "wdt", "pclk", PCLKCON, 11, 0, 0), > + GATE(PCLK_PWM, "pwm", "pclk", PCLKCON, 10, 0, 0), > + GATE(PCLK_I2S0, "i2s0", "pclk", PCLKCON, 9, 0, 0), > + GATE(PCLK_AC97, "ac97", "pclk", PCLKCON, 8, 0, 0), > + GATE(PCLK_ADC, "adc", "pclk", PCLKCON, 7, 0, 0), > + GATE(PCLK_SPI0, "spi0", "pclk", PCLKCON, 6, 0, 0), > + GATE(PCLK_I2C0, "i2c0", "pclk", PCLKCON, 4, 0, 0), > + GATE(PCLK_UART3, "uart3", "pclk", PCLKCON, 3, 0, 0), > + GATE(PCLK_UART2, "uart2", "pclk", PCLKCON, 2, 0, 0), > + GATE(PCLK_UART1, "uart1", "pclk", PCLKCON, 1, 0, 0), > + GATE(PCLK_UART0, "uart0", "pclk", PCLKCON, 0, 0, 0), > +}; > + > +struct samsung_clock_alias s3c2443_common_aliases[] __initdata =3D { > + ALIAS(HCLK, NULL, "hclk"), > + ALIAS(HCLK_SSMC, NULL, "nand"), > + ALIAS(PCLK_UART0, "s3c2440-uart.0", "uart"), > + ALIAS(PCLK_UART1, "s3c2440-uart.1", "uart"), > + ALIAS(PCLK_UART2, "s3c2440-uart.2", "uart"), > + ALIAS(PCLK_UART3, "s3c2440-uart.3", "uart"), > + ALIAS(EXT_UART, NULL, "clk_uart_baud1"), > + ALIAS(PCLK_UART0, "s3c2440-uart.0", "clk_uart_baud2"), > + ALIAS(PCLK_UART1, "s3c2440-uart.1", "clk_uart_baud2"), > + ALIAS(PCLK_UART2, "s3c2440-uart.2", "clk_uart_baud2"), > + ALIAS(PCLK_UART3, "s3c2440-uart.3", "clk_uart_baud2"), > + ALIAS(SCLK_UART, NULL, "clk_uart_baud3"), > + ALIAS(PCLK_PWM, NULL, "timers"), > + ALIAS(PCLK_RTC, NULL, "rtc"), > + ALIAS(PCLK_WDT, NULL, "watchdog"), > + ALIAS(PCLK_ADC, NULL, "adc"), > + ALIAS(PCLK_I2C0, "s3c2410-i2c.0", "i2c"), > + ALIAS(HCLK_USBD, NULL, "usb-device"), > + ALIAS(HCLK_USBH, NULL, "usb-host"), > + ALIAS(SCLK_USBH, NULL, "usb-bus-host"), > + ALIAS(PCLK_SPI0, "s3c2443-spi.0", "spi"), > + ALIAS(PCLK_SPI0, "s3c2443-spi.0", "spi_busclk0"), > + ALIAS(HCLK_HSMMC1, "s3c-sdhci.1", "hsmmc"), > + ALIAS(HCLK_HSMMC1, "s3c-sdhci.1", "mmc_busclk.0"), > + ALIAS(PCLK_I2S0, "samsung-i2s.0", "iis"), > + ALIAS(SCLK_I2S0, NULL, "i2s-if"), > + ALIAS(HCLK_LCD, NULL, "lcd"), > + ALIAS(SCLK_FIMD, NULL, "sclk_fimd"), > +}; > + > +/* S3C2416 specific clocks */ > + > +PNAME(s3c2416_hsmmc0_p) =3D { "sclk_hsmmc0", "sclk_hsmmcext" }; > +PNAME(s3c2416_hsmmc1_p) =3D { "sclk_hsmmc1", "sclk_hsmmcext" }; > +PNAME(s3c2416_hsspi0_p) =3D { "hsspi0_epll", "hsspi0_mpll" }; > + > +static struct clk_div_table armdiv_s3c2416_d[] =3D { > + { .val =3D 0, .div =3D 1 }, > + { .val =3D 1, .div =3D 2 }, > + { .val =3D 2, .div =3D 3 }, > + { .val =3D 3, .div =3D 4 }, > + { .val =3D 5, .div =3D 6 }, > + { .val =3D 7, .div =3D 8 }, > + { .div =3D 0 }, > +}; > + > +struct samsung_div_clock s3c2416_dividers[] __initdata =3D { > + DIV_T(ARMDIV, "armdiv", "msysclk", CLKDIV0, 9, 3, armdiv_s3c2416_d)= , > + DIV(0, "div_hsspi0_mpll", "msysclk", CLKDIV2, 0, 4), > + DIV(0, "div_hsmmc0", "esysclk", CLKDIV2, 6, 2), > +}; > + > +struct samsung_mux_clock s3c2416_muxes[] __initdata =3D { > + MUX(MUX_HSMMC0, "mux_hsmmc0", s3c2416_hsmmc0_p, CLKSRC, 16, 1), > + MUX(MUX_HSMMC1, "mux_hsmmc1", s3c2416_hsmmc1_p, CLKSRC, 17, 1), > + MUX(MUX_HSSPI0, "mux_hsspi0", s3c2416_hsspi0_p, CLKSRC, 18, 1), > +}; > + > +struct samsung_gate_clock s3c2416_gates[] __initdata =3D { > + GATE(0, "hsspi0_mpll", "div_hsspi0_mpll", SCLKCON, 19, 0, 0), > + GATE(0, "hsspi0_epll", "div_hsspi0_epll", SCLKCON, 14, 0, 0), > + GATE(0, "sclk_hsmmc0", "div_hsmmc0", SCLKCON, 6, 0, 0), > + GATE(HCLK_2D, "2d", "hclk", HCLKCON, 20, 0, 0), > + GATE(HCLK_HSMMC0, "hsmmc0", "hclk", HCLKCON, 15, 0, 0), > + GATE(HCLK_IROM, "irom", "hclk", HCLKCON, 13, CLK_IGNORE_UNUSED, 0), > + GATE(PCLK_PCM, "pcm", "pclk", PCLKCON, 19, 0, 0), > +}; > + > +struct samsung_clock_alias s3c2416_aliases[] __initdata =3D { > + ALIAS(HCLK_HSMMC0, "s3c-sdhci.0", "hsmmc"), > + ALIAS(HCLK_HSMMC0, "s3c-sdhci.0", "mmc_busclk.0"), > + ALIAS(MUX_HSMMC0, "s3c-sdhci.0", "mmc_busclk.2"), > + ALIAS(MUX_HSMMC1, "s3c-sdhci.1", "mmc_busclk.2"), > + ALIAS(MUX_HSSPI0, "s3c2443-spi.0", "spi_busclk2"), > + ALIAS(ARMDIV, NULL, "armdiv"), > +}; > + > +/* S3C2443 specific clocks */ > + > +static struct clk_div_table armdiv_s3c2443_d[] =3D { > + { .val =3D 0, .div =3D 1 }, > + { .val =3D 8, .div =3D 2 }, > + { .val =3D 2, .div =3D 3 }, > + { .val =3D 9, .div =3D 4 }, > + { .val =3D 10, .div =3D 6 }, > + { .val =3D 11, .div =3D 8 }, > + { .val =3D 13, .div =3D 12 }, > + { .val =3D 15, .div =3D 16 }, > + { .div =3D 0 }, > +}; > + > +struct samsung_div_clock s3c2443_dividers[] __initdata =3D { > + DIV_T(ARMDIV, "armdiv", "msysclk", CLKDIV0, 9, 4, armdiv_s3c2443_d)= , > + DIV(0, "div_cam", "esysclk", CLKDIV1, 26, 4), > +}; > + > +struct samsung_gate_clock s3c2443_gates[] __initdata =3D { > + GATE(SCLK_HSSPI0, "sclk_hsspi0", "div_hsspi0_epll", SCLKCON, 14, 0,= 0), > + GATE(SCLK_CAM, "sclk_cam", "div_cam", SCLKCON, 11, 0, 0), > + GATE(HCLK_CFC, "cfc", "hclk", HCLKCON, 17, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_CAM, "cam", "hclk", HCLKCON, 8, 0, 0), > + GATE(PCLK_SPI1, "spi1", "pclk", PCLKCON, 15, 0, 0), > + GATE(PCLK_SDI, "sdi", "pclk", PCLKCON, 5, 0, 0), > +}; > + > +struct samsung_clock_alias s3c2443_aliases[] __initdata =3D { > + ALIAS(SCLK_HSSPI0, "s3c2443-spi.0", "spi_busclk2"), > + ALIAS(SCLK_HSMMC1, "s3c-sdhci.1", "mmc_busclk.2"), > + ALIAS(SCLK_CAM, NULL, "camif-upll"), > + ALIAS(PCLK_SPI1, "s3c2410-spi.0", "spi"), > + ALIAS(PCLK_SDI, NULL, "sdi"), > + ALIAS(HCLK_CFC, NULL, "cfc"), > + ALIAS(ARMDIV, NULL, "armdiv"), > +}; > + > +/* S3C2450 specific clocks */ > + > +PNAME(s3c2450_cam_p) =3D { "div_cam", "hclk" }; > +PNAME(s3c2450_hsspi1_p) =3D { "hsspi1_epll", "hsspi1_mpll" }; > +PNAME(i2s1_p) =3D { "div_i2s1", "ext_i2s", "epllref", "epllref" }; > + > +struct samsung_div_clock s3c2450_dividers[] __initdata =3D { > + DIV(0, "div_cam", "esysclk", CLKDIV1, 26, 4), > + DIV(0, "div_hsspi1_epll", "esysclk", CLKDIV2, 24, 2), > + DIV(0, "div_hsspi1_mpll", "msysclk", CLKDIV2, 16, 4), > + DIV(0, "div_i2s1", "esysclk", CLKDIV2, 12, 4), > +}; > + > +struct samsung_mux_clock s3c2450_muxes[] __initdata =3D { > + MUX(0, "mux_cam", s3c2450_cam_p, CLKSRC, 20, 1), > + MUX(MUX_HSSPI1, "mux_hsspi1", s3c2450_hsspi1_p, CLKSRC, 19, 1), > + MUX(0, "mux_i2s1", i2s1_p, CLKSRC, 12, 2), > +}; > + > +struct samsung_gate_clock s3c2450_gates[] __initdata =3D { > + GATE(SCLK_I2S1, "sclk_i2s1", "div_i2s1", SCLKCON, 5, 0, 0), > + GATE(HCLK_CFC, "cfc", "hclk", HCLKCON, 17, 0, 0), > + GATE(HCLK_CAM, "cam", "hclk", HCLKCON, 8, 0, 0), > + GATE(HCLK_DMA7, "dma7", "hclk", HCLKCON, 7, CLK_IGNORE_UNUSED, 0), > + GATE(HCLK_DMA6, "dma6", "hclk", HCLKCON, 6, CLK_IGNORE_UNUSED, 0), > + GATE(PCLK_I2S1, "i2s1", "pclk", PCLKCON, 17, 0, 0), > + GATE(PCLK_I2C1, "i2c1", "pclk", PCLKCON, 16, 0, 0), > + GATE(PCLK_SPI1, "spi1", "pclk", PCLKCON, 14, 0, 0), > +}; > + > +struct samsung_clock_alias s3c2450_aliases[] __initdata =3D { > + ALIAS(PCLK_SPI1, "s3c2443-spi.1", "spi"), > + ALIAS(PCLK_SPI1, "s3c2443-spi.1", "spi_busclk0"), > + ALIAS(MUX_HSSPI1, "s3c2443-spi.1", "spi_busclk2"), > + ALIAS(PCLK_I2C1, "s3c2410-i2c.1", "i2c"), > +}; > + > +/* > + * This function allows non-dt platforms to specify the clock speed = of the > + * xti and ext clocks. > + */ > +void __init s3c2443_clk_register_fixed_ext(unsigned long xti_f, > + unsigned long ext_f, unsigned long i2s_f, > + unsigned long uart_f) > +{ > + s3c2443_common_frate_clks[0].fixed_rate =3D xti_f; > + s3c2443_common_frate_clks[1].fixed_rate =3D ext_f; > + s3c2443_common_frate_clks[2].fixed_rate =3D i2s_f; > + s3c2443_common_frate_clks[3].fixed_rate =3D uart_f; > + > + samsung_clk_register_fixed_rate(s3c2443_common_frate_clks, > + ARRAY_SIZE(s3c2443_common_frate_clks)); > +} > + > +static __initdata struct of_device_id ext_clk_match[] =3D { > + { .compatible =3D "samsung,clock-xti", .data =3D (void *)0, }, > + { .compatible =3D "samsung,clock-ext", .data =3D (void *)1, }, > + { .compatible =3D "samsung,clock-ext-i2s", .data =3D (void *)2, }, > + { .compatible =3D "samsung,clock-ext-uart", .data =3D (void *)3, }, > + {}, > +}; I wonder if we shouldn't use the generic clock bindings to define these= instead=20 of introducing platform specific binding. I know this has been already = done for=20 Exynos (and even in my patches for S3C64xx - I need to fix this), but I= =20 think the preferred way now (or maybe even the only allowed) is to use = the=20 generic bindings. > + > +void __init s3c2443_common_clk_init(struct device_node *np, int > current_soc, + void __iomem *reg_base) > +{ > + struct clk *mpll, *epll; > + > + if (np) { > + reg_base =3D of_iomap(np, 0); > + if (!reg_base) > + panic("%s: failed to map registers\n", __func__); > + } > + > + samsung_clk_init(np, reg_base, NR_CLKS, > + s3c2443_clk_regs, ARRAY_SIZE(s3c2443_clk_regs), NULL, 0); > + > + if (np) > + samsung_clk_of_register_fixed_ext(s3c2443_common_frate_clks, > + ARRAY_SIZE(s3c2443_common_frate_clks), > + ext_clk_match); > + > + samsung_clk_register_fixed_factor(s3c2443_common_ffactor, > + ARRAY_SIZE(s3c2443_common_ffactor)); > + > + if (current_soc =3D=3D S3C2416 || current_soc =3D=3D S3C2450) { > + mpll =3D samsung_clk_register_pll6552x("mpll", "mpllref", > + reg_base + MPLLCON); > + epll =3D samsung_clk_register_pll6553x("epll", "epllref", > + reg_base + EPLLCON); > + } else { > + mpll =3D samsung_clk_register_pll3000x("mpll", "mpllref", > + reg_base + MPLLCON); > + epll =3D samsung_clk_register_pll2126x("epll", "epllref", > + reg_base + EPLLCON); > + } > + > + samsung_clk_register_mux(s3c2443_common_muxes, > + ARRAY_SIZE(s3c2443_common_muxes)); > + samsung_clk_register_div(s3c2443_common_dividers, > + ARRAY_SIZE(s3c2443_common_dividers)); > + samsung_clk_register_gate(s3c2443_common_gates, > + ARRAY_SIZE(s3c2443_common_gates)); > + samsung_clk_register_alias(s3c2443_common_aliases, > + ARRAY_SIZE(s3c2443_common_aliases)); > + > + if (current_soc =3D=3D S3C2416 || current_soc =3D=3D S3C2450) { > + samsung_clk_register_div(s3c2416_dividers, > + ARRAY_SIZE(s3c2416_dividers)); > + samsung_clk_register_mux(s3c2416_muxes, > + ARRAY_SIZE(s3c2416_muxes)); > + samsung_clk_register_gate(s3c2416_gates, > + ARRAY_SIZE(s3c2416_gates)); > + samsung_clk_register_alias(s3c2416_aliases, > + ARRAY_SIZE(s3c2416_aliases)); > + } else { > + samsung_clk_register_div(s3c2443_dividers, > + ARRAY_SIZE(s3c2443_dividers)); > + samsung_clk_register_gate(s3c2443_gates, > + ARRAY_SIZE(s3c2443_gates)); > + samsung_clk_register_alias(s3c2443_aliases, > + ARRAY_SIZE(s3c2443_aliases)); > + } > + > + /* s3c2450 extends the s3c2416 clocks */ > + if (current_soc =3D=3D S3C2450) { > + samsung_clk_register_div(s3c2450_dividers, > + ARRAY_SIZE(s3c2450_dividers)); > + samsung_clk_register_mux(s3c2450_muxes, > + ARRAY_SIZE(s3c2450_muxes)); > + samsung_clk_register_gate(s3c2450_gates, > + ARRAY_SIZE(s3c2450_gates)); > + samsung_clk_register_alias(s3c2450_aliases, > + ARRAY_SIZE(s3c2450_aliases)); > + } nit: What about using a switch statement, like: switch (current_soc) { case S3C2450: /* register 2450 specific clocks */ /* fall through */ case S3C2416: /* register clocks common to 2416 and 2450 */ break; case S3C2443: /* register 2443 specific clocks */ break; } Still, I guess it's just a matter of preference, so just feel free to l= eave it=20 as it is. Best regards, Tomasz