Re: [PATCH v3 01/11] clk: davinci - add main PLL clock driver

2012-10-31 Thread Sekhar Nori
Hi Murali,

On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This is the driver for the main PLL clock hardware found on DM SoCs.
 This driver borrowed code from arch/arm/mach-davinci/clock.c and
 implemented the driver as per common clock provider API. The main PLL
 hardware typically has a multiplier, a pre-divider and a post-divider.
 Some of the SoCs has the divider fixed meaning they can not be
 configured through a register. HAS_PREDIV and HAS_POSTDIV flags are used
 to tell the driver if a hardware has these dividers present or not.
 Driver is configured through the struct clk_pll_data that has the
 SoC specific clock data.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---

 diff --git a/drivers/clk/davinci/clk-pll.c b/drivers/clk/davinci/clk-pll.c

 +static unsigned long clk_pllclk_recalc(struct clk_hw *hw,
 + unsigned long parent_rate)
 +{
 + struct clk_pll *pll = to_clk_pll(hw);
 + struct clk_pll_data *pll_data = pll-pll_data;
 + u32 mult = 1, prediv = 1, postdiv = 1;

No need to initialize mult here. I gave this comment last time around as
well.

 + unsigned long rate = parent_rate;
 +
 + mult = readl(pll_data-reg_pllm);
 +
 + /*
 +  * if fixed_multiplier is non zero, multiply pllm value by this
 +  * value.
 +  */
 + if (pll_data-fixed_multiplier)
 + mult =  pll_data-fixed_multiplier *
 + (mult  pll_data-pllm_mask);
 + else
 + mult = (mult  pll_data-pllm_mask) + 1;

Hmm, this is interpreting the 'mult' register field differently in both
cases. In one case it is 'actual multiplier - 1' and in other case it is
the 'actual multiplier' itself. Can we be sure that the mult register
definition will change whenever there is a fixed multiplier in the PLL
block? I don't think any of the existing DaVinci devices have a fixed
multiplier. Is this on keystone?

 +struct clk *clk_register_davinci_pll(struct device *dev, const char *name,
 + const char *parent_name,
 + struct clk_pll_data *pll_data)
 +{
 + struct clk_init_data init;
 + struct clk_pll *pll;
 + struct clk *clk;
 +
 + if (!pll_data)
 + return ERR_PTR(-ENODEV);

-EINVAL? Clearly you are treating NULL value as an invalid argument here.

Thanks,
Sekhar
--
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 v3 01/11] clk: davinci - add main PLL clock driver

2012-11-01 Thread Sekhar Nori
On 10/31/2012 7:16 PM, Murali Karicheri wrote:
 On 10/31/2012 08:29 AM, Sekhar Nori wrote:

 +/*
 + * if fixed_multiplier is non zero, multiply pllm value by this
 + * value.
 + */
 +if (pll_data-fixed_multiplier)
 +mult =  pll_data-fixed_multiplier *
 +(mult  pll_data-pllm_mask);
 +else
 +mult = (mult  pll_data-pllm_mask) + 1;
 Hmm, this is interpreting the 'mult' register field differently in both
 cases. In one case it is 'actual multiplier - 1' and in other case it is
 the 'actual multiplier' itself. Can we be sure that the mult register
 definition will change whenever there is a fixed multiplier in the PLL
 block? I don't think any of the existing DaVinci devices have a fixed
 multiplier. Is this on keystone?
 Read section 6.4.3 (PLL Mode) in DM365 documentation (SPRUFG5a.pdf) that
 states PLL multiplies the clock by 2x the value in the PLLM. In the old
 code this is handled by a if cpu_is_* macro that we can't use in the
 driver. So this is represented by a fixed_multiplier that can be set to
 2 for DM365 and zero on other SoCs.

Thanks for the clarification.

Regards,
Sekhar
--
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 v3 03/11] clk: davinci - common clk utilities to init clk driver

2012-11-01 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This is the common clk driver initialization functions for DaVinci
 SoCs and other SoCs that uses similar hardware architecture.
 clock.h also defines struct types for clock definitions in a SoC
 and clock data type for configuring clk-mux. The initialization
 functions are used by clock initialization code in a specific
 platform/SoC.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com

 +struct clk *davinci_plldiv_clk(const char *name, const char *parent,
 + struct clk_plldiv_data *data)
 +{
 + /*
 +  * This is a PLL divider clock with divider specified by
 +  * div_reg in pll_div_data.
 +  */
 + data-reg = ioremap(data-phys_div_reg, 4);
 + if (WARN_ON(!data-reg))
 + return NULL;
 +
 + return clk_register_davinci_plldiv(NULL, name, parent, data, _lock);

This function does not exist at this point. Looks like you need to swap
3/11 with 4/11. Also, you should also add build infrastructure
(makefile, Kconfig) changes in the same patch that creates the file.
There is no point in adding those separately.

Thanks,
Sekhar
--
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 v3 04/11] clk: davinci - add pll divider clock driver

2012-11-02 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:

 pll dividers are present in the pll controller of DaVinci and Other
 SoCs that re-uses the same hardware IP. This has a enable bit for
 bypass the divider or enable the driver. This is a sub class of the
 clk-divider clock checks the enable bit to calculare the rate and
 invoke the recalculate() function of the clk-divider if enabled.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
  drivers/clk/davinci/clk-div.c |  124 
 +
  drivers/clk/davinci/clk-div.h |   42 ++
  2 files changed, 166 insertions(+)
  create mode 100644 drivers/clk/davinci/clk-div.c
  create mode 100644 drivers/clk/davinci/clk-div.h
 
 diff --git a/drivers/clk/davinci/clk-div.c b/drivers/clk/davinci/clk-div.c
 new file mode 100644
 index 000..8147d99
 --- /dev/null
 +++ b/drivers/clk/davinci/clk-div.c
 @@ -0,0 +1,124 @@
 +/*
 + * Copyright 2012 Freescale Semiconductor, Inc.
 + * Copyright 2012 Texas instuments
 + *
 + * The code contained herein is licensed under the GNU General Public
 + * License. You may obtain a copy of the GNU General Public License
 + * Version 2 or later at the following locations:

incomplete sentence.

 +/**
 + * clk_register_davinci_plldiv - register function for DaVinci PLL divider 
 clk
 + *
 + * @dev: device ptr
 + * @name: name of the clock
 + * @parent_name: name of parent clock
 + * @plldiv_data: ptr to pll divider data
 + * @lock: ptr to spinlock passed to divider clock
 + */
 +struct clk *clk_register_davinci_plldiv(struct device *dev,

Why do you need a dev pointer here and which device does it point to? In
the only usage of this API in the series, you pass a NULL here. I should
have probably asked this question on one of the earlier patches itself.

 + const char *name, const char *parent_name,
 + struct clk_plldiv_data *plldiv_data,
 + spinlock_t *lock)
 +{
 + struct clk_div *div;
 + struct clk *clk;
 + struct clk_init_data init;
 +
 + div = kzalloc(sizeof(*div), GFP_KERNEL);
 + if (!div)
 + return ERR_PTR(-ENOMEM);
 +
 + init.name = name;
 + init.ops = clk_div_ops;
 + init.flags = plldiv_data-flags;
 + init.parent_names = (parent_name ? parent_name : NULL);
 + init.num_parents = (parent_name ? 1 : 0);
 +
 + div-reg = plldiv_data-reg;
 + div-en_id = plldiv_data-en_id;
 +
 + div-divider.reg = plldiv_data-reg;
 + div-divider.shift = plldiv_data-shift;
 + div-divider.width = plldiv_data-width;
 + div-divider.flags = plldiv_data-divider_flags;
 + div-divider.lock = lock;
 + div-divider.hw.init = init;
 + div-ops = clk_divider_ops;
 +
 + clk = clk_register(NULL, div-divider.hw);

Shouldn't you be calling clk_register_divider() here which in turn will
do clk_register()?

Thanks,
Sekhar
--
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 v3 0/2] ARM: davinci: da850/omap-l138: add support for VPIF driver

2012-10-03 Thread Sekhar Nori
Hi Prabhakar,

On 9/11/2012 11:55 AM, Prabhakar Lad wrote:
 This patch series adds support for VPIF
 capture and display driver on da850/omap-l138.
 Enables SD capture and display.
 
 This patch series is dependent on the following patch:
 https://patchwork.kernel.org/patch/1332311/
 
 Changes for v3:
 1: Clubbed the code for DA850_UI_SD_VIDEO_PORT config
as pointed by Sekhar.
 2: Define  the resource structure outside the function as pointed
by Sekhar.
 
 Changes for v2:
 1: Avoid breaking of print messages.
 2: Removed the handlers which just returned zero (which did nothing).
 3: Clubbed the code where ever possible for DA850_UI_SD_VIDEO_PORT
config option.
 4: Removed the dma_declare_coherent_memory() calls and used
global CMA.
 5: Added the base address in increasing order.
 
 Manjunath Hadli (2):
   ARM: da850/omap-l138: Add SoC related definitions for VPIF
   ARM: da850/omap-l138: Add EVM specific code for VPIF to work
 
 Manjunath Hadli (2):
   ARM: da850/omap-l138: Add SoC related definitions for VPIF
   ARM: da850/omap-l138: Add EVM specific code for VPIF to work
 
  arch/arm/mach-davinci/Kconfig  |7 ++
  arch/arm/mach-davinci/board-da850-evm.c|  156 
 
  arch/arm/mach-davinci/da850.c  |  152 +++
  arch/arm/mach-davinci/include/mach/da8xx.h |   11 ++
  arch/arm/mach-davinci/include/mach/mux.h   |   42 
  arch/arm/mach-davinci/include/mach/psc.h   |1 +
  6 files changed, 369 insertions(+), 0 deletions(-)

The patches look good to me. In case you want to queue them through the
media tree to manage the dependencies, feel free to add my:

Acked-by: Sekhar Nori nsek...@ti.com

Note that the subject like prefix for DA850 soc patches should be

ARM: davinci: da850: ..

and that for DA850 EVM patches should be:

ARM: davinci: da850 evm: ..

instead of what you have used here. Please fix these if you are queuing
them and take care of this in future.

Thanks,
Sekhar
--
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] davinci: vpbe: replace V4L2_OUT_CAP_CUSTOM_TIMINGS with V4L2_OUT_CAP_DV_TIMINGS

2012-10-03 Thread Sekhar Nori
On 10/3/2012 12:02 PM, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar@ti.com
 
 This patch replaces V4L2_OUT_CAP_CUSTOM_TIMINGS macro with
 V4L2_OUT_CAP_DV_TIMINGS. As V4L2_OUT_CAP_CUSTOM_TIMINGS is being phased
 out.
 
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Cc: Sekhar Nori nsek...@ti.com
 Cc: Hans Verkuil hansv...@cisco.com
 Cc: Mauro Carvalho Chehab mche...@redhat.com

For the DaVinci platform change:

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
--
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] ARM: davinci: dm644x: fix out range signal for ED

2012-10-03 Thread Sekhar Nori
On 10/3/2012 12:05 PM, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar@ti.com
 
 while testing display on dm644x, for ED out-range signals
 was observed. This patch fixes appropriate clock setting
 for ED.

Can you please clarify what you mean by out range signal? Are there
any user visible artifacts on the display? What was the clock being
provided earlier and what is the value after this patch?

Also, is the issue severe enough that this patch should be applied to
stable tree as well?

Thanks,
Sekhar
--
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] ARM: davinci: dm644x: fix out range signal for ED

2012-10-04 Thread Sekhar Nori
On 10/4/2012 10:22 AM, Prabhakar Lad wrote:
 Hi Sekhar,
 
 On Wed, Oct 3, 2012 at 4:08 PM, Sekhar Nori nsek...@ti.com wrote:
 On 10/3/2012 12:05 PM, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar@ti.com

 while testing display on dm644x, for ED out-range signals
 was observed. This patch fixes appropriate clock setting
 for ED.

 Can you please clarify what you mean by out range signal? Are there
 any user visible artifacts on the display? What was the clock being
 provided earlier and what is the value after this patch?

 Also, is the issue severe enough that this patch should be applied to
 stable tree as well?

 Ideally a clock of 54Mhz is required for  enhanced definition to
 work, which it was actually set to but while testing I noticed
 out-of-range signal. The out-of-range signal is often caused
 when the field rate is above the rate that the television will handle.
 When this is the case the TV or monitor displays Out-Of-Range Signal.
 
 Reducing the clock fixed it. now the clock is set to 27Mhz.

So, is the requirement for ED 54MHz or lower? Still trying to explain
myself how 27MHz is working where a 54MHz is required. I guess there is
also a lower limit on what the frequency should be?

Sorry I am not familiar will all the video concepts, hence the questions.

Thanks,
Sekhar
--
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 v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-04 Thread Sekhar Nori
On 10/3/2012 8:25 PM, Matt Porter wrote:
 From: Ben Gardiner bengardi...@nanometrics.ca
 
 The current davinci init sets up SRAM in iotables. There has been an observed
 failure to boot a da850 with 128K specified in the iotable.
 
 Make the davinci sram allocator -- now based on RMK's consolidated SRAM
 support -- do an ioremap of the region specified by the entries in

The part about being based on RMK's consolidated SRAM support should be
dropped.

 davinci_soc_info before registering with gen_pool_add_virt().
 
 This commit breaks runtime of davinci boards since the regions that
 the sram init is now trying to ioremap have been iomapped by their
 iotable entries. The iotable entries will be removed in the patches
 to come.

I would prefer merging 2/6 into this for this reason.

 
 Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
 [rebased to mainline as the consolidated SRAM support was dropped]
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/mach-davinci/sram.c |   17 ++---
  1 file changed, 14 insertions(+), 3 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
 index db0f778..0e8ca4f 100644
 --- a/arch/arm/mach-davinci/sram.c
 +++ b/arch/arm/mach-davinci/sram.c
 @@ -10,6 +10,7 @@
   */
  #include linux/module.h
  #include linux/init.h
 +#include linux/io.h
  #include linux/genalloc.h
  
  #include mach/common.h
 @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
   return NULL;
  
   if (dma)
 - *dma = dma_base + (vaddr - SRAM_VIRT);
 + *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
   return (void *)vaddr;
  
  }
 @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
   */
  static int __init sram_init(void)
  {
 + phys_addr_t phys = davinci_soc_info.sram_dma;
   unsigned len = davinci_soc_info.sram_len;
   int status = 0;
 + void *addr;
  
   if (len) {
   len = min_t(unsigned, len, SRAM_SIZE);
 @@ -62,8 +65,16 @@ static int __init sram_init(void)
   if (!sram_pool)
   status = -ENOMEM;
   }
 - if (sram_pool)
 - status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
 +
 + if (sram_pool) {
 + addr = ioremap(phys, len);
 + if (!addr)
 + return -ENOMEM;
 + if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
 +phys, len, -1)))

Nit: prefer to set status outside of if().

Looks good otherwise. Thanks for reviving this.

Thanks,
Sekhar
--
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 v3 2/6] ARM: davinci: da850-dm646x: remove the SRAM_VIRT iotable entry

2012-10-04 Thread Sekhar Nori
On 10/3/2012 8:25 PM, Matt Porter wrote:
 From: Ben Gardiner bengardi...@nanometrics.ca
 
 The sram regions defined for da850-dm646x in their iotable entries are also
 defined in their davinci_soc_info's.
 
 Remove this duplicate information which is now uneccessary since sram
 init will ioremap the regions defined by their davinci_soc_info's.
 
 Since this removal completely removes all uses of SRAM_VIRT, also remove
 the SRAM_VIRT definition.
 
 Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
 Tested-by: Matt Porter mpor...@ti.com

What testing was done with this patch? Can you please add that
information to the commit text as well.

Thanks,
Sekhar
--
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 v3 3/6] ARM: davinci: da850: changed SRAM allocator to shared ram.

2012-10-04 Thread Sekhar Nori
Matt,

On 10/3/2012 8:25 PM, Matt Porter wrote:
 From: Subhasish Ghosh subhas...@mistralsolutions.com
 
 This patch modifies the sram allocator to allocate memory
 from the DA8XX shared RAM.
 
 Signed-off-by: Subhasish Ghosh subhas...@mistralsolutions.com
 [rebased onto consolidated SRAM patches]
 Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
 [rebased to mainline as consolidated SRAM patches were dropped]
 Signed-off-by: Matt Porter mpor...@ti.com

Were you able to test PM with this change or you need my help?

Thanks,
Sekhar
--
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 v3 5/6] ARM: davinci: Add support for PRUSS on DA850

2012-10-04 Thread Sekhar Nori
On 10/3/2012 8:25 PM, Matt Porter wrote:
 Adds PRUSS clock, registers the L3RAM pool, and registers the
 platform device for uio_pruss on DA850.
 
 Signed-off-by: Matt Porter mpor...@ti.com

I am interested in knowing how this patch was tested.

 ---
  arch/arm/mach-davinci/board-da850-evm.c|   12 +
  arch/arm/mach-davinci/da850.c  |7 +++
  arch/arm/mach-davinci/devices-da8xx.c  |   66 
 
  arch/arm/mach-davinci/include/mach/da8xx.h |2 +
  4 files changed, 87 insertions(+)
 
 diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
 b/arch/arm/mach-davinci/board-da850-evm.c
 index 1295e61..acc6e84 100644
 --- a/arch/arm/mach-davinci/board-da850-evm.c
 +++ b/arch/arm/mach-davinci/board-da850-evm.c
 @@ -29,6 +29,7 @@
  #include linux/regulator/machine.h
  #include linux/regulator/tps6507x.h
  #include linux/input/tps6507x-ts.h
 +#include linux/platform_data/uio_pruss.h
  #include linux/spi/spi.h
  #include linux/spi/flash.h
  #include linux/delay.h
 @@ -42,6 +43,7 @@
  #include mach/da8xx.h
  #include linux/platform_data/mtd-davinci.h
  #include mach/mux.h
 +#include mach/sram.h
  #include linux/platform_data/mtd-davinci-aemif.h
  #include linux/platform_data/spi-davinci.h

I know you did not introduce the mess, but can you include a patch to
fix the mixture of mach/ and linux/ includes here? mach/ includes should
follow the linux/ includes.

  
 @@ -1253,6 +1255,10 @@ static __init int da850_wl12xx_init(void)
  
  #endif /* CONFIG_DA850_WL12XX */
  
 +struct uio_pruss_pdata da8xx_pruss_uio_pdata = {
 + .pintc_base = 0x4000,
 +};
 +
  #define DA850EVM_SATA_REFCLKPN_RATE  (100 * 1000 * 1000)
  
  static __init void da850_evm_init(void)
 @@ -1339,6 +1345,12 @@ static __init void da850_evm_init(void)
   pr_warning(da850_evm_init: lcdcntl mux setup failed: %d\n,
   ret);
  
 + da8xx_pruss_uio_pdata.l3ram_pool = sram_get_gen_pool();

I wonder if this platform data should still be called l3ram pool. L3RAM
is OMAP terminology. May be just call it sram_pool? Also this patch
should follow 6/6 to prevent build breakage.

 + ret = da8xx_register_pruss_uio(da8xx_pruss_uio_pdata);
 + if (ret)
 + pr_warning(pruss_uio initialization failed: %d\n,
 + ret);
 +
   /* Handle board specific muxing for LCD here */
   ret = davinci_cfg_reg_list(da850_evm_lcdc_pins);
   if (ret)
 diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
 index d8d69de..7c01d31 100644
 --- a/arch/arm/mach-davinci/da850.c
 +++ b/arch/arm/mach-davinci/da850.c

Can you separate out board and SoC changes into different patches?

 @@ -212,6 +212,12 @@ static struct clk tptc2_clk = {
   .flags  = ALWAYS_ENABLED,
  };
  
 +static struct clk pruss_clk = {
 + .name   = pruss,
 + .parent = pll0_sysclk2,
 + .lpsc   = DA8XX_LPSC0_PRUSS,
 +};
 +
  static struct clk uart0_clk = {
   .name   = uart0,
   .parent = pll0_sysclk2,
 @@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
   CLK(NULL,   tptc1,tptc1_clk),
   CLK(NULL,   tpcc1,tpcc1_clk),
   CLK(NULL,   tptc2,tptc2_clk),
 + CLK(NULL,   pruss,pruss_clk),

This is actually incorrect since we should use device name rather than
con_id for matching the clock. If there is just one clock that the
driver needs, connection id should be NULL. Looking at the driver now,
the clk_get() call seems to pass a valid device pointer. So, I wonder
how you are able to look up the clock even with a NULL device name.

   CLK(NULL,   uart0,uart0_clk),
   CLK(NULL,   uart1,uart1_clk),
   CLK(NULL,   uart2,uart2_clk),
 diff --git a/arch/arm/mach-davinci/devices-da8xx.c 
 b/arch/arm/mach-davinci/devices-da8xx.c
 index bd2f72b..7c2e0d2 100644
 --- a/arch/arm/mach-davinci/devices-da8xx.c
 +++ b/arch/arm/mach-davinci/devices-da8xx.c
 @@ -518,6 +518,72 @@ void __init da8xx_register_mcasp(int id, struct 
 snd_platform_data *pdata)
   }
  }
  
 +#define DA8XX_PRUSS_MEM_BASE 0x01C3

In this file all base addresses are added at the top of the file. The
defines are sorted in ascending order of address. If that's broken, its
all my fault, but please don't add to the breakage when adding this entry :)

 +
 +static struct resource da8xx_pruss_resources[] = {
 + {
 + .start  = DA8XX_PRUSS_MEM_BASE,
 + .end= DA8XX_PRUSS_MEM_BASE + 0x,
 + .flags  = IORESOURCE_MEM,
 + },
 + {
 + .start  = IRQ_DA8XX_EVTOUT0,
 + .end= IRQ_DA8XX_EVTOUT0,
 + .flags  = IORESOURCE_IRQ,
 + },
 + {
 + .start  = IRQ_DA8XX_EVTOUT1,
 + .end= IRQ_DA8XX_EVTOUT1,
 + .flags  = IORESOURCE_IRQ,
 +  

Re: [PATCH v3 0/6] uio_pruss cleanup and platform support

2012-10-04 Thread Sekhar Nori
On 10/4/2012 6:12 PM, Matt Porter wrote:
 On Thu, Oct 04, 2012 at 11:11:45AM +0200, Philipp Zabel wrote:
 Hi Matt,

 On 10/3/12, Matt Porter mpor...@ti.com wrote:
 This series enables uio_pruss on DA850 and removes use of the
 private SRAM API by the driver. The driver previously was not
 enabled by any platform and the private SRAM API was accessing
 an invalid SRAM bank.

 have you seen my SRAM patch series at https://lkml.org/lkml/2012/9/7/281
 Add device tree support for on-chip SRAM ?
 
 Yes.
 
 I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
 could be useful to map the L3RAM on Davinci.
 With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
 uio_pruss driver could then use the gen_pool_find_by_phys() (or
 of_get_named_gen_pool() for initialization from device tree) to
 retrieve the struct gen_pool*.

 This way you could avoid handing it over via platform data and you could
 get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
 
 I did miss the gen_pool_find_by_phys() call in that series. That does
 look useful. I actually mentioned your series in an earlier posting
 since I like it, but since the initialization of the driver was inherently
 tied to DT it's not usable for DaVinci that's just starting to convert
 to DT and needs !DT support as well.
 
 I do see it moving to your driver exclusively, but I wanted to make this
 series focused on only getting rid of the private SRAM API using the
 existing pdata framework that's already there. I think once
 gen_pool_find_by_phys() goes upstream we can switch to that and get the
 address from a resource in the !DT case. I guess we should see if Sekhar
 would like to see this happen in two steps or just have us depend on
 the gen_pool_find_by_phys() patch now.

I prefer going with this series now and switching to the SRAM driver
once it is available mainline.

Thanks,
Sekhar
--
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 14/17] ARM: davinci: don't mark da850_register_cpufreq as __init

2012-10-04 Thread Sekhar Nori
On 10/2/2012 10:06 PM, Arnd Bergmann wrote:
 The mityomapl138_cpufreq_init and read_factory_config function in
 board-mityomapl138.c are not __init functions and might be called
 at a later stage, so da850_register_cpufreq must not be __init either.
 
 Without this patch, building da8xx_omapl_defconfig results in:
 
 WARNING: arch/arm/mach-davinci/built-in.o(.text+0x2eb4): Section mismatch in 
 reference from the function read_factory_config() to the function 
 .init.text:da850_register_cpufreq()
 The function read_factory_config() references
 the function __init da850_register_cpufreq().
 This is often because read_factory_config lacks a __init
 annotation or the annotation of da850_register_cpufreq is wrong.
 
 Signed-off-by: Arnd Bergmann a...@arndb.de
 Cc: Sekhar Nori nsek...@ti.com
 Cc: Kevin Hilman khil...@ti.com

Acked-by: Sekhar Nori nsek...@ti.com

Regards,
Sekhar
--
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 v3 5/6] ARM: davinci: Add support for PRUSS on DA850

