Re: [PATCH v3 05/11] clk: davinci - add dm644x clock initialization

2012-11-06 Thread Sekhar Nori
On 11/6/2012 4:53 AM, Murali Karicheri wrote:
 On 11/03/2012 09:30 AM, Sekhar Nori wrote:
 On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This patch adds dm644x clock initialization code that consists of
 clocks data for various clocks and clock register callouts to
 various clock drivers. It uses following clk drivers for this

   1. clk-fixed-rate - for ref clock
   2. clk-mux - for mux at the input and output of main pll
   3. davinci specific clk-pll for main pll clock
   4. davinci specific clk-div for pll divider clock
   5. clk-fixed-factor for fixed factor clock such as auxclk
   6. davinci specific clk-psc for psc clocks

 This patch also moves all of the PLL and PSC register definitions
 from clock.h and psc.h under davinci to the clk/davinci folder so
 that various soc specific clock initialization code can share these
 definitions.
 Except this patch does not move the defines, it creates a copy of them
 (which is bad since you quickly lose track of which is the correct
 copy). Is this done to avoid including mach/ header files here? It will
 actually be better to include the mach/ files here as a temporary
 solution and then remove the include mach/ files once all the SoCs have
 been converted over.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
   drivers/clk/davinci/dm644x-clock.c |  304
 
   drivers/clk/davinci/pll.h  |   83 ++
   drivers/clk/davinci/psc.h  |  215 +
   3 files changed, 602 insertions(+)
   create mode 100644 drivers/clk/davinci/dm644x-clock.c
   create mode 100644 drivers/clk/davinci/pll.h
   create mode 100644 drivers/clk/davinci/psc.h

 +/* all clocks available in DM644x SoCs */
 +enum dm644x_clk {
 +clkin, oscin, ref_clk_mux, pll1, pll1_plldiv_clk_mux, auxclk,
 +clk_pll1_sysclk1, clk_pll1_sysclk2, clk_pll1_sysclk3,
 clk_pll1_sysclk4,
 +clk_pll1_sysclk5, clk_pll1_sysclkbp, pll2, pll2_plldiv_clk_mux,
 +clk_pll2_sysclk1, clk_pll2_sysclk2, clk_pll2_sysclkbp, dsp, arm,
 vicp,
 +vpss_master, vpss_slave, uart0, uart1, uart2, emac, i2c, ide, asp,
 +mmcsd, spi, gpio, usb, vlynq, aemif, pwm0, pwm1, pwm2, timer0,
 timer1,
 +timer2, clk_max
 +};
 +
 +static struct davinci_clk *psc_clocks[] = {
 +clk_dsp, clk_arm, clk_vicp, clk_vpss_master, clk_vpss_slave,
 +clk_uart0, clk_uart1, clk_uart2, clk_emac, clk_i2c, clk_ide,
 +clk_asp0, clk_mmcsd, clk_spi, clk_gpio, clk_usb, clk_vlynq,
 +clk_aemif, clk_pwm0, clk_pwm1, clk_pwm2, clk_timer0,
 clk_timer1,
 +clk_timer2
 +};
 You rely on perfect order between this array and dm644x_clk enum above.
 Can you initialize this array using the enum as the index so that it is
 clear. Current method is too error prone.
 Are you expecting something like this?
 
 static struct davinci_clk *psc_clocks[] = {
 [dsp - dsp]= clk_dsp,
 [arm - dsp]= clk_arm,
 [vicp - dsp]= clk_vicp,
 [vpss_maste - dsp]= clk_vpss_master,
 [vpss_slave - dsp]= clk_vpss_slave,
 [uart0 - dsp]= clk_uart0,
 [uart1 - dsp]= clk_uart1,

Well, sort of! But the '- dsp' is really ugly. You can either define the
array for the full list of clocks (like 'clks') or move the psc clocks
to the beginning of the enum (less preferable to me).

Thanks,
Sekhar
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v3 05/11] clk: davinci - add dm644x clock initialization

