RE: [PATCH 2/2] can:ti_hecc: board specific hookup on AM3517EVM

2010-04-30 Thread Govindarajan, Sriramakrishnan
> -Original Message-
> From: Govindarajan, Sriramakrishnan
> Sent: Tuesday, February 23, 2010 12:08 PM
> To: linux-omap@vger.kernel.org
> Cc: Gole, Anant; Govindarajan, Sriramakrishnan
> Subject: [PATCH 2/2] can:ti_hecc: board specific hookup on AM3517EVM
> 
> Add board specific hookup for TI HECC driver on
> AM3517 EVM
> 
> Signed-off-by: Sriramakrishnan 
> Acked-by: Anant Gole 
> ---
> The driver requires that CAN_STB signal be driven low to enable
> CAN PHY. Currently this is being managed from U-boot. Will submit a patch
> for handling this as part of board init sequence.
> 
>  arch/arm/mach-omap2/board-am3517evm.c |   38
> +
>  arch/arm/mach-omap2/include/mach/am35xx.h |   10 +++
>  2 files changed, 48 insertions(+), 0 deletions(-)
> 
[Sriram] Tony, there hasn't been any review comments on this patch for long. Do 
you any comments on this patch? If not can please apply this patch

Regards
Sriram
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCHv3 1/3] TCA6416 keypad : Implement keypad driver for keys interfaced to TCA6416

2010-04-30 Thread Govindarajan, Sriramakrishnan
> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Friday, April 16, 2010 10:55 AM
> To: Govindarajan, Sriramakrishnan
> Cc: linux-omap@vger.kernel.org; linux-in...@vger.kernel.org
> Subject: Re: [PATCHv3 1/3] TCA6416 keypad : Implement keypad driver for
> keys interfaced to TCA6416
> 
> On Tue, Mar 23, 2010 at 08:40:33PM +0530, Sriramakrishnan wrote:
> > This patch implements a simple Keypad driver that functions
> > as an I2C client. It handles key press events for keys
> > connected to TCA6416 I2C based IO expander.
> >
> > Signed-off-by: Sriramakrishnan 
> > ---
> >  drivers/input/keyboard/Kconfig  |   16 ++
> >  drivers/input/keyboard/Makefile |1 +
> >  drivers/input/keyboard/tca6416-keypad.c |  354
> +++
> >  include/linux/tca6416_keypad.h  |   34 +++
> >  4 files changed, 405 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/input/keyboard/tca6416-keypad.c
> >  create mode 100644 include/linux/tca6416_keypad.h
snip
Does the driver still work if you aplly the patch below on top?
[Sriram] Dmitry, I was able to test with the patch below(again scope restricted 
to Polling mode) and it works but with following minor changes. Please review.
> 
> Thanks.
> 
> --
> Dmitry
> 
> Input: tca6416 - misc fixes
> 
> Signed-off-by: Dmitry Torokhov 
> ---
> 
>  drivers/input/keyboard/tca6416-keypad.c |  327 +++---
> -
>  1 files changed, 160 insertions(+), 167 deletions(-)
> 
> 
> diff --git a/drivers/input/keyboard/tca6416-keypad.c
> b/drivers/input/keyboard/tca6416-keypad.c
> index 17df832..0943af3 100644
> --- a/drivers/input/keyboard/tca6416-keypad.c
> +++ b/drivers/input/keyboard/tca6416-keypad.c
> @@ -10,16 +10,17 @@
>   * published by the Free Software Foundation.
>   */
---snip---

