Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-19 Thread Mark Brown 
On Thu, Nov 19, 2009 at 11:44:14AM +0900, jassi brar wrote:

 Let each platform code(SoC specific) define names of all possible
 source clocks and let the board init code pass on the potential source
 clocks by some bit-mask(or some other mechanism) while setting the
 platform data.

This is going to set off warnings from a clock API point of view -
passing clock names around in platform data is usually a sign that
something is very wrong.  Keeping the mapping inside the clock API
(still controlled by the board driver but by telling the clock API that
device X should use clock Y).
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-19 Thread jassi brar 
On Thu, Nov 19, 2009 at 7:33 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 On Thu, Nov 19, 2009 at 11:44:14AM +0900, jassi brar wrote:

 Let each platform code(SoC specific) define names of all possible
 source clocks and let the board init code pass on the potential source
 clocks by some bit-mask(or some other mechanism) while setting the
 platform data.

 This is going to set off warnings from a clock API point of view -
 passing clock names around in platform data is usually a sign that
 something is very wrong.  Keeping the mapping inside the clock API
 (still controlled by the board driver but by telling the clock API that
 device X should use clock Y).
no clock pointer needs to be passed, just a pointer to an array of _strings_
There is no need to even include any clock header in platform code
for the purpose.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-19 Thread jassi brar 
On Thu, Nov 19, 2009 at 8:08 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 On Thu, Nov 19, 2009 at 08:05:29PM +0900, jassi brar wrote:
 On Thu, Nov 19, 2009 at 7:33 PM, Mark Brown

  This is going to set off warnings from a clock API point of view -
  passing clock names around in platform data is usually a sign that
  something is very wrong.  Keeping the mapping inside the clock API
  (still controlled by the board driver but by telling the clock API that
  device X should use clock Y).

 no clock pointer needs to be passed, just a pointer to an array of _strings_
 There is no need to even include any clock header in platform code
 for the purpose.

 Yes, that's what I was commenting on - like I say, passing clock names
 tends to set off the same alarm bells as passing a struct clk.  Like I
 say, the general model for this has been that the fixups will be done by
 having the machine code talk to the clock API ratehr than bouncing the
 data about the clock to use through the driver.
The case here is that UART controller can source from various platform clocks
to generate the UART clocks. The drivers/serial/samsung.c chooses a source
clock that can result in closest match to the requested rate.
How could we do that from machine code(except for writing callbacks for each
machine using the SoC)?

Or maybe i cud understand better looking at some example.
Cud u suggest some please?
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-19 Thread Russell King - ARM Linux 
On Thu, Nov 19, 2009 at 11:08:27AM +, Mark Brown wrote:
 On Thu, Nov 19, 2009 at 08:05:29PM +0900, jassi brar wrote:
  On Thu, Nov 19, 2009 at 7:33 PM, Mark Brown
 
   This is going to set off warnings from a clock API point of view -
   passing clock names around in platform data is usually a sign that
   something is very wrong.  Keeping the mapping inside the clock API
   (still controlled by the board driver but by telling the clock API that
   device X should use clock Y).
 
  no clock pointer needs to be passed, just a pointer to an array of _strings_
  There is no need to even include any clock header in platform code
  for the purpose.
 
 Yes, that's what I was commenting on - like I say, passing clock names
 tends to set off the same alarm bells as passing a struct clk.  Like I
 say, the general model for this has been that the fixups will be done by
 having the machine code talk to the clock API ratehr than bouncing the
 data about the clock to use through the driver.

I'm not sure what you're commenting on precisely, but the Samsung code as
a whole doesn't use the clk API very well, and I suspect that is starting
to cause people to have to pass clock names around.

I've tried to persuade Ben to tidy this up and convert over to clkdev
before it becomes too big a problem, but I've had little success.  I
suspect it's now grown too big to be tackled sanely.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-19 Thread Mark Brown 
On Thu, Nov 19, 2009 at 11:38:41AM +, Russell King - ARM Linux wrote:

 I'm not sure what you're commenting on precisely, but the Samsung code as
 a whole doesn't use the clk API very well, and I suspect that is starting
 to cause people to have to pass clock names around.

That's what I'd thought originally but it's not quite the problem here.
The issue is that Samsung serial port clock is chosen from a mux and the
logic to select which of the inputs to that mux should be used is in the
serial driver.  This means that if the set of parent clocks changes then
the serial driver needs to be told what they are.

My suggestion was to push this logic down into the clock API so the
serial driver just requests a rate and then clock API picks the best
option from the mux.  As well as being nicer from the clock API point of
view this would also allow other drivers to use the same logic since
these muxes are a standard idiom for Samsung SoCs.

 I've tried to persuade Ben to tidy this up and convert over to clkdev
 before it becomes too big a problem, but I've had little success.  I
 suspect it's now grown too big to be tackled sanely.