2012-11-06 Thread Sekhar Nori
On 11/5/2012 9:12 PM, Murali Karicheri wrote:
 On 11/03/2012 09:30 AM, Sekhar Nori wrote:
 On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This patch adds dm644x clock initialization code that consists of
 clocks data for various clocks and clock register callouts to
 various clock drivers. It uses following clk drivers for this

   1. clk-fixed-rate - for ref clock
   2. clk-mux - for mux at the input and output of main pll
   3. davinci specific clk-pll for main pll clock
   4. davinci specific clk-div for pll divider clock
   5. clk-fixed-factor for fixed factor clock such as auxclk
   6. davinci specific clk-psc for psc clocks

 This patch also moves all of the PLL and PSC register definitions
 from clock.h and psc.h under davinci to the clk/davinci folder so
 that various soc specific clock initialization code can share these
 definitions.
 Except this patch does not move the defines, it creates a copy of them
 (which is bad since you quickly lose track of which is the correct
 copy). Is this done to avoid including mach/ header files here?
 Yes.
 It will
 actually be better to include the mach/ files here as a temporary
 solution and then remove the include mach/ files once all the SoCs have
 been converted over.
 
 I was thinking we are not allowed to include mach/* header files in
 driver files. But most of the clk drivers
 such clk-imx28, spear6xx_clock.c. versatile/clk-integrator.c etc are
 including mach/ headers. One issue is that the definitions in pll.h are
 re-usable across other machines falling under c6x and Keystone (new
 device we are working on) as well. Where do we keep includes that can be
 re-used across different architectures? include/linux/platform_data/ ?

In this case, it is not really platform data or even an interface for
drivers to use, so I prefer include/linux/clk/davinci-pll.h

 I
 see clk-integrator.h, clk-nomadik.h and clk-u300 sitting there. So I
 suggest moving any header files that defines utility functions, register
 definitions across different architectures to
 include/linux/platform_data. Candidate files would be clock.h, pll.h,
 clk-psc.h, clk-pll.h and clk-div.h. This way these can be used across

It is not clear to me why you would move these files outside of
drivers/clk/davinci. They are not used by any other code outside of this
directory.

 the above machines that use the above architectures. Can we do this in
 my next version? This way we don't have to make another move later. All
 these CLK IPs are re-used across multiple architectures and make perfect
 sense to me to move to the above folder.

Thanks,
Sekhar
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v3 05/11] clk: davinci - add dm644x clock initialization

2012-11-05 Thread Murali Karicheri

On 11/03/2012 09:30 AM, Sekhar Nori wrote:

On 10/25/2012 9:41 PM, Murali Karicheri wrote:

This patch adds dm644x clock initialization code that consists of
clocks data for various clocks and clock register callouts to
various clock drivers. It uses following clk drivers for this

  1. clk-fixed-rate - for ref clock
  2. clk-mux - for mux at the input and output of main pll
  3. davinci specific clk-pll for main pll clock
  4. davinci specific clk-div for pll divider clock
  5. clk-fixed-factor for fixed factor clock such as auxclk
  6. davinci specific clk-psc for psc clocks

This patch also moves all of the PLL and PSC register definitions
from clock.h and psc.h under davinci to the clk/davinci folder so
that various soc specific clock initialization code can share these
definitions.

Except this patch does not move the defines, it creates a copy of them
(which is bad since you quickly lose track of which is the correct
copy). Is this done to avoid including mach/ header files here?

Yes.

It will
actually be better to include the mach/ files here as a temporary
solution and then remove the include mach/ files once all the SoCs have
been converted over.


I was thinking we are not allowed to include mach/* header files in 
driver files. But most of the clk drivers
such clk-imx28, spear6xx_clock.c. versatile/clk-integrator.c etc are 
including mach/ headers. One issue is that the definitions in pll.h are 
re-usable across other machines falling under c6x and Keystone (new 
device we are working on) as well. Where do we keep includes that can be 
re-used across different architectures? include/linux/platform_data/ ? I 
see clk-integrator.h, clk-nomadik.h and clk-u300 sitting there. So I 
suggest moving any header files that defines utility functions, register 
definitions across different architectures to 
include/linux/platform_data. Candidate files would be clock.h, pll.h, 
clk-psc.h, clk-pll.h and clk-div.h. This way these can be used across 
the above machines that use the above architectures. Can we do this in 
my next version? This way we don't have to make another move later. All 
these CLK IPs are re-used across multiple architectures and make perfect 
sense to me to move to the above folder.



Signed-off-by: Murali Karicheri m-kariche...@ti.com
---
  drivers/clk/davinci/dm644x-clock.c |  304 
  drivers/clk/davinci/pll.h  |   83 ++
  drivers/clk/davinci/psc.h  |  215 +
  3 files changed, 602 insertions(+)
  create mode 100644 drivers/clk/davinci/dm644x-clock.c
  create mode 100644 drivers/clk/davinci/pll.h
  create mode 100644 drivers/clk/davinci/psc.h

+/* all clocks available in DM644x SoCs */
+enum dm644x_clk {
+   clkin, oscin, ref_clk_mux, pll1, pll1_plldiv_clk_mux, auxclk,
+   clk_pll1_sysclk1, clk_pll1_sysclk2, clk_pll1_sysclk3, clk_pll1_sysclk4,
+   clk_pll1_sysclk5, clk_pll1_sysclkbp, pll2, pll2_plldiv_clk_mux,
+   clk_pll2_sysclk1, clk_pll2_sysclk2, clk_pll2_sysclkbp, dsp, arm, vicp,
+   vpss_master, vpss_slave, uart0, uart1, uart2, emac, i2c, ide, asp,
+   mmcsd, spi, gpio, usb, vlynq, aemif, pwm0, pwm1, pwm2, timer0, timer1,
+   timer2, clk_max
+};
+
+static struct davinci_clk *psc_clocks[] = {
+   clk_dsp, clk_arm, clk_vicp, clk_vpss_master, clk_vpss_slave,
+   clk_uart0, clk_uart1, clk_uart2, clk_emac, clk_i2c, clk_ide,
+   clk_asp0, clk_mmcsd, clk_spi, clk_gpio, clk_usb, clk_vlynq,
+   clk_aemif, clk_pwm0, clk_pwm1, clk_pwm2, clk_timer0, clk_timer1,
+   clk_timer2
+};

