Hi Cyril, On Fri, Mar 26, 2010 at 03:13:58, Chemparathy, Cyril wrote: > TNETV107X is a Texas Instruments SOC that shares a number of common features > with the Davinci architecture. Some of the key differences between > traditional Davincis and this new SOC are as follow: > > 1. The SOCs clock architecture includes a new spread-spectrum PLL. Some > elements of the clock architecture are reused from Davinci (e.g. LPSC), but > the PLL related code is overridden using existing interfaces in "struct clk". > > 2. The MMR layout on this SOC is substantially different from Davinci. > Consequently, the fixed I/O map is a whole lot more convoluted (more so than > DA8xx). The net impact here is that IO_ADDRESS() will not work on this SoC, > and therefore all mappings have to be through ioremap(). > > 3. The Davinci reset mechanism (via Timer64 watchdog) will not work on this > SoC. This difference has not been addressed here (dummy davinci_wdt_device), > but a more "extensible" reset mechanism will need to be established > eventually.
It will be nice to point out which of the existing platforms were tested to be working after this series was applied (with some indication of the type of testing done). > > Signed-off-by: Cyril Chemparathy <[email protected]> > --- [...] > + > +static struct resource wdt_resources[] = { > + { > + .start = TNETV107X_TIMER1_BASE, > + .end = TNETV107X_TIMER1_BASE + SZ_4K - 1, > + .flags = IORESOURCE_MEM, > + }, > +}; > + > +struct platform_device davinci_wdt_device = { > + .name = "watchdog", > + .id = -1, > + .num_resources = ARRAY_SIZE(wdt_resources), > + .resource = wdt_resources, > +}; IMO, a better way to overcome the 'no watchdog timer' limitation would be to make the watchdog reset code not use the wdt platform data directly, but instead 'search' for watchdog device using bus_for_each_dev() iterator on platform bus. In your case, the watchdog device wouldn't be found and the reset function should exist gracefully. [...] > + > +void __init tnetv107x_edma_init(void) > +{ > + platform_device_register(&edma_device); > +} > + > +void __init tnetv107x_serial_init(struct plat_serial8250_port* ports) > +{ > + int i; > + char name[16]; > + struct clk *uart_clk; > + > + for (i = 0; ports[i].flags; i++) { > + sprintf(name, "uart%d", i); > + uart_clk = clk_get(NULL, name); > + if (IS_ERR(uart_clk)) > + printk(KERN_ERR "%s:%d: failed to get UART%d clock\n", > + __func__, __LINE__, i); > + else { > + clk_enable(uart_clk); > + ports[i].uartclk = clk_get_rate(uart_clk); > + } > + } > +} An explanation on why davinci_serial_init() is no good for this SoC would be good. [...] > diff --git a/arch/arm/mach-davinci/tnetv107x.c > b/arch/arm/mach-davinci/tnetv107x.c > new file mode 100644 > index 0000000..30ff23b > --- /dev/null > +++ b/arch/arm/mach-davinci/tnetv107x.c > @@ -0,0 +1,1222 @@ > +/* > + * TI TNETV107X chip specific setup > + * > + * Author: Cyril Chemparathy <[email protected]> > + * > + * 2009 (c) Texas Instruments, Inc. This file is licensed under > + * the terms of the GNU General Public License version 2. This program > + * is licensed "as is" without any warranty of any kind, whether express > + * or implied. > + */ > + > +/* > + * TNETV107X clock control implementation doesn't (at this point) reuse > + * much of Davinci's generic implementation. This is because of the > + * SSPLL nastiness that is present on this part. > + */ > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/clk.h> > +#include <linux/io.h> > + > +#include <asm/mach/map.h> > + > +#include <mach/common.h> > +#include <mach/time.h> > +#include <mach/cputype.h> > +#include <mach/psc.h> > +#include <mach/cp_intc.h> > +#include <mach/irqs.h> > +#include <mach/hardware.h> > +#include <mach/tnetv107x.h> > + > +#include "clock.h" > +#include "mux.h" > + > +/* Reference clock frequencies */ > +#define OSC_FREQ_ONCHIP (24000 * 1000) > +#define OSC_FREQ_OFFCHIP_SYS (25000 * 1000) > +#define OSC_FREQ_OFFCHIP_ETH (25000 * 1000) > +#define OSC_FREQ_OFFCHIP_TDM (19200 * 1000) > + > +/* PLL Types */ > +enum pll_type { > + SYS_PLL, > + TDM_PLL, > + ETH_PLL, > + N_PLLS > +}; > + > +/* Clock Control Registers */ > +struct clk_ctrl_regs { > + u32 pll_bypass; > + u32 _reserved0; > + u32 gem_lrst; > + u32 _reserved1; > + u32 pll_unlock_stat; > + u32 sys_unlock; > + u32 eth_unlock; > + u32 tdm_unlock; > +}; > + > +/* SSPLL Registers */ > +struct sspll_regs { > + u32 modes; > + u32 post_div; > + u32 pre_div; > + u32 mult_factor; > + u32 divider_range; > + u32 bw_divider; > + u32 spr_amount; > + u32 spr_rate_div; > + u32 diag; > +}; > + > +static struct clk_ctrl_regs __iomem *clk_ctrl_regs; > + > +static struct sspll_regs __iomem *sspll_regs[N_PLLS]; > +static int sspll_regs_base[N_PLLS] = { 0x40, 0x80, 0xc0 }; > + > +/* PLL Control Registers (same as in Davinci) */ > +static void __iomem *pllctl_regs[N_PLLS]; > +static int pllctl_regs_base[N_PLLS] = { 0x600, 0x200, 0x400 }; > + > +/* PLL bypass bit shifts in clk_ctrl_regs->pll_bypass register */ > +static u32 bypass_mask[N_PLLS] = { BIT(0), BIT(2), BIT(1) }; > + > +/* different PLLs have different limits on the divider max */ > +static u32 div_mask[] = {0x01ff, 0x00ff, 0x00ff}; > + > +/* offchip (external) reference clock frequencies */ > +static u32 pll_ext_freq[] = { > + OSC_FREQ_OFFCHIP_SYS, > + OSC_FREQ_OFFCHIP_TDM, > + OSC_FREQ_OFFCHIP_ETH > +}; > + > +/* PSC control registers */ > +static void __iomem *psc_regs[1]; > + > +/* Host map for interrupt controller */ > +static u32 intc_host_map[] = { 0x01010000, 0x01010101, -1 }; > + > +static unsigned long clk_sspll_recalc(struct clk *clk); > +static unsigned long clk_div_recalc(struct clk *clk); > +static unsigned long clk_null_recalc(struct clk *clk); > + > +static struct pll_data pll_sys_data = { .num = SYS_PLL }; > +static struct pll_data pll_eth_data = { .num = ETH_PLL }; > +static struct pll_data pll_tdm_data = { .num = TDM_PLL }; > + [...] > + > + > +/* LPSC Gated Clocks */ > +static struct clk clk_arm = { > + .name = "clk_arm", [...] > + > +static struct clk clk_ddr2_vctl_rst = { > + .name = "clk_ddr2_vctl_rst", No need of the 'clk_' prefix for all the LPSC clock names. [...] > + > +/* > + * TNETV107X platforms do not use the static mappings from Davinci > + * IO_PHYS/IO_VIRT. This SOC's interesting MMRs are at different addresses, > + * and changing IO_PHYS would break away from existing Davinci SOCs. So why not redefine the IO_PHYS for this SoC? I believe you are not able to build a single image for TNETV107x and DaVinci anyway. That should also avoid the EDMA issue you highlight below. > + * > + * The primary impact of the current model is that IO_ADDRESS() is not to be > + * used to map registers on TNETV107X. With the exception of early boot code > + * in tnetv107x_init() (where fixed maps are necessary), all other pieces of > + * code absolutely _must_ use ioremap(). > + * > + * 1. The first section in here is specifically for edma, even > through the > + * edma driver code properly ioremap()s its register space. This is > + * because the edma MMRs on tnetv107x unfortunately fall within the > + * davinci IO_PHYS - (IO_PHYS + IO_SIZE) range, and therefore the edma > + * driver's ioremap() gets subverted by davinci_ioremap(). Yikes! > + * > + * 2. The second chunk is for INTC - no major issues here. If the > cp_intc > + * code were to someday ioremap() internally, we could very well get rid > + * of this map. > + * > + * 3. The third chunk maps in register areas that need to be > populated into > + * davinci_soc_info. Note that alignment restrictions come into play if > + * low-level debug is enabled (see note in <mach/tnetv107x.h>). > + */ > +static struct map_desc tnetv107x_io_desc[] = { > + { /* EDMA */ > + .virtual = IO_VIRT, > + .pfn = __phys_to_pfn(IO_PHYS), > + .length = SZ_128K, > + .type = MT_DEVICE > + }, > + { /* INTC */ > + .virtual = IO_VIRT + SZ_128K, > + .pfn = __phys_to_pfn(TNETV107X_INTC_BASE), > + .length = SZ_16K, > + .type = MT_DEVICE > + }, > + { /* Most of the rest */ > + .virtual = TNETV107X_IO_VIRT, > + .pfn = __phys_to_pfn(TNETV107X_IO_BASE), > + .length = IO_SIZE - SZ_1M, > + .type = MT_DEVICE > + }, > +}; > + [...] > + > +static void __iomem *fixed_ioremap(unsigned long p, size_t size) > +{ > + struct map_desc *map = tnetv107x_soc_info.io_desc; > + int i; > + > + for (i = 0; i < tnetv107x_soc_info.io_desc_num; i++) { > + unsigned long iophys = __pfn_to_phys(map[i].pfn); > + unsigned long iosize = map[i].length; > + > + if (p >= iophys && (p + size) <= (iophys + iosize)) > + return IOMEM(map[i].virtual + p - iophys); > + } > + > + panic("attempt to map invalid physical range %lx-%lx\n", > + p, p + size); > +} This sounds like something that should be useful on all DaVinci SoCs. Why not make davinci_ioremap to do exactly this? > + > +void __init tnetv107x_init(void) > +{ > + void __iomem *tmp; > + int i; > + > + /* > + * Figure out virtual addresses for necessary peripherals, but do not > + * access any of these here. iotable_init() needs to happen for the > + * mappings to actually get setup > + */ > + clk_ctrl_regs = tmp = > + fixed_ioremap(TNETV107X_CLOCK_CONTROL_BASE, SZ_4K); > + > + for (i = 0; i < N_PLLS; i++) { > + sspll_regs[i] = tmp + sspll_regs_base[i]; > + pllctl_regs[i] = tmp + pllctl_regs_base[i]; > + } > + > + psc_regs[0] = fixed_ioremap(TNETV107X_PSC_BASE, SZ_4K); > + > + tmp = fixed_ioremap(TNETV107X_CHIP_CFG_BASE, SZ_4K); > + tnetv107x_soc_info.jtag_id_base = tmp + 0x018; > + tnetv107x_soc_info.pinmux_base = tmp + 0x150; > + > + tnetv107x_soc_info.intc_base = > + fixed_ioremap(TNETV107X_INTC_BASE, SZ_16K); > + > + tnetv107x_timer_instance[0].base = > + fixed_ioremap(TNETV107X_TIMER0_BASE, 0x100); > + > + tnetv107x_timer_instance[1].base = > + fixed_ioremap(TNETV107X_TIMER1_BASE, 0x100); All the fixed_ioremap stuff should disappear if you can implement the comment above. > + > + davinci_common_init(&tnetv107x_soc_info); > +} > + [...] > + > +static unsigned long clk_sspll_recalc(struct clk *clk) > +{ > + int pll; > + unsigned long mult = 0, prediv = 1, postdiv = 1; > + unsigned long ref = OSC_FREQ_ONCHIP, ret; > + u32 tmp; > + > + if (WARN_ON(!clk->pll_data)) > + return clk->rate; > + > + pll = clk->pll_data->num; > + > + tmp = __raw_readl(&clk_ctrl_regs->pll_bypass); > + if (!(tmp & bypass_mask[pll])) { > + mult = __raw_readl(&sspll_regs[pll]->mult_factor); > + prediv = __raw_readl(&sspll_regs[pll]->pre_div) + 1; > + postdiv = __raw_readl(&sspll_regs[pll]->post_div) + 1; > + } > + > + tmp = __raw_readl(pllctl_regs[pll] + PLLCTL); > + if (tmp & PLLCTL_CLKMODE) > + ref = pll_ext_freq[pll]; > + > + clk->pll_data->input_rate = ref; > + > + tmp = __raw_readl(pllctl_regs[pll] + PLLCTL); > + if (!(tmp & PLLCTL_PLLEN)) > + return ref; > + > + ret = ref; > + if (mult) > + ret += ((unsigned long long)ref * mult) / 256; > + > + ret /= (prediv * postdiv); > + > + return ret; > +} How about moving this to clock.c and using a flag in clk structure to determine whether to use clk_pllclk_recalc() or clk_sspll_recalc()? > + > +static unsigned long clk_div_recalc(struct clk *clk) > +{ > + struct pll_data *pll; > + unsigned long rate = clk->rate; > + unsigned long div; > + > + if (WARN_ON(!clk->parent)) > + return rate; > + > + rate = clk->parent->rate; > + > + /* Otherwise, the parent must be a PLL */ > + if (WARN_ON(!clk->parent->pll_data)) > + return rate; > + > + pll = clk->parent->pll_data; > + > + if (!clk->div_reg) > + return rate; > + > + div = __raw_readl(pllctl_regs[pll->num] + clk->div_reg); > + if (div & PLLDIV_EN) { > + div &= div_mask[pll->num]; > + rate /= (div + 1); > + } > + > + return rate; > +} Is the div_mask the only reason you could not use clock.c:clk_sysclk_recalc() here? How about making the mask a part of PLL data and using PLLDIV_RATIO_MASK if no mask is initialized (zero)? > + > +static unsigned long clk_null_recalc(struct clk *clk) > +{ > + BUG_ON(!clk->parent); > + return clk->parent->rate; > +} There is already a function in clock.c which does exactly this (clk_leafclk_recalc). Thanks, Sekhar _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