2012-10-05 Thread Sekhar Nori
On 10/4/2012 10:05 PM, Matt Porter wrote:
 On Thu, Oct 04, 2012 at 05:52:45PM +0530, Sekhar Nori wrote:
 On 10/3/2012 8:25 PM, Matt Porter wrote:
 +static struct clk pruss_clk = {
 +   .name   = pruss,
 +   .parent = pll0_sysclk2,
 +   .lpsc   = DA8XX_LPSC0_PRUSS,
 +};
 +
  static struct clk uart0_clk = {
 .name   = uart0,
 .parent = pll0_sysclk2,
 @@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
 CLK(NULL,   tptc1,tptc1_clk),
 CLK(NULL,   tpcc1,tpcc1_clk),
 CLK(NULL,   tptc2,tptc2_clk),
 +   CLK(NULL,   pruss,pruss_clk),

 This is actually incorrect since we should use device name rather than
 con_id for matching the clock. If there is just one clock that the
 driver needs, connection id should be NULL. Looking at the driver now,
 the clk_get() call seems to pass a valid device pointer. So, I wonder
 how you are able to look up the clock even with a NULL device name.
 
 I doublechecked the clk_get() find implentation to confirm that I indeed
 did get lucky here. Since pruss has only one instance and the clk_find()
 implementation looks for the best found match, it's able to find the
 clock just by the con_id, though it's not the best possible match in
 the find implementation...it's the best found match.. As you noted,
 this is incorrect though for a clock expected to be used from a driver
 context so I will address this in the update.

Thanks for debugging and clarifying.

Regards,
Sekhar
--
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 v4 7/7] ARM: davinci: register pruss_uio device on DA850 EVM

2012-10-08 Thread Sekhar Nori
On 10/5/2012 10:34 PM, Matt Porter wrote:
 Configures the required pdata and registers the pruss_uio
 platform device on the DA850 EVM.
 
 Tested on AM180x-EVM using the PRU_memAccessPRUDataRam and
 PRU_memAccessL3andDDR examples from the PRU userspace tools
 available from http://www.ti.com/tool/sprc940
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/mach-davinci/board-da850-evm.c |   12 
  1 file changed, 12 insertions(+)
 
 diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
 b/arch/arm/mach-davinci/board-da850-evm.c
 index 7359375..6f9478b 100644
 --- a/arch/arm/mach-davinci/board-da850-evm.c
 +++ b/arch/arm/mach-davinci/board-da850-evm.c
 @@ -31,6 +31,7 @@
  #include linux/platform_data/mtd-davinci.h
  #include linux/platform_data/mtd-davinci-aemif.h
  #include linux/platform_data/spi-davinci.h
 +#include linux/platform_data/uio_pruss.h
  #include linux/regulator/machine.h
  #include linux/regulator/tps6507x.h
  #include linux/spi/spi.h
 @@ -40,6 +41,7 @@
  #include mach/cp_intc.h
  #include mach/da8xx.h
  #include mach/mux.h
 +#include mach/sram.h
  
  #include asm/mach-types.h
  #include asm/mach/arch.h
 @@ -1253,6 +1255,10 @@ static __init int da850_wl12xx_init(void)
  
  #endif /* CONFIG_DA850_WL12XX */
  
 +struct uio_pruss_pdata da8xx_pruss_uio_pdata = {
 + .pintc_base = 0x4000,
 +};
 +
  #define DA850EVM_SATA_REFCLKPN_RATE  (100 * 1000 * 1000)
  
  static __init void da850_evm_init(void)
 @@ -1339,6 +1345,12 @@ static __init void da850_evm_init(void)
   pr_warning(da850_evm_init: lcdcntl mux setup failed: %d\n,
   ret);
  
 + da8xx_pruss_uio_pdata.sram_pool = sram_get_gen_pool();
 + ret = da8xx_register_pruss_uio(da8xx_pruss_uio_pdata);
 + if (ret)
 + pr_warning(pruss_uio initialization failed: %d\n,
 + ret);

I failed to mention this last time around, but is there any reason
platform data will change from board to board? Looks like the pintc_base
is SoC specific and that all platforms would use SRAM when using the
pruss driver. Looks like you can make da8xx_register_pruss_uio() take no
parameters at all.

Thanks,
Sekhar
--
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 v4 0/7] uio_pruss cleanup and platform support

2012-10-08 Thread Sekhar Nori
On 10/5/2012 10:34 PM, Matt Porter wrote:

 This series enables uio_pruss on DA850 and removes use of the
 private SRAM API by the driver. The driver previously was not
 enabled by any platform and the private SRAM API was accessing
 an invalid SRAM bank.
 
 It is regression tested on AM180x EVM with suspend/resume due
 to the new use of the shared SRAM for both PM and PRUSS. The
 uio_pruss driver is tested on the same platform using the
 PRU_memAccessPRUDataRam and PRU_memAccessL3andDDR examples from
 the PRU userspace tools available from http://www.ti.com/tool/sprc940

I applied patches 2/7, 3/7 and 6/7 of this series for v3.8. I have some
comments on the board patch. Rest of the patches depend on acceptance of
1/7 so I will take them only after that is accepted.

Thanks,
Sekhar
--
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 v4 6/7] ARM: davinci: clean up DA850 EVM include ordering

2012-10-08 Thread Sekhar Nori
On 10/5/2012 10:34 PM, Matt Porter wrote:
 Reorder includes so they are grouped by linux/mach/asm
 
 Signed-off-by: Matt Porter mpor...@ti.com

Thanks for the clean-up. While committing, I changed the subject to:

ARM: davinci: da850 evm: clean up include ordering

which is the convention I prefer.

Thanks,
Sekhar
--
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 v4] Enable USB peripheral on dm365 EVM

2012-10-08 Thread Sekhar Nori
Hi Constantine,

On 10/4/2012 9:52 PM, Constantine Shulyupin wrote:
 From: Constantine Shulyupin co...@makelinux.com
 
 Signed-off-by: Constantine Shulyupin co...@makelinux.com
 ---
 
 Note:
 
 USBPHY_CTL_PADDR and USBPHY_CLKFREQ_24MHZ are defined in board-dm365-evm.c 
 because davinci.h can't be included from drivers/usb/musb/. May be davinci.h 
 should be renamed and moved to arch/arm/mach-davinci/include/mach/usb.h like 
 arch/arm/plat-omap/include/plat/usb.h
 
 Tested with usb gadget g_zero.
 
 Changelog 
 
 Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html:
 - removed optional altering of pr_info
 
 Changes since v2 http://article.gmane.org/gmane.linux.kernel/1159868/
 - reordered code
 - removed alternation of GPIO33, which is multiplexed with DRVVBUS, because 
 is not need for peripheral USB
 
 Changes since v1 http://marc.info/?l=linux-kernelm=130894150803661w=2:
 - removed optional code and reordered
 
 This patch is based on code from Arago, Angstom, and RidgeRun projects.
 Original patch by miguel.agui...@ridgerun.com is three years ago:
 - 
 http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html
 
 ---
  arch/arm/mach-davinci/board-dm365-evm.c |8 
  arch/arm/mach-davinci/usb.c |2 ++
  drivers/usb/musb/davinci.h  |1 +
  drivers/usb/musb/musb_core.c|   20 
  4 files changed, 23 insertions(+), 8 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/board-dm365-evm.c 
 b/arch/arm/mach-davinci/board-dm365-evm.c
 index 3a4743b..dfcb67f 100644
 --- a/arch/arm/mach-davinci/board-dm365-evm.c
 +++ b/arch/arm/mach-davinci/board-dm365-evm.c
 @@ -38,6 +38,8 @@
  #include mach/mmc.h
  #include mach/nand.h
  #include mach/keyscan.h
 +#include mach/usb.h
 +#include mach/hardware.h
  
  #include media/tvp514x.h
  
 @@ -92,6 +94,9 @@ static inline int have_tvp7002(void)
  #define CPLD_CCD_DIR3CPLD_OFFSET(0x3f,0)
  #define CPLD_CCD_IO3 CPLD_OFFSET(0x3f,1)
  
 +#define USBPHY_CTL_PADDR 0x01c40034
 +#define USBPHY_CLKFREQ_24MHZ BIT(13)
 +
  static void __iomem *cpld;
  
  
 @@ -613,6 +618,9 @@ static __init void dm365_evm_init(void)
  
   dm365_init_spi0(BIT(0), dm365_evm_spi_info,
   ARRAY_SIZE(dm365_evm_spi_info));
 + writel(readl(IO_ADDRESS(USBPHY_CTL_PADDR)) | USBPHY_CLKFREQ_24MHZ,
 + IO_ADDRESS(USBPHY_CTL_PADDR));
 + davinci_setup_usb(500, 8);

Add a comment here on why current is limited to 500 mA? Probably coming
because of an on-board component?

  }
  
  MACHINE_START(DAVINCI_DM365_EVM, DaVinci DM365 EVM)
 diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
 index 23d2b6d..664c689 100644
 --- a/arch/arm/mach-davinci/usb.c
 +++ b/arch/arm/mach-davinci/usb.c
 @@ -49,6 +49,8 @@ static struct musb_hdrc_platform_data usb_data = {
   .mode   = MUSB_PERIPHERAL,
  #elif defined(CONFIG_USB_MUSB_HOST)
   .mode   = MUSB_HOST,
 +#else
 + .mode   = MUSB_OTG,

This does not look to be directly related to DM365 support and should be
separated into a different patch.


  #endif
   .clock  = usb,
   .config = musb_config,
 diff --git a/drivers/usb/musb/davinci.h b/drivers/usb/musb/davinci.h
 index 371baa0..e737d97 100644
 --- a/drivers/usb/musb/davinci.h
 +++ b/drivers/usb/musb/davinci.h
 @@ -16,6 +16,7 @@
  
  /* Integrated highspeed/otg PHY */
  #define USBPHY_CTL_PADDR 0x01c40034
 +#define USBPHY_CLKFREQ_24MHZ BIT(13)

You have added this repeat definition and do not use it anywhere.

Looks like all PHY related configuration is currently happening in
drivers/usb/musb/davinci.c and the same register is also being written
to for other platforms. Can you move the code you have included in board
file to the driver? As we move towards DT, we need to avoid register
configurations from board files anyway.

Thanks,
Sekhar
--
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 v3 04/11] clk: davinci - add pll divider clock driver

2012-11-03 Thread Sekhar Nori
On 11/2/2012 7:23 PM, Murali Karicheri wrote:
 On 11/02/2012 07:33 AM, Sekhar Nori wrote:
 On 10/25/2012 9:41 PM, Murali Karicheri wrote:

 pll dividers are present in the pll controller of DaVinci and Other
 SoCs that re-uses the same hardware IP. This has a enable bit for
 bypass the divider or enable the driver. This is a sub class of the
 clk-divider clock checks the enable bit to calculare the rate and
 invoke the recalculate() function of the clk-divider if enabled.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---

 +/**
 + * clk_register_davinci_plldiv - register function for DaVinci PLL
 divider clk
 + *
 + * @dev: device ptr
 + * @name: name of the clock
 + * @parent_name: name of parent clock
 + * @plldiv_data: ptr to pll divider data
 + * @lock: ptr to spinlock passed to divider clock
 + */
 +struct clk *clk_register_davinci_plldiv(struct device *dev,
 Why do you need a dev pointer here and which device does it point to? In
 the only usage of this API in the series, you pass a NULL here. I should
 have probably asked this question on one of the earlier patches itself.

 I did a grep in the drivers/clk directory. All of the platform drivers
 are having the device ptr and all of them are called with NULL. I am not
 sure what is the intent of this arg in the API.  As per documentation of

I just took a look at the mxs example you referenced below and it does
not take a dev pointer.

struct clk *mxs_clk_div(const char *name, const char *parent_name,
void __iomem *reg, u8 shift, u8 width, u8 busy)
{

 the clk_register() API, the device ptr points to the device that is
 registering this clk. So if a specific device driver ever has to
 register a PLL div clk, this will be non NULL. In  the normal use case,
 clk is registered in a platform specific code and is always passed NULL.
 
 The platform/SoC specific clock initialization code will be using
 davinci_plldiv_clk() that doesn't have a device ptr arg.
 So this can be changed in future in sync with other drivers (assuming
 this will get removed if unused), and changes
 doesn't impact the platform code that initialize the clock. So IMO, we
 should keep this arg so that it is in sync with other driver APIs.

I think you should get rid of this unused arg and introduce it when you
actually need it. That way we are clear about why we need it.

 
 +const char *name, const char *parent_name,
 +struct clk_plldiv_data *plldiv_data,
 +spinlock_t *lock)
 +{
 +struct clk_div *div;
 +struct clk *clk;
 +struct clk_init_data init;
 +
 +div = kzalloc(sizeof(*div), GFP_KERNEL);
 +if (!div)
 +return ERR_PTR(-ENOMEM);
 +
 +init.name = name;
 +init.ops = clk_div_ops;
 +init.flags = plldiv_data-flags;
 +init.parent_names = (parent_name ? parent_name : NULL);
 +init.num_parents = (parent_name ? 1 : 0);
 +
 +div-reg = plldiv_data-reg;
 +div-en_id = plldiv_data-en_id;
 +
 +div-divider.reg = plldiv_data-reg;
 +div-divider.shift = plldiv_data-shift;
 +div-divider.width = plldiv_data-width;
 +div-divider.flags = plldiv_data-divider_flags;
 +div-divider.lock = lock;
 +div-divider.hw.init = init;
 +div-ops = clk_divider_ops;
 +
 +clk = clk_register(NULL, div-divider.hw);
 
 Shouldn't you be calling clk_register_divider() here which in turn will
 do clk_register()?
 As stated in the top of the file, this is a subclass driver of clk-div
 similar in line with mxs/clk-div.c. The
 driver registers the clock instead of calling clk_register_divider() so
 that it's ops function has a chance to do whatever it wants to do and
 call the divider ops function after that.

I see that now. I should have paid more attention.

Regards,
Sekhar
--
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 v3 02/11] clk: davinci - add PSC clock driver

2012-11-03 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This is the driver for the Power Sleep Controller (PSC) hardware
 found on DM SoCs as well Keystone SoCs (c6x). This driver borrowed
 code from arch/arm/mach-davinci/psc.c and implemented the driver
 as per common clock provider API. The PSC module is responsible for
 enabling/disabling the Power Domain and Clock domain for different IPs
 present in the SoC. The driver is configured through the clock data
 passed to the driver through struct clk_psc_data.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---

 +/**
 + * struct clk_psc - DaVinci PSC clock driver data
 + *
 + * @hw: clk_hw for the psc
 + * @psc_data: Driver specific data
 + */
 +struct clk_psc {
 + struct clk_hw hw;
 + struct clk_psc_data *psc_data;

 + spinlock_t *lock;

Unused member? I don't see this being used.

Thanks,
Sekhar
--
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 v3 03/11] clk: davinci - common clk utilities to init clk driver

2012-11-03 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This is the common clk driver initialization functions for DaVinci
 SoCs and other SoCs that uses similar hardware architecture.
 clock.h also defines struct types for clock definitions in a SoC
 and clock data type for configuring clk-mux. The initialization
 functions are used by clock initialization code in a specific
 platform/SoC.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
  drivers/clk/davinci/clock.c |  112 
 +++
  drivers/clk/davinci/clock.h |   80 +++
  2 files changed, 192 insertions(+)
  create mode 100644 drivers/clk/davinci/clock.c
  create mode 100644 drivers/clk/davinci/clock.h
 
 diff --git a/drivers/clk/davinci/clock.c b/drivers/clk/davinci/clock.c
 new file mode 100644
 index 000..ad02149
 --- /dev/null
 +++ b/drivers/clk/davinci/clock.c
 @@ -0,0 +1,112 @@
 +/*
 + * clock.c - davinci clock initialization functions for various clocks
 + *
 + * Copyright (C) 2006-2012 Texas Instruments.
 + * Copyright (C) 2008-2009 Deep Root Systems, LLC
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +#include linux/init.h
 +#include linux/clk-provider.h
 +#include linux/clkdev.h
 +#include linux/io.h
 +#include linux/slab.h
 +
 +#include clk-pll.h
 +#include clk-psc.h
 +#include clk-div.h
 +#include clock.h
 +
 +static DEFINE_SPINLOCK(_lock);
 +
 +#ifdef   CONFIG_CLK_DAVINCI_PLL
 +struct clk *davinci_pll_clk(const char *name, const char *parent,
 + u32 phys_pllm, u32 phys_prediv, u32 phys_postdiv,
 + struct clk_pll_data *pll_data)
 +{
 + struct clk *clkp = NULL;
 +
 + pll_data-reg_pllm = ioremap(phys_pllm, 4);
 + if (WARN_ON(!pll_data-reg_pllm))
 + return clkp;

I would prefer ERR_PTR(-ENOMEM) here. Same comment applies to other
instances elsewhere in the patch.

 diff --git a/drivers/clk/davinci/clock.h b/drivers/clk/davinci/clock.h
 new file mode 100644
 index 000..73204b8
 --- /dev/null
 +++ b/drivers/clk/davinci/clock.h
 @@ -0,0 +1,80 @@
 +/*
 + * TI DaVinci Clock definitions -  Contains Macros and Types used for
 + * defining various clocks on a DaVinci SoC
 + *
 + * 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 as
 + * published by the Free Software Foundation version 2.
 + *
 + * This program is distributed as is WITHOUT ANY WARRANTY of any
 + * kind, whether express or implied; without even the implied warranty
 + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +#ifndef __DAVINCI_CLOCK_H
 +#define __DAVINCI_CLOCK_H
 +
 +#include linux/types.h
 +
 +/* general flags: */
 +#define ALWAYS_ENABLED   BIT(0)

This is not used in this patch. Can you add the define along with its
usage so it is immediately clear why you need it?

 +/**
 + * struct davinci_clk - struct for defining DaVinci clocks for a SoC.
 + *
 + * @name: name of the clock
 + * @parent: name of parent clock
 + * @flags: General flags for all drivers used by platform clock init code
 + * @data: data specific to a clock used by the driver
 + * @dev_id: dev_id used to look up this clock. If this is NULL
 + *   clock name is used for lookup.
 + */
 +struct davinci_clk {
 + const char  *name;
 + const char  *parent;
 + u32 flags;
 + void*data;
 + char*dev_id;

Similarly dont see this being used as well.

Thanks,
Sekhar
--
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 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
--
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 v3 10/11] ARM: davinci - migrate to common clock

2012-11-04 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 Currently migrate only DM644x as this is being reviewed. Once all
 platforms are migrated, the Makefile will be cleaned up to remove
 obsoleted files clock.o and psc.o
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
  arch/arm/Kconfig   |1 +
  arch/arm/mach-davinci/Kconfig  |2 ++
  arch/arm/mach-davinci/Makefile |   11 ++-
  3 files changed, 13 insertions(+), 1 deletion(-)
 
 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index c5f9ae5..4611987 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -967,6 +967,7 @@ config ARCH_DAVINCI
   select ARCH_REQUIRE_GPIOLIB
   select ZONE_DMA
   select HAVE_IDE
 + select COMMON_CLK
   select CLKDEV_LOOKUP
   select GENERIC_ALLOCATOR
   select GENERIC_IRQ_CHIP

This list has to be sorted. Russell has a patch which fixed this all
across arch/arm/* which got merged for v3.7. This patch does not apply
to v3.7-rc3 anymore due to the same reason.

Thanks,
Sekhar
--
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 v3 06/11] clk: davinci - add build infrastructure for DaVinci clock drivers

2012-11-04 Thread Sekhar Nori
On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This updates clk Makefile and Kconfig to integrate the DaVinci specific
 clock drivers. Also add new Kconfig and Makefile for these drivers.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com

As mentioned before, this should be folded into previous patches which
actually add the particular functionality.

 ---
  drivers/clk/Kconfig  |2 ++
  drivers/clk/Makefile |1 +
  drivers/clk/davinci/Kconfig  |   44 
 ++
  drivers/clk/davinci/Makefile |5 +
  4 files changed, 52 insertions(+)
  create mode 100644 drivers/clk/davinci/Kconfig
  create mode 100644 drivers/clk/davinci/Makefile
 
 diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
 index 7f0b5ca..1ad2ab0 100644
 --- a/drivers/clk/Kconfig
 +++ b/drivers/clk/Kconfig
 @@ -33,6 +33,8 @@ config COMMON_CLK_DEBUG
 clk_flags, clk_prepare_count, clk_enable_count 
 clk_notifier_count.
  
 +source drivers/clk/davinci/Kconfig
 +
  config COMMON_CLK_WM831X
   tristate Clock driver for WM831x/2x PMICs
   depends on MFD_WM831X
 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index 5869ea3..b127b6f 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -10,6 +10,7 @@ obj-$(CONFIG_ARCH_SOCFPGA)  += socfpga/
  obj-$(CONFIG_PLAT_SPEAR) += spear/
  obj-$(CONFIG_ARCH_U300)  += clk-u300.o
  obj-$(CONFIG_ARCH_INTEGRATOR)+= versatile/
 +obj-$(CONFIG_DAVINCI_CLKS)   += davinci/
  
  # Chip specific
  obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
 diff --git a/drivers/clk/davinci/Kconfig b/drivers/clk/davinci/Kconfig
 new file mode 100644
 index 000..e53bbc3
 --- /dev/null
 +++ b/drivers/clk/davinci/Kconfig
 @@ -0,0 +1,44 @@
 +menu TI DaVinci Clock drivers
 + depends on COMMON_CLK
 +
 +config  CLK_DAVINCI_PSC
 + bool TI DaVici PSC clock driver
 + default n
 + ---help---
 +   Selects clock driver for DaVinci PSC clocks. This clock
 +   hardware is found on TI DaVinci SoCs and other SoCs that
 +   uses this hardware IP. This hardware has a local power
 +   sleep control module that gate the clock to the IP.
 +
 +config  CLK_DAVINCI_PLL
 + bool DaVici main PLL clock
 + ---help---
 +   Selects clock driver for DaVinci main PLL. This clock
 +   hardware is found on TI DaVinci SoCs. This typically has
 +   a multiplier, a pre divider and post driver. Some of the
 +   SoCs has the the dividers fixed, and others have it
 +   programmable
 +
 +config  CLK_DAVINCI_PLLDIV
 + bool DaVici PLL divider clock
 + ---help---
 +   Selects clock driver for DaVinci PLL divider. This clock
 +   hardware is found on TI DaVinci SoCs. This typically has
 +   a divider and an enable bit to bypass or enable the
 +   divider.
 +
 +config DAVINCI_CLKS
 + bool TI DaVinci common clocks
 + default n
 + select CLK_DAVINCI_PSC
 + select CLK_DAVINCI_PLLDIV
 + ---help---
 +   Selects common clock drivers for DaVinci. These clocks
 +   are re-used across many TI SoCs that are based on DaVinci and
 +   Keystone (c6x) families. This config option is used to select
 +   the common clock driver for DaVinci based SoCs. SoCs specific
 +   Kconfig option needs to select the driver for clocks specific
 +   to the SoC.
 +
 +endmenu

I wonder if all these configurations are really required. For complete
clock functionality you anyway require all this functionality so I
wonder why anyone would not select any of these. And to differentiate
between architecture changes, conditional build based on architecture
can be used:

obj-$(CONFIG_ARCH_DAVINCI)  += clk-pll.o
obj-$(CONFIG_ARCH_KEYSTONE) += clk-pll-keystone.o

Thanks,
Sekhar
--
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 v3 07/11] ARM: davinci - restructure header files for common clock migration

2012-11-04 Thread Sekhar Nori


On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 pll.h is added to migrate some of the PLL controller defines for sleep.S.
 psc.h is modified to keep only PSC modules definitions needed by sleep.S
 after migrating to common clock. The definitions under
 ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch.
 davinci_watchdog_reset prototype is moved to time.h as clock.h is
 being obsoleted. sleep.S and pm.c is modified to include the new header
 file replacements.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
  arch/arm/mach-davinci/devices.c   |2 ++
  arch/arm/mach-davinci/include/mach/pll.h  |   46 
 +
  arch/arm/mach-davinci/include/mach/psc.h  |4 +++
  arch/arm/mach-davinci/include/mach/time.h |4 ++-
  arch/arm/mach-davinci/pm.c|4 +++
  arch/arm/mach-davinci/sleep.S |4 +++
  6 files changed, 63 insertions(+), 1 deletion(-)
  create mode 100644 arch/arm/mach-davinci/include/mach/pll.h

With this patch a _third_ copy of PLL definitions is created in kernel
sources. The existing PLL definitions in clock.h inside mach-davinci
should be moved to mach/pll.h and the pll.h you introduced inside
drivers/clk in 5/11 should be removed (this patch should appear before
5/11).

The biggest disadvantage of this approach is inclusion of mach/ includes
in drivers/clk. But duplicating code is definitely not the fix for this.
Anyway, mach/ includes are not uncommon in drivers/clk (they are all
probably suffering from the same issue).

$ grep -rl include mach/ drivers/clk/*
drivers/clk/clk-u300.c
drivers/clk/mmp/clk-pxa168.c
drivers/clk/mmp/clk-mmp2.c
drivers/clk/mmp/clk-pxa910.c
drivers/clk/mxs/clk-imx23.c
drivers/clk/mxs/clk-imx28.c
drivers/clk/spear/spear6xx_clock.c
drivers/clk/spear/spear3xx_clock.c
drivers/clk/spear/spear1340_clock.c
drivers/clk/spear/spear1310_clock.c
drivers/clk/ux500/clk-prcc.c
drivers/clk/versatile/clk-integrator.c
drivers/clk/versatile/clk-realview.c

pll.h can probably be moved to include/linux/clk/ to avoid this. Would
like to hear from Mike on this before going ahead.

Anyway, instead of just commenting, I though I will be more useful and
went ahead and made some of the changes I have been talking about. I
fixed the multiple PLL definitions issue, the build infrastructure issue
and the commit ordering too.

I pushed the patches I fixed to devel-common-clk branch of my git tree.
It is build tested using davinci_all_defconfig but its not runtime tested.

Can you start from here and provide me incremental changes on top of
this? That way we can collaborate to finish this faster.

Thanks,
Sekhar
--
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: [RFC PATCH 1/2] memory: davinci - add aemif controller platform driver

2012-11-05 Thread Sekhar Nori
On 11/6/2012 2:38 AM, Murali Karicheri wrote:
 On 11/04/2012 08:52 AM, Rob Herring wrote:
 OMAP GPMC
 Could you send me a link please?

https://www.google.com/search?q=RFC+OMAP+GPMC+DT+bindings

The patches series is sent by Daniel Mack. v3 was the last version sent.

Thanks,
Sekhar
--
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 v3 03/11] clk: davinci - common clk utilities to init clk driver

2012-11-06 Thread Sekhar Nori


On 11/5/2012 8:50 PM, Murali Karicheri wrote:
 On 11/03/2012 08:35 AM, Sekhar Nori wrote:
 On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This is the common clk driver initialization functions for DaVinci
 SoCs and other SoCs that uses similar hardware architecture.
 clock.h also defines struct types for clock definitions in a SoC
 and clock data type for configuring clk-mux. The initialization
 functions are used by clock initialization code in a specific
 platform/SoC.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
   drivers/clk/davinci/clock.c |  112
 +++
   drivers/clk/davinci/clock.h |   80 +++
   2 files changed, 192 insertions(+)
   create mode 100644 drivers/clk/davinci/clock.c
   create mode 100644 drivers/clk/davinci/clock.h

 diff --git a/drivers/clk/davinci/clock.c b/drivers/clk/davinci/clock.c
 new file mode 100644
 index 000..ad02149
 --- /dev/null
 +++ b/drivers/clk/davinci/clock.c
 @@ -0,0 +1,112 @@
 +/*
 + * clock.c - davinci clock initialization functions for various clocks
 + *
 + * Copyright (C) 2006-2012 Texas Instruments.
 + * Copyright (C) 2008-2009 Deep Root Systems, LLC
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +#include linux/init.h
 +#include linux/clk-provider.h
 +#include linux/clkdev.h
 +#include linux/io.h
 +#include linux/slab.h
 +
 +#include clk-pll.h
 +#include clk-psc.h
 +#include clk-div.h
 +#include clock.h
 +
 +static DEFINE_SPINLOCK(_lock);
 +
 +#ifdefCONFIG_CLK_DAVINCI_PLL
 +struct clk *davinci_pll_clk(const char *name, const char *parent,
 +u32 phys_pllm, u32 phys_prediv, u32 phys_postdiv,
 +struct clk_pll_data *pll_data)
 +{
 +struct clk *clkp = NULL;
 +
 +pll_data-reg_pllm = ioremap(phys_pllm, 4);
 +if (WARN_ON(!pll_data-reg_pllm))
 +return clkp;
 I would prefer ERR_PTR(-ENOMEM) here. Same comment applies to other
 instances elsewhere in the patch.

 diff --git a/drivers/clk/davinci/clock.h b/drivers/clk/davinci/clock.h
 new file mode 100644
 index 000..73204b8
 --- /dev/null
 +++ b/drivers/clk/davinci/clock.h
 @@ -0,0 +1,80 @@
 +/*
 + * TI DaVinci Clock definitions -  Contains Macros and Types used for
 + * defining various clocks on a DaVinci SoC
 + *
 + * 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 as
 + * published by the Free Software Foundation version 2.
 + *
 + * This program is distributed as is WITHOUT ANY WARRANTY of any
 + * kind, whether express or implied; without even the implied warranty
 + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +#ifndef __DAVINCI_CLOCK_H
 +#define __DAVINCI_CLOCK_H
 +
 +#include linux/types.h
 +
 +/* general flags: */
 +#define ALWAYS_ENABLEDBIT(0)
 This is not used in this patch. Can you add the define along with its
 usage so it is immediately clear why you need it?
 This is used on the next patch as this adds a bunch of utilities or
 types for the platform specific clock code. Do you want to combine this
 patch with 05/11?  It will become a bigger patch then? Is that fine.
 IMO, this is a logical group that can be a standalone patch than
 combining with 05/11.

It is more important to divide patches in logical functional blocks
rather than split it based on files. Reading this patch, I have no idea
what that define is for.

Thanks,
Sekhar
--
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 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
--
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 v3 06/11] clk: davinci - add build infrastructure for DaVinci clock drivers

