Re: [PATCH 2/4] serial: s5pv210: Add device tree support

2011-08-10 Thread Thomas Abraham
Hi Ben,

On 3 August 2011 14:42, Ben Dooks ben-li...@fluff.org wrote:
 On Wed, Aug 03, 2011 at 12:08:27AM +0100, Thomas Abraham wrote:
 For device tree based probe, the dependecy on pdev-id to attach a
 corresponding default port info to the driver's private data is
 removed. The fifosize parameter is obtained from the device tree
 node and the next available instance of port info is updated
 with the fifosize value and attached to the driver's private data.
 The default platform data is selected based on the compatible property.

 CC: Ben Dooks ben-li...@fluff.org
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
  .../devicetree/bindings/serial/samsung_uart.txt    |   16 +++
  drivers/tty/serial/s5pv210.c                       |   43 
 +++-
  drivers/tty/serial/samsung.c                       |    5 ++-
  3 files changed, 62 insertions(+), 2 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/serial/samsung_uart.txt


[...]

  /* device management */
  static int s5p_serial_probe(struct platform_device *pdev)
  {
 -     return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev-id]);
 +     static unsigned int probe_index;
 +     unsigned int port = pdev-id;
 +     const struct of_device_id *match;
 +     struct s3c2410_uartcfg *cfg;
 +
 +     if (pdev-dev.of_node) {
 +             if (of_property_read_u32(pdev-dev.of_node,
 +                             samsung,uart-fifosize,
 +                             s5p_uart_inf[probe_index]-fifosize))
 +                     return -EINVAL;

 I'd rather see the fifo size either being a property of the soc itself
 or being inferred by the compatible field.

When using the compatible field to infer the fifosize of the
controller, the code looks as listed below.

struct s3c24xx_uart_dt_compat_data {
unsigned int fifosize;
struct s3c2410_uartcfg *uartcfg;
};

static struct s3c2410_uartcfg s5pv310_uart_defcfg = {
.ucon   = 0x3c5,
.ufcon  = 0x111,
.flags  = NO_NEED_CHECK_CLKSRC,
.has_fracval = 1,
};

static struct s3c24xx_uart_dt_compat_data s5pv210_compat_fs256 = {
.fifosize = 256;
.uartcfg = s5pv310_uart_defcfg;
};

static struct s3c24xx_uart_dt_compat_data s5pv210_compat_fs64 = {
.fifosize = 64;
.uartcfg = s5pv310_uart_defcfg;
};

static struct s3c24xx_uart_dt_compat_data s5pv210_compat_fs16 = {
.fifosize = 16;
.uartcfg = s5pv310_uart_defcfg;
};

static const struct of_device_id s5pv210_uart_dt_match[] = {
{ .compatible = samsung,s5pv310-uart-fs256, .data =
s5pv210_compat_fs256 },
{ .compatible = samsung,s5pv310-uart-fs64, .data =
s5pv210_compat_fs64 },
{ .compatible = samsung,s5pv310-uart-fs16, .data =
s5pv210_compat_fs16 },
{},
};
MODULE_DEVICE_TABLE(of, s5pv210_uart_match);


This requires a new structure definition and additional data in the
driver. So I still prefer to use a property in the uart device node to
define the fifosize of the controller.

What would you be your preference? Or is there a better way to obtain
the fifosize?

Thanks,
Thomas.


       .driver         = {
               .name   = s5pv210-uart,
               .owner  = THIS_MODULE,
 +             .of_match_table = s5pv210_uart_dt_match,

 I think maybe doing something like

                .of_match_table = of_match_ptr(5pv210_uart_dt_match),

 so we can avoid having the #else and #define 5pv210_uart_dt_match NULL
 in a number of places.

       },
  };

 --
 Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

 Large Hadron Colada: A large Pina Colada that makes the universe disappear.


--
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 2/4] serial: s5pv210: Add device tree support