clkdev would be nice but I don't think it'd deal with the issue here.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-19 Thread Russell King - ARM Linux 
On Thu, Nov 19, 2009 at 11:48:33AM +, Mark Brown wrote:
 On Thu, Nov 19, 2009 at 11:38:41AM +, Russell King - ARM Linux wrote:
 
  I'm not sure what you're commenting on precisely, but the Samsung code as
  a whole doesn't use the clk API very well, and I suspect that is starting
  to cause people to have to pass clock names around.
 
 That's what I'd thought originally but it's not quite the problem here.
 The issue is that Samsung serial port clock is chosen from a mux and the
 logic to select which of the inputs to that mux should be used is in the
 serial driver.  This means that if the set of parent clocks changes then
 the serial driver needs to be told what they are.
 
 My suggestion was to push this logic down into the clock API so the
 serial driver just requests a rate and then clock API picks the best
 option from the mux.  As well as being nicer from the clock API point of
 view this would also allow other drivers to use the same logic since
 these muxes are a standard idiom for Samsung SoCs.

Yes, I agree with that approach.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-18 Thread jassi brar 
On Wed, Nov 18, 2009 at 10:33 PM, Marek Szyprowski
m.szyprow...@samsung.com wrote:
 From: Kyungmin Park kyungmin.p...@samsung.com

 Samsung S5PC110 SoCs have UART that differs a bit from the one known
 from the previous Samsung SoCs. This patch adds support for this new
 driver.

 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com


 ---
  arch/arm/plat-s3c/include/plat/regs-serial.h |   31 ++
  drivers/serial/Kconfig                       |    7 ++
  drivers/serial/Makefile                      |    1 +
  drivers/serial/s5pc110.c                     |  143 
 ++
  4 files changed, 182 insertions(+), 0 deletions(-)
  create mode 100644 drivers/serial/s5pc110.c

 diff --git a/arch/arm/plat-s3c/include/plat/regs-serial.h 
 b/arch/arm/plat-s3c/include/plat/regs-serial.h
 index 66af75a..910cfba 100644
 --- a/arch/arm/plat-s3c/include/plat/regs-serial.h
 +++ b/arch/arm/plat-s3c/include/plat/regs-serial.h
 @@ -194,6 +194,37 @@
  #define S3C64XX_UINTSP         0x34
  #define S3C64XX_UINTM          0x38

 +/* S5PC110 UCON */
 +#define S5PC110_UCON_CLKMASK   (110)
 +#define S5PC110_UCON_PCLK      (010)
 +#define S5PC110_UCON_SCLK_UART (110)
 +
 +/* S5PC110 FIFO trigger levels */
 +#define S5PC110_UFCON_RXTRIG1  (04)
 +#define S5PC110_UFCON_RXTRIG4  (14)
 +#define S5PC110_UFCON_RXTRIG8  (24)
 +#define S5PC110_UFCON_RXTRIG16 (34)
 +#define S5PC110_UFCON_RXTRIG32 (44)
 +#define S5PC110_UFCON_RXTRIG64 (54)
 +#define S5PC110_UFCON_RXTRIG128        (64)
 +#define S5PC110_UFCON_RXTRIG256        (74)
 +
 +#define S5PC110_UFCON_TXTRIG1  (08)
 +#define S5PC110_UFCON_TXTRIG4  (18)
 +#define S5PC110_UFCON_TXTRIG8  (28)
 +#define S5PC110_UFCON_TXTRIG16 (38)
 +#define S5PC110_UFCON_TXTRIG32 (48)
 +#define S5PC110_UFCON_TXTRIG64 (58)
 +#define S5PC110_UFCON_TXTRIG128        (68)
 +#define S5PC110_UFCON_TXTRIG256        (78)
 +
 +#define S5PC110_UFSTAT_TXFULL  (124)
 +#define S5PC110_UFSTAT_RXFULL  (18)
 +#define S5PC110_UFSTAT_TXSHIFT (16)
 +#define S5PC110_UFSTAT_RXSHIFT (0)
 +#define S5PC110_UFSTAT_TXMASK  (25516)
 +#define S5PC110_UFSTAT_RXMASK  (255)
 +
  #ifndef __ASSEMBLY__

  /* struct s3c24xx_uart_clksrc
 diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
 index e522572..d119cac 100644
 --- a/drivers/serial/Kconfig
 +++ b/drivers/serial/Kconfig
 @@ -540,6 +540,13 @@ config SERIAL_S5PC100
        help
          Serial port support for the Samsung S5PC100 SoCs

 +config SERIAL_S5PC110
 +       tristate Samsung S5PC110 Serial port support
 +       depends on SERIAL_SAMSUNG  CPU_S5PC110
 +       default y
 +       help
 +         Serial port support for the Samsung S5PC110 SoCs
 +
  config SERIAL_MAX3100
        tristate MAX3100 support
        depends on SPI
 diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
 index d21d5dd..43d6123 100644
 --- a/drivers/serial/Makefile
 +++ b/drivers/serial/Makefile
 @@ -45,6 +45,7 @@ obj-$(CONFIG_SERIAL_S3C2440) += s3c2440.o
  obj-$(CONFIG_SERIAL_S3C24A0) += s3c24a0.o
  obj-$(CONFIG_SERIAL_S3C6400) += s3c6400.o
  obj-$(CONFIG_SERIAL_S5PC100) += s3c6400.o
 +obj-$(CONFIG_SERIAL_S5PC110) += s5pc110.o
  obj-$(CONFIG_SERIAL_MAX3100) += max3100.o
  obj-$(CONFIG_SERIAL_IP22_ZILOG) += ip22zilog.o
  obj-$(CONFIG_SERIAL_MUX) += mux.o
 diff --git a/drivers/serial/s5pc110.c b/drivers/serial/s5pc110.c
 new file mode 100644
 index 000..1e1e229
 --- /dev/null
 +++ b/drivers/serial/s5pc110.c
 @@ -0,0 +1,143 @@
 +/*
 + * linux/drivers/serial/s5pc110.c
 + *
 + * Driver for Samsung S5PC110 SoC onboard UARTs.
 + *
 + *  Copyright 2009 Samsung Electronics
 + *  Kyungin Park kyungmin.p...@samsung.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + */
 +
 +#include linux/module.h
 +#include linux/ioport.h
 +#include linux/io.h
 +#include linux/platform_device.h
 +#include linux/init.h
 +#include linux/serial_core.h
 +#include linux/serial.h
 +
 +#include asm/irq.h
 +#include mach/hardware.h
 +
 +#include plat/regs-serial.h
 +
 +#include samsung.h
 +
 +static int s5pc110_serial_setsource(struct uart_port *port,
 +                                   struct s3c24xx_uart_clksrc *clk)
 +{
 +       unsigned long ucon = rd_regl(port, S3C2410_UCON);
 +
 +       if (strcmp(clk-name, uclk0) == 0)
 +               ucon |= S5PC110_UCON_SCLK_UART;
 +       else if (strcmp(clk-name, pclk) == 0)
 +               /* See notes about transitioning from UCLK to PCLK */
 +               ucon = ~S5PC110_UCON_SCLK_UART;
 +       else {
 +               printk(KERN_ERR unknown clock source %s\n, clk-name);
 +               return -EINVAL;
 +       }
 +
 +       wr_regl(port, S3C2410_UCON, ucon);
 +       return 0;
 +}