You rely on perfect order between this array and dm644x_clk enum above.
Can you initialize this array using the enum as the index so that it is
clear. Current method is too error prone.


Ok. Will do.


Thanks,
Sekhar



___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v3 05/11] clk: davinci - add dm644x clock initialization

2012-11-05 Thread Murali Karicheri

On 11/03/2012 09:30 AM, Sekhar Nori wrote:

On 10/25/2012 9:41 PM, Murali Karicheri wrote:

This patch adds dm644x clock initialization code that consists of
clocks data for various clocks and clock register callouts to
various clock drivers. It uses following clk drivers for this

  1. clk-fixed-rate - for ref clock
  2. clk-mux - for mux at the input and output of main pll
  3. davinci specific clk-pll for main pll clock
  4. davinci specific clk-div for pll divider clock
  5. clk-fixed-factor for fixed factor clock such as auxclk
  6. davinci specific clk-psc for psc clocks

This patch also moves all of the PLL and PSC register definitions
from clock.h and psc.h under davinci to the clk/davinci folder so
that various soc specific clock initialization code can share these
definitions.

Except this patch does not move the defines, it creates a copy of them
(which is bad since you quickly lose track of which is the correct
copy). Is this done to avoid including mach/ header files here? It will
actually be better to include the mach/ files here as a temporary
solution and then remove the include mach/ files once all the SoCs have
been converted over.


Signed-off-by: Murali Karicheri m-kariche...@ti.com
---
  drivers/clk/davinci/dm644x-clock.c |  304 
  drivers/clk/davinci/pll.h  |   83 ++
  drivers/clk/davinci/psc.h  |  215 +
  3 files changed, 602 insertions(+)
  create mode 100644 drivers/clk/davinci/dm644x-clock.c
  create mode 100644 drivers/clk/davinci/pll.h
  create mode 100644 drivers/clk/davinci/psc.h

+/* all clocks available in DM644x SoCs */
+enum dm644x_clk {
+   clkin, oscin, ref_clk_mux, pll1, pll1_plldiv_clk_mux, auxclk,
+   clk_pll1_sysclk1, clk_pll1_sysclk2, clk_pll1_sysclk3, clk_pll1_sysclk4,
+   clk_pll1_sysclk5, clk_pll1_sysclkbp, pll2, pll2_plldiv_clk_mux,
+   clk_pll2_sysclk1, clk_pll2_sysclk2, clk_pll2_sysclkbp, dsp, arm, vicp,
+   vpss_master, vpss_slave, uart0, uart1, uart2, emac, i2c, ide, asp,
+   mmcsd, spi, gpio, usb, vlynq, aemif, pwm0, pwm1, pwm2, timer0, timer1,
+   timer2, clk_max
+};
+
+static struct davinci_clk *psc_clocks[] = {
+   clk_dsp, clk_arm, clk_vicp, clk_vpss_master, clk_vpss_slave,
+   clk_uart0, clk_uart1, clk_uart2, clk_emac, clk_i2c, clk_ide,
+   clk_asp0, clk_mmcsd, clk_spi, clk_gpio, clk_usb, clk_vlynq,
+   clk_aemif, clk_pwm0, clk_pwm1, clk_pwm2, clk_timer0, clk_timer1,
+   clk_timer2
+};

You rely on perfect order between this array and dm644x_clk enum above.
Can you initialize this array using the enum as the index so that it is
clear. Current method is too error prone.

Are you expecting something like this?