2011-08-03 Thread Ben Dooks
On Wed, Aug 03, 2011 at 12:08:27AM +0100, Thomas Abraham wrote:
 For device tree based probe, the dependecy on pdev-id to attach a
 corresponding default port info to the driver's private data is
 removed. The fifosize parameter is obtained from the device tree
 node and the next available instance of port info is updated
 with the fifosize value and attached to the driver's private data.
 The default platform data is selected based on the compatible property.
 
 CC: Ben Dooks ben-li...@fluff.org
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
  .../devicetree/bindings/serial/samsung_uart.txt|   16 +++
  drivers/tty/serial/s5pv210.c   |   43 
 +++-
  drivers/tty/serial/samsung.c   |5 ++-
  3 files changed, 62 insertions(+), 2 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/serial/samsung_uart.txt
 
 diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.txt 
 b/Documentation/devicetree/bindings/serial/samsung_uart.txt
 new file mode 100644
 index 000..9174499
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/serial/samsung_uart.txt
 @@ -0,0 +1,16 @@
 +* Samsung's UART Controller
 +
 +The Samsung's UART controller is used for serial communications on all of
 +Samsung's s3c24xx, s3c64xx and s5p series application processors.
 +
 +Required properties:
 +- compatible: should be specific to the application processor
 +- samsung,s5pv310-uart , for Exynos4 processors.
 +
 +- reg: base physical address of the controller and length of memory mapped
 +  region.
 +
 +- interrupts : Three interrupt numbers should be specified in the following
 +  order - Rx interrupt, Tx interrupt, Error Interrupt.
 +
 +- samsung,uart-fifosize: Size of the tx/rx fifo used in the controller.
 diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
 index fb0806c..a44b599 100644
 --- a/drivers/tty/serial/s5pv210.c
 +++ b/drivers/tty/serial/s5pv210.c
 @@ -19,6 +19,7 @@
  #include linux/serial_core.h
  #include linux/serial.h
  #include linux/delay.h
 +#include linux/of.h
  
  #include asm/irq.h
  #include mach/hardware.h
 @@ -132,10 +133,49 @@ static struct s3c24xx_uart_info *s5p_uart_inf[] = {
   [3] = s5p_port_fifo16,
  };
  
 +#ifdef CONFIG_OF
 +static struct s3c2410_uartcfg s5pv310_uart_defcfg = {
 + .ucon   = 0x3c5,
 + .ufcon  = 0x111,
 + .flags  = NO_NEED_CHECK_CLKSRC,
 + .has_fracval = 1,
 +};
 +
 +static const struct of_device_id s5pv210_uart_dt_match[] = {
 + { .compatible = samsung,s5pv310-uart, .data = s5pv310_uart_defcfg },
 + {},
 +};
 +MODULE_DEVICE_TABLE(of, s5pv210_uart_match);
 +#else
 +#define s5pv210_uart_match NULL
 +#endif
 +
  /* device management */
  static int s5p_serial_probe(struct platform_device *pdev)
  {
 - return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev-id]);
 + static unsigned int probe_index;
 + unsigned int port = pdev-id;
 + const struct of_device_id *match;
 + struct s3c2410_uartcfg *cfg;
 +
 + if (pdev-dev.of_node) {
 + if (of_property_read_u32(pdev-dev.of_node,
 + samsung,uart-fifosize,
 + s5p_uart_inf[probe_index]-fifosize))
 + return -EINVAL;

I'd rather see the fifo size either being a property of the soc itself
or being inferred by the compatible field. 

   .driver = {
   .name   = s5pv210-uart,
   .owner  = THIS_MODULE,
 + .of_match_table = s5pv210_uart_dt_match,

I think maybe doing something like

.of_match_table = of_match_ptr(5pv210_uart_dt_match),

so we can avoid having the #else and #define 5pv210_uart_dt_match NULL
in a number of places.

   },
  };

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

--
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 2/4] serial: s5pv210: Add device tree support

2011-08-03 Thread Thomas Abraham
Hi Ben,

On 3 August 2011 10:12, Ben Dooks ben-li...@fluff.org wrote:
 On Wed, Aug 03, 2011 at 12:08:27AM +0100, Thomas Abraham wrote:
 For device tree based probe, the dependecy on pdev-id to attach a
 corresponding default port info to the driver's private data is
 removed. The fifosize parameter is obtained from the device tree
 node and the next available instance of port info is updated
 with the fifosize value and attached to the driver's private data.
 The default platform data is selected based on the compatible property.

 CC: Ben Dooks ben-li...@fluff.org
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
  .../devicetree/bindings/serial/samsung_uart.txt    |   16 +++
  drivers/tty/serial/s5pv210.c                       |   43 
 +++-
  drivers/tty/serial/samsung.c                       |    5 ++-
  3 files changed, 62 insertions(+), 2 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/serial/samsung_uart.txt

[...]


 +
 +     if (pdev-dev.of_node) {
 +             if (of_property_read_u32(pdev-dev.of_node,
 +                             samsung,uart-fifosize,
 +                             s5p_uart_inf[probe_index]-fifosize))
 +                     return -EINVAL;

 I'd rather see the fifo size either being a property of the soc itself
 or being inferred by the compatible field.

Ok. I missed that. I will make it part of the SoC data.


       .driver         = {
               .name   = s5pv210-uart,
               .owner  = THIS_MODULE,
 +             .of_match_table = s5pv210_uart_dt_match,

 I think maybe doing something like

                .of_match_table = of_match_ptr(5pv210_uart_dt_match),

 so we can avoid having the #else and #define 5pv210_uart_dt_match NULL
 in a number of places.

Ok. I will change it.

Thanks,
Thomas.


       },
  };

 --
 Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

 Large Hadron Colada: A large Pina Colada that makes the universe disappear.