Not just about this c110 patch: I think this string comparison thing
isn't very neat.
I think we'd better be doing it by indexing into an array of clock

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-18 Thread Ben Dooks 
On Wed, Nov 18, 2009 at 11:14:52PM +0900, jassi brar wrote:
 On Wed, Nov 18, 2009 at 10:33 PM, Marek Szyprowski
 m.szyprow...@samsung.com wrote:
  From: Kyungmin Park kyungmin.p...@samsung.com
 
  Samsung S5PC110 SoCs have UART that differs a bit from the one known
  from the previous Samsung SoCs. This patch adds support for this new
  driver.
 
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com

 Not just about this c110 patch: I think this string comparison thing
 isn't very neat.
 I think we'd better be doing it by indexing into an array of clock
 names(which can be
 defined in some platform specific code).

I don't mind changing to using a table, but the table is probably best
off here, closest to the UART drivers in question.

-- 
Ben

Q:  What's a light-year?
A:  One-third less calories than a regular year.

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

Re: [PATCH 09/19] drivers: serial: add support for Samsung S5PC110 SoC uart

2009-11-18 Thread jassi brar 
On Thu, Nov 19, 2009 at 7:13 AM, Ben Dooks ben-li...@fluff.org wrote:
 On Wed, Nov 18, 2009 at 11:14:52PM +0900, jassi brar wrote:
 On Wed, Nov 18, 2009 at 10:33 PM, Marek Szyprowski
 m.szyprow...@samsung.com wrote:
  From: Kyungmin Park kyungmin.p...@samsung.com
 
  Samsung S5PC110 SoCs have UART that differs a bit from the one known
  from the previous Samsung SoCs. This patch adds support for this new
  driver.
 
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com

 Not just about this c110 patch: I think this string comparison thing
 isn't very neat.
 I think we'd better be doing it by indexing into an array of clock
 names(which can be
 defined in some platform specific code).

 I don't mind changing to using a table, but the table is probably best
 off here, closest to the UART drivers in question.
well as per current implementation of drivers/serial/samsung.c we can't
do much about it.

Ideally, each SoC(if not yet, maybe in future) can have different
number/names of possible source clocks for signal generation. Let us
not depend upon even defaults(fclk, pclk etc)
Let each platform code(SoC specific) define names of all possible
source clocks and let the board init code pass on the potential source
clocks by some bit-mask(or some other mechanism) while setting the
platform data.
The samsung.c could then simply go thru the array of source clock
names and the bit-mask, in platform_data, while selecting the best possible
 source for the baud rate under consideration.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html