[PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Govindraj.R]

>From 8dae69c5e17421123b88414021289974e968fd02 Mon Sep 17 00:00:00 2001
From: Govindraj R <govindraj.raja@ti.com>
Date: Fri, 13 Nov 2009 12:09:38 +0530
Subject: [PATCH] OMAP UART: Adding support for omap-serial driver.

The Following series of patch adds support for OMAP3430-HIGH SPEED UART Controller.

Govindraj R (3):
  OMAP UART: Add omap-serial driver support.
  OMAP UART: Add platform data for omap-serial driver.
  OMAP UART: Add omap-serial driver support in omap_3430sdp_defconfig

 arch/arm/configs/omap_3430sdp_defconfig       |    1 +
 arch/arm/mach-omap2/serial.c                  |  335 +++++--
 arch/arm/plat-omap/include/plat/omap-serial.h |  115 +++
 drivers/serial/Kconfig                        |   23 +
 drivers/serial/Makefile                       |    1 +
 drivers/serial/omap-serial.c                  | 1330 +++++++++++++++++++++++++
 include/linux/serial_core.h                   |    3 +
 7 files changed, 1714 insertions(+), 94 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/omap-serial.h
 create mode 100644 drivers/serial/omap-serial.c

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Govindraj]

I have not seen any comments on this patch series yet.

Tony,
Is there something I need to modify?

Or it can be unstreamed now?

---
Regards,
Govindraj.R


On Fri, Nov 13, 2009 at 12:23 PM, Govindraj.R <govindraj.raja@ti.com> wrote:
>
> From 8dae69c5e17421123b88414021289974e968fd02 Mon Sep 17 00:00:00 2001
> From: Govindraj R <govindraj.raja@ti.com>
> Date: Fri, 13 Nov 2009 12:09:38 +0530
> Subject: [PATCH] OMAP UART: Adding support for omap-serial driver.
>
> The Following series of patch adds support for OMAP3430-HIGH SPEED UART Controller.
>
> Govindraj R (3):
>  OMAP UART: Add omap-serial driver support.
>  OMAP UART: Add platform data for omap-serial driver.
>  OMAP UART: Add omap-serial driver support in omap_3430sdp_defconfig
>
>  arch/arm/configs/omap_3430sdp_defconfig       |    1 +
>  arch/arm/mach-omap2/serial.c                  |  335 +++++--
>  arch/arm/plat-omap/include/plat/omap-serial.h |  115 +++
>  drivers/serial/Kconfig                        |   23 +
>  drivers/serial/Makefile                       |    1 +
>  drivers/serial/omap-serial.c                  | 1330 +++++++++++++++++++++++++
>  include/linux/serial_core.h                   |    3 +
>  7 files changed, 1714 insertions(+), 94 deletions(-)
>  create mode 100644 arch/arm/plat-omap/include/plat/omap-serial.h
>  create mode 100644 drivers/serial/omap-serial.c
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Olof Johansson]

On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
> I have not seen any comments on this patch series yet.
> 
> Tony,
> Is there something I need to modify?

Govindaj,

I'm having problems with these patches on Zoom2 with debug board, since
they don't seem to coexist very well together with 8250. Enabling both
options will result in build errors.

Also, you have hardcoded and are using the same major/minor versions as
the 8250 driver, which results in failure to allocate those devices when
probing 8250. That needs to be corrected.


-Olof

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Olof Johansson]

On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
> I have not seen any comments on this patch series yet.
> 
> Tony,
> Is there something I need to modify?
> 
> Or it can be unstreamed now?

Oh, and by the way, serial drivers are preferrably merged through Greg
K-H (gregkh@suse.de). So Cc him on the next revision of the patch.


-Olof

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Kevin Hilman]

Olof Johansson <olof@lixom.net> writes:

> On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
>> I have not seen any comments on this patch series yet.
>> 
>> Tony,
>> Is there something I need to modify?
>> 
>> Or it can be unstreamed now?
>
> Oh, and by the way, serial drivers are preferrably merged through Greg
> K-H (gregkh@suse.de). So Cc him on the next revision of the patch.

FWIW, I had asked Govindraj to not post more broadly until we get some
sort of better consensus/design around this driver.  I don't think
we're quite there yet.

Kevin

RE: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[G, Manjunath Kondaiah]

> -----Original Message-----
> From: Olof Johansson [mailto:olof@lixom.net] 
> Sent: Monday, November 23, 2009 10:36 PM
> To: Govindraj; g@lixom.net
> Cc: Tony Lindgren; linux-omap@vger.kernel.org; Pandita, 
> Vikram; Raja, Govindraj; G, Manjunath Kondaiah
> Subject: Re: [PATCHv3 0/3] OMAP UART: Adding support for 
> omap-serial driver.
> 
> On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
> > I have not seen any comments on this patch series yet.
> > 
> > Tony,
> > Is there something I need to modify?
> 
> Govindaj,
> 
> I'm having problems with these patches on Zoom2 with debug 
> board, since
> they don't seem to coexist very well together with 8250. Enabling both
> options will result in build errors.