2012-11-06 Thread Sekhar Nori


On 11/5/2012 9:47 PM, Murali Karicheri wrote:
 On 11/04/2012 08:34 AM, Sekhar Nori wrote:
 On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 This updates clk Makefile and Kconfig to integrate the DaVinci specific
 clock drivers. Also add new Kconfig and Makefile for these drivers.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 As mentioned before, this should be folded into previous patches which
 actually add the particular functionality.
 Yes. Agreed.,
 ---
   drivers/clk/Kconfig  |2 ++
   drivers/clk/Makefile |1 +
   drivers/clk/davinci/Kconfig  |   44
 ++
   drivers/clk/davinci/Makefile |5 +
   4 files changed, 52 insertions(+)
   create mode 100644 drivers/clk/davinci/Kconfig
   create mode 100644 drivers/clk/davinci/Makefile

 diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
 index 7f0b5ca..1ad2ab0 100644
 --- a/drivers/clk/Kconfig
 +++ b/drivers/clk/Kconfig
 @@ -33,6 +33,8 @@ config COMMON_CLK_DEBUG
 clk_flags, clk_prepare_count, clk_enable_count 
 clk_notifier_count.
   +source drivers/clk/davinci/Kconfig
 +
   config COMMON_CLK_WM831X
   tristate Clock driver for WM831x/2x PMICs
   depends on MFD_WM831X
 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index 5869ea3..b127b6f 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -10,6 +10,7 @@ obj-$(CONFIG_ARCH_SOCFPGA)+= socfpga/
   obj-$(CONFIG_PLAT_SPEAR)+= spear/
   obj-$(CONFIG_ARCH_U300)+= clk-u300.o
   obj-$(CONFIG_ARCH_INTEGRATOR)+= versatile/
 +obj-$(CONFIG_DAVINCI_CLKS)+= davinci/
 # Chip specific
   obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
 diff --git a/drivers/clk/davinci/Kconfig b/drivers/clk/davinci/Kconfig
 new file mode 100644
 index 000..e53bbc3
 --- /dev/null
 +++ b/drivers/clk/davinci/Kconfig
 @@ -0,0 +1,44 @@
 +menu TI DaVinci Clock drivers
 +depends on COMMON_CLK
 +
 +config  CLK_DAVINCI_PSC
 +bool TI DaVici PSC clock driver
 +default n
 +---help---
 +  Selects clock driver for DaVinci PSC clocks. This clock
 +  hardware is found on TI DaVinci SoCs and other SoCs that
 +  uses this hardware IP. This hardware has a local power
 +  sleep control module that gate the clock to the IP.
 +
 +config  CLK_DAVINCI_PLL
 +bool DaVici main PLL clock
 +---help---
 +  Selects clock driver for DaVinci main PLL. This clock
 +  hardware is found on TI DaVinci SoCs. This typically has
 +  a multiplier, a pre divider and post driver. Some of the
 +  SoCs has the the dividers fixed, and others have it
 +  programmable
 +
 +config  CLK_DAVINCI_PLLDIV
 +bool DaVici PLL divider clock
 +---help---
 +  Selects clock driver for DaVinci PLL divider. This clock
 +  hardware is found on TI DaVinci SoCs. This typically has
 +  a divider and an enable bit to bypass or enable the
 +  divider.
 +
 +config DAVINCI_CLKS
 +bool TI DaVinci common clocks
 +default n
 +select CLK_DAVINCI_PSC
 +select CLK_DAVINCI_PLLDIV
 +---help---
 +  Selects common clock drivers for DaVinci. These clocks
 +  are re-used across many TI SoCs that are based on DaVinci and
 +  Keystone (c6x) families. This config option is used to select
 +  the common clock driver for DaVinci based SoCs. SoCs specific
 +  Kconfig option needs to select the driver for clocks specific
 +  to the SoC.
 +
 +endmenu
 I wonder if all these configurations are really required. For complete
 clock functionality you anyway require all this functionality so I
 wonder why anyone would not select any of these. And to differentiate
 between architecture changes, conditional build based on architecture
 can be used:
 I know this is a grey area I need to improve and had a discussion with
 Mike sometime back. Are you also suggesting including davinci folder
 unconditionally under drivers/clk/Makefile? I think it will work. And
 then in clk/davinci/Makefile we can include on a per architecture basis.
 But I am not sure if calling the folder as davinci make sense and these
 drivers are used on non DaVinci architectures as well. I had initially
 named it as ti to include all ti drivers. But Mike had commented that
 current directory structure use platform/arch name instead of vendor
 name. Also OMAP is going to move the clk drivers to this folder soon. So
 for now, davinci may be used to house all of the non OMAP drivers. Later
 we can re-visit this as needed (wouldn't be that hard to do this within
 the clk driver folder later). We can keep the davinci folder name and
 unconditionally include it in clk/Makefile.
 
 clk/Makefile
 
 add
 obj-y  += davinci/

I am not sure about this unconditional inclusion. Are you sure the
following will create problems when DaVinci and keystone are enabled
together?

obj-$(CONFIG_ARCH_DAVINCI)  += davinci/
obj-$(CONFIG_ARCH_KEYSTONE) += davinci/

 
 clk/davinci/Makefile
 
 obj

Re: [PATCH v3 07/11] ARM: davinci - restructure header files for common clock migration

2012-11-06 Thread Sekhar Nori
On 11/6/2012 12:41 AM, Murali Karicheri wrote:
 On 11/04/2012 09:05 AM, Sekhar Nori wrote:

 On 10/25/2012 9:41 PM, Murali Karicheri wrote:
 pll.h is added to migrate some of the PLL controller defines for
 sleep.S.
 psc.h is modified to keep only PSC modules definitions needed by sleep.S
 after migrating to common clock. The definitions under
 ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch.
 davinci_watchdog_reset prototype is moved to time.h as clock.h is
 being obsoleted. sleep.S and pm.c is modified to include the new header
 file replacements.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 ---
   arch/arm/mach-davinci/devices.c   |2 ++
   arch/arm/mach-davinci/include/mach/pll.h  |   46
 +
   arch/arm/mach-davinci/include/mach/psc.h  |4 +++
   arch/arm/mach-davinci/include/mach/time.h |4 ++-
   arch/arm/mach-davinci/pm.c|4 +++
   arch/arm/mach-davinci/sleep.S |4 +++
   6 files changed, 63 insertions(+), 1 deletion(-)
   create mode 100644 arch/arm/mach-davinci/include/mach/pll.h
 With this patch a _third_ copy of PLL definitions is created in kernel
 sources. The existing PLL definitions in clock.h inside mach-davinci
 should be moved to mach/pll.h and the pll.h you introduced inside
 drivers/clk in 5/11 should be removed (this patch should appear before
 5/11).

 The biggest disadvantage of this approach is inclusion of mach/ includes
 in drivers/clk. But duplicating code is definitely not the fix for this.
 Anyway, mach/ includes are not uncommon in drivers/clk (they are all
 probably suffering from the same issue).
 Sekhar,
 
 I have replied to patch 5/11 that also refers to this. The main reason
 we are not able to do this cleanly is the code in sleep.c and pm.c. That
 is something related to Power management. Could you take a look and see
 if you can do some clean up on this code? I believe It is more than just
 moving the header files.

I am not sure what more is required than moving header files? Can you
elaborate?

 
 Murali

 $ grep -rl include mach/ drivers/clk/*
 drivers/clk/clk-u300.c
 drivers/clk/mmp/clk-pxa168.c
 drivers/clk/mmp/clk-mmp2.c
 drivers/clk/mmp/clk-pxa910.c
 drivers/clk/mxs/clk-imx23.c
 drivers/clk/mxs/clk-imx28.c
 drivers/clk/spear/spear6xx_clock.c
 drivers/clk/spear/spear3xx_clock.c
 drivers/clk/spear/spear1340_clock.c
 drivers/clk/spear/spear1310_clock.c
 drivers/clk/ux500/clk-prcc.c
 drivers/clk/versatile/clk-integrator.c
 drivers/clk/versatile/clk-realview.c

 pll.h can probably be moved to include/linux/clk/ to avoid this. Would
 like to hear from Mike on this before going ahead.

 Anyway, instead of just commenting, I though I will be more useful and
 went ahead and made some of the changes I have been talking about. I
 fixed the multiple PLL definitions issue, the build infrastructure issue
 and the commit ordering too.

 I pushed the patches I fixed to devel-common-clk branch of my git tree.
 It is build tested using davinci_all_defconfig but its not runtime
 tested.

 Can you start from here and provide me incremental changes on top of
 this? That way we can collaborate to finish this faster.
 Thanks for offering some help. Yes I can provide you incremental patch.
 But then could you also help me to squash/rebase and send patches to the
 list for review?

No, no. I want you to own the submissions. Towards this, if it is easier
for you to build upon my work in your own tree, by all means, go ahead
and do it. Just share the branch/tree details so I can take a look and
contribute as well.

Thanks,
Sekhar
--
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 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
--
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] usb: remove CONFIG_USB_MUSB_HOST etc

2012-10-23 Thread Sekhar Nori
On 10/8/2012 6:47 PM, Constantine Shulyupin wrote:
 From: Constantine Shulyupin co...@makelinux.com
 
 Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly 
 CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST 
 and set MUSB_OTG configuration by default
 because this configuration options are removed from Kconfig.
 
 Signed-off-by: Constantine Shulyupin co...@makelinux.com

Queuing this patch for v3.8. Since the config options are removed there
is no use having code which refers to them. The patch has been tested on
DM644x and DM365 in both host and gadget mode (I will add this
information to commit text while committing). Without this patch .mode
seems to be defaulting to MUSB_UNDEFINED which I think is definitely wrong.

Thanks,
Sekhar

  
 ---
  arch/arm/mach-davinci/usb.c |6 --
  1 file changed, 6 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
 index f77b953..34509ff 100644
 --- a/arch/arm/mach-davinci/usb.c
 +++ b/arch/arm/mach-davinci/usb.c
 @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
  };
  
  static struct musb_hdrc_platform_data usb_data = {
 -#if defined(CONFIG_USB_MUSB_OTG)
   /* OTG requires a Mini-AB connector */
   .mode   = MUSB_OTG,
 -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
 - .mode   = MUSB_PERIPHERAL,
 -#elif defined(CONFIG_USB_MUSB_HOST)
 - .mode   = MUSB_HOST,
 -#endif
   .clock  = usb,
   .config = musb_config,
  };
 
--
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] usb: remove CONFIG_USB_MUSB_HOST etc

2012-10-24 Thread Sekhar Nori
On 10/23/2012 6:09 PM, Felipe Balbi wrote:
 Hi,
 
 On Tue, Oct 23, 2012 at 06:04:53PM +0530, Sekhar Nori wrote:
 On 10/8/2012 6:47 PM, Constantine Shulyupin wrote:
 From: Constantine Shulyupin co...@makelinux.com

 Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly 
 CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST 
 and set MUSB_OTG configuration by default
 because this configuration options are removed from Kconfig.

 Signed-off-by: Constantine Shulyupin co...@makelinux.com

 Queuing this patch for v3.8. Since the config options are removed there
 is no use having code which refers to them. The patch has been tested on
 DM644x and DM365 in both host and gadget mode (I will add this
 information to commit text while committing). Without this patch .mode
 seems to be defaulting to MUSB_UNDEFINED which I think is definitely wrong.
 
 sorry for the delay, this looks ok:
 
 Acked-by: Felipe Balbi ba...@ti.com

Thanks Felipe, I added your ack.

Constantine,

Patches touching arch/arm/* should be prefixed with 'ARM:' and those
touching mach-davinci should be prefixed with 'davinci:'. I added these
two while committing the patch this time. Please take care next time on.

Thanks,
Sekhar
--
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 v2] ARM: dm365: replace V4L2_OUT_CAP_CUSTOM_TIMINGS with V4L2_OUT_CAP_DV_TIMINGS

2012-10-24 Thread Sekhar Nori
On 10/23/2012 6:47 PM, Prabhakar Lad wrote:
 From: Lad, Prabhakar prabhakar@ti.com
 
 This patch replaces V4L2_OUT_CAP_CUSTOM_TIMINGS macro with
 V4L2_OUT_CAP_DV_TIMINGS. As V4L2_OUT_CAP_CUSTOM_TIMINGS is being phased
 out.
 
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Cc: Sekhar Nori nsek...@ti.com
 Cc: Sergei Shtylyov sshtyl...@mvista.com

Patches for mach-davinci should have a 'davinci:' prefix after 'ARM:'.
Can you please resend with that fixed?

Thanks,
Sekhar
--
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 v4 1/2] ARM: davinci: dm365: add support for v4l2 video display

2012-10-24 Thread Sekhar Nori
Hi Prabhakar,

Sorry about the late feedback on this patch.

On 8/8/2012 6:20 PM, Prabhakar Lad wrote:
 From: Manjunath Hadli manjunath.ha...@ti.com
 
 Create platform devices for various video modules like venc,osd,
 vpbe and v4l2 driver for dm365.
 
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 ---
  arch/arm/mach-davinci/board-dm365-evm.c |4 +-
  arch/arm/mach-davinci/davinci.h |2 +-
  arch/arm/mach-davinci/dm365.c   |  203 
 +--
  3 files changed, 196 insertions(+), 13 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/board-dm365-evm.c 
 b/arch/arm/mach-davinci/board-dm365-evm.c
 index 688a9c5..ac1f20d 100644
 --- a/arch/arm/mach-davinci/board-dm365-evm.c
 +++ b/arch/arm/mach-davinci/board-dm365-evm.c
 @@ -564,8 +564,6 @@ static struct davinci_uart_config uart_config __initdata 
 = {
  
  static void __init dm365_evm_map_io(void)
  {
 - /* setup input configuration for VPFE input devices */
 - dm365_set_vpfe_config(vpfe_cfg);
   dm365_init();
  }
  
 @@ -597,6 +595,8 @@ static __init void dm365_evm_init(void)
  
   davinci_setup_mmc(0, dm365evm_mmc_config);
  
 + dm365_init_video(vpfe_cfg, NULL);
 +
   /* maybe setup mmc1/etc ... _after_ mmc0 */
   evm_init_cpld();
  
 diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
 index 8db0fc6..94d867f 100644
 --- a/arch/arm/mach-davinci/davinci.h
 +++ b/arch/arm/mach-davinci/davinci.h
 @@ -84,7 +84,7 @@ void __init dm365_init_ks(struct davinci_ks_platform_data 
 *pdata);
  void __init dm365_init_rtc(void);
  void dm365_init_spi0(unsigned chipselect_mask,
   const struct spi_board_info *info, unsigned len);
 -void dm365_set_vpfe_config(struct vpfe_config *cfg);
 +int __init dm365_init_video(struct vpfe_config *, struct vpbe_config *);
  
  /* DM644x function declarations */
  void __init dm644x_init(void);
 diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
 index a50d49d..7c0cc66 100644
 --- a/arch/arm/mach-davinci/dm365.c
 +++ b/arch/arm/mach-davinci/dm365.c
 @@ -1232,6 +1232,199 @@ static struct platform_device dm365_isif_dev = {
   },
  };
  
 +#define DM365_OSD_REG_BASE   0x01C71C00

In this file all base addresses are defined at the beginning of the
file. Please move it there. Keep the list sorted by address.

 +
 +static struct resource dm365_osd_resources[] = {
 + {
 + .start  = DM365_OSD_REG_BASE,
 + .end= DM365_OSD_REG_BASE + 0x100,
 + .flags  = IORESOURCE_MEM,
 + },
 +};
 +
 +static u64 dm365_video_dma_mask = DMA_BIT_MASK(32);
 +
 +static struct osd_platform_data dm365_osd_data = {
 + .vpbe_type  = VPBE_VERSION_2,
 +};

The better way to handle multiple versions of IP is to define different
platform names for each variant and then pass the right name from
platform code. Have a look at 'fec_devtype' in
drivers/net/ethernet/freescale/fec.c to see how this is done to handle
different variants of Ethernet IP in freescale Ethernet driver.