static struct davinci_clk *psc_clocks[] = {
[dsp - dsp]= clk_dsp,
[arm - dsp]= clk_arm,
[vicp - dsp]= clk_vicp,
[vpss_maste - dsp]= clk_vpss_master,
[vpss_slave - dsp]= clk_vpss_slave,
[uart0 - dsp]= clk_uart0,
[uart1 - dsp]= clk_uart1,

Murali


Thanks,
Sekhar



___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v3 05/11] clk: davinci - add dm644x clock initialization

2012-11-03 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This patch adds dm644x clock initialization code that consists of
 clocks data for various clocks and clock register callouts to
 various clock drivers. It uses following clk drivers for this
 
  1. clk-fixed-rate - for ref clock
  2. clk-mux - for mux at the input and output of main pll
  3. davinci specific clk-pll for main pll clock
  4. davinci specific clk-div for pll divider clock
  5. clk-fixed-factor for fixed factor clock such as auxclk
  6. davinci specific clk-psc for psc clocks
 
 This patch also moves all of the PLL and PSC register definitions
 from clock.h and psc.h under davinci to the clk/davinci folder so
 that various soc specific clock initialization code can share these
 definitions.

Except this patch does not move the defines, it creates a copy of them
(which is bad since you quickly lose track of which is the correct
copy). Is this done to avoid including mach/ header files here? It will
actually be better to include the mach/ files here as a temporary
solution and then remove the include mach/ files once all the SoCs have
been converted over.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
  drivers/clk/davinci/dm644x-clock.c |  304 
 
  drivers/clk/davinci/pll.h  |   83 ++
  drivers/clk/davinci/psc.h  |  215 +
  3 files changed, 602 insertions(+)
  create mode 100644 drivers/clk/davinci/dm644x-clock.c
  create mode 100644 drivers/clk/davinci/pll.h
  create mode 100644 drivers/clk/davinci/psc.h
 

 +/* all clocks available in DM644x SoCs */
 +enum dm644x_clk {
 + clkin, oscin, ref_clk_mux, pll1, pll1_plldiv_clk_mux, auxclk,
 + clk_pll1_sysclk1, clk_pll1_sysclk2, clk_pll1_sysclk3, clk_pll1_sysclk4,
 + clk_pll1_sysclk5, clk_pll1_sysclkbp, pll2, pll2_plldiv_clk_mux,
 + clk_pll2_sysclk1, clk_pll2_sysclk2, clk_pll2_sysclkbp, dsp, arm, vicp,
 + vpss_master, vpss_slave, uart0, uart1, uart2, emac, i2c, ide, asp,
 + mmcsd, spi, gpio, usb, vlynq, aemif, pwm0, pwm1, pwm2, timer0, timer1,
 + timer2, clk_max
 +};
 +
 +static struct davinci_clk *psc_clocks[] = {
 + clk_dsp, clk_arm, clk_vicp, clk_vpss_master, clk_vpss_slave,
 + clk_uart0, clk_uart1, clk_uart2, clk_emac, clk_i2c, clk_ide,
 + clk_asp0, clk_mmcsd, clk_spi, clk_gpio, clk_usb, clk_vlynq,
 + clk_aemif, clk_pwm0, clk_pwm1, clk_pwm2, clk_timer0, clk_timer1,
 + clk_timer2
 +};

You rely on perfect order between this array and dm644x_clk enum above.
Can you initialize this array using the enum as the index so that it is
clear. Current method is too error prone.

Thanks,
Sekhar
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


[PATCH v3 05/11] clk: davinci - add dm644x clock initialization

2012-10-25 Thread Murali Karicheri
This patch adds dm644x clock initialization code that consists of
clocks data for various clocks and clock register callouts to
various clock drivers. It uses following clk drivers for this

 1. clk-fixed-rate - for ref clock
 2. clk-mux - for mux at the input and output of main pll
 3. davinci specific clk-pll for main pll clock
 4. davinci specific clk-div for pll divider clock
 5. clk-fixed-factor for fixed factor clock such as auxclk
 6. davinci specific clk-psc for psc clocks

This patch also moves all of the PLL and PSC register definitions
from clock.h and psc.h under davinci to the clk/davinci folder so
that various soc specific clock initialization code can share these
definitions.

Signed-off-by: Murali Karicheri m-kariche...@ti.com
---
 drivers/clk/davinci/dm644x-clock.c |  304 
 drivers/clk/davinci/pll.h  |   83 ++
 drivers/clk/davinci/psc.h  |  215 +
 3 files changed, 602 insertions(+)
 create mode 100644 drivers/clk/davinci/dm644x-clock.c
 create mode 100644 drivers/clk/davinci/pll.h
 create mode 100644 drivers/clk/davinci/psc.h