--
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


[PATCH 2/4] serial: s5pv210: Add device tree support

2011-08-02 Thread Thomas Abraham
For device tree based probe, the dependecy on pdev-id to attach a
corresponding default port info to the driver's private data is
removed. The fifosize parameter is obtained from the device tree
node and the next available instance of port info is updated
with the fifosize value and attached to the driver's private data.
The default platform data is selected based on the compatible property.

CC: Ben Dooks ben-li...@fluff.org
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
 .../devicetree/bindings/serial/samsung_uart.txt|   16 +++
 drivers/tty/serial/s5pv210.c   |   43 +++-
 drivers/tty/serial/samsung.c   |5 ++-
 3 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/samsung_uart.txt

diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.txt 
b/Documentation/devicetree/bindings/serial/samsung_uart.txt
new file mode 100644
index 000..9174499
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/samsung_uart.txt
@@ -0,0 +1,16 @@
+* Samsung's UART Controller
+
+The Samsung's UART controller is used for serial communications on all of
+Samsung's s3c24xx, s3c64xx and s5p series application processors.
+
+Required properties:
+- compatible: should be specific to the application processor
+- samsung,s5pv310-uart , for Exynos4 processors.
+
+- reg: base physical address of the controller and length of memory mapped
+  region.
+
+- interrupts : Three interrupt numbers should be specified in the following
+  order - Rx interrupt, Tx interrupt, Error Interrupt.
+
+- samsung,uart-fifosize: Size of the tx/rx fifo used in the controller.
diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
index fb0806c..a44b599 100644
--- a/drivers/tty/serial/s5pv210.c
+++ b/drivers/tty/serial/s5pv210.c
@@ -19,6 +19,7 @@
 #include linux/serial_core.h
 #include linux/serial.h
 #include linux/delay.h
+#include linux/of.h
 
 #include asm/irq.h
 #include mach/hardware.h
@@ -132,10 +133,49 @@ static struct s3c24xx_uart_info *s5p_uart_inf[] = {
[3] = s5p_port_fifo16,
 };
 
+#ifdef CONFIG_OF
+static struct s3c2410_uartcfg s5pv310_uart_defcfg = {
+   .ucon   = 0x3c5,
+   .ufcon  = 0x111,
+   .flags  = NO_NEED_CHECK_CLKSRC,
+   .has_fracval = 1,
+};
+
+static const struct of_device_id s5pv210_uart_dt_match[] = {
+   { .compatible = samsung,s5pv310-uart, .data = s5pv310_uart_defcfg },
+   {},
+};
+MODULE_DEVICE_TABLE(of, s5pv210_uart_match);
+#else
+#define s5pv210_uart_match NULL
+#endif
+
 /* device management */
 static int s5p_serial_probe(struct platform_device *pdev)
 {
-   return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev-id]);
+   static unsigned int probe_index;
+   unsigned int port = pdev-id;
+   const struct of_device_id *match;
+   struct s3c2410_uartcfg *cfg;
+
+   if (pdev-dev.of_node) {
+   if (of_property_read_u32(pdev-dev.of_node,
+   samsung,uart-fifosize,
+   s5p_uart_inf[probe_index]-fifosize))
+   return -EINVAL;
+
+   cfg = devm_kzalloc(pdev-dev, sizeof(*cfg), GFP_KERNEL);
+   if (!cfg)
+   return -ENOMEM;
+
+   match = of_match_node(s5pv210_uart_dt_match, pdev-dev.of_node);
+   *cfg = *((struct s3c2410_uartcfg *)match-data);
+   s5p_uart_inf[probe_index]-cfg = cfg;
+   cfg-hwport = probe_index;
+   port = probe_index++;
+   }
+
+   return s3c24xx_serial_probe(pdev, s5p_uart_inf[port]);
 }
 
 static struct platform_driver s5p_serial_driver = {
@@ -144,6 +184,7 @@ static struct platform_driver s5p_serial_driver = {
.driver = {
.name   = s5pv210-uart,
.owner  = THIS_MODULE,
+   .of_match_table = s5pv210_uart_dt_match,
},
 };
 
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 47fee73..49ba4a5a 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1070,10 +1070,13 @@ static int s3c24xx_serial_init_port(struct 
s3c24xx_uart_port *ourport,
 
/*
 * If platform data is supplied, keep a copy of the location of
-* platform data in the driver's private data.
+* platform data in the driver's private data. In case of device tree
+* based probe, the location of platform data is already set.
 */
if (cfg)
info-cfg = cfg;
+   else
+   cfg = info-cfg;
 
if (cfg-hwport  CONFIG_SERIAL_SAMSUNG_UARTS) {
printk(KERN_ERR %s: port %d bigger than %d\n, __func__,
-- 
1.7.4.1

--
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