I understand this is how the driver has defined it, but I suggest fixing
this now. Taking the above approach will also make the DT transition easier.

 +
 +static struct platform_device dm365_osd_dev = {
 + .name   = VPBE_OSD_SUBDEV_NAME,
 + .id = -1,
 + .num_resources  = ARRAY_SIZE(dm365_osd_resources),
 + .resource   = dm365_osd_resources,
 + .dev= {
 + .dma_mask   = dm365_video_dma_mask,
 + .coherent_dma_mask  = DMA_BIT_MASK(32),
 + .platform_data  = dm365_osd_data,
 + },
 +};
 +
 +#define DM365_VENC_REG_BASE  0x01C71E00
 +#define DM3XX_VDAC_CONFIG0x01C4002C

Similar to above comment, please move this to top of the file.

 +
 +static struct resource dm365_venc_resources[] = {
 + {
 + .start  = IRQ_VENCINT,
 + .end= IRQ_VENCINT,
 + .flags  = IORESOURCE_IRQ,
 + },
 + /* venc registers io space */
 + {
 + .start  = DM365_VENC_REG_BASE,
 + .end= DM365_VENC_REG_BASE + 0x180,
 + .flags  = IORESOURCE_MEM,
 + },
 + /* vdaccfg registers io space */
 + {
 + .start  = DM3XX_VDAC_CONFIG,
 + .end= DM3XX_VDAC_CONFIG + 4,
 + .flags  = IORESOURCE_MEM,
 + },
 +};
 +
 +static struct resource dm365_v4l2_disp_resources[] = {
 + {
 + .start  = IRQ_VENCINT,
 + .end= IRQ_VENCINT,
 + .flags  = IORESOURCE_IRQ,
 + },
 + /* venc registers io space */
 + {
 + .start  = DM365_VENC_REG_BASE,
 + .end= DM365_VENC_REG_BASE + 0x180,
 + .flags  = IORESOURCE_MEM,
 + },
 +};
 +
 +static int dm365_vpbe_setup_pinmux(enum v4l2_mbus_pixelcode if_type,
 + int field)
 +{
 + int ret = 0;
 +
 +  

Re: [PATCH 1/1] spi:clk: preparation for switch to common clock framework

2012-10-09 Thread Sekhar Nori
On 9/17/2012 10:12 PM, Murali Karicheri wrote:
 As a first step towards migrating davinci platforms to use common clock
 framework, replace all instances of clk_enable() with clk_prepare_enable()
 and clk_disable() with clk_disable_unprepare(). Until the platform is
 switched to use the CONFIG_HAVE_CLK_PREPARE Kconfig variable, this just
 adds a might_sleep() call and would work without any issues.
 
 This will make it easy later to switch to common clk based implementation
 of clk driver from DaVinci specific driver.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 Reviewed-by: Mike Turquette mturque...@linaro.org

This patch had to be applied manually and with some fuzz when I applied
it to latest linus/master. There were no conflicts though and once I
applied it, I was able to test SPI flash on DA850 EVM successfully using it.

Acked-by: Sekhar Nori nsek...@ti.com

I am hoping this patch can still go in v3.7. I realize we are in the
middle of the merge window though.

Thanks,
Sekhar
--
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 1/1] mtd:nand:clk: preparation for switch to common clock framework

2012-10-09 Thread Sekhar Nori
On 9/17/2012 10:07 PM, Murali Karicheri wrote:
 As a first step towards migrating davinci platforms to use common clock
 framework, replace all instances of clk_enable() with clk_prepare_enable()
 and clk_disable() with clk_disable_unprepare(). Until the platform is
 switched to use the CONFIG_HAVE_CLK_PREPARE Kconfig variable, this just
 adds a might_sleep() call and would work without any issues.
 
 This will make it easy later to switch to common clk based implementation
 of clk driver from DaVinci specific driver.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 Reviewed-by: Mike Turquette mturque...@linaro.org

Tested this using NAND flash on DA850 EVM.

Acked-by: Sekhar Nori nsek...@ti.com

Any chance this can make into the v3.7 kernel?

Thanks,
Sekhar
--
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 1/1] mtd:nand:clk: preparation for switch to common clock framework

2012-10-09 Thread Sekhar Nori
On 10/9/2012 6:33 PM, Sekhar Nori wrote:
 On 9/17/2012 10:07 PM, Murali Karicheri wrote:
 As a first step towards migrating davinci platforms to use common clock
 framework, replace all instances of clk_enable() with clk_prepare_enable()
 and clk_disable() with clk_disable_unprepare(). Until the platform is
 switched to use the CONFIG_HAVE_CLK_PREPARE Kconfig variable, this just
 adds a might_sleep() call and would work without any issues.

 This will make it easy later to switch to common clk based implementation
 of clk driver from DaVinci specific driver.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 Reviewed-by: Mike Turquette mturque...@linaro.org
 
 Tested this using NAND flash on DA850 EVM.
 
 Acked-by: Sekhar Nori nsek...@ti.com
 
 Any chance this can make into the v3.7 kernel?

Just noticed another thread where Artem replied saying this patch is
pushed. Sorry about the noise.

Thanks,
Sekhar
--
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 01/13] clk: davinci - add Main PLL clock driver

2012-10-10 Thread Sekhar Nori
Hi Murali,

On 9/26/2012 11:37 PM, Murali Karicheri wrote:
 This is the driver for the main PLL clock hardware found on DM SoCs.
 This driver borrowed code from arch/arm/mach-davinci/clock.c and
 implemented the driver as per common clock provider API. The main PLL
 hardware typically has a multiplier, a pre-divider and a post-divider.
 Some of the SoCs has the divider fixed meaning they can not be
 configured through a register. HAS_PREDIV and HAS_POSTDIV flags are used
 to tell the driver if a hardware has these dividers present or not.
 Driver is configured through the structure clk_davinci_pll_data
 that has the platform data for the driver.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com

Are you using git-format-patch to generate the patches? It should have
added a diffstat here by default which is very useful in quickly
understanding what the patch is touching.
 
 diff --git a/drivers/clk/davinci/clk-davinci-pll.c 
 b/drivers/clk/davinci/clk-davinci-pll.c
 new file mode 100644
 index 000..13e1690
 --- /dev/null
 +++ b/drivers/clk/davinci/clk-davinci-pll.c
 @@ -0,0 +1,128 @@
 +/*
 + * PLL clk driver DaVinci devices
 + *
 + * Copyright (C) 2006-2012 Texas Instruments.
 + * Copyright (C) 2008-2009 Deep Root Systems, LLC
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + * TODO - Add set_parent_rate()
 + */
 +#include linux/clk.h
 +#include linux/clk-provider.h
 +#include linux/delay.h
 +#include linux/err.h
 +#include linux/io.h
 +#include linux/slab.h
 +#include linux/platform_data/clk-davinci-pll.h

It will be nice to keep these sorted in alphabetical order. You got it
almost right, except the last one. Keeping it sorted keeps out duplicates.

 +
 +#include mach/cputype.h

Hmm, why is this needed? mach/ includes in driver files make it tough to
use this on other architectures/machines. You have plan to use this on
driver on keystone and c6x as well, right? It will also break
multi-platform ARM build.

 +
 +#define PLLM 0x110
 +#define PLLM_PLLM_MASK  0xff
 +#define PREDIV  0x114
 +#define POSTDIV 0x128
 +#define PLLDIV_EN   BIT(15)

Can you use tabs for indentation?

 +
 +/**
 + * struct clk_davinci_pll - DaVinci Main pll clock

no capitalization on 'M' needed, I think.

 + * @hw: clk_hw for the pll
 + * @pll_data: PLL driver specific data
 + */
 +struct clk_davinci_pll {
 + struct clk_hw hw;
 + struct clk_davinci_pll_data *pll_data;
 +};
 +
 +#define to_clk_pll(_hw) container_of(_hw, struct clk_davinci_pll, hw)
 +
 +static unsigned long clk_pllclk_recalc(struct clk_hw *hw,
 + unsigned long parent_rate)
 +{
 + struct clk_davinci_pll *pll = to_clk_pll(hw);
 + struct clk_davinci_pll_data *pll_data = pll-pll_data;
 + u32 mult = 1, prediv = 1, postdiv = 1;

No need to initialize mult here since you are doing it right away below.

 + unsigned long rate = parent_rate;
 +
 + /* If there is a device specific recalc defined invoke it. Otherwise
 +  * fallback to default one
 +  */

This is not following the multi-line comment style defined in
Documentation/CodingStyle.

 + mult = __raw_readl(pll_data-pllm);

Do not use __raw_ variants since they are not safe on ARMv7. Use
readl/writel instead.

 + if (pll_data-pllm_multiplier)
 + mult =  pll_data-pllm_multiplier *
 + (mult  pll_data-pllm_mask);
 + else
 + mult = (mult  pll_data-pllm_mask) + 1;
 +
 + if (pll_data-flags  CLK_DAVINCI_PLL_HAS_PREDIV) {
 + /* pre-divider is fixed, take prediv value from pll_data  */
 + if (pll_data-fixed_prediv)
 + prediv = pll_data-fixed_prediv;

Since else is multi-line, if needs braces as well.

 + else {
 + prediv = __raw_readl(pll_data-prediv);
 + if (prediv  PLLDIV_EN)
 + prediv = (prediv  pll_data-prediv_mask) + 1;
 + else
 + prediv = 1;
 + }
 + }
 +
 + if (pll_data-flags  CLK_DAVINCI_PLL_HAS_POSTDIV) {
 + postdiv = __raw_readl(pll_data-postdiv);
 + if (postdiv  PLLDIV_EN)
 + postdiv = (postdiv  pll_data-postdiv_mask) + 1;
 + else
 + postdiv = 1;
 + }
 +
 + rate /= prediv;
 + rate *= mult;
 + rate /= postdiv;
 +
 + pr_debug(PLL%d: input = %lu MHz [ ,
 +  pll_data-num, parent_rate / 100);
 + if (prediv  1)
 + pr_debug(/ %d , prediv);
 + if (mult  1)
 + pr_debug(* %d , mult);
 + if (postdiv  1)
 + pr_debug(/ %d , postdiv);
 + pr_debug(] -- %lu MHz output.\n, rate / 100);
 + 

Re: [PATCH 02/13] clk: davinci - add PSC clock driver

2012-10-10 Thread Sekhar Nori
On 9/26/2012 11:37 PM, Murali Karicheri wrote:
 This is the driver for the Power Sleep Controller (PSC) hardware
 found on DM SoCs as well Keystone SoCs (c6x). This driver borrowed
 code from arch/arm/mach-davinci/psc.c and implemented the driver
 as per common clock provider API. The PSC module is responsible for
 enabling/disabling the Power Domain and Clock domain for different IPs
 present in the SoC. The driver is configured through the platform
 data structure struct clk_davinci_psc_data.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 
 diff --git a/drivers/clk/davinci/clk-davinci-psc.c 
 b/drivers/clk/davinci/clk-davinci-psc.c
 new file mode 100644
 index 000..b7aa332
 --- /dev/null
 +++ b/drivers/clk/davinci/clk-davinci-psc.c
 @@ -0,0 +1,197 @@
 +/*
 + * PSC clk driver for DaVinci devices
 + *
 + * Copyright (C) 2006-2012 Texas Instruments.
 + * Copyright (C) 2008-2009 Deep Root Systems, LLC
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +#include linux/clk.h
 +#include linux/clk-provider.h
 +#include linux/delay.h
 +#include linux/err.h
 +#include linux/io.h
 +#include linux/slab.h
 +#include linux/platform_data/clk-davinci-psc.h

Sort these alphabetically.

 +
 +/* PSC register offsets */
 +#define EPCPR0x070
 +#define PTCMD0x120
 +#define PTSTAT   0x128
 +#define PDSTAT   0x200
 +#define PDCTL0x300
 +#define MDSTAT   0x800
 +#define MDCTL0xA00
 +
 +/* PSC module states */
 +#define PSC_STATE_SWRSTDISABLE   0
 +#define PSC_STATE_SYNCRST1
 +#define PSC_STATE_DISABLE2
 +#define PSC_STATE_ENABLE 3
 +
 +#define MDSTAT_STATE_MASK0x3f
 +#define PDSTAT_STATE_MASK0x1f
 +#define MDCTL_FORCE  BIT(31)
 +#define PDCTL_NEXT   BIT(0)
 +#define PDCTL_EPCGOODBIT(8)
 +
 +/* PSC flags */
 +#define PSC_SWRSTDISABLE BIT(0) /* Disable state is SwRstDisable */
 +#define PSC_FORCEBIT(1) /* Force module state transtition */
 +#define PSC_HAS_EXT_POWER_CNTL   BIT(2) /* PSC has external power control
 + * available (for DM6446 SoC) */

Can you try and keep the comment above on a single line?

 +/**
 + * struct clk_psc - DaVinci PSC clock
 + * @hw: clk_hw for the psc
 + * @psc_data: PSC driver specific data
 + * @lock: Spinlock used by the driver
 + */
 +struct clk_psc {
 + struct clk_hw hw;
 + struct clk_davinci_psc_data *psc_data;
 + spinlock_t *lock;
 +};
 +
 +#define to_clk_psc(_hw) container_of(_hw, struct clk_psc, hw)
 +
 +/* Enable or disable a PSC domain */
 +static void clk_psc_config(void __iomem *base, unsigned int domain,
 + unsigned int id, bool enable, u32 flags)
 +{
 + u32 epcpr, ptcmd, ptstat, pdstat, pdctl, mdstat, mdctl;
 + u32 next_state = PSC_STATE_ENABLE;
 + void __iomem *psc_base = base;
 +
 + if (!enable) {
 + if (flags  PSC_SWRSTDISABLE)
 + next_state = PSC_STATE_SWRSTDISABLE;
 + else
 + next_state = PSC_STATE_DISABLE;
 + }
 +
 + mdctl = __raw_readl(psc_base + MDCTL + 4 * id);

Please convert all __raw_ variants to simple readl/writel() calls.

 + mdctl = ~MDSTAT_STATE_MASK;
 + mdctl |= next_state;
 + if (flags  PSC_FORCE)
 + mdctl |= MDCTL_FORCE;
 + __raw_writel(mdctl, psc_base + MDCTL + 4 * id);
 +
 + pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain);
 + if ((pdstat  PDSTAT_STATE_MASK) == 0) {
 + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
 + pdctl |= PDCTL_NEXT;
 + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
 +
 + ptcmd = 1  domain;
 + __raw_writel(ptcmd, psc_base + PTCMD);
 +
 + if (flags  PSC_HAS_EXT_POWER_CNTL) {
 + do {
 + epcpr = __raw_readl(psc_base + EPCPR);
 + } while epcpr  domain)  1) == 0));
 + }
 +
 + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
 + pdctl |= 0x100;
 + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
 +
 + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
 + pdctl |= PDCTL_EPCGOOD;
 + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
 + } else {
 + ptcmd = 1  domain;
 + __raw_writel(ptcmd, psc_base + PTCMD);
 + }
 +
 + do {
 + ptstat = __raw_readl(psc_base + PTSTAT);
 + } while (!(((ptstat  domain)  1) == 0));
 +
 + do {
 + mdstat = __raw_readl(psc_base + MDSTAT + 4 * id);
 + } while (!((mdstat  MDSTAT_STATE_MASK) == next_state));
 +}
 +
 +static int 

Re: [PATCH 02/13] clk: davinci - add PSC clock driver

2012-10-10 Thread Sekhar Nori
On 10/10/2012 6:05 PM, Sekhar Nori wrote:

 +struct clk *clk_register_davinci_psc(struct device *dev, const char *name,
 +const char *parent_name,
 +struct clk_davinci_psc_data *psc_data,
 +spinlock_t *lock)
 
 Why do you need the lock to be provided from outside of this file? You
 can initialize a lock for serializing writes to PSC registers within
 this file, no?

Looking again, it seems like the common clock framework defines an
enable_lock in drivers/clk/clk.c to serialize the clock enable/disable
calls. Unless I am missing something, this lock seems unnecessary.

Thanks,
Sekhar
--
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] usb: remove CONFIG_USB_MUSB_HOST etc

2012-10-11 Thread Sekhar Nori
On 10/11/2012 12:05 PM, Heiko Schocher wrote:
 Hello Manjunathappa
 
 On 11.10.2012 07:42, Manjunathappa, Prakash wrote:
 Hi,
 On Mon, Oct 08, 2012 at 18:47:07, Constantine Shulyupin wrote:
 From: Constantine Shulyupinco...@makelinux.com

 Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
 CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
 and set MUSB_OTG configuration by default
 because this configuration options are removed from Kconfig.

 Signed-off-by: Constantine Shulyupinco...@makelinux.com

 ---
   arch/arm/mach-davinci/usb.c |6 --
   1 file changed, 6 deletions(-)

 diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
 index f77b953..34509ff 100644
 --- a/arch/arm/mach-davinci/usb.c
 +++ b/arch/arm/mach-davinci/usb.c
 @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
   };

   static struct musb_hdrc_platform_data usb_data = {
 -#if defined(CONFIG_USB_MUSB_OTG)
   /* OTG requires a Mini-AB connector */
   .mode   = MUSB_OTG,
 -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
 -.mode   = MUSB_PERIPHERAL,
 -#elif defined(CONFIG_USB_MUSB_HOST)
 -.mode   = MUSB_HOST,
 -#endif
   .clock= usb,
   .config=musb_config,
   };

 Tested it on DM6446-EVM for host mode with MSC thumb drive and gadget
 mode with g-ether. It works.

 Acked-by: Manjunathappa, Prakashprakash...@ti.com
 
 I sent a similiar patch here:
 
 http://comments.gmane.org/gmane.linux.usb.general/54512
 
 If the issues, mentioned from Sergei for my patch, nullified I add my:

The last outstanding issue from Sergei seems to be additional comments
describing why MUSB_OTG is OK to use.

Prakash/Constantine,

Did you have to make any hardware changes when testing host/gadget on
DM6446 EVM? Or change in kernel configuration?

It appears that there is no way to choose any of the config option
affecting the mode setting. Right now it seems to be just defaulting to
MUSB_UNDEFINED. Setting it to MUSB_OTG would be better than that.

Thanks,
Sekhar
--
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: [REMINDER: PATCH 1/1] mtd:nand:clk: preparation for switch to common clock framework

2012-10-11 Thread Sekhar Nori
Hi Artem,

On 10/11/2012 3:27 PM, Artem Bityutskiy wrote:
 On Fri, 2012-09-28 at 16:10 +, Karicheri, Muralidharan wrote:
 -Original Message-
 From: Karicheri, Muralidharan
 Sent: Monday, September 17, 2012 12:38 PM
 To: dw...@infradead.org; artem.bityuts...@linux.intel.com; h...@denx.de;
 miked...@newsguy.com; linux-...@lists.infradead.org; 
 linux-kernel@vger.kernel.org;
 davinci-linux-open-sou...@linux.davincidsp.com; 
 linux-arm-ker...@lists.infradead.org
 Cc: Karicheri, Muralidharan
 Subject: [PATCH 1/1] mtd:nand:clk: preparation for switch to common clock 
 framework
 
 I do not see this patch in my mailbox. Can you please re-send with all
 the acks?

This patch is already applied to linux-next. Do you need it still?

http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=ea73fe7f0d562154975a77fe77ae3da6ab4d3e77

Thanks,
Sekhar
--
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 01/13] calk: davinci - add Main PLL clock driver

2012-10-11 Thread Sekhar Nori
On 10/10/2012 8:04 PM, Karicheri, Muralidharan wrote:

 +struct clk *clk_register_davinci_pll(struct device *dev, const char *name,
 +  const char *parent_name,
 +  struct clk_davinci_pll_data *pll_data) {
 +  struct clk_init_data init;
 +  struct clk_davinci_pll *pll;
 +  struct clk *clk;
 +
 +  if (!pll_data)
 +  return ERR_PTR(-ENODEV);
 +
 +  pll = kzalloc(sizeof(*pll), GFP_KERNEL);
 +  if (!pll)
 +  return ERR_PTR(-ENOMEM);
 +  init.name = name;
 +  init.ops = clk_pll_ops;
 +  init.flags = pll_data-flags;
 +  init.parent_names = (parent_name ? parent_name : NULL);
 +  init.num_parents = (parent_name ? 1 : 0);
 +
 +  pll-pll_data   = pll_data;
 +  pll-hw.init = init;
 +
 +  clk = clk_register(NULL, pll-hw);
 +  if (IS_ERR(clk))
 +  kfree(pll);
 +
 +  return clk;
 +}

 I guess there is an an unregister required as well which will free the 
 pll memory
 allocated above and unregister the clock? Not sure if you would ever 
 unregister a PLL,
 but providing this will probably help symmetry.
 Sekhar,
 
 clk_unregister() itself is a null statement in clk.c. Besides none of the clk 
 drivers presently have implemented the unregister(). So I believe this is 
 unnecessary. 

I am ok with this.

 BTW, please review the v2 patch for the rest of the series. For the one you 
 have already reviewed, it should be fine.

Okay. I see those now. BTW, this series also has a v2 in its 0/13. Are
there any differences between this and the other v2, or is that merely a
resend?

Thanks,
Sekhar
--
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 01/13] clk: davinci - add Main PLL clock driver

2012-10-11 Thread Sekhar Nori
On 10/10/2012 5:32 PM, Sekhar Nori wrote:
 Hi Murali,
 
 On 9/26/2012 11:37 PM, Murali Karicheri wrote:
 This is the driver for the main PLL clock hardware found on DM SoCs.
 This driver borrowed code from arch/arm/mach-davinci/clock.c and
 implemented the driver as per common clock provider API. The main PLL
 hardware typically has a multiplier, a pre-divider and a post-divider.
 Some of the SoCs has the divider fixed meaning they can not be
 configured through a register. HAS_PREDIV and HAS_POSTDIV flags are used
 to tell the driver if a hardware has these dividers present or not.
 Driver is configured through the structure clk_davinci_pll_data
 that has the platform data for the driver.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 
 Are you using git-format-patch to generate the patches? It should have
 added a diffstat here by default which is very useful in quickly
 understanding what the patch is touching.

 diff --git a/drivers/clk/davinci/clk-davinci-pll.c 
 b/drivers/clk/davinci/clk-davinci-pll.c

Looking at how common clock framework for mxs has been implemented, this
file should simply be clk-pll.c. That makes sense as you are creating a
davinci folder anyway. Similar change required for psc as well.

Thanks,
Sekhar
--
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 v2 01/13] clk: davinci - add Main PLL clock driver

2012-10-11 Thread Sekhar Nori
On 9/26/2012 11:40 PM, Murali Karicheri wrote:

 +struct clk_davinci_pll_data {
 + /* physical addresses set by platform code */
 + u32 phy_pllm;
 + /* if PLL has a prediv register this should be non zero */
 + u32 phy_prediv;
 + /* if PLL has a postdiv register this should be non zero */
 + u32 phy_postdiv;

phys is typically used for physical addresses. I couldn't immediately
related this to physical address when I looked at this variable outside
of this file.

Also, physical addresses should be of type phys_addr_t.

 + /* mapped addresses. should be initialized by  */
 + void __iomem *pllm;
 + void __iomem *prediv;
 + void __iomem *postdiv;
 + u32 pllm_mask;
 + u32 prediv_mask;
 + u32 postdiv_mask;
 + u32 num;

Wondering what num is for? May be it will be clear once you document the
complete structure as Linus suggested.

 + /* framework flags */
 + u32 flags;
 + /* pll flags */
 + u32 pll_flags;
 +   /* use this value for prediv */
 + u32 fixed_prediv;

Why is this called fixed. Isn't this the pre-divider value that user
can configure? Also, shouldn't there be a post-divider value that you
need as well.

 + /* multiply PLLM by this factor. By default most SOC set this to zero
 +  * that translates to a multiplier of 1 and incrementer of 1.
 +  * To override default, set this factor
 +  */
 + u32 pllm_multiplier;

'm' in pllm stands for multiplier. So, instead of calling it
pllm_multiplier, call it pllm_val instead?

Sorry about late comments on this, looks like I missed looking at this
structure altogether yesterday and had to go back to this only when I
tried understanding whats going on in 4/13

Thanks,
Sekhar
--
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 v2 09/13] ARM: davinci - update the dm644x soc code to use common clk drivers