Why do we need to enable both 8250 and omap-serial for console. The console
UART can be either external UART or any one of the omap UART right?

-Manjunath

> 
> Also, you have hardcoded and are using the same major/minor 
> versions as
> the 8250 driver, which results in failure to allocate those 
> devices when
> probing 8250. That needs to be corrected.
> 
> 
> -Olof
> 
> 

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Olof Johansson]

On Tue, Nov 24, 2009 at 03:04:09PM +0530, G, Manjunath Kondaiah wrote:
> 
> 
> > -----Original Message-----
> > From: Olof Johansson [mailto:olof@lixom.net] 
> > Sent: Monday, November 23, 2009 10:36 PM
> > To: Govindraj; g@lixom.net
> > Cc: Tony Lindgren; linux-omap@vger.kernel.org; Pandita, 
> > Vikram; Raja, Govindraj; G, Manjunath Kondaiah
> > Subject: Re: [PATCHv3 0/3] OMAP UART: Adding support for 
> > omap-serial driver.
> > 
> > On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
> > > I have not seen any comments on this patch series yet.
> > > 
> > > Tony,
> > > Is there something I need to modify?
> > 
> > Govindaj,
> > 
> > I'm having problems with these patches on Zoom2 with debug 
> > board, since
> > they don't seem to coexist very well together with 8250. Enabling both
> > options will result in build errors.
> 
> Why do we need to enable both 8250 and omap-serial for console. The console
> UART can be either external UART or any one of the omap UART right?

You should allow both of them to be enabled at the same time, so the same
kernel can for example be booted on a ZOOM2 with debug board attached
(8250 on GPMC), or on a beagle/overo board.

Making them exclusive would be a step backwards.


-Olof

RE: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Shilimkar, Santosh]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Olof Johansson
> Sent: Tuesday, November 24, 2009 10:52 PM
> To: G, Manjunath Kondaiah
> Cc: Govindraj; g@lixom.net; Tony Lindgren; linux-
> omap@vger.kernel.org; Pandita, Vikram; Raja, Govindraj
> Subject: Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial
> driver.
> 
> On Tue, Nov 24, 2009 at 03:04:09PM +0530, G, Manjunath Kondaiah
> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Olof Johansson [mailto:olof@lixom.net]
> > > Sent: Monday, November 23, 2009 10:36 PM
> > > To: Govindraj; g@lixom.net
> > > Cc: Tony Lindgren; linux-omap@vger.kernel.org; Pandita,
> > > Vikram; Raja, Govindraj; G, Manjunath Kondaiah
> > > Subject: Re: [PATCHv3 0/3] OMAP UART: Adding support for
> > > omap-serial driver.
> > >
> > > On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
> > > > I have not seen any comments on this patch series yet.
> > > >
> > > > Tony,
> > > > Is there something I need to modify?
> > >
> > > Govindaj,
> > >
> > > I'm having problems with these patches on Zoom2 with debug
> > > board, since
> > > they don't seem to coexist very well together with 8250.
> Enabling both
> > > options will result in build errors.
> >
> > Why do we need to enable both 8250 and omap-serial for console.
> The console
> > UART can be either external UART or any one of the omap UART
> right?
> 
> You should allow both of them to be enabled at the same time, so the
> same
> kernel can for example be booted on a ZOOM2 with debug board
> attached
> (8250 on GPMC), or on a beagle/overo board.
> 
> Making them exclusive would be a step backwards.

More so, selecting both config option really shouldn't break the build. If one expect a build break because only one of the options can be enabled then clearly the dependency should be used so that user can select only one option at a time.

Regards,
Santosh

RE: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[G, Manjunath Kondaiah]

> -----Original Message-----
> From: Shilimkar, Santosh 
> Sent: Wednesday, November 25, 2009 12:06 PM
> To: Olof Johansson; G, Manjunath Kondaiah
> Cc: Govindraj; g@lixom.net; Tony Lindgren; 
> linux-omap@vger.kernel.org; Pandita, Vikram; Raja, Govindraj
> Subject: RE: [PATCHv3 0/3] OMAP UART: Adding support for 
> omap-serial driver.
> 
> 
> > -----Original Message-----
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Olof Johansson
> > Sent: Tuesday, November 24, 2009 10:52 PM
> > To: G, Manjunath Kondaiah
> > Cc: Govindraj; g@lixom.net; Tony Lindgren; linux-
> > omap@vger.kernel.org; Pandita, Vikram; Raja, Govindraj
> > Subject: Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial
> > driver.
> > 
> > On Tue, Nov 24, 2009 at 03:04:09PM +0530, G, Manjunath Kondaiah
> > wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Olof Johansson [mailto:olof@lixom.net]
> > > > Sent: Monday, November 23, 2009 10:36 PM
> > > > To: Govindraj; g@lixom.net
> > > > Cc: Tony Lindgren; linux-omap@vger.kernel.org; Pandita,
> > > > Vikram; Raja, Govindraj; G, Manjunath Kondaiah
> > > > Subject: Re: [PATCHv3 0/3] OMAP UART: Adding support for
> > > > omap-serial driver.
> > > >
> > > > On Fri, Nov 20, 2009 at 03:21:26PM +0530, Govindraj wrote:
> > > > > I have not seen any comments on this patch series yet.
> > > > >
> > > > > Tony,
> > > > > Is there something I need to modify?
> > > >
> > > > Govindaj,
> > > >
> > > > I'm having problems with these patches on Zoom2 with debug
> > > > board, since
> > > > they don't seem to coexist very well together with 8250.
> > Enabling both
> > > > options will result in build errors.
> > >
> > > Why do we need to enable both 8250 and omap-serial for console.
> > The console
> > > UART can be either external UART or any one of the omap UART
> > right?
> > 
> > You should allow both of them to be enabled at the same time, so the
> > same
> > kernel can for example be booted on a ZOOM2 with debug board
> > attached
> > (8250 on GPMC), or on a beagle/overo board.
> > 
> > Making them exclusive would be a step backwards.
> 
> More so, selecting both config option really shouldn't break 
> the build. If one expect a build break because only one of 
> the options can be enabled then clearly the dependency should 
> be used so that user can select only one option at a time.