> +static int tca6416_keys_open(struct input_dev *dev)
> +{
> + struct tca6416_keypad_chip *chip = input_get_drvdata(dev);
> +
> + /* Get initial device state in case it has switches */
> + tca6416_keys_scan(chip);
> 
>   if (chip->use_polling)
>   schedule_delayed_work(&chip->dwork, msecs_to_jiffies(100));
>   else
>   enable_irq(chip->irqnum);
> 
> + return 0;
> +}
> +
> +static void tca6416_keys_close(struct input_dev *dev)
> +{
> + struct tca6416_keypad_chip *chip = input_get_drvdata(dev);
> +
> + if (chip->use_polling)
> + cancel_delayed_work_sync(&chip->dwork);
> + else
> + free_irq(chip->irqnum, chip);
[Sriram] replaced free_irq() with disable_irq() instead. This should take care 
of multiple (concurrent) opens.

>  }
> 
> +static int __devinit tca6416_setup_registers(struct tca6416_keypad_chip
> *chip)
> +{
> + int error;
> +
> + error = tca6416_read_reg(chip, TCA6416_OUTPUT, &chip->reg_output);
> + if (error)
> + return error;
> +
> + error = tca6416_read_reg(chip, TCA6416_DIRECTION, &chip-
> >reg_direction);
> + if (error)
> + return error;
> +
> + /* ensure that keypad pins are set to input */
> + error = tca6416_write_reg(chip, TCA6416_DIRECTION,
> +   chip->reg_direction | chip->pinmask);
> + if (error)
> + return error;
> +
> + error = tca6416_read_reg(chip, TCA6416_DIRECTION, &chip-
> >reg_direction);
> + if (error)
> + return error;
> +
> + error = tca6416_read_reg(chip, TCA6416_INPUT, &chip->reg_input);
> + if (error)
> + return error;

[Sriram] reg_input cached value needs to be masked with pinmask - otherwise on 
device open the driver reports continuous stream of key_down(repeat) events 
until a key is pressed.

> + return 0;
> +}
> 
[Sriram] Dmitry, once you have reviewed the changes, should I re-post the patch 
series with your patch added to the series?

Regards
Sriram
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 06/12] AM3517: Add nand platform data for am3517evm

2010-04-14 Thread Govindarajan, Sriramakrishnan
> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Stanley.Miao
> Sent: Wednesday, April 14, 2010 2:00 PM
> To: linux-omap@vger.kernel.org
> Cc: t...@atomide.com
> Subject: [PATCH 06/12] AM3517: Add nand platform data for am3517evm
> 
> Add nand flash init code for am3517evm.
> 
> Signed-off-by: Stanley.Miao 
> ---
>  arch/arm/mach-omap2/board-am3517evm.c |   84
> +
>  1 files changed, 84 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-
> omap2/board-am3517evm.c
> index 879c13f..a76ff11 100644
> --- a/arch/arm/mach-omap2/board-am3517evm.c
> +++ b/arch/arm/mach-omap2/board-am3517evm.c
> @@ -23,6 +23,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> 
>  #include 
>  #include 
> @@ -35,6 +38,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> 
>  #include "mux.h"
> 
> @@ -42,6 +47,84 @@
>  #define LCD_PANEL_BKLIGHT_PWR182
>  #define LCD_PANEL_PWM181
> 
> +#define GPMC_CS0_BASE  0x60
> +#define GPMC_CS_SIZE   0x30
> +
> +#define NAND_BLOCK_SIZESZ_128K
> +
> +static struct mtd_partition am3517evm_nand_partitions[] = {
> + /* All the partition sizes are listed in terms of NAND block size */
> + {
> + .name   = "xloader",
> + .offset = 0,
> + .size   = 4 * (SZ_128K),
> + .mask_flags = MTD_WRITEABLE
> + },
> + {
> + .name   = "uboot",
> + .offset = MTDPART_OFS_APPEND,
> + .size   = 14 * (SZ_128K),
> + .mask_flags = MTD_WRITEABLE
> + },
> + {
> + .name   = "uboot-params",
> + .offset = MTDPART_OFS_APPEND,
> + .size   = 2 * (SZ_128K)
> + },
> + {
> + .name   = "linux-kernel",
> + .offset = MTDPART_OFS_APPEND,
> + .size   = 40 * (SZ_128K)
> + },
> + {
> + .name   = "rootfs",
> + .size   = MTDPART_SIZ_FULL,
> + .offset = MTDPART_OFS_APPEND,
> + },
> +};
--- snip ---
[Sriram] Similar patch was posted earlier - pending re-work (see Tony's 
comments)
https://patchwork.kernel.org/patch/56485/

If you deriving your work from existing implementation, attribute the original 
author in your submissions 

Regards
Sriram
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 02/12] AM3517: Add platform init code for EMAC driver