2012-10-11 Thread Sekhar Nori
Murali,

On 9/26/2012 11:40 PM, Murali Karicheri wrote:
 The clock tree for dm644x is defined using the new structure davinci_clk.
 The SoC specific code re-uses clk-fixed-rate, clk-divider and clk-mux
 drivers in addition to the davinci specific clk drivers, clk-davinci-pll
 and clk-davinci-psc. Macros are defined to define the various clocks in
 the SoC.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com

You have chosen to keep all clock related data in platform files while
using the common clock framework to provide just the infrastructure. If
you look at how mxs and spear have been migrated, they have migrated the
soc specific clock data to drivers/clk as well. See
drivers/clk/spear/spear3xx_clock.c or drivers/clk/mxs/clk-imx23.c. I
feel the latter way is better and I also think it will simplify some of
the look-up infrastructure you had to build. This will also help some
real code reduction from arch/arm/mach-davinci/.

Thanks,
Sekhar
--
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 v2 04/13] clk: davinci - common clk driver initialization

2012-10-11 Thread Sekhar Nori
Hi Murali,

I have given this patch a partial review. I suspect this patch will
change much based on the comment I gave for 9/13.

On 9/26/2012 11:40 PM, Murali Karicheri wrote:
 This is the common clk driver initialization function for DaVinci
 SoCs and other SoCs that uses similar hardware architecture.
 Different clocks present in the SoC are passed to this function
 using a clk table and this function initialize the various drivers.
 Existing driver such as clk-fixed-rate, clk-divider, clk-mux and
 DaVinci specific clk drivers are initialized by this function.
 Also adds clock lookup for shared clocks used by various drivers.
 davinci-clock.h declares the various structures used for defining
 the DaVinci clocks.
 
 Signed-off-by: Murali Karicheri m-kariche...@ti.com
 
 diff --git a/drivers/clk/davinci/davinci-clock.c 
 b/drivers/clk/davinci/davinci-clock.c
 new file mode 100644
 index 000..cbd5201
 --- /dev/null
 +++ b/drivers/clk/davinci/davinci-clock.c
 @@ -0,0 +1,232 @@
 +/*
 + * Clock initialization code for DaVinci devices
 + *
 + * Copyright (C) 2006-2012 Texas Instruments.
 + * Copyright (C) 2008-2009 Deep Root Systems, LLC
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +#include linux/init.h
 +#include linux/clk-provider.h
 +#include linux/clkdev.h
 +#include linux/slab.h
 +#include linux/io.h
 +#include linux/platform_data/clk-davinci-pll.h
 +#include linux/platform_data/clk-keystone-pll.h
 +#include linux/platform_data/clk-davinci-psc.h
 +#include linux/platform_data/davinci-clock.h

Needs to be kept sorted.

 +
 +static DEFINE_SPINLOCK(_lock);

No need of the underscore prefix.

 +
 +struct clk *davinci_lookup_clk(struct davinci_clk_lookup *clocks,
 + const char *con_id)
 +{
 + struct davinci_clk_lookup *c;
 +
 + for (c = clocks; c-_clk; c++) {
 + if (c-con_id  !strcmp(c-con_id, con_id))
 + return c-clk;
 + }
 + return NULL;
 +}

This is a global function, but it is not exported in a header file.
Should this be static instead?

 +
 +#ifdef   CONFIG_CLK_DAVINCI_PLL
 +static void register_davinci_pll_clk(struct davinci_clk_lookup *c,
 + struct clk_davinci_pll_data *pll_data)
 +{
 +
 + WARN_ON(!pll_data-phy_pllm);

I don't think it is right to check for zero physical address. It is not
true on any of the DaVinci platforms, but it is possible to design
hardware where physical address of the PLL multipler is zero.

 + pll_data-pllm = ioremap(pll_data-phy_pllm, 4);
 + WARN_ON(!pll_data-pllm);

Along with the warning for users, you also need to return an -ENOMEM so
callers are aware? Or you can skip the WARN_ON and simply return -ENOMEM.

 + if (pll_data-phy_prediv) {
 + pll_data-prediv = ioremap(pll_data-phy_prediv, 4);
 + WARN_ON(!pll_data-prediv);
 + }
 + if (pll_data-phy_postdiv) {
 + pll_data-postdiv = ioremap(pll_data-phy_postdiv, 4);
 + WARN_ON(!pll_data-postdiv);
 + }

Comments above apply here as well.

 + c-clk = clk_register_davinci_pll(NULL,
 + c-_clk-name, c-_clk-parent-name,
 + pll_data);
 +}
 +#else
 +static void register_davinci_pll_clk(struct davinci_clk_lookup *c,
 + struct clk_davinci_pll_data *pll_data)
 +{
 + return;
 +}
 +#endif
 +
 +#ifdef   CONFIG_CLK_KEYSTONE_PLL
 +static void register_keystone_pll_clk(struct davinci_clk_lookup *c,
 + struct clk_keystone_pll_data *pll_data)
 +{
 + WARN_ON(!pll_data-phy_pllm);
 + pll_data-pllm = ioremap(pll_data-phy_pllm, 4);
 + WARN_ON(!pll_data-pllm);
 + WARN_ON(!pll_data-phy_main_pll_ctl0);
 + pll_data-main_pll_ctl0 =
 + ioremap(pll_data-phy_main_pll_ctl0, 4);
 + WARN_ON(!pll_data-main_pll_ctl0);
 + c-clk = clk_register_keystone_pll(NULL,
 + c-_clk-name, c-_clk-parent-name,
 +  pll_data);
 +}
 +#else
 +static void register_keystone_pll_clk(struct davinci_clk_lookup *c,
 + struct clk_keystone_pll_data *pll_data)
 +{
 + return;
 +}
 +#endif
 +
 +int __init davinci_common_clk_init(struct davinci_clk_lookup *clocks,
 + struct davinci_dev_lookup *dev_clk_lookups,
 + u8 num_gpscs, u32 *psc_bases)

psc_bases should be of type phys_add_t.

 +{
 + void __iomem **base = NULL, *reg;
 + struct davinci_clk_lookup *c;
 + struct davinci_clk *_clk;
 + unsigned long rate;
 + int i, skip;
 +
 + WARN_ON(!num_gpscs);

Need to return error if this is hit. In general revisit the need to
return an error wherever you have WARN_ON().

Thanks,
Sekhar
--
To unsubscribe from this list: send the 

Re: [PATCH v2 09/13] ARM: davinci - update the dm644x soc code to use common clk drivers

2012-10-12 Thread Sekhar Nori
Hi Murali,

On 10/11/2012 8:28 PM, Karicheri, Muralidharan wrote:
 -Original Message-
 From: Nori, Sekhar
 Sent: Thursday, October 11, 2012 8:25 AM
 To: Karicheri, Muralidharan
 Cc: mturque...@linaro.org; a...@arndb.de; a...@linux-foundation.org;
 shawn@linaro.org; rob.herr...@calxeda.com; linus.wall...@linaro.org;
 viresh.li...@gmail.com; linux-kernel@vger.kernel.org; Hilman, Kevin;
 li...@arm.linux.org.uk; davinci-linux-open-sou...@linux.davincidsp.com; 
 linux-arm-
 ker...@lists.infradead.org; linux-keyst...@list.ti.com - Linux developers 
 for Keystone
 family of devices (May contain non-TIers); linux-c6x-...@linux-c6x.org; 
 Chemparathy,
 Cyril
 Subject: Re: [PATCH v2 09/13] ARM: davinci - update the dm644x soc code to 
 use
 common clk drivers

 Murali,

 On 9/26/2012 11:40 PM, Murali Karicheri wrote:
 The clock tree for dm644x is defined using the new structure davinci_clk.
 The SoC specific code re-uses clk-fixed-rate, clk-divider and clk-mux
 drivers in addition to the davinci specific clk drivers,
 clk-davinci-pll and clk-davinci-psc. Macros are defined to define the
 various clocks in the SoC.

 Signed-off-by: Murali Karicheri m-kariche...@ti.com

 You have chosen to keep all clock related data in platform files while 
 using the common
 clock framework to provide just the infrastructure. If you look at how mxs 
 and spear
 have been migrated, they have migrated the soc specific clock data to 
 drivers/clk as well.
 See drivers/clk/spear/spear3xx_clock.c or drivers/clk/mxs/clk-imx23.c 
 
 I have to disagree on this one. I had investigated these code already and came
 up with a way that we can re-use code across all of the davinci platforms as 
 well as other architectures that re-uses the clk hardware IPs.

Which code you are talking about here? Even if you introduce
clk-dm644x.c, clk-keystone.c etc in drivers/clk/davinci/ you can reuse
the code you introduce in patches 1-3. I cant see how that will be
prevented.

 spear3xx_clock.c has initialization code for each of the platforms
 and so is the case with imx23.c.

By each of the platforms, you mean they all cater to a family of
devices? This depends on how close together the family of devices are.
Otherwise, there would be a file per soc. DM644x also represents a
family for that matter.

 By using platform_data approach, we are able to define clks for each of the 
 SoC and then use davinci_common_clk_init() to do initialize the clk drivers 
 based on platform data.

You need to define and register the clocks present on each SoC either
which way. I don't see why just the platform_data approach allows this.
And looking closely, you have defined platform data, but don't actually
have a platform device, making things more confusing.

 Later once we migrate to device tree, davinci_common_clk_init() will go way 
 and also the clk structures defined in the SoC file. I have prototyped this 
 on one of the device that I am working on. davinci_common_clk_init() will be 
 replaced with a of_davinci_clk_init() that will use device tree to get all of 
 the platform data for the clk providers and do the initialization based on 
 that. See highbank_clocks_init() in clk-highbank.c. I have used this model 
 for device
 tree based clk initialization.

I don't think we should wait till DT migration to get rid of clock data
from platform code. For many of the older DaVinci platforms, DT
migration is a big if and when. This approach you gave above might work
for newer DT-only platforms, but even if there is one board that is not
migrated to DT, the entire clock data will have to stay. I have very
less hope this will happen for DaVinci (at least in the near term). So,
I would rather take the opportunity of common clock tree migration to
move clock data out of mach-davinci.

Also, just moving soc-specific clk data to drivers/clk/davinci/* does
not impede a future DT conversion, no?

 So it make sense to keep the design the way it is. Otherwise we will end up 
 writing dm644x_clk_init(), dm355_clk_init(), etc for each of the platforms 
 and these code will get thrown away once we migrate to
 device tree. 

I still don't see why davinci/keystone cannot follow the same approach
taken by multiple other socs - spear, mxs and ux500. I am unconvinced
that we have a significantly different case.

 . I feel the
 latter way is better and I also think it will simplify some of the look-up 
 infrastructure you
 had to build. This will also help some real code reduction from 
 arch/arm/mach-davinci/.

 
 The look-up infrastructure is pretty much re-use of the existing code base in 
 SoC specific file.

Yes, but that's no reason to keep maintaining it.

 About code reduction, I can't say I agree, as we need to add 
 platform_specific clock initialization code if we follow spear3xx_clock.c 
 model and end up probably adding more code.
 SoC specific file (for example dm644x.c) has only data structures and all of 
 SoC will re-use davinci_common_clk_init() to do the 

Re: [PATCH] davinci: fix return value check by using IS_ERR in tnetv107x_devices_init()

2012-10-12 Thread Sekhar Nori
Hi Wei,

On 9/21/2012 11:42 AM, Wei Yongjun wrote:
 From: Wei Yongjun yongjun_...@trendmicro.com.cn
 
 In case of error, the function clk_get() returns ERR_PTR() not
 NULL pointer. The NULL test in the error handling should be
 replaced with IS_ERR().
 
 dpatch engine is used to auto generated this patch.
 (https://github.com/weiyj/dpatch)
 
 Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn

Queuing this for v3.8. While committing, I added ARM: prefix to the
subject line as is the norm with all arch/arm/* patches.

Thanks,
Sekhar
--
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] rtc: davinci: clean up probe/remove routines

2012-10-14 Thread Sekhar Nori
On 10/14/2012 3:43 AM, Hannu Heikkinen wrote:
 Use the devres managed resource functions in the probe routine.
 Also affects the remove routine where the previously used free and
 release functions are not needed.
 
 The devm_* functions eliminate the need for manual resource releasing and
 simplify error handling.  Resources allocated by devm_* are freed
 automatically on driver detach.
 
 Signed-off-by: Hannu Heikkinen hann...@iki.fi
 ---
  drivers/rtc/rtc-davinci.c | 56 
 +--
  1 file changed, 15 insertions(+), 41 deletions(-)
 
 diff --git a/drivers/rtc/rtc-davinci.c b/drivers/rtc/rtc-davinci.c
 index 14c2109..ffb09c2 100644
 --- a/drivers/rtc/rtc-davinci.c
 +++ b/drivers/rtc/rtc-davinci.c
 @@ -119,8 +119,6 @@ static DEFINE_SPINLOCK(davinci_rtc_lock);
  struct davinci_rtc {
   struct rtc_device   *rtc;
   void __iomem*base;
 - resource_size_t pbase;
 - size_t  base_size;
   int irq;
  };
  
 @@ -482,22 +480,16 @@ static int __init davinci_rtc_probe(struct 
 platform_device *pdev)
  {
   struct device *dev = pdev-dev;
   struct davinci_rtc *davinci_rtc;
 - struct resource *res, *mem;
 + struct resource *res;
   int ret = 0;
  
 - davinci_rtc = kzalloc(sizeof(struct davinci_rtc), GFP_KERNEL);
 + davinci_rtc = devm_kzalloc(pdev-dev, sizeof(struct tegra_rtc_info),

This line causes a build breakage because of wrong structure name.


 + GFP_KERNEL);
   if (!davinci_rtc) {
   dev_dbg(dev, could not allocate memory for private data\n);
   return -ENOMEM;
   }
  
 - davinci_rtc-irq = platform_get_irq(pdev, 0);
 - if (davinci_rtc-irq  0) {
 - dev_err(dev, no RTC irq\n);
 - ret = davinci_rtc-irq;
 - goto fail1;
 - }
 -
   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   if (!res) {
   dev_err(dev, no mem resource\n);
 @@ -505,23 +497,16 @@ static int __init davinci_rtc_probe(struct 
 platform_device *pdev)
   goto fail1;

This goto target does not exist anymore after this patch. This causes
build breakage too.

I was not able to test this on a DM365 EVM though. hwclock returned with
this error:

select() to /dev/rtc to wait for clock tick timed out

but this is not related to this patch since the same error occurs even
without this patch. May something to do with the particular board I
used, need to debug and check.

Thanks,
Sekhar
--
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 v5 7/7] ARM: davinci: da850 evm: register uio_pruss device

2012-10-25 Thread Sekhar Nori
On 10/8/2012 7:24 PM, Matt Porter wrote:
 Registers the uio_pruss platform device on the DA850 EVM.
 
 Tested on AM180x-EVM using the PRU_memAccessPRUDataRam and
 PRU_memAccessL3andDDR examples from the PRU userspace tools
 available from http://www.ti.com/tool/sprc940
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/mach-davinci/board-da850-evm.c |6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
 b/arch/arm/mach-davinci/board-da850-evm.c
 index 7359375..9e7f954 100644
 --- a/arch/arm/mach-davinci/board-da850-evm.c
 +++ b/arch/arm/mach-davinci/board-da850-evm.c
 @@ -31,6 +31,7 @@
  #include linux/platform_data/mtd-davinci.h
  #include linux/platform_data/mtd-davinci-aemif.h
  #include linux/platform_data/spi-davinci.h
 +#include linux/platform_data/uio_pruss.h
  #include linux/regulator/machine.h
  #include linux/regulator/tps6507x.h
  #include linux/spi/spi.h
 @@ -1339,6 +1340,11 @@ static __init void da850_evm_init(void)
   pr_warning(da850_evm_init: lcdcntl mux setup failed: %d\n,
   ret);
  
 + ret = da8xx_register_uio_pruss();
 + if (ret)
 + pr_warning(da850_evm_init: pruss initialization failed: %d\n,

So this gave a checkpatch warning asking you to use pr_warn() instead. I
fixed this locally.

Thanks,
Sekhar
--
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 v4 0/7] uio_pruss cleanup and platform support

2012-10-25 Thread Sekhar Nori
On Wed, Oct 24, 2012 at 10:24 PM, Matt Porter mpor...@ti.com wrote:
 On Mon, Oct 08, 2012 at 04:27:20PM +0530, Sekhar Nori wrote:
 On 10/5/2012 10:34 PM, Matt Porter wrote:

  This series enables uio_pruss on DA850 and removes use of the
  private SRAM API by the driver. The driver previously was not
  enabled by any platform and the private SRAM API was accessing
  an invalid SRAM bank.
 
  It is regression tested on AM180x EVM with suspend/resume due
  to the new use of the shared SRAM for both PM and PRUSS. The
  uio_pruss driver is tested on the same platform using the
  PRU_memAccessPRUDataRam and PRU_memAccessL3andDDR examples from
  the PRU userspace tools available from http://www.ti.com/tool/sprc940

 I applied patches 2/7, 3/7 and 6/7 of this series for v3.8. I have some
 comments on the board patch. Rest of the patches depend on acceptance of
 1/7 so I will take them only after that is accepted.

 Ok, Hans has accepted 1/7, will you take the entire series through the
 Davinci tree?

Yes.

Thanks,
Sekhar
--
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] ARM: davinci: dm644x: fix out range signal for ED

2012-10-26 Thread Sekhar Nori
On 10/4/2012 7:23 PM, Prabhakar Lad wrote:
 Sekhar
 
 On Thu, Oct 4, 2012 at 12:43 PM, Sekhar Nori nsek...@ti.com wrote:
 On 10/4/2012 10:22 AM, Prabhakar Lad wrote:
 Hi Sekhar,

 On Wed, Oct 3, 2012 at 4:08 PM, Sekhar Nori nsek...@ti.com wrote:
 On 10/3/2012 12:05 PM, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar@ti.com

 while testing display on dm644x, for ED out-range signals
 was observed. This patch fixes appropriate clock setting
 for ED.

 Can you please clarify what you mean by out range signal? Are there
 any user visible artifacts on the display? What was the clock being
 provided earlier and what is the value after this patch?

 Also, is the issue severe enough that this patch should be applied to
 stable tree as well?

 Ideally a clock of 54Mhz is required for  enhanced definition to
 work, which it was actually set to but while testing I noticed
 out-of-range signal. The out-of-range signal is often caused
 when the field rate is above the rate that the television will handle.
 When this is the case the TV or monitor displays Out-Of-Range Signal.

 Reducing the clock fixed it. now the clock is set to 27Mhz.

 So, is the requirement for ED 54MHz or lower? Still trying to explain
 myself how 27MHz is working where a 54MHz is required. I guess there is
 also a lower limit on what the frequency should be?

 Ideally its 54Mhz, but I see in the datasheet for AD7342/3 [1] it can also
 work at 27Mhz too.

So, based on this discussion I think a better description would be:


Fix the video clock setting when custom timings are used with pclock  =
27MHz. Existing clock selection uses PLL2 mode which results in a 54MHz
clock where as using the MXI mode results in a 27MHz clock (which is the
one actually desired).

This affects the Enhanced Definition (ED) support on DM644x. Without
this patch, out-range signals errors are were observed on the TV. An
out-of-range signal is often caused when the field rate is above the
rate that the television will handle.


Is this accurate? In future, please try to be descriptive in patch
description as it will help understand what the patch is doing and why
(which in turn will lead to the patch getting accepted faster).

Also I assume you need this patch in v3.7? Or can I send it for v3.8?

Thanks,
Sekhar
--
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 v2] arch/arm/mach-davinci/board-dm646x-evm.c: Remove unecessary semicolon

2012-10-26 Thread Sekhar Nori
On 9/18/2012 10:29 PM, Peter Senna Tschudin wrote:
 Found by http://coccinelle.lip6.fr/
 
 Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

Queuing this for v3.8

Thanks,
Sekhar
--
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] ARM: davinci: dm644x: move the dereference below the NULL test

2012-10-26 Thread Sekhar Nori
On 9/10/2012 10:19 AM, Wei Yongjun wrote:
 From: Wei Yongjun yongjun_...@trendmicro.com.cn
 
 The dereference should be moved below the NULL test.
 
 spatch with a semantic match is used to found this.
 (http://coccinelle.lip6.fr/)
 
 Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn

Queuing this for inclusion in v3.8

Thanks,
Sekhar
--
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] davinci: fix the uart number in the comment

2012-10-26 Thread Sekhar Nori
On 9/13/2012 11:34 PM, Henrique Camargo wrote:
 The bit 0 of the field is uart0 and the bit 1 is uart1 and so on.
 
 Signed-off-by: Henrique Camargo henri...@henriquecamargo.com

Queuing this for v3.8. Added an ARM: prefix to subject line since this
is an arch/arm/ patch. Please take care of this next time on.

Thanks,
Sekhar
--
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] davinci: check for presence of channel controller on slot alloc

2012-10-26 Thread Sekhar Nori
+ Matt, who is doing the DMA engine conversion for EDMA

On 9/12/2012 11:44 PM, Cyril Chemparathy wrote:
 This patch adds a check for the presence of the channel controller when
 trying to allocate a slot.  Without this fix, the kernel panics with a NULL
 pointer dereference when the dma-engine drivers are probed.
 
 Signed-off-by: Cyril Chemparathy cy...@ti.com
 ---
  arch/arm/mach-davinci/dma.c |3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
 index a685e97..2fee31e 100644
 --- a/arch/arm/mach-davinci/dma.c
 +++ b/arch/arm/mach-davinci/dma.c
 @@ -743,6 +743,9 @@ EXPORT_SYMBOL(edma_free_channel);
   */
  int edma_alloc_slot(unsigned ctlr, int slot)
  {
 + if (!edma_cc[ctlr])
 + return -ENODEV;

I am ok with this patch, although I wonder if a better error is -ENXIO
or even -EINVAL (since its most likely the result of a wrong argument).

This patch will conflict with the EDMA movement series that Matt is
doing and he should probably include it in his series to avoid
conflicts. From the description it appears to be not required for v3.7
anyway.

Thanks,
Sekhar
--
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: [RESEND-PATCH] media:davinci: clk - {prepare/unprepare} for common clk

2012-10-27 Thread Sekhar Nori
Hi Murali,

On 10/26/2012 9:22 PM, Murali Karicheri wrote:
 On 10/25/2012 09:12 AM, Prabhakar Lad wrote:
 Hi Murali,

 Thanks for the patch.  I'll  queue this patch for 3.8.
 Please check with Sekhar as well. This is a preparation patch for common
 clk framework support. ALso fixes some bugs on the existing code. As the
 clk
 patches are dependent on these patches, I would suggest you queue this
 against 3.7 rcx.

The -rc cycle is for fixes only so this cannot get merged into v3.7
as-is. If the patch has some fixes embedded, its a good idea to separate
them out (and have the feature parts come after the fixes in the patch
series) so they can be considered for -rc cycle. The current description
does not detail what the issue is and what its impact is so when you do
separate it out, please mention those as well. It will help determine
the severity of the issue and convince maintainers to include it in v3.7.

Thanks,
Sekhar
--
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: [RFC PATCH v3 02/16] ARM: davinci: move private EDMA API to arm/common

2012-10-28 Thread Sekhar Nori
On 10/18/2012 6:56 PM, Matt Porter wrote:
 Move mach-davinci/dma.c to common/edma.c so it can be used
 by OMAP (specifically AM33xx) as well. This just moves the
 private EDMA API but does not support OMAP.
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---

 diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
 index 4c48a36..f45d591 100644
 --- a/arch/arm/mach-davinci/devices.c
 +++ b/arch/arm/mach-davinci/devices.c
 @@ -19,9 +19,10 @@
  #include mach/irqs.h
  #include mach/cputype.h
  #include mach/mux.h
 -#include mach/edma.h
  #include linux/platform_data/mmc-davinci.h
  #include mach/time.h
 +#include linux/platform_data/edma.h

Can you please introduce a patch to clean this mixture of linux/ and
mach/ includes?

 +
  
  #include davinci.h
  #include clock.h
 @@ -141,10 +142,10 @@ static struct resource mmcsd0_resources[] = {
   },
   /* DMA channels: RX, then TX */
   {
 - .start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCRXEVT),
 + .start = EDMA_CTLR_CHAN(0, 26),

Instead of just replacing the event #defines with plain numbers, can you
introduce a mach-davinci local edma.h which can then be included in the
davinci platform files which refer to edma channel numbers?

 diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
 index 7cd56ed..153fab8 100644
 --- a/arch/arm/plat-omap/Kconfig
 +++ b/arch/arm/plat-omap/Kconfig
 @@ -28,6 +28,7 @@ config ARCH_OMAP2PLUS
   select OMAP_DM_TIMER
   select PROC_DEVICETREE if PROC_FS
   select SPARSE_IRQ
 + select TI_PRIV_EDMA

This hunk does not seem to belong to subject of this patch.

   select USE_OF
   help
 Systems based on OMAP2, OMAP3, OMAP4 or OMAP5

 diff --git a/include/linux/platform_data/edma.h 
 b/include/linux/platform_data/edma.h
 new file mode 100644
 index 000..7396f0b3
 --- /dev/null
 +++ b/include/linux/platform_data/edma.h
 @@ -0,0 +1,198 @@
 +/*
 + *  TI DAVINCI dma definitions
 + *
 + *  Copyright (C) 2006-2009 Texas Instruments.
 + *
 + *  This program is free software; you can redistribute  it and/or modify it
 + *  under  the terms of  the GNU General  Public License as published by the
 + *  Free Software Foundation;  either version 2 of the  License, or (at your
 + *  option) any later version.
 + *
 + *  THIS  SOFTWARE  IS PROVIDED   ``AS  IS'' AND   ANY  EXPRESS OR IMPLIED
 + *  WARRANTIES,   INCLUDING, BUT NOT  LIMITED  TO, THE IMPLIED WARRANTIES OF
 + *  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN
 + *  NO  EVENT  SHALL   THE AUTHOR  BELIABLE FOR ANY   DIRECT, INDIRECT,
 + *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
 + *  NOT LIMITED   TO, PROCUREMENT OF  SUBSTITUTE GOODS  OR SERVICES; LOSS OF
 + *  USE, DATA,  OR PROFITS; OR  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
 + *  ANY THEORY OF LIABILITY, WHETHER IN  CONTRACT, STRICT LIABILITY, OR TORT
 + *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 + *  THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 + *  You should have received a copy of the  GNU General Public License along
 + *  with this program; if not, write  to the Free Software Foundation, Inc.,
 + *  675 Mass Ave, Cambridge, MA 02139, USA.

This part can be dropped, I suppose ;-)

Thanks,
Sekhar
--
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: [RFC PATCH v3 04/16] ARM: edma: add DT and runtime PM support for AM33XX

2012-10-28 Thread Sekhar Nori
On 10/18/2012 6:56 PM, Matt Porter wrote:
 Adds support for parsing the TI EDMA DT data into the required
 EDMA private API platform data.
 
 Calls runtime PM API only in the DT case in order to unidle the
 associated hwmods on AM33XX.

Runtime PM is supported on DaVinci now, so if that was the reason for
this choice, then it doesn't need to be that way.

 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/common/edma.c  |  255 
 +--
  arch/arm/mach-davinci/board-da830-evm.c |4 +-
  arch/arm/mach-davinci/board-da850-evm.c |8 +-
  arch/arm/mach-davinci/board-dm646x-evm.c|4 +-
  arch/arm/mach-davinci/board-omapl138-hawk.c |8 +-
  arch/arm/mach-davinci/devices-da8xx.c   |8 +-
  arch/arm/mach-davinci/devices-tnetv107x.c   |4 +-
  arch/arm/mach-davinci/dm355.c   |4 +-
  arch/arm/mach-davinci/dm365.c   |4 +-
  arch/arm/mach-davinci/dm644x.c  |4 +-
  arch/arm/mach-davinci/dm646x.c  |4 +-
  include/linux/platform_data/edma.h  |8 +-
  12 files changed, 272 insertions(+), 43 deletions(-)
 
 diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
 index a3d189d..6d2a590 100644
 --- a/arch/arm/common/edma.c
 +++ b/arch/arm/common/edma.c
 @@ -24,6 +24,13 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/edma.h
 +#include linux/err.h
 +#include linux/of_address.h
 +#include linux/of_device.h
 +#include linux/of_dma.h
 +#include linux/of_irq.h
 +#include linux/pm_runtime.h
  
  #include linux/platform_data/edma.h
  
 @@ -1366,31 +1373,237 @@ void edma_clear_event(unsigned channel)
  EXPORT_SYMBOL(edma_clear_event);
  
  /*---*/
 +static int edma_of_read_u32_to_s8_array(const struct device_node *np,
 +  const char *propname, s8 *out_values,
 +  size_t sz)
 +{
 + struct property *prop = of_find_property(np, propname, NULL);
 + const __be32 *val;
 +
 + if (!prop)
 + return -EINVAL;
 + if (!prop-value)
 + return -ENODATA;
 + if ((sz * sizeof(u32))  prop-length)
 + return -EOVERFLOW;
 +
 + val = prop-value;
 +
 + while (sz--)
 + *out_values++ = (s8)(be32_to_cpup(val++)  0xff);
 +
 + /* Terminate it */
 + *out_values++ = -1;
 + *out_values++ = -1;
 +
 + return 0;
 +}
 +
 +static int edma_of_read_u32_to_s16_array(const struct device_node *np,
 +  const char *propname, s16 *out_values,
 +  size_t sz)
 +{
 + struct property *prop = of_find_property(np, propname, NULL);
 + const __be32 *val;
 +
 + if (!prop)
 + return -EINVAL;
 + if (!prop-value)
 + return -ENODATA;
 + if ((sz * sizeof(u32))  prop-length)
 + return -EOVERFLOW;
 +
 + val = prop-value;
 +
 + while (sz--)
 + *out_values++ = (s16)(be32_to_cpup(val++)  0x);
 +
 + /* Terminate it */
 + *out_values++ = -1;
 + *out_values++ = -1;
 +
 + return 0;
 +}

I think these helper functions will have some general use beyond EDMA
and can be kept in drivers/of/base.c. Grant/Rob need to agree though.

 diff --git a/arch/arm/mach-davinci/board-da830-evm.c 
 b/arch/arm/mach-davinci/board-da830-evm.c
 index 95b5e10..ffcbec1 100644
 --- a/arch/arm/mach-davinci/board-da830-evm.c
 +++ b/arch/arm/mach-davinci/board-da830-evm.c
 @@ -512,7 +512,7 @@ static struct davinci_i2c_platform_data 
 da830_evm_i2c_0_pdata = {
   * example: Timer, GPIO, UART events etc) on da830/omap-l137 EVM, hence
   * they are being reserved for codecs on the DSP side.
   */
 -static const s16 da830_dma_rsv_chans[][2] = {
 +static s16 da830_dma_rsv_chans[][2] = {

I wonder why you had to remove const here and in other places. You seem
to be allocating new memory for DT case anyway. Its also not a good idea
to modify the passed platform data.

Thanks,
Sekhar
--
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: [RFC PATCH v3 05/16] ARM: edma: add AM33XX crossbar event support

2012-10-28 Thread Sekhar Nori
On 10/18/2012 6:56 PM, Matt Porter wrote:
 Adds support for the per-EDMA channel event mux. This is required
 for any peripherals using DMA crossbar mapped events.
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/common/edma.c |   63 
 +++-
  include/linux/platform_data/edma.h |1 +
  2 files changed, 63 insertions(+), 1 deletion(-)
 
 diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
 index 6d2a590..b761b7a 100644
 --- a/arch/arm/common/edma.c
 +++ b/arch/arm/common/edma.c
 @@ -1425,6 +1425,53 @@ static int edma_of_read_u32_to_s16_array(const struct 
 device_node *np,
   return 0;
  }
  
 +static int edma_xbar_event_map(struct device *dev,
 +struct device_node *node,
 +struct edma_soc_info *pdata, int len)
 +{
 + int ret = 0;
 + int i;
 + struct resource res;
 + void *xbar;
 + s16 (*xbar_chans)[2];
 + u32 shift, offset, mux;
 +
 + xbar_chans = devm_kzalloc(dev,
 +   len/sizeof(s16) + 2*sizeof(s16),
 +   GFP_KERNEL);
 + if (!xbar_chans)
 + return -ENOMEM;
 +
 + ret = of_address_to_resource(node, 1, res);
 + if (IS_ERR_VALUE(ret))
 + return -EIO;
 +
 + xbar = devm_ioremap(dev, res.start, resource_size(res));
 + if (!xbar)
 + return -EIO;

-ENOMEM is more appropiate for ioremap failures.

 +
 + ret = edma_of_read_u32_to_s16_array(node,
 + ti,edma-xbar-event-map,
 + (s16 *)xbar_chans,
 + len/sizeof(u32));
 + if (IS_ERR_VALUE(ret))
 + return -EIO;
 +
 + for (i = 0; xbar_chans[i][0] != -1; i++) {
 + shift = (xbar_chans[i][1] % 4) * 8;
 + offset = xbar_chans[i][1]  2;
 + offset = 2;
 + mux = __raw_readl((void *)((u32)xbar + offset));

Dont use __raw* variants of io accessors. There will be ordering issues
on ARMv7.

 + mux = (~(0xff  shift));
 + mux |= (xbar_chans[i][0]  shift);

Unnecessary parens above.

 + __raw_writel(mux, (void *)((u32)xbar + offset));
 + }
 +
 + pdata-xbar_chans = xbar_chans;
 +
 + return 0;
 +}
 +
  static int edma_of_parse_dt(struct device *dev,
   struct device_node *node,
   struct edma_soc_info *pdata)
 @@ -1453,7 +1500,6 @@ static int edma_of_parse_dt(struct device *dev,
   pdata-n_slot = value;
  
   pdata-n_cc = 1;
 - /* This is unused */

The comment should have not been part of 4/16?

Thanks,
Sekhar
--
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: [RFC PATCH v3 08/16] ARM: dts: add AM33XX EDMA support

2012-10-28 Thread Sekhar Nori
On 10/18/2012 6:56 PM, Matt Porter wrote:
 Adds AM33XX EDMA support to the am33xx.dtsi as documented in
 Documentation/devicetree/bindings/dma/ti-edma.txt
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/boot/dts/am33xx.dtsi |   31 +++
  1 file changed, 31 insertions(+)
 
 diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
 index bb31bff..ab9c78f 100644
 --- a/arch/arm/boot/dts/am33xx.dtsi
 +++ b/arch/arm/boot/dts/am33xx.dtsi
 @@ -62,6 +62,37 @@
   reg = 0x4820 0x1000;
   };
  
 + edma: edma@4900 {
 + compatible = ti,edma3;
 + ti,hwmods = tpcc, tptc0, tptc1, tptc2;
 + reg =   0x4900 0x1,
 + 0x44e10f90 0x10;
 + interrupt-parent = intc;
 + interrupts = 12 13 14;
 + #dma-cells = 1;
 + dma-channels = 64;
 + ti,edma-regions = 4;
 + ti,edma-slots = 256;
 + ti,edma-reserved-channels = 0  2
 +  14 2
 +  26 6
 +  48 4
 +  56 8;
 + ti,edma-reserved-slots = 0  2
 +   14 2
 +   26 6
 +   48 4
 +   56 8
 +   64 127;

No need to reserve any channels or slots on AM335x, I think. This is
used on DaVinci devices to share channels with DSP. I am not sure the
cortex-M3 or PRU on the AM335x need to (or even can) have EDMA access.

Thanks,
Sekhar
--
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: [RFC PATCH v3 16/16] ARM: dts: add AM33XX SPI support

2012-10-28 Thread Sekhar Nori
On 10/18/2012 6:56 PM, Matt Porter wrote:
 Adds AM33XX SPI support for am335x-bone and am335x-evm.
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/boot/dts/am335x-bone.dts |   17 +++
  arch/arm/boot/dts/am335x-evm.dts  |9 
  arch/arm/boot/dts/am33xx.dtsi |   43 
 +
  3 files changed, 69 insertions(+)
 
 diff --git a/arch/arm/boot/dts/am335x-bone.dts 
 b/arch/arm/boot/dts/am335x-bone.dts
 index 5510979..23edfd8 100644
 --- a/arch/arm/boot/dts/am335x-bone.dts
 +++ b/arch/arm/boot/dts/am335x-bone.dts
 @@ -18,6 +18,17 @@
   reg = 0x8000 0x1000; /* 256 MB */
   };
  
 + am3358_pinmux: pinmux@44e10800 {
 + spi1_pins: pinmux_spi1_pins {
 + pinctrl-single,pins = 
 + 0x190 0x13  /* mcasp0_aclkx.spi1_sclk, 
 OUTPUT_PULLUP | MODE3 */
 + 0x194 0x33  /* mcasp0_fsx.spi1_d0, 
 INPUT_PULLUP | MODE3 */
 + 0x198 0x13  /* mcasp0_axr0.spi1_d1, 
 OUTPUT_PULLUP | MODE3 */
 + 0x19c 0x13  /* mcasp0_ahclkr.spi1_cs0, 
 OUTPUT_PULLUP | MODE3 */
 + ;

Is there a single pinmux setting that provides SPI functionality on the
bone headers? Or this is specific to a cape you tested with?

Thanks,
Sekhar
--
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: [RFC PATCH v3 11/16] mmc: omap_hsmmc: limit max_segs with the EDMA DMAC

2012-10-29 Thread Sekhar Nori
On 10/18/2012 6:56 PM, Matt Porter wrote:
 The EDMA DMAC has a hardware limitation that prevents supporting
 scatter gather lists with any number of segments. Since the EDMA
 DMA Engine driver sets the maximum segments to 16, we do the
 same.
 
 TODO: this will be replaced once the DMA Engine API supports an
 API to query the DMAC's segment size limit.
 
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  drivers/mmc/host/omap_hsmmc.c |   10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
 index b327cd0..52bab01 100644
 --- a/drivers/mmc/host/omap_hsmmc.c
 +++ b/drivers/mmc/host/omap_hsmmc.c
 @@ -1828,6 +1828,16 @@ static int __devinit omap_hsmmc_probe(struct 
 platform_device *pdev)
* as we want. */
   mmc-max_segs = 1024;
  
 + /* Eventually we should get our max_segs limitation for EDMA by
 +  * querying the dmaengine API */

Nit picking: This is not as per multi-line comment style in
Documentation/CodingStyle.

Thanks,
Sekhar
--
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 v2] rtc: davinci: clean up probe/remove routines

2012-10-14 Thread Sekhar Nori
On 10/14/2012 3:13 PM, Hannu Heikkinen wrote:
 Use the devres managed resource functions in the probe routine.
 Also affects the remove routine where the previously used free and
 release functions are not needed.
 
 The devm_* functions eliminate the need for manual resource releasing and
 simplify error handling.  Resources allocated by devm_* are freed
 automatically on driver detach.
 
 Signed-off-by: Hannu Heikkinen hann...@iki.fi
 ---
  drivers/rtc/rtc-davinci.c | 56 
 +--
  1 file changed, 15 insertions(+), 41 deletions(-)
 
 diff --git a/drivers/rtc/rtc-davinci.c b/drivers/rtc/rtc-davinci.c
 index 14c2109..d24b573 100644
 --- a/drivers/rtc/rtc-davinci.c
 +++ b/drivers/rtc/rtc-davinci.c
 @@ -119,8 +119,6 @@ static DEFINE_SPINLOCK(davinci_rtc_lock);
  struct davinci_rtc {
   struct rtc_device   *rtc;
   void __iomem*base;
 - resource_size_t pbase;
 - size_t  base_size;
   int irq;
  };
  
 @@ -482,22 +480,16 @@ static int __init davinci_rtc_probe(struct 
 platform_device *pdev)
  {
   struct device *dev = pdev-dev;
   struct davinci_rtc *davinci_rtc;
 - struct resource *res, *mem;
 + struct resource *res;
   int ret = 0;
  
 - davinci_rtc = kzalloc(sizeof(struct davinci_rtc), GFP_KERNEL);
 + davinci_rtc = devm_kzalloc(pdev-dev, sizeof(struct davinci_rtc),
 + GFP_KERNEL);
   if (!davinci_rtc) {
   dev_dbg(dev, could not allocate memory for private data\n);
   return -ENOMEM;
   }
  
 - davinci_rtc-irq = platform_get_irq(pdev, 0);
 - if (davinci_rtc-irq  0) {
 - dev_err(dev, no RTC irq\n);
 - ret = davinci_rtc-irq;
 - goto fail1;
 - }
 -
   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   if (!res) {
   dev_err(dev, no mem resource\n);
 @@ -505,23 +497,16 @@ static int __init davinci_rtc_probe(struct 
 platform_device *pdev)
   goto fail1;

As mentioned last time, this will have a build break here because fail1
is being removed down below.

Thanks,
Sekhar
--
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 v2 09/13] ARM: davinci - update the dm644x soc code to use common clk drivers

2012-10-15 Thread Sekhar Nori
Hi Murali,

On 10/15/2012 9:21 PM, Karicheri, Muralidharan wrote:
 --Cut
 
 Subject: Re: [PATCH v2 09/13] ARM: davinci - update the dm644x soc code to 
 use
 common clk drivers

 You have chosen to keep all clock related data in platform files
 while using the common clock framework to provide just the
 infrastructure. If you look at how mxs and spear have been migrated, 
 they have
 migrated the soc specific clock data to drivers/clk as well.
 See drivers/clk/spear/spear3xx_clock.c or
 drivers/clk/mxs/clk-imx23.c

 I have to disagree on this one. I had investigated these code already
 and came up with a way that we can re-use code across all of the
 davinci platforms as well as other architectures that re-uses the clk 
 hardware IPs.

 Which code you are talking about here? Even if you introduce clk-dm644x.c, 
 clk-
 keystone.c etc in drivers/clk/davinci/ you can reuse the code you introduce 
 in patches 1-
 3. I cant see how that will be prevented.
 
 I was talking about re-use of davinci_common_clk_init in 
 drivers/clk/davinci/davinci-clock.c.
 This is meant to be re-used across all of the DaVinci devices.
 

 spear3xx_clock.c has initialization code for each of the platforms and
 so is the case with imx23.c.

 By each of the platforms, you mean they all cater to a family of devices? 
 This depends on
 how close together the family of devices are.
 Otherwise, there would be a file per soc. DM644x also represents a family 
 for that matter.

 By using platform_data approach, we are able to define clks for each of 
 the SoC and
 then use davinci_common_clk_init() to do initialize the clk drivers based 
 on platform
 data.

 You need to define and register the clocks present on each SoC either which 
 way. I don't
 see why just the platform_data approach allows this.
 And looking closely, you have defined platform data, but don't actually 
 have a platform
 device, making things more confusing.

 
 Ok. There are multiple ways to implement this software. We had discussed this
 internally and picked the platform_data approach. The clk drivers are written 
 not
 following the platform driver model. But I don't see why we can't use 
 platform data
 to configure this drivers. Down below, you have made two interesting points, 
 one is
 ARM code reduction. This patch already does this by moving the API that 
 initializes
 the clk drivers (davinci_common_clk_init()) out of ARM to 
 drivers/clk/davinci. So
 this + removal of existing clk driver under arm/mach-davinci/clock.[ch], we 
 have
 achieved this goal. The second point is the moving of SoC specific clk data 
 out of SoC
 code to drive. Are you 100% sure this is the right thing to do for these 
 drivers. If so,
 I can start working on this change right away. As I am working on this as a 
 background
 activity, I want to be double or triple sure before doing the rework of these 
 patches :).
 So please confirm.

Yes, this is the right way to go. And I don't see it as something
breaking new ground since there are already multiple SoCs in mainline
which are following this same approach. May be to start with just
convert one SoC and send for review.

Thanks for taking this up and helping clean-up mach-davinci.

Regards,
Sekhar
--
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 v5] ARM: davinci: da850: add EHRPWM ECAP DT node

2013-04-15 Thread Sekhar Nori
On 4/10/2013 5:42 PM, Philip Avinash wrote:
 Add da850 EHRPWM  ECAP DT node along with pin-mux details.
 Also adds OF_DEV_AUXDATA for EHRPWM  ECAP driver to use EHRPWM  ECAP
 clock.
 
 Signed-off-by: Philip Avinash avinashphi...@ti.com

Looks good to me. Will try and see if this can make it to v3.10.

Thanks,
Sekhar
--
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 v3 0/6] Generic PHY Framework

2013-04-19 Thread Sekhar Nori
Hi Kishon,

On 3/20/2013 2:41 PM, Kishon Vijay Abraham I wrote:
 Added a generic PHY framework that provides a set of APIs for the PHY drivers
 to create/destroy a PHY and APIs for the PHY users to obtain a reference to
 the PHY with or without using phandle. To obtain a reference to the PHY
 without using phandle, the platform specfic intialization code (say from board
 file) should have already called phy_bind with the binding information. The
 binding information consists of phy's device name, phy user device name and an
 index. The index is used when the same phy user binds to mulitple phys.
 
 This framework will be of use only to devices that uses external PHY (PHY
 functionality is not embedded within the controller).

From a top level what you are doing looks closely related to External
connector (extcon).

I understand a connector is not the same as a phy, but it will still be
useful to know why extcon framework (or some extension of it) will not
suffice your needs.

You can probably note it in the cover-letter so folks like me get their
answer.

Thanks,
Sekhar
--
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 v3 0/3] Platform support for EHRPWM ECAP devices in DAVINCI.

2013-04-03 Thread Sekhar Nori
On 4/4/2013 10:09 AM, Philip, Avinash wrote:
 On Tue, Apr 02, 2013 at 14:05:09, Nori, Sekhar wrote:
 On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add platform support for EHRPWM and ECAP by providing clock nodes and
 device tree nodes.
 This series depends on [1] and [2] and is available for testing at [3].
 Tested for back light support in da850 EVM with EHRPWM PWM device.

 [1] 
 http://gitorious.org/linux-davinci/linux-davinci/trees/davinci-for-v3.9/dt-2
 [2] https://gitorious.org/linux-pwm/linux-pwm/trees/for-next
 [3] 
 https://github.com/avinashphilip/am335x_linux/tree/davinci-for-v3.9_soc_pwm

 Note:
 DT support for EHRPWM backlight has not been added in da850-evm.dts as 
 there is
 conflicting pin-mux requirement with SPI flash.

 Can you check if this is really true even in newer boards (have a look
 at the latest schematics)? I remember this used to be a problem in very
 early versions but was fixed later.
 
 On looking schematics, panel has three power controls LCD_BACKLIGHT_PWR, 
 LCD_PANEL_PWR,
 LCD_PWM0. On latest schematic, LCD_PWM0 is connected to ECAP instance 2. So 
 backlight
 can control through ECAP2 (not conflicting with SPI1 cs0). Still for 
 controlling
 backlight, require support for LCD_BACKLIGHT_PWR  LCD_PANEL_PWR. These 
 signals

By controlling above you mean switching on/off? Otherwise this seems
to be contradicting the statement just before.

 to be controlled by GPIO 2[8]  GPIO 2[15]. In release platform callbacks used
 to control GPIO functionality. But with DT support, I have to check how 
 platform
 callbacks can be used.

Platform callbacks are not possible with DT. You can look at what
freescale mxsfb.c does. Look for panel-enable-gpios DT binding.

Thanks,
Sekhar
--
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 v7 1/2] ARM: davinci: dm355: add support for v4l2 video display