Ack. There should be dependency in Kconfig so that user can select
either 8250 console(through Quad-uart for zoom2/zoom3) or OMAP-serial
Console(through OMAP UART's).

-Manjunath

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Olof Johansson]

G, Manjunath Kondaiah wrote:
>>> You should allow both of them to be enabled at the same time, so the
>>> same
>>> kernel can for example be booted on a ZOOM2 with debug board
>>> attached
>>> (8250 on GPMC), or on a beagle/overo board.
>>>
>>> Making them exclusive would be a step backwards.
>>>       
>> More so, selecting both config option really shouldn't break 
>> the build. If one expect a build break because only one of 
>> the options can be enabled then clearly the dependency should 
>> be used so that user can select only one option at a time.
>>     
>
> Ack. There should be dependency in Kconfig so that user can select
> either 8250 console(through Quad-uart for zoom2/zoom3) or OMAP-serial
> Console(through OMAP UART's).
>   

No. The drivers MUST handle co-existing. You can have a bunch of drivers 
configured to be _potential_ consoles, which one will end up being used 
depends on, among other things, the bootargs. But it should be allowed 
to configure both 8250 and omap-serial at the same time.


-Olof

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Tony Lindgren]

* Olof Johansson <olof@lixom.net> [091124 23:05]:
> G, Manjunath Kondaiah wrote:
> >>>You should allow both of them to be enabled at the same time, so the
> >>>same
> >>>kernel can for example be booted on a ZOOM2 with debug board
> >>>attached
> >>>(8250 on GPMC), or on a beagle/overo board.
> >>>
> >>>Making them exclusive would be a step backwards.
> >>More so, selecting both config option really shouldn't break the
> >>build. If one expect a build break because only one of the
> >>options can be enabled then clearly the dependency should be
> >>used so that user can select only one option at a time.
> >
> >Ack. There should be dependency in Kconfig so that user can select
> >either 8250 console(through Quad-uart for zoom2/zoom3) or OMAP-serial
> >Console(through OMAP UART's).
> 
> No. The drivers MUST handle co-existing. You can have a bunch of
> drivers configured to be _potential_ consoles, which one will end up
> being used depends on, among other things, the bootargs. But it
> should be allowed to configure both 8250 and omap-serial at the same
> time.

Exactly. Even the same board must be capable of using 8250 and
omap-serial. That's why it must be called ttyO instead of ttyS.
The 8250 drivers are often needed for hotpluggable buses.

Regards,

Tony

Re: [PATCHv3 0/3] OMAP UART: Adding support for omap-serial driver.

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Olof Johansson <olof@lixom.net> [091124 23:05]:
>> G, Manjunath Kondaiah wrote:
>> >>>You should allow both of them to be enabled at the same time, so the
>> >>>same
>> >>>kernel can for example be booted on a ZOOM2 with debug board
>> >>>attached
>> >>>(8250 on GPMC), or on a beagle/overo board.
>> >>>
>> >>>Making them exclusive would be a step backwards.
>> >>More so, selecting both config option really shouldn't break the
>> >>build. If one expect a build break because only one of the
>> >>options can be enabled then clearly the dependency should be
>> >>used so that user can select only one option at a time.
>> >
>> >Ack. There should be dependency in Kconfig so that user can select
>> >either 8250 console(through Quad-uart for zoom2/zoom3) or OMAP-serial
>> >Console(through OMAP UART's).
>> 
>> No. The drivers MUST handle co-existing. You can have a bunch of
>> drivers configured to be _potential_ consoles, which one will end up
>> being used depends on, among other things, the bootargs. But it
>> should be allowed to configure both 8250 and omap-serial at the same
>> time.
>
> Exactly. Even the same board must be capable of using 8250 and
> omap-serial. That's why it must be called ttyO instead of ttyS.

I completely agree with the need for these drivers to co-exist and for
a given board to use both drivers.  For example, Zoom2 would want to
use omap-serial for the on-chip UARTs, and 8250 for the external UART
used for the console.

In my review I complained about using ttyO instead of ttyS, but now I
see more clearly the reason for that.

Kevin