diff --git a/drivers/clk/davinci/dm644x-clock.c 
b/drivers/clk/davinci/dm644x-clock.c
new file mode 100644
index 000..8f74f72
--- /dev/null
+++ b/drivers/clk/davinci/dm644x-clock.c
@@ -0,0 +1,304 @@
+/*
+ * DM644x clock initialization
+ *
+ * Copyright (C) 2012 Texas Instruments.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#include linux/clk.h
+#include linux/clkdev.h
+#include linux/clk-provider.h
+#include linux/err.h
+#include linux/io.h
+#include linux/module.h
+
+#include clk-pll.h
+#include clk-psc.h
+#include clk-div.h
+#include clock.h
+#include pll.h
+#include psc.h
+
+#define DM644X_CLKIN_FREQ 2700
+#define DM644X_OSCIN_FREQ 2700
+
+/* all clocks available in DM644x SoCs */
+enum dm644x_clk {
+   clkin, oscin, ref_clk_mux, pll1, pll1_plldiv_clk_mux, auxclk,
+   clk_pll1_sysclk1, clk_pll1_sysclk2, clk_pll1_sysclk3, clk_pll1_sysclk4,
+   clk_pll1_sysclk5, clk_pll1_sysclkbp, pll2, pll2_plldiv_clk_mux,
+   clk_pll2_sysclk1, clk_pll2_sysclk2, clk_pll2_sysclkbp, dsp, arm, vicp,
+   vpss_master, vpss_slave, uart0, uart1, uart2, emac, i2c, ide, asp,
+   mmcsd, spi, gpio, usb, vlynq, aemif, pwm0, pwm1, pwm2, timer0, timer1,
+   timer2, clk_max
+};
+
+static struct clk *clks[clk_max];
+
+static u32 dm644x_psc_bases[] = { DAVINCI_PWR_SLEEP_CNTRL_BASE };
+
+static const char *ref_clk_mux_parents[] = {clkin, oscin};
+
+static struct clk_mux_data ref_clk_mux_data = {
+   .shift  = PLLCTL_CLKMODE_SHIFT,
+   .width  = PLLCTL_CLKMODE_WIDTH,
+   .phys_reg   = DAVINCI_PLL1_BASE + PLLCTL,
+};
+
+static const char *pll1_plldiv_clk_mux_parents[] = {
+   ref_clk_mux, pll1};
+
+static struct clk_pll_data pll1_data = {
+   .pllm_mask  = PLLM_PLLM_MASK,
+   .prediv_mask= PLLDIV_RATIO_MASK,
+   .postdiv_mask   = PLLDIV_RATIO_MASK,
+   .num= 1,
+};
+
+static struct clk_mux_data pll1_plldiv_clk_mux_data = {
+   .shift  = PLLCTL_PLLEN_SHIFT,
+   .width  = PLLCTL_PLLEN_WIDTH,
+   .phys_reg   = DAVINCI_PLL1_BASE + PLLCTL,
+};
+
+#define define_pll1_div_clk(__name, __parent_name, __div)  \
+   static struct clk_plldiv_data pll1_div_data##__div = {  \
+   .phys_div_reg   = DAVINCI_PLL1_BASE + PLLDIV##__div,\
+   .width  = 5,\
+   .en_id  = 15,   \
+   };  \
+   \
+   static struct davinci_clk __name = {\
+   .name   = #__name,  \
+   .parent = #__parent_name,   \
+   .data   =  pll1_div_data##__div,   \
+   };
+
+define_pll1_div_clk(pll1_sysclk1, pll1_plldiv_clk_mux, 1);
+define_pll1_div_clk(pll1_sysclk2, pll1_plldiv_clk_mux, 2);
+define_pll1_div_clk(pll1_sysclk3, pll1_plldiv_clk_mux, 3);
+define_pll1_div_clk(pll1_sysclk4, pll1_plldiv_clk_mux, 4);
+define_pll1_div_clk(pll1_sysclk5, pll1_plldiv_clk_mux, 5);
+
+static struct clk_plldiv_data pll1_sysclkbp_data = {
+   .phys_div_reg   = DAVINCI_PLL1_BASE + BPDIV,
+   .width  = 5,
+   .en_id  = 15,
+};
+
+static struct davinci_clk pll1_sysclkbp = {
+   .name   = pll1_sysclkbp,
+   .parent = ref_clk_mux,
+   .data   = pll1_sysclkbp_data,
+};
+
+static struct davinci_clk *pll1_plldiv_clocks[] = {
+   pll1_sysclk1, pll1_sysclk2, pll1_sysclk3, pll1_sysclk4,
+   pll1_sysclk5,