2013-04-04 Thread Sekhar Nori
On 4/2/2013 6:45 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 Create platform devices for various video modules like venc,osd,
 vpbe and v4l2 driver for dm355.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  arch/arm/mach-davinci/board-dm355-evm.c |4 +-
  arch/arm/mach-davinci/davinci.h |3 +-
  arch/arm/mach-davinci/dm355.c   |  173 
 ++-
  3 files changed, 172 insertions(+), 8 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/board-dm355-evm.c 
 b/arch/arm/mach-davinci/board-dm355-evm.c
 index 147b8e1..37d12cc 100644
 --- a/arch/arm/mach-davinci/board-dm355-evm.c
 +++ b/arch/arm/mach-davinci/board-dm355-evm.c
 @@ -253,8 +253,6 @@ static struct davinci_uart_config uart_config __initdata 
 = {
  
  static void __init dm355_evm_map_io(void)
  {
 - /* setup input configuration for VPFE input devices */
 - dm355_set_vpfe_config(vpfe_cfg);
   dm355_init();
  }
  
 @@ -344,6 +342,8 @@ static __init void dm355_evm_init(void)
   davinci_setup_mmc(0, dm355evm_mmc_config);
   davinci_setup_mmc(1, dm355evm_mmc_config);
  
 + dm355_init_video(vpfe_cfg, NULL);
 +
   dm355_init_spi0(BIT(0), dm355_evm_spi_info,
   ARRAY_SIZE(dm355_evm_spi_info));
  
 diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
 index a9de512..5402592 100644
 --- a/arch/arm/mach-davinci/davinci.h
 +++ b/arch/arm/mach-davinci/davinci.h
 @@ -39,6 +39,7 @@
  #define SYSMOD_VDAC_CONFIG   0x2c
  #define SYSMOD_VIDCLKCTL 0x38
  #define SYSMOD_VPSS_CLKCTL   0x44
 +#define VPSS_MUXSEL_EXTCLK_ENABLEBIT(1)
  #define VPSS_VENCCLKEN_ENABLEBIT(3)
  #define VPSS_DACCLKEN_ENABLE BIT(4)
  #define VPSS_PLLC2SYSCLK5_ENABLE BIT(5)
 @@ -79,7 +80,7 @@ void __init dm355_init(void);
  void dm355_init_spi0(unsigned chipselect_mask,
   const struct spi_board_info *info, unsigned len);
  void __init dm355_init_asp1(u32 evt_enable, struct snd_platform_data *pdata);
 -void dm355_set_vpfe_config(struct vpfe_config *cfg);
 +int __init dm355_init_video(struct vpfe_config *, struct vpbe_config *);

Why do this especially after sending a big patch cleaning __init
annotation in function declaration all over mach-davinci?

  
  /* DM365 function declarations */
  void __init dm365_init(void);
 diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
 index 8e98bb0..174ad02 100644
 --- a/arch/arm/mach-davinci/dm355.c
 +++ b/arch/arm/mach-davinci/dm355.c
 @@ -36,6 +36,10 @@
  
  #define DM355_UART2_BASE (IO_PHYS + 0x206000)
  
 +#define DM355_OSD_BASE   0x01c70200
 +

Drop the empty line here and after DM355_UART2_BASE. And be consistent
and add new definitions as (IO_PHYS + N)?

 +#define DM355_VENC_BASE  0x01c70400
 +
  /*
   * Device specific clocks
   */
 @@ -744,11 +748,150 @@ static struct platform_device vpfe_capture_dev = {
   },
  };
  
 -void dm355_set_vpfe_config(struct vpfe_config *cfg)
 +static struct resource dm355_osd_resources[] = {
 + {
 + .start  = DM355_OSD_BASE,
 + .end= DM355_OSD_BASE + 0x17f,
 + .flags  = IORESOURCE_MEM,
 + },
 +};
 +
 +static struct platform_device dm355_osd_dev = {
 + .name   = DM355_VPBE_OSD_SUBDEV_NAME,
 + .id = -1,
 + .num_resources  = ARRAY_SIZE(dm355_osd_resources),
 + .resource   = dm355_osd_resources,
 + .dev= {
 + .dma_mask   = vpfe_capture_dma_mask,
 + .coherent_dma_mask  = DMA_BIT_MASK(32),
 + },
 +};
 +
 +static struct resource dm355_venc_resources[] = {
 + {
 + .start  = IRQ_VENCINT,
 + .end= IRQ_VENCINT,
 + .flags  = IORESOURCE_IRQ,
 + },
 + /* venc registers io space */
 + {
 + .start  = DM355_VENC_BASE,
 + .end= DM355_VENC_BASE + 0x17f,
 + .flags  = IORESOURCE_MEM,
 + },
 + /* VDAC config register io space */
 + {
 + .start  = DAVINCI_SYSTEM_MODULE_BASE + SYSMOD_VDAC_CONFIG,
 + .end= DAVINCI_SYSTEM_MODULE_BASE + SYSMOD_VDAC_CONFIG + 3,
 + .flags  = IORESOURCE_MEM,
 + },
 +};
 +
 +static struct resource dm355_v4l2_disp_resources[] = {
 + {
 + .start  = IRQ_VENCINT,
 + .end= IRQ_VENCINT,
 + .flags  = IORESOURCE_IRQ,
 + },
 + /* venc registers io space */
 + {
 + .start  = DM355_VENC_BASE,
 + .end= DM355_VENC_BASE + 0x17f,
 + .flags  = IORESOURCE_MEM,
 + },
 +};
 +
 +static int dm355_vpbe_setup_pinmux(enum v4l2_mbus_pixelcode if_type,
 + int field)
  {
 - vpfe_capture_dev.dev.platform_data = cfg;
 + switch (if_type) {
 + case V4L2_MBUS_FMT_SGRBG8_1X8:
 + 

Re: [PATCH v7 2/2] ARM: davinci: dm355 EVM: add support for VPBE display

2013-04-04 Thread Sekhar Nori
On 4/2/2013 6:45 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 add support for V4L2 video display to DM355 EVM.
 Support for SD modes is provided, along with Composite
 output
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Looks good. Since you mentioned that you will take this through media
tree to manage driver dependencies, please add:

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
--
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 v8 1/2] ARM: davinci: dm365: add support for v4l2 video display

2013-04-04 Thread Sekhar Nori
On 4/2/2013 5:24 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 Create platform devices for various video modules like venc,osd,
 vpbe and v4l2 driver for dm365.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Minor nits below:

 diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
 index c61dd94..786c860 100644
 --- a/arch/arm/mach-davinci/dm365.c
 +++ b/arch/arm/mach-davinci/dm365.c
 @@ -40,10 +40,14 @@
  
  #define DM365_REF_FREQ   2400/* 24 MHz on the DM365 
 EVM */
  
 +#define DM365_RTC_BASE   0x01c69000
 +
  /* Base of key scan register bank */
  #define DM365_KEYSCAN_BASE   0x01c69400
  
 -#define DM365_RTC_BASE   0x01c69000
 +#define DM365_OSD_BASE   0x01c71c00
 +
 +#define DM365_VENC_BASE  0x01c71e00
  
  #define DAVINCI_DM365_VC_BASE0x01d0c000

No need of empty lines between these definitions. While at it you can
also remove the useless comment /* Base of key scan register bank */

 +static int dm365_vpbe_setup_pinmux(enum v4l2_mbus_pixelcode if_type,
 + int field)
 +{
 + switch (if_type) {
 + case V4L2_MBUS_FMT_SGRBG8_1X8:
 + davinci_cfg_reg(DM365_VOUT_FIELD_G81);
 + davinci_cfg_reg(DM365_VOUT_COUTL_EN);
 + davinci_cfg_reg(DM365_VOUT_COUTH_EN);
 + break;
 +

No need of these empty lines after 'break'. Here and other places below.

The patch looks good overall though so you can fix the nits and add:

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
--
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 v8 2/2] ARM: davinci: dm365 EVM: add support for VPBE display

2013-04-04 Thread Sekhar Nori
On 4/2/2013 5:24 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 add support for V4L2 video display to DM365 EVM.
 Support for SD and ED modes is provided, along with Composite
 and Component outputs.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
--
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 v2 1/3] media: davinci: vpss: enable vpss clocks

2013-04-08 Thread Sekhar Nori
On 4/2/2013 5:14 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 By default the VPSS clocks were enabled in capture driver
 for davinci family which creates duplicates for dm355/dm365/dm644x.
 This patch adds support to enable the VPSS clocks in VPSS driver,
 which avoids duplication of code and also adding clock aliases.
 
 This patch uses PM runtime API to enable/disable instead common clock
 framework. con_ids for master and slave clocks of vpss is added in pm_domain

Common clock framework in not (yet) used on DaVinci, so this is misleading.

 diff --git a/arch/arm/mach-davinci/pm_domain.c 
 b/arch/arm/mach-davinci/pm_domain.c
 index c90250e..445b10b 100644
 --- a/arch/arm/mach-davinci/pm_domain.c
 +++ b/arch/arm/mach-davinci/pm_domain.c
 @@ -53,7 +53,7 @@ static struct dev_pm_domain davinci_pm_domain = {
  
  static struct pm_clk_notifier_block platform_bus_notifier = {
   .pm_domain = davinci_pm_domain,
 - .con_ids = { fck, NULL, },
 + .con_ids = { fck, master, slave, NULL, },

NULL is sentinel so you can drop the ',' after that. Apart from that,
for the mach-davinci parts:

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
--
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 v8 0/2] ARM: davinci: dm355: add support for vpbe display

2013-04-08 Thread Sekhar Nori
On 4/8/2013 2:56 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 This patch series enables VPBE display driver on DM355.

These (this and the DM365 one) patches look good to me. I need to get an
immutable branch from Mauro where dependencies are queued and then I can
generate a pull request for these for ARM SoC.

Thanks,
Sekhar

--
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 v8 0/2] ARM: davinci: dm355: add support for vpbe display

2013-04-08 Thread Sekhar Nori
Hi Maruo,

On 4/8/2013 4:26 PM, Mauro Carvalho Chehab wrote:
 Hi Sekhar,
 
 Em Mon, 8 Apr 2013 16:06:24 +0530
 Sekhar Nori nsek...@ti.com escreveu:
 
 On 4/8/2013 2:56 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 This patch series enables VPBE display driver on DM355.

 These (this and the DM365 one) patches look good to me. I need to get an
 immutable branch from Mauro where dependencies are queued and then I can
 generate a pull request for these for ARM SoC.
 
 Are you mean a branch at the media development tree for you to sent
 pull requests for me? If so, just use the master branch at the media

The pull request will be sent to my upstreams (ie the ARM SoC folks -
Arnd and Olof). Since the platform data patches need driver patches to
be applied first, I need a non-rebasing branch containing the driver
stuff which I can use a dependency and apply platform patches on top.

 tree:
   http://git.linuxtv.org/media_tree.git
 
 The master branch there is never rebased. I'll likely start to have
 topic branches on the next Kernel cycle, also at the same tree.

Ideally I will use the immutable branch which you will also use to send
pull request to Linus so there are not unnecessary merge conflicts. When
will those branches be ready?

Thanks,
Sekhar
--
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 v2 1/3] media: davinci: vpss: enable vpss clocks

2013-04-08 Thread Sekhar Nori


On 4/8/2013 5:08 PM, Prabhakar Lad wrote:
 Sekhar,
 
 On Mon, Apr 8, 2013 at 3:56 PM, Sekhar Nori nsek...@ti.com wrote:
 On 4/2/2013 5:14 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 By default the VPSS clocks were enabled in capture driver
 for davinci family which creates duplicates for dm355/dm365/dm644x.
 This patch adds support to enable the VPSS clocks in VPSS driver,
 which avoids duplication of code and also adding clock aliases.

 This patch uses PM runtime API to enable/disable instead common clock
 framework. con_ids for master and slave clocks of vpss is added in pm_domain

 Common clock framework in not (yet) used on DaVinci, so this is misleading.

 OK, I'll make it 'This patch uses PM runtime API to enable/disable
 clock, instead
 of Davinci specific clock framework. con_ids for master and slave

may be just call it DaVinci clock framework

Thanks,
Sekhar
--
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 v3 3/3] ARM: davinci: da850: add EHRPWM ECAP DT node

2013-04-08 Thread Sekhar Nori

On 4/8/2013 2:39 PM, Philip, Avinash wrote:
 On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote:
 On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add da850 EHRPWM  ECAP DT node.
 Also adds OF_DEV_AUXDATA for EHRPWM  ECAP driver to use EHRPWM  ECAP
 clock.

 This looks fine to me but I will wait for the bindings to get accepted
 before taking this one.
 
 Sekhar,
 
 Binding document got accepted in PWM tree [1].
 Can you accept this patch?

Can you also add the pinmux definitions and resend just this patch?
Sorry I did not notice those were missing earlier.

Thanks,
Sekhar
--
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 v3 2/3] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver

2013-04-09 Thread Sekhar Nori


On 4/8/2013 5:49 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 The vpss clocks were enabled by calling a exported function from a driver
 in a machine code. calling driver code from platform code is incorrect way.
 
 This patch fixes this issue and calls the function from driver code itself.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/media/platform/davinci/vpbe_venc.c |   25 +
  1 files changed, 25 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/media/platform/davinci/vpbe_venc.c 
 b/drivers/media/platform/davinci/vpbe_venc.c
 index f15f211..91d0272 100644
 --- a/drivers/media/platform/davinci/vpbe_venc.c
 +++ b/drivers/media/platform/davinci/vpbe_venc.c
 @@ -202,6 +202,25 @@ static void venc_enabledigitaloutput(struct v4l2_subdev 
 *sd, int benable)
   }
  }
  
 +static void
 +venc_enable_vpss_clock(int venc_type,
 +enum vpbe_enc_timings_type type,
 +unsigned int pclock)
 +{
 + if (venc_type == VPBE_VERSION_1)
 + return;
 +
 + if (venc_type == VPBE_VERSION_2  (type == VPBE_ENC_STD ||
 + (type == VPBE_ENC_DV_TIMINGS  pclock = 2700))) {

checkpatch --strict will throw a Alignment should match open
parenthesis check here. You may want to fix before you send the pull
request. No need to resend the patch just for this.

Thanks,
Sekhar
--
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 v8 0/2] ARM: davinci: dm355: add support for vpbe display

2013-04-09 Thread Sekhar Nori
On 4/8/2013 6:26 PM, Mauro Carvalho Chehab wrote:
 Em Mon, 8 Apr 2013 17:17:34 +0530
 Sekhar Nori nsek...@ti.com escreveu:
 
 Hi Maruo,

 On 4/8/2013 4:26 PM, Mauro Carvalho Chehab wrote:
 Hi Sekhar,

 Em Mon, 8 Apr 2013 16:06:24 +0530
 Sekhar Nori nsek...@ti.com escreveu:

 On 4/8/2013 2:56 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 This patch series enables VPBE display driver on DM355.

 These (this and the DM365 one) patches look good to me. I need to get an
 immutable branch from Mauro where dependencies are queued and then I can
 generate a pull request for these for ARM SoC.

 Are you mean a branch at the media development tree for you to sent
 pull requests for me? If so, just use the master branch at the media

 The pull request will be sent to my upstreams (ie the ARM SoC folks -
 Arnd and Olof). Since the platform data patches need driver patches to
 be applied first, I need a non-rebasing branch containing the driver
 stuff which I can use a dependency and apply platform patches on top.
 
 Hmm... I generally wait for arch patches to be applied first, before
 sending driver patches. If you're willing to do that, I suggest that
 Arnd/Olof should put those patches on a separate topic branch, and
 sending an upstream pull request for them after mine.

The patches can be put on a separate branch, but the branch wont build
without the driver changes (eg need defintion of VPBE_ENC_DV_TIMINGS
from include/media/davinci/vpbe_types.h).

So this will need a immutable branch from you to depend on so Linus's
tree won't have duplicate commits when Arnd/Olof send their pull request.

 
 If the patches are trivial, maybe the better would be to put both
 media and arm patches at the same tree, with the other maintainers'
 ack.

This seems to be the easiest path at the moment. I will ack the platform
parts and Prabhakar can issue a pull request for you. I have just
checked that these patches won't cause a merge conflict with anything
already queued through my tree.

Thanks,
Sekhar
--
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 v3 3/3] ARM: davinci: da850: add EHRPWM ECAP DT node

2013-04-09 Thread Sekhar Nori
On 4/9/2013 2:12 PM, Philip, Avinash wrote:
 On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote:

 On 4/8/2013 2:39 PM, Philip, Avinash wrote:
 On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote:
 On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add da850 EHRPWM  ECAP DT node.
 Also adds OF_DEV_AUXDATA for EHRPWM  ECAP driver to use EHRPWM  ECAP
 clock.

 This looks fine to me but I will wait for the bindings to get accepted
 before taking this one.

 Sekhar,

 Binding document got accepted in PWM tree [1].
 Can you accept this patch?

 Can you also add the pinmux definitions and resend just this patch?
 Sorry I did not notice those were missing earlier.
 
 According to latest schematics, ECAP instance 2 being used for PWM backlight
 control. Should I add pin-mux only for ECAP2 or for all PWM instances?

I meant add definitions in .dtsi. Since there is only one pin a given
functionality can be present on in DaVinci, it can be done in a board
independent manner. See examples for other peripherals in existing
da850.dtsi file.

Thanks,
Sekhar
--
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 v9 1/2] ARM: davinci: dm365: add support for v4l2 video display

2013-04-09 Thread Sekhar Nori
On 4/8/2013 2:47 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 Create platform devices for various video modules like venc,osd,
 vpbe and v4l2 driver for dm365.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Acked-by: Sekhar Nori nsek...@ti.com
 ---

 diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
 index 12d544b..a9de512 100644
 --- a/arch/arm/mach-davinci/davinci.h
 +++ b/arch/arm/mach-davinci/davinci.h
 @@ -36,8 +36,13 @@
  #include media/davinci/vpbe_osd.h
  
  #define DAVINCI_SYSTEM_MODULE_BASE   0x01c4
 +#define SYSMOD_VDAC_CONFIG   0x2c
  #define SYSMOD_VIDCLKCTL 0x38
  #define SYSMOD_VPSS_CLKCTL   0x44
 +#define VPSS_VENCCLKEN_ENABLEBIT(3)
 +#define VPSS_DACCLKEN_ENABLE BIT(4)
 +#define VPSS_PLLC2SYSCLK5_ENABLE BIT(5)
 +

Prabhakar, I noticed it only after I applied these patches, but these
bit definitions in between list of register offsets is distracting. Can
you please move them down after the register offsets when you send the
pull request? You can add a comment /* VPSS CLKCTL bit definitions */
before you start the bit definitions.

  #define SYSMOD_VDD3P3VPWDN   0x48
  #define SYSMOD_VSCLKDIS  0x6c
  #define SYSMOD_PUPDCTL1  0x7c

Thanks,
Sekhar
--
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 v3 3/3] ARM: davinci: da850: add EHRPWM ECAP DT node

2013-04-09 Thread Sekhar Nori
On 4/10/2013 11:00 AM, Philip, Avinash wrote:
 On Tue, Apr 09, 2013 at 17:05:25, Nori, Sekhar wrote:
 On 4/9/2013 2:12 PM, Philip, Avinash wrote:
 On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote:

 On 4/8/2013 2:39 PM, Philip, Avinash wrote:
 On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote:
 On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add da850 EHRPWM  ECAP DT node.
 Also adds OF_DEV_AUXDATA for EHRPWM  ECAP driver to use EHRPWM  ECAP
 clock.

 This looks fine to me but I will wait for the bindings to get accepted
 before taking this one.

 Sekhar,

 Binding document got accepted in PWM tree [1].
 Can you accept this patch?

 Can you also add the pinmux definitions and resend just this patch?
 Sorry I did not notice those were missing earlier.

 According to latest schematics, ECAP instance 2 being used for PWM backlight
 control. Should I add pin-mux only for ECAP2 or for all PWM instances?

 I meant add definitions in .dtsi. Since there is only one pin a given
 functionality can be present on in DaVinci, it can be done in a board
 independent manner.
 
 I think here the expectation would be that .dtsi should populate the complete
 pin-mux for SOC and board files should just be able to re-use it (add it as a 
 phandler).

Yes, that's the idea.

 Also as per the above description .dtsi file will end up contain majorly 
 pin-mux info
 rather than the hardware data. Is it a good idea?

Pinmux is also hardware data, no? Thats why its present in DT.

 On looking da850.dtsi file NAND pins were defined for 8-bit part. 
 In case of NAND flash, the device might be sitting under different 
 chip-select or may
 have 16 bit part on  different boards. So pin-mux defined in soc.dtsi has to 
 be split
 separately for CS, DATA, Address.

The idea is to define pin groups that most of the time can be reused by
.dts file as-is and if there are any board specific extra pins needed
then they can be handled directly in .dts files. But the common cases
don't have to be repeated in all boards. In case of NAND, CS and the top
8-pins when using a 16-bit bus can be moved to a different group. So, I
agree instead of nand_cs3_pins, we could have had nand_pins and moved cs
definitions to another re-usable group.

 So it is always challenging to create pin-mux info in .dtsi file. So more 
 useful/meaningful
 way is to actually create pin-mux in board file rather in .dtsi file.

I don't see why it is so challenging. Repeating the same pinmux
information over multiple .dts file (while making errors copying) will
be challenging. And its not as if this is my original idea. imx (and I
think some others) are doing it as well. See how pinmux is defined in
imx53.dtsi and reused in a number of boards like evk, qsb, smd and so on.

 See examples for other peripherals in existing
 da850.dtsi file.
 
 I have gone through .dtsi. But it didn't describe the complete pin-mux like 
 I2C1, MMC1, etc.

pinmux should be added for whatever nodes are added since pimux is part
of node.

 So the expectation here is only to add ECAP2 pin-mux. Is it correct?

No, please add pinmux information for all the IP nodes you are adding. I
am not insisting that you add all IP nodes at the same time. You can add
whatever you have tested.

Thanks,
Sekhar
--
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 v4] ARM: davinci: da850: add EHRPWM ECAP DT node

2013-04-10 Thread Sekhar Nori
Avinash,

On 4/10/2013 1:32 PM, Philip Avinash wrote:
 Add da850 EHRPWM  ECAP DT node along with pin-mux details for EHRPWM1.
 Also adds OF_DEV_AUXDATA for EHRPWM  ECAP driver to use EHRPWM  ECAP
 clock.
 
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 ---
 Changes since v3:
   - add pin mux info for EHRPWM1.

I think you misunderstood what I asked. Please add pinmux information
for all the nodes you are adding (ecap0-2 and ehrpwm0). Without pinmux
setup, these IPs cannot be used anyway. So why leave just the pinmux to
be added by someone else?

Thanks,
Sekhar

 
 Changes since v1:
   - Reusing ti,am33xxecap/ehrpwm as compatible field as both IP's are
 similar with am33xx platform and da850 has no platform specific
 dependency.
 
  arch/arm/boot/dts/da850.dtsi |   42 
 ++
  arch/arm/mach-davinci/da8xx-dt.c |5 +
  2 files changed, 47 insertions(+)
 
 diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
 index 3ade343..3bff30d 100644
 --- a/arch/arm/boot/dts/da850.dtsi
 +++ b/arch/arm/boot/dts/da850.dtsi
 @@ -71,6 +71,18 @@
   0x28 0x0022  0x00ff
   ;
   };
 + ehrpwm1a_pins: pinmux_ehrpwm1a_pins {
 + pinctrl-single,bits = 
 + /* EPWM1A */
 + 0x14 0x0002 0x000f
 + ;
 + };
 + ehrpwm1b_pins: pinmux_ehrpwm1b_pins {
 + pinctrl-single,bits = 
 + /* EPWM1B */
 + 0x14 0x0020 0x00f0
 + ;
 + };
   };
   serial0: serial@1c42000 {
   compatible = ns16550a;
 @@ -122,6 +134,36 @@
   interrupts = 16;
   status = disabled;
   };
 + ehrpwm0: ehrpwm@01f0 {
 + compatible = ti,da850-ehrpwm, ti,am33xx-ehrpwm;
 + #pwm-cells = 3;
 + reg = 0x30 0x2000;
 + status = disabled;
 + };
 + ehrpwm1: ehrpwm@01f02000 {
 + compatible = ti,da850-ehrpwm, ti,am33xx-ehrpwm;
 + #pwm-cells = 3;
 + reg = 0x302000 0x2000;
 + status = disabled;
 + };
 + ecap0: ecap@01f06000 {
 + compatible = ti,da850-ecap, ti,am33xx-ecap;
 + #pwm-cells = 3;
 + reg = 0x306000 0x80;
 + status = disabled;
 + };
 + ecap1: ecap@01f07000 {
 + compatible = ti,da850-ecap, ti,am33xx-ecap;
 + #pwm-cells = 3;
 + reg = 0x307000 0x80;
 + status = disabled;
 + };
 + ecap2: ecap@01f08000 {
 + compatible = ti,da850-ecap, ti,am33xx-ecap;
 + #pwm-cells = 3;
 + reg = 0x308000 0x80;
 + status = disabled;
 + };
   };
   nand_cs3@6200 {
   compatible = ti,davinci-nand;
 diff --git a/arch/arm/mach-davinci/da8xx-dt.c 
 b/arch/arm/mach-davinci/da8xx-dt.c
 index d83de8f..05bb9a2 100644
 --- a/arch/arm/mach-davinci/da8xx-dt.c
 +++ b/arch/arm/mach-davinci/da8xx-dt.c
 @@ -41,6 +41,11 @@ struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
   OF_DEV_AUXDATA(ti,davinci-i2c, 0x01c22000, i2c_davinci.1, NULL),
   OF_DEV_AUXDATA(ti,davinci-wdt, 0x01c21000, watchdog, NULL),
   OF_DEV_AUXDATA(ti,da830-mmc, 0x01c4, da830-mmc.0, NULL),
 + OF_DEV_AUXDATA(ti,da850-ehrpwm, 0x01f0, ehrpwm, NULL),
 + OF_DEV_AUXDATA(ti,da850-ehrpwm, 0x01f02000, ehrpwm, NULL),
 + OF_DEV_AUXDATA(ti,da850-ecap, 0x01f06000, ecap, NULL),
 + OF_DEV_AUXDATA(ti,da850-ecap, 0x01f07000, ecap, NULL),
 + OF_DEV_AUXDATA(ti,da850-ecap, 0x01f08000, ecap, NULL),
   {}
  };
  
 
--
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 v8 1/2] ARM: davinci: dm355: add support for v4l2 video display

2013-04-12 Thread Sekhar Nori
On 4/8/2013 2:56 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 Create platform devices for various video modules like venc,osd,
 vpbe and v4l2 driver for dm355.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

For patches 1/2 and 2/2:

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
--
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] ARM: daVinci: dm644x/dm355/dm365: replace V4L2_STD_525_60/625_50 with V4L2_STD_NTSC/PAL

2013-04-12 Thread Sekhar Nori


On 4/12/2013 4:35 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 This patch replaces V4L2_STD_525_60/625_50 with V4L2_STD_NTSC/PAL
 respectively as this are the proper video standards.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Reported-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Sekhar Nori nsek...@ti.com

I assume you have tested it, some testing information would have been
good. The patch as such is OK with me. I assume this will go through
media tree for sake of dependencies.

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar

 ---
  Note:- This patch is on top of following pull
   https://patchwork.linuxtv.org/patch/17888/
 
  arch/arm/mach-davinci/board-dm355-evm.c  |4 ++--
  arch/arm/mach-davinci/board-dm365-evm.c  |4 ++--
  arch/arm/mach-davinci/board-dm644x-evm.c |4 ++--
  3 files changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/board-dm355-evm.c 
 b/arch/arm/mach-davinci/board-dm355-evm.c
 index 1043506..886481c 100644
 --- a/arch/arm/mach-davinci/board-dm355-evm.c
 +++ b/arch/arm/mach-davinci/board-dm355-evm.c
 @@ -247,7 +247,7 @@ static struct vpbe_enc_mode_info 
 dm355evm_enc_preset_timing[] = {
   {
   .name   = ntsc,
   .timings_type   = VPBE_ENC_STD,
 - .std_id = V4L2_STD_525_60,
 + .std_id = V4L2_STD_NTSC,
   .interlaced = 1,
   .xres   = 720,
   .yres   = 480,
 @@ -259,7 +259,7 @@ static struct vpbe_enc_mode_info 
 dm355evm_enc_preset_timing[] = {
   {
   .name   = pal,
   .timings_type   = VPBE_ENC_STD,
 - .std_id = V4L2_STD_625_50,
 + .std_id = V4L2_STD_PAL,
   .interlaced = 1,
   .xres   = 720,
   .yres   = 576,
 diff --git a/arch/arm/mach-davinci/board-dm365-evm.c 
 b/arch/arm/mach-davinci/board-dm365-evm.c
 index 0518ce5..2a66743 100644
 --- a/arch/arm/mach-davinci/board-dm365-evm.c
 +++ b/arch/arm/mach-davinci/board-dm365-evm.c
 @@ -381,7 +381,7 @@ static struct vpbe_enc_mode_info 
 dm365evm_enc_std_timing[] = {
   {
   .name   = ntsc,
   .timings_type   = VPBE_ENC_STD,
 - .std_id = V4L2_STD_525_60,
 + .std_id = V4L2_STD_NTSC,
   .interlaced = 1,
   .xres   = 720,
   .yres   = 480,
 @@ -393,7 +393,7 @@ static struct vpbe_enc_mode_info 
 dm365evm_enc_std_timing[] = {
   {
   .name   = pal,
   .timings_type   = VPBE_ENC_STD,
 - .std_id = V4L2_STD_625_50,
 + .std_id = V4L2_STD_PAL,
   .interlaced = 1,
   .xres   = 720,
   .yres   = 576,
 diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c 
 b/arch/arm/mach-davinci/board-dm644x-evm.c
 index a021800..745280d 100644
 --- a/arch/arm/mach-davinci/board-dm644x-evm.c
 +++ b/arch/arm/mach-davinci/board-dm644x-evm.c
 @@ -622,7 +622,7 @@ static struct vpbe_enc_mode_info 
 dm644xevm_enc_std_timing[] = {
   {
   .name   = ntsc,
   .timings_type   = VPBE_ENC_STD,
 - .std_id = V4L2_STD_525_60,
 + .std_id = V4L2_STD_NTSC,
   .interlaced = 1,
   .xres   = 720,
   .yres   = 480,
 @@ -634,7 +634,7 @@ static struct vpbe_enc_mode_info 
 dm644xevm_enc_std_timing[] = {
   {
   .name   = pal,
   .timings_type   = VPBE_ENC_STD,
 - .std_id = V4L2_STD_625_50,
 + .std_id = V4L2_STD_PAL,
   .interlaced = 1,
   .xres   = 720,
   .yres   = 576,
 
--
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] davinci: vpif: add pm_runtime support

2013-04-01 Thread Sekhar Nori
On 3/28/2013 3:50 PM, Prabhakar Lad wrote:
 Hi Laurent,
 
 On Thu, Mar 28, 2013 at 3:40 PM, Laurent Pinchart
 laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 On Thursday 28 March 2013 15:36:11 Prabhakar Lad wrote:
 On Thu, Mar 28, 2013 at 2:39 PM, Laurent Pinchart wrote:
 On Thursday 28 March 2013 14:20:32 Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 Add pm_runtime support to the TI Davinci VPIF driver.
 Along side this patch replaces clk_get() with devm_clk_get()
 to simplify the error handling.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---

  drivers/media/platform/davinci/vpif.c |   21 +++--
  1 files changed, 11 insertions(+), 10 deletions(-)

 diff --git a/drivers/media/platform/davinci/vpif.c
 b/drivers/media/platform/davinci/vpif.c index 28638a8..7d14625 100644
 --- a/drivers/media/platform/davinci/vpif.c
 +++ b/drivers/media/platform/davinci/vpif.c

 [snip]

 @@ -439,12 +440,17 @@ static int vpif_probe(struct platform_device *pdev)
   goto fail;
   }

 - vpif_clk = clk_get(pdev-dev, vpif);
 + vpif_clk = devm_clk_get(pdev-dev, vpif);
   if (IS_ERR(vpif_clk)) {
   status = PTR_ERR(vpif_clk);
   goto clk_fail;
   }

 - clk_prepare_enable(vpif_clk);
 + clk_put(vpif_clk);

 Why do you need to call clk_put() here ?

 The above check is to see if the clock is provided, once done
 we free it using clk_put().

 In that case you shouldn't use devm_clk_get(), otherwise clk_put() will be
 called again automatically at remove() time.

 Yes agreed it should be clk_get() only.
 
 + pm_runtime_enable(pdev-dev);
 + pm_runtime_resume(pdev-dev);
 +
 + pm_runtime_get(pdev-dev);

 Does runtime PM automatically handle your clock ? If so can't you remove
 clock handling from the driver completely ?

 Yes  pm runtime take care of enabling/disabling the clocks
 so that we don't have to do it in drivers. I believe clock
 handling is removed with this patch, with just  devm_clk_get() remaining ;)

 When is the clk_get() call expected to fail ? If the clock is provided by the
 SoC and always available, can't the check be removed completely ?

 Yes I agree with you it can be removed completely assuming the clock
 is always available from the Soc.
 But may be I need feedback from others Hans/Sekhar what do you suggest ?

Unless you need the clk handle to get the clock rate or something, you
should simply rely on runtime PM calls to enable clocks for you and not
have any clk API call at all in your driver.

Thanks,
Sekhar
--
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] ARM: davinci: use is IS_ENABLED macro

2013-04-01 Thread Sekhar Nori
On 3/28/2013 10:07 AM, Prabhakar Lad wrote:
 Hi Sekhar,
 
 On Wed, Mar 27, 2013 at 4:13 PM, Stephen Rothwell s...@canb.auug.org.au 
 wrote:
 Hi,

 On Mon, 25 Mar 2013 16:51:35 +0530 Prabhakar lad 
 prabhakar.cse...@gmail.com wrote:

 --- a/arch/arm/mach-davinci/board-da830-evm.c
 +++ b/arch/arm/mach-davinci/board-da830-evm.c
 @@ -298,7 +298,7 @@ static const short da830_evm_emif25_pins[] = {
   -1
  };

 -#if defined(CONFIG_MMC_DAVINCI) || defined(CONFIG_MMC_DAVINCI_MODULE)
 +#if IS_ENABLED(CONFIG_MMC_DAVINCI)
  #define HAS_MMC  1
  #else
  #define HAS_MMC  0

 Why not

 #define HAS_MMC IS_ENABLED(CONFIG_MMC_DAVINCI)

 and similarly in the rest?

 Stephen's suggestion looks good to me, if you haven’t queued it i'll post a 
 v2,
 or if you have already queued it I'll create a patch on top of the
 same patch fixing it.
 lemme know how would you want it.

Please post a v2. I will take that instead.

Thanks,
Sekhar
--
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 v3 2/3] arm: davinci: clock node support for ECAP EHRPWM

2013-04-02 Thread Sekhar Nori
On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add clock node support for ECAP and EHRPWM modules.
 Also adds TBCLK for EHRWPM TBCLK to comply with pwm-tiehrpwm
 driver.
 
 Signed-off-by: Philip Avinash avinashphi...@ti.com

Queuing this for v3.10

Thanks,
Sekhar
--
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 v3 3/3] ARM: davinci: da850: add EHRPWM ECAP DT node

2013-04-02 Thread Sekhar Nori
On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add da850 EHRPWM  ECAP DT node.
 Also adds OF_DEV_AUXDATA for EHRPWM  ECAP driver to use EHRPWM  ECAP
 clock.

This looks fine to me but I will wait for the bindings to get accepted
before taking this one.

Thanks,
Sekhar
--
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 v3 0/3] Platform support for EHRPWM ECAP devices in DAVINCI.

2013-04-02 Thread Sekhar Nori
On 3/25/2013 1:19 PM, Philip Avinash wrote:
 Add platform support for EHRPWM and ECAP by providing clock nodes and
 device tree nodes.
 This series depends on [1] and [2] and is available for testing at [3].
 Tested for back light support in da850 EVM with EHRPWM PWM device.
 
 [1] 
 http://gitorious.org/linux-davinci/linux-davinci/trees/davinci-for-v3.9/dt-2
 [2] https://gitorious.org/linux-pwm/linux-pwm/trees/for-next
 [3] 
 https://github.com/avinashphilip/am335x_linux/tree/davinci-for-v3.9_soc_pwm
 
 Note:
 DT support for EHRPWM backlight has not been added in da850-evm.dts as there 
 is
 conflicting pin-mux requirement with SPI flash.

Can you check if this is really true even in newer boards (have a look
at the latest schematics)? I remember this used to be a problem in very
early versions but was fixed later.

Thanks,
Sekhar
--
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 v2] ARM: davinci: use is IS_ENABLED macro

2013-04-02 Thread Sekhar Nori
On 4/1/2013 12:43 PM, Prabhakar lad wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 This patches replaces #if defined() by IS_ENABLED macro, which provides
 better readability.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Cc: Sekhar Nori nsek...@ti.com
 Cc: Stephen Rothwell s...@canb.auug.org.au
 Cc: Randy Dunlap rdun...@infradead.org

I dropped earlier version of this patch and queued this one instead.

Thanks,
Sekhar
--
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] ARM: davinci: remove test for undefined Kconfig macro

2013-04-02 Thread Sekhar Nori
On 3/29/2013 6:03 PM, Paul Bolle wrote:
 [Dropped Kevin Hilman, because Kevin's address bounces.]

Use khil...@deeprootsystems.com as updated in MAINTAINERS.

 
 On Fri, 2013-03-29 at 12:47 +0100, Paul Bolle wrote:
 The DaVinci debugging macro contains a check for
 CONFIG_DEBUG_DAVINCI_DA8XX_UART0. But there's corresponding Kconfig
 
   [...] there's no corresponding [...]
 
 symbol, so this test will always evaluate to false. That Kconfig symbol
 is not needed because, as __arch_decomp_setup() shows, there are no
 DaVinci DA8XX boards that use UART0 for debugging. We can remove two
 lines of unneeded code.

Queuing this for v3.10 with updated commit text indicated above.

Thanks,
Sekhar
--
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 v9 0/9] DMA Engine support for AM33XX

2013-04-03 Thread Sekhar Nori
Hi Matt,

On 3/6/2013 9:45 PM, Matt Porter wrote:

 This series adds DMA Engine support for AM33xx, which uses
 an EDMA DMAC. The EDMA DMAC has been previously supported by only
 a private API implementation (much like the situation with OMAP
 DMA) found on the DaVinci family of SoCs.

 Matt Porter (9):
   ARM: davinci: move private EDMA API to arm/common
   ARM: edma: remove unused transfer controller handlers

If you are going to be late in re-spinning the entire series in time for
v3.10, then I suggest I queue the first two patches through davinci tree
for v3.10 and then you can work on rest of patches for next (v3.11)
merge window.

Let me know what you think.

   ARM: edma: add AM33XX support to the private EDMA API
   dmaengine: edma: enable build for AM33XX
   dmaengine: edma: Add TI EDMA device tree binding
   ARM: dts: add AM33XX EDMA support
   spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
   spi: omap2-mcspi: add generic DMA request support to the DT binding
   ARM: dts: add AM33XX SPI DMA support

Thanks,
Sekhar
--
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 v5 2/2] ARM: davinci: dm365 EVM: add support for VPBE display

2013-03-14 Thread Sekhar Nori
On 3/14/2013 2:09 PM, Prabhakar Lad wrote:
 Hi Sekhar,
 
 On Tue, Dec 4, 2012 at 6:52 PM, Sekhar Nori nsek...@ti.com wrote:
 On 12/3/2012 1:51 PM, Prabhakar Lad wrote:
 From: Manjunath Hadli manjunath.ha...@ti.com

 add support for V4L2 video display to DM365 EVM.
 Support for SD and ED modes is provided, along with Composite
 and Component outputs.

 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 ---
  arch/arm/mach-davinci/board-dm365-evm.c |  177 
 ++-
  1 files changed, 176 insertions(+), 1 deletions(-)

 diff --git a/arch/arm/mach-davinci/board-dm365-evm.c 
 b/arch/arm/mach-davinci/board-dm365-evm.c
 index 0c3dae6..10f2a85 100644
 --- a/arch/arm/mach-davinci/board-dm365-evm.c
 +++ b/arch/arm/mach-davinci/board-dm365-evm.c
 @@ -27,6 +27,7 @@
  #include linux/input.h
  #include linux/spi/spi.h
  #include linux/spi/eeprom.h
 +#include linux/v4l2-dv-timings.h

  #include asm/mach-types.h
  #include asm/mach/arch.h
 @@ -374,6 +375,180 @@ static struct vpfe_config vpfe_cfg = {
   .ccdc = ISIF,
  };

 +/* venc standards timings */
 +static struct vpbe_enc_mode_info dm365evm_enc_std_timing[] = {
 + {
 + .name   = ntsc,
 + .timings_type   = VPBE_ENC_STD,
 + .std_id = V4L2_STD_525_60,
 + .interlaced = 1,
 + .xres   = 720,
 + .yres   = 480,
 + .aspect = {11, 10},
 + .fps= {3, 1001},
 + .left_margin= 0x79,
 + .right_margin   = 0,
 + .upper_margin   = 0x10,
 + .lower_margin   = 0,
 + .hsync_len  = 0,
 + .vsync_len  = 0,
 + .flags  = 0,

 I wonder what makes this entire information board specific? Shouldn't
 these (or at least most of these) be same across all devices which
 support NTSC and hence should be coming from some common code instead of
 being replicated for each platform that supports NTSC?

 most of the structure members are board specific,
 
   struct v4l2_fract aspect;
   struct v4l2_fract fps;
   unsigned int left_margin;
   unsigned int right_margin;
   unsigned int upper_margin;
   unsigned int lower_margin;
   unsigned int hsync_len;
   unsigned int vsync_len;
   unsigned int flags;
 
 And as of now there is no ready made structure within v4l2 which
 provides the common data. So I'll be keeping this as it is.

I am fine with this approach.

Regards,
Sekhar
--
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 1/3] pwm: davinci: Add Kconfig support for ECAP EHRPWM devices

2013-03-14 Thread Sekhar Nori
On 3/14/2013 4:02 PM, Philip Avinash wrote:
 Add EHRPWM and ECAP support build support for DAVINCI_DA850 platforms.
 
 Also, since DAVINCI platforms doesn't support TI-PWM-Subsystem module,
 remove the select option for CONFIG_PWM_TIPWMSS.
 
 Also, update CONFIG_PWM_TIPWMSS compiler directive appropriately in
 pwm-tipwmss.h to fix the below compiler error upon removal of
 CONFIG_PWM_TIPWMSS for Davinci platforms.
 
   drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_probe':
   drivers/pwm/pwm-tiecap.c:263:4: error: 'PWMSS_ECAPCLK_EN' undeclared
   (first use in this function)
   drivers/pwm/pwm-tiecap.c:263:4: note: each undeclared identifier
   is reported only once for each function it appears in
   drivers/pwm/pwm-tiecap.c:264:17: error: 'PWMSS_ECAPCLK_EN_ACK'
   undeclared (first use in this function)
   drivers/pwm/pwm-tiecap.c: In function 'ecap_pwm_remove':
   drivers/pwm/pwm-tiecap.c:291:49: error: 'PWMSS_ECAPCLK_STOP_REQ'
   undeclared (first use in this function)
   make[2]: *** [drivers/pwm/pwm-tiecap.o] Error 1
   make[1]: *** [drivers/pwm] Error 2
   make: *** [drivers] Error 2
 
 Signed-off-by: Philip Avinash avinashphi...@ti.com

  config  PWM_TIECAP
   tristate ECAP PWM support
 - depends on SOC_AM33XX
 - select PWM_TIPWMSS
 + depends on SOC_AM33XX || ARCH_DAVINCI_DA850

Having such narrow dependencies is wrong. The same device is present on
DaVinci DA830 too. A depends on should not be required at all since the
driver should build on all architectures. But I have seen resistance to
doing that since users don't like to see configuration options totally
irrelevant for the architecture they are building for. So may be take a
middle path and do 'depends on ARCH_ARM'?

Thanks,
Sekhar
--
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 2/3] pwm: pwm-tiecap: Add device-tree binding support for da850 SOC

2013-03-14 Thread Sekhar Nori
On 3/14/2013 4:02 PM, Philip Avinash wrote:
 ECAP IP is used in da850 SOC's also. Hence adds ECAP device tree binding
 support for da850.
 
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: Rob Landley r...@landley.net
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 ---
 :100644 100644 131e8c1... fcbd3c1... M
 Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
 :100644 100644 22e96e2... e0d96c8... Mdrivers/pwm/pwm-tiecap.c
  .../devicetree/bindings/pwm/pwm-tiecap.txt |2 +-
  drivers/pwm/pwm-tiecap.c   |1 +
  2 files changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt 
 b/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
 index 131e8c1..fcbd3c1 100644
 --- a/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
 +++ b/Documentation/devicetree/bindings/pwm/pwm-tiecap.txt
 @@ -1,7 +1,7 @@
  TI SOC ECAP based APWM controller
  
  Required properties:
 -- compatible: Must be ti,am33xx-ecap
 +- compatible: Must be ti,am33xx-ecap or ti,da850-ecap
  - #pwm-cells: Should be 3. Number of cells being used to specify PWM 
 property.
First cell specifies the per-chip index of the PWM to use, the second
cell is the period in nanoseconds and bit 0 in the third cell is used to
 diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
 index 22e96e2..e0d96c8 100644
 --- a/drivers/pwm/pwm-tiecap.c
 +++ b/drivers/pwm/pwm-tiecap.c
 @@ -197,6 +197,7 @@ static const struct pwm_ops ecap_pwm_ops = {
  
  static const struct of_device_id ecap_of_match[] = {
   { .compatible   = ti,am33xx-ecap },
 + { .compatible   = ti,da850-ecap },
   {},

You add a new compatible, but don't really show any changes in driver in
this series. So why can't we simply use ti,am33xx-ecap on DA850 too?

Thanks,
Sekhar
--
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 1/3] arm: davinci: clock node support for ECAP EHRPWM

2013-03-14 Thread Sekhar Nori

On 3/14/2013 4:07 PM, Philip Avinash wrote:
 Add clock node support for ECAP and EHRPWM modules.
 Also adds dummy clock for EHRWPM TBCLK to comply with pwm-tiehrpwm
 driver.

This is not right. So the version of IP used on AM335x uses a TBCLK and
that's absent on the version used on DA850? If yes, the driver should
never request clock on DA850.

Thanks,
Sekhar
--
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/


  1   2   3   4   5   6   7   8   9   10   >