2010-04-14 Thread Govindarajan, Sriramakrishnan
> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Stanley.Miao
> Sent: Wednesday, April 14, 2010 2:00 PM
> To: linux-omap@vger.kernel.org
> Cc: t...@atomide.com
> Subject: [PATCH 02/12] AM3517: Add platform init code for EMAC driver
> 
> Add platform init code for EMAC driver.
> 
> Signed-off-by: Stanley.Miao 
> ---
>  arch/arm/mach-omap2/board-am3517evm.c |  114
> +
>  arch/arm/mach-omap2/include/mach/am35xx.h |9 ++
>  2 files changed, 123 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-
> omap2/board-am3517evm.c
> index 6ae8805..db542b2 100644
> --- a/arch/arm/mach-omap2/board-am3517evm.c
> +++ b/arch/arm/mach-omap2/board-am3517evm.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  #include 
> @@ -30,6 +31,7 @@
> 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 

-- snip --

This patch has been posted earlier. Refer,
https://patchwork.kernel.org/patch/84921/
https://patchwork.kernel.org/patch/84919/

Tony,
Any updates on these

Regards
Sriram


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCHv3 0/3] Add support for TCA6416 based Keypad driver.

2010-04-01 Thread Govindarajan, Sriramakrishnan


> -Original Message-
> From: Govindarajan, Sriramakrishnan
> Sent: Tuesday, March 23, 2010 8:41 PM
> To: linux-omap@vger.kernel.org; linux-in...@vger.kernel.org
> Cc: Govindarajan, Sriramakrishnan
> Subject: [PATCHv3 0/3] Add support for TCA6416 based Keypad driver.
> 
> AM3517 EVM with APPS board includes keys interfaced to TCA6416 IO expander
> User keys are connected as GPIO lines to TCA6416 IO expander. Unlike the
> case with generic gpio-keypad driver individual keys do not generate an
> interrupt event. Hence we implement a simple keypad driver, that
> registers as direct I2C client.
> 
> The implementation has been tested on AM3517 EVM with the driver tested
> in polling mode.
> 
> Version3 is refreshed against the tip of linux-omap and
> file mode issues from the previous version is fixed.
> 
> Sriramakrishnan (3):
>   TCA6416 keypad : Implement keypad driver for keys interfaced to
> TCA6416
>   AM3517: Board hookup for TCA6416 keypad driver.
>   AM3517 EVM : Enable TCA6416 keypad.
> 
>  arch/arm/configs/am3517_evm_defconfig   |   16 ++-
>  arch/arm/mach-omap2/board-am3517evm.c   |   47 -
>  drivers/input/keyboard/Kconfig  |   16 ++
>  drivers/input/keyboard/Makefile |1 +
>  drivers/input/keyboard/tca6416-keypad.c |  354
> +++
>  include/linux/tca6416_keypad.h  |   34 +++
>  6 files changed, 462 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/input/keyboard/tca6416-keypad.c
>  create mode 100644 include/linux/tca6416_keypad.h

[Sriram] All,
Any feedback on this updated patch series?
Thanks
Sriram

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 0/2] Enable CAN peripheral support on AM3517

2010-04-01 Thread Govindarajan, Sriramakrishnan


> -Original Message-
> From: Govindarajan, Sriramakrishnan
> Sent: Friday, February 26, 2010 5:37 PM
> To: linux-omap@vger.kernel.org
> Cc: Gole, Anant; Govindarajan, Sriramakrishnan
> Subject: [PATCH v2 0/2] Enable CAN peripheral support on AM3517
> 
> AM3517 platform includes the ti-hecc CAN peripheral. This patch
> series enables support for the ti-hecc peripheral on AM3517 platform.
> This patch series has been validated on AM3517EVM.
> 
> This patch is generated against tip of linux-omap.
> 
> Sriramakrishnan (2):
>   can:ti_hecc: board specific hookup on AM3517EVM
>   can:ti_hecc: Enable CAN support on AM3517.
> 
>  arch/arm/configs/am3517_evm_defconfig |   22 +---
>  arch/arm/mach-omap2/board-am3517evm.c |   38
> +
>  arch/arm/mach-omap2/include/mach/am35xx.h |9 +++
>  3 files changed, 65 insertions(+), 4 deletions(-)

[Sriram]  Tony, 
Any comments on this patch
Thanks,
Sriram

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCHv2 0/3] Add support for TCA6416 based Keypad driver.

2010-03-12 Thread Govindarajan, Sriramakrishnan


> -Original Message-
> From: Grazvydas Ignotas [mailto:nota...@gmail.com]
> Sent: Friday, March 12, 2010 4:54 PM
> To: Govindarajan, Sriramakrishnan
> Cc: linux-omap@vger.kernel.org; linux-in...@vger.kernel.org
> Subject: Re: [PATCHv2 0/3] Add support for TCA6416 based Keypad driver.
> 
> On Fri, Mar 12, 2010 at 11:18 AM, Sriramakrishnan  wrote:
> > AM3517 EVM with APPS board includes keys interfaced to TCA6416 IO
> expander
> > User keys are connected as GPIO lines to TCA6416 IO expander. Unlike the
> > case with generic gpio-keypad driver individual keys do not generate an
> > interrupt event. Hence we implement a simple keypad driver, that
> > registers as direct I2C client.
> >
> > The implementation has been tested on AM3517 EVM with the driver tested
> > in polling mode.
> >
> > Version2 of the patch series addresses review comments from the earlier
> > posting - specifically redesigned to eliminate overhead of using
> gpio_keys
> > data structures.
> >
> > Sriramakrishnan (3):
> >  TCA6416 keypad : Implement keypad driver for keys interfaced to
> >    TCA6416
> >  AM3517: Board hookup for TCA6416 keypad driver.
> >  AM3517 EVM : Enable TCA6416 keypad.
> >
> >  arch/arm/configs/am3517_evm_defconfig   |   16 ++-
> >  arch/arm/mach-omap2/board-am3517evm.c   |   47 -
> >  drivers/input/keyboard/Kconfig          |   16 ++
> >  drivers/input/keyboard/Makefile         |    1 +
> >  drivers/input/keyboard/tca6416-keypad.c |  354
> +++
> >  include/linux/tca6416_keypad.h          |   34 +++
> >  6 files changed, 462 insertions(+), 6 deletions(-)
> >  mode change 100644 => 100755 arch/arm/mach-omap2/board-am3517evm.c
> >  create mode 100755 drivers/input/keyboard/tca6416-keypad.c
> >  create mode 100755 include/linux/tca6416_keypad.h
> 
> Please fix mode - remove +x flags.
[Sriram] Thanks for pointing out. I will await further review comments
And post the updated version thereafter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/3] TCA6416 keypad : Implement keypad driver for keys interfaced to TCA6416

2010-03-05 Thread Govindarajan, Sriramakrishnan


> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Saturday, February 27, 2010 2:30 PM
> To: Govindarajan, Sriramakrishnan
> Cc: Felipe Balbi; linux-omap@vger.kernel.org; linux-in...@vger.kernel.org
> Subject: Re: [PATCH 1/3] TCA6416 keypad : Implement keypad driver for keys
> interfaced to TCA6416

---snip---
> > > > I see,
> > > >
> > > > shouldn't you instead provide a gpiolib driver for tca6416 and use
> the
> > > > generic gpio_keys driver ??
> > > >
> > >
> > > Right. The fact that the driver precludes all otehr gpios from being
> > > used is a major drawback.
> 
> > [Sriram] gpio_keys implementation assumes that every key can generate
> > an interrupt to the processor and also the state machine to process
> > events will handle each line individually. Imagine if 16 keys are
> > connected to the TCA controller and you have to query(assume it is
> > polling, interrupt mode not possible - as you have single interrupt
> > line to the processor for event on any of the 16 input lines) 16 lines
> > individually over i2c bus - the associated overhead is too high.
> > Instead this implementation handles the necessary book-keeping in one
> > simple function (get status of all 16 lines in one read transaction).
> > Building on a GPIO implementation and using gpio-keys above will be
> > both clumsy and incur runtime overhead as against a direct keypad
> > driver.
> >
> > More often(as on AM3517EVM) the keypad keys are not mixed with other
> > GPIO keys. The initial implementation includes key_mask to identify
> > the keys to be used for keypad and the others are logically not
> > modified. Hence the implementation can be extended if need arises to
> > overcome this restriction.
> 
> In this case you should not build on gpio_keys at all but select some
> other keyboard as an example that smply uses keytable with set of
> keycodes, single interrupt, etc. The overhead of all gpio_keys data
> structures is not needed here.
> 
[Sriram] I am working on cleaning up the driver to use a simpler book keeping 
structure(move away from gpio_keys). Will post the reworked patch shortly.

---snip ---
> 
> > [Sriram] All the core logic (including I2C transactions) is
> > implemented through a work queue implementation. The ISR is slim and
> > only schedules the work queue. This also enables the implementation to
> > be easily adaptable for both interrupt/polled mode.
> >
> 
> Fair enough but you need to ensure that you do not leave irq unbalances
> (in regards to enable/disable) when you using workqueue and disabling
> interrupt in the ISR.

[Sriram] Will ensure that this is addressed appropriately (including cleanup on 
error conditions).

---
Sriram
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/3] TCA6416 keypad : Implement keypad driver for keys interfaced to TCA6416

2010-02-26 Thread Govindarajan, Sriramakrishnan

> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Friday, February 26, 2010 1:58 PM
> To: Felipe Balbi
> Cc: Govindarajan, Sriramakrishnan; linux-omap@vger.kernel.org; linux-
> in...@vger.kernel.org
> Subject: Re: [PATCH 1/3] TCA6416 keypad : Implement keypad driver for keys
> interfaced to TCA6416
> 
> On Thu, Feb 25, 2010 at 08:47:03PM +0200, Felipe Balbi wrote:
> > Hi,
> >
> > On Thu, Feb 25, 2010 at 06:44:59PM +0530, Sriramakrishnan wrote:
> > > This patch implements a simple Keypad driver that functions
> > > as an I2C client. It handles key press events for keys
> > > connected to TCA6416 I2C based IO expander.
> >
> > what's wrong with gpio-keys ??
> >
> > > + * Implementation based on drivers/input/keyboard/gpio_keys.c
> >
> > I see,
> >
> > shouldn't you instead provide a gpiolib driver for tca6416 and use the
> > generic gpio_keys driver ??
> >
> 
> Right. The fact that the driver precludes all otehr gpios from being
> used is a major drawback.
[Sriram] gpio_keys implementation assumes that every key can generate an 
interrupt to the processor and also the state machine to process events will 
handle each line individually. Imagine if 16 keys are connected to the TCA 
controller and you have to query(assume it is polling, interrupt mode not 
possible - as you have single interrupt line to the processor for event on any 
of the 16 input lines) 16 lines individually over i2c bus - the associated 
overhead is too high. Instead this implementation handles the necessary 
book-keeping in one simple function (get status of all 16 lines in one read 
transaction). Building on a GPIO implementation and using gpio-keys above will 
be both clumsy and incur runtime overhead as against a direct keypad driver.

More often(as on AM3517EVM) the keypad keys are not mixed with other GPIO keys. 
The initial implementation includes key_mask to identify the keys to be used 
for keypad and the others are logically not modified. Hence the implementation 
can be extended if need arises to overcome this restriction.
> 
> 
> > > + if (!chip->use_polling) {
> >
> > IMO, you should only use polling if the irq line isn't connected.
> >
> > > + if (pdata->irq_is_gpio)
> > > + chip->irqnum = gpio_to_irq(pdata->irqnum);
> >
> > you can pass the irq number via i2c_board_info. Use it.
> >
> > > + else
> > > + chip->irqnum = pdata->irqnum;
> > > +
> > > + ret = request_irq(chip->irqnum, tca6416_keys_isr,
> >
> > it's an i2c driver!!! this should be request_threaded_irq()
> >
> 
> Threaded IRQ probably does not fit well when you want to support both
> interrupt and polling in the same driver...
[Sriram] All the core logic (including I2C transactions) is implemented through 
a work queue implementation. The ISR is slim and only schedules the work queue. 
This also enables the implementation to be easily adaptable for both 
interrupt/polled mode.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/2] can:ti_hecc: board specific hookup on AM3517EVM

2010-02-26 Thread Govindarajan, Sriramakrishnan

> -Original Message-
> From: Tony Lindgren [mailto:t...@atomide.com]
> Sent: Friday, February 26, 2010 3:34 AM
> To: Govindarajan, Sriramakrishnan
> Cc: linux-omap@vger.kernel.org; Gole, Anant
> Subject: Re: [PATCH 2/2] can:ti_hecc: board specific hookup on AM3517EVM
> 
> * Sriramakrishnan  [100222 22:35]:
> > Add board specific hookup for TI HECC driver on
> > AM3517 EVM
> >
> > Signed-off-by: Sriramakrishnan 
> > Acked-by: Anant Gole 
> > ---
> > The driver requires that CAN_STB signal be driven low to enable
> > CAN PHY. Currently this is being managed from U-boot. Will submit a
> patch
> > for handling this as part of board init sequence.
> >
> >  arch/arm/mach-omap2/board-am3517evm.c |   38
> +
> >  arch/arm/mach-omap2/include/mach/am35xx.h |   10 +++
> >  2 files changed, 48 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-
> omap2/board-am3517evm.c
> > index af99faf..42013b5 100644
> > --- a/arch/arm/mach-omap2/board-am3517evm.c
> > +++ b/arch/arm/mach-omap2/board-am3517evm.c
> > @@ -20,6 +20,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
--- snip 
> > +
> > +static struct ti_hecc_platform_data am3517_evm_hecc_pdata = {
> > +   .scc_hecc_offset= AM35XX_HECC_SCC_HECC_OFFSET,
> > +   .scc_ram_offset = AM35XX_HECC_SCC_RAM_OFFSET,
> > +   .hecc_ram_offset= AM35XX_HECC_RAM_OFFSET,
> > +   .mbx_offset= AM35XX_HECC_MBOX_OFFSET,
> > +   .int_line   = AM35XX_HECC_INT_LINE,
> > +   .version= AM35XX_HECC_VERSION,
> > +};
> 
> The formatting above should use tabs instead of spaces. Please
> check and run checkpatch.pl --strict on this.
> 
> Also, sounds like the Kconfig changes should be 2/2, not 1/2
> to enable this.
> 
> Tony
> 
[Sriram] Tony, I did verify the patch with checkpatch(log below) but 
Somehow, it didn't flag any formatting errors. I realize that there are spaces 
that need to be converted to tabs. I will re-generate the patch and also 
re-order the patches as pointed out.

$scripts/checkpatch.pl --strict 
2-2-can-ti_hecc-board-specific-hookup-on-AM3517EVM.patch
total: 0 errors, 0 warnings, 0 checks, 70 lines checked

2-2-can-ti_hecc-board-specific-hookup-on-AM3517EVM.patch has no obvious style 
problems and is ready for submission.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] ARCH OMAP : enable ARCH_HAS_HOLES_MEMORYMODEL for OMAP

2010-01-11 Thread Govindarajan, Sriramakrishnan
From: Menon, Nishanth
> Sent: Friday, January 08, 2010 8:33 PM
> To: Govindarajan, Sriramakrishnan
> Cc: linux-omap@vger.kernel.org; Syed Mohammed, Khasim; Nori, Sekhar
> Subject: Re: [PATCH] ARCH OMAP : enable ARCH_HAS_HOLES_MEMORYMODEL for
> OMAP
> 
> Govindarajan, Sriramakrishnan had written, on 01/08/2010 02:15 AM, the
> following:
> >
> >> From: Menon, Nishanth
> >> Govindarajan, Sriramakrishnan had written, on 01/07/2010 06:30 AM, the
> >> following:
> >>> From: Sriram 
> >>>
> >>> OMAP platforms(like OMAP3530) include DSP or other co-processors
> >>> for media acceleration. when carving out memory for the
> >>> accelerators we can end up creating a hole in the memory map
> >>> of sort:
> >>> 
> >>>
> >>> To handle such a memory configuration ARCH_HAS_HOLES_MEMORYMODEL
> >>> has to be enabled. For further information refer discussion at:
> >>> http://www.mail-archive.com/linux-o...@vger.kernl.org/msg15262.html.
> >> pls check the link: I see "The document you were looking for was not
> >> found."
> >
> > The URL is spelt incorrectly. Here is the correct URL:
> > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg15262.html
> >
> >>>   select GENERIC_TIME
> >>>   select GENERIC_CLOCKEVENTS
> >>> + select ARCH_HAS_HOLES_MEMORYMODEL
> >> why enable this for all OMAPs?
> >
> > I have tested this on several OMAP3 platforms and this feature is
> required
> > Wherever some memory needs to be reserved for media accelerators - hence
> it
> > Would be handy for earlier OMAP platforms as well
> I understand that you have tested with OMAP3. my question is you have
> handled this as per the diff
> @@ -699,6 +699,7 @@ config ARCH_OMAP
> to ARCH_OMAP
> 
> is this good for OMAP1510, 1610, 1710, 2420,2430, 770 etc..?

I do not have other OMAP platforms for me to validate this patch on.
I believe the nature of change is generic and benefit other OMAP
Platforms as well.

Regards
Sriram

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] ARCH OMAP : enable ARCH_HAS_HOLES_MEMORYMODEL for OMAP

2010-01-08 Thread Govindarajan, Sriramakrishnan


> From: Menon, Nishanth
> Govindarajan, Sriramakrishnan had written, on 01/07/2010 06:30 AM, the
> following:
> > From: Sriram 
> >
> > OMAP platforms(like OMAP3530) include DSP or other co-processors
> > for media acceleration. when carving out memory for the
> > accelerators we can end up creating a hole in the memory map
> > of sort:
> > 
> >
> > To handle such a memory configuration ARCH_HAS_HOLES_MEMORYMODEL
> > has to be enabled. For further information refer discussion at:
> > http://www.mail-archive.com/linux-o...@vger.kernl.org/msg15262.html.
> 
> pls check the link: I see "The document you were looking for was not
> found."

The URL is spelt incorrectly. Here is the correct URL:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg15262.html

> > select GENERIC_TIME
> > select GENERIC_CLOCKEVENTS
> > +   select ARCH_HAS_HOLES_MEMORYMODEL
> why enable this for all OMAPs?

I have tested this on several OMAP3 platforms and this feature is required
Wherever some memory needs to be reserved for media accelerators - hence it
Would be handy for earlier OMAP platforms as well

Regards
Sriram
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] omap3evm: Migrate to smsc911x ethernet driver.

2009-11-11 Thread Govindarajan, Sriramakrishnan
Tony,
Are there comments with respect to this patch. Will this be merged in the next 
window?
Regards
Sriram

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Govindarajan, Sriramakrishnan
> Sent: Wednesday, October 28, 2009 7:19 PM
> To: linux-omap@vger.kernel.org
> Cc: Govindarajan, Sriramakrishnan
> Subject: [PATCH] omap3evm: Migrate to smsc911x ethernet driver.
> 
> Migrate to smsc911x ethernet driver instead of smc911x driver.
> The smsc911x ethernet driver supports NAPI and performs better
> under heavy traffic. With the smc911x driver we were witnessing
> very high iowait time for high IO load over NFS.
> 
> Signed-off-by: Sriramakrishnan 
> ---
> This patch is generated against tip of for-next branch.
> 
>  arch/arm/configs/omap3_evm_defconfig |4 +-
>  arch/arm/mach-omap2/board-omap3evm.c |   36
> +
>  2 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/configs/omap3_evm_defconfig
> b/arch/arm/configs/omap3_evm_defconfig
> index d5ff477..50afc67 100644
> --- a/arch/arm/configs/omap3_evm_defconfig
> +++ b/arch/arm/configs/omap3_evm_defconfig
> @@ -617,8 +617,8 @@ CONFIG_MII=y
>  # CONFIG_DM9000 is not set
>  # CONFIG_ENC28J60 is not set
>  # CONFIG_ETHOC is not set
> -CONFIG_SMC911X=y
> -# CONFIG_SMSC911X is not set
> +# CONFIG_SMC911X is not set
> +CONFIG_SMSC911X=y
>  # CONFIG_DNET is not set
>  # CONFIG_IBM_NEW_EMAC_ZMII is not set
>  # CONFIG_IBM_NEW_EMAC_RGMII is not set
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-
> omap2/board-omap3evm.c
> index 5d2310e..9bcdaf7 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -21,11 +21,13 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> @@ -51,7 +53,8 @@
>  #define OMAP3EVM_ETHR_GPIO_IRQ   176
>  #define OMAP3EVM_SMC911X_CS  5
> 
> -static struct resource omap3evm_smc911x_resources[] = {
> +#if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
> +static struct resource omap3evm_smsc911x_resources[] = {
>   [0] =   {
>   .start  = OMAP3EVM_ETHR_START,
>   .end= (OMAP3EVM_ETHR_START + OMAP3EVM_ETHR_SIZE - 1),
> @@ -60,18 +63,28 @@ static struct resource
> omap3evm_smc911x_resources[] = {
>   [1] =   {
>   .start  = OMAP_GPIO_IRQ(OMAP3EVM_ETHR_GPIO_IRQ),
>   .end= OMAP_GPIO_IRQ(OMAP3EVM_ETHR_GPIO_IRQ),
> - .flags  = IORESOURCE_IRQ,
> + .flags  = (IORESOURCE_IRQ | IRQF_TRIGGER_LOW),
>   },
>  };
> 
> -static struct platform_device omap3evm_smc911x_device = {
> - .name   = "smc911x",
> +static struct smsc911x_platform_config smsc911x_config = {
> + .phy_interface  = PHY_INTERFACE_MODE_MII,
> + .irq_polarity   = SMSC911X_IRQ_POLARITY_ACTIVE_LOW,
> + .irq_type   = SMSC911X_IRQ_TYPE_OPEN_DRAIN,
> + .flags  = (SMSC911X_USE_32BIT |
> SMSC911X_SAVE_MAC_ADDRESS),
> +};
> +
> +static struct platform_device omap3evm_smsc911x_device = {
> + .name   = "smsc911x",
>   .id = -1,
> - .num_resources  = ARRAY_SIZE(omap3evm_smc911x_resources),
> - .resource   = &omap3evm_smc911x_resources[0],
> + .num_resources  = ARRAY_SIZE(omap3evm_smsc911x_resources),
> + .resource   = &omap3evm_smsc911x_resources[0],
> + .dev= {
> + .platform_data = &smsc911x_config,
> + },
>  };
> 
> -static inline void __init omap3evm_init_smc911x(void)
> +static inline void __init omap3evm_init_smsc911x(void)
>  {
>   int eth_cs;
>   struct clk *l3ck;
> @@ -92,8 +105,14 @@ static inline void __init
> omap3evm_init_smc911x(void)
>   }
> 
>   gpio_direction_input(OMAP3EVM_ETHR_GPIO_IRQ);
> +
> + platform_device_register(&omap3evm_smsc911x_device);
>  }
> 
> +#else
> +static inline void __init omap3evm_init_smsc911x(void) { return; }
> +#endif
> +
>  static struct regulator_consumer_supply omap3evm_vmmc1_supply = {
>   .supply = "vmmc",
>  };
> @@ -335,12 +354,10 @@ static void __init omap3_evm_init_irq(void)
>   omap2_init_common_hw(mt46h32m32lf6_sdrc_params, NULL);
>   omap_init_irq();
>   omap_gpio_init();
> - omap3evm_init_smc911x();
>  }
> 
>  static struct platform_device *omap3_evm_devices[] __initdata = {
>   &omap3_evm_lcd_device,
> - &omap3evm_smc911x_device,
>

High iowait time with NFS on OMAP3EVM

2009-05-14 Thread Govindarajan, Sriramakrishnan
Hi,
When I copy a large file to/from NFS mounted filesystem, the system becomes
unresponsive and top command shows very high iowait(80%+) time. Though the IO 
operation goes through, typical throughput is < 8Mbps/sec.

On the same system, testing for Ethernet driver throughput using iperf yields 
close to 50Mb/sec throughput. (I am using the tip of linux-omap and running on 
OMAP3EVM using smc911x driver)

Any pointers on what could result in degradation of throughput when using NFS 
and the resulting high iowait time?

Regards
Sriram

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html