Re: [PATCH 4/8] tty: atmel_serial: prepare clk before calling enable

2013-06-20 Thread Nicolas Ferre

On 20/06/2013 10:06, boris brezillon :

On 20/06/2013 09:48, Nicolas Ferre wrote:

On 19/06/2013 13:17, Boris BREZILLON :

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON 


Acked-by: Nicolas Ferre 

(one comment though)


---
  drivers/tty/serial/atmel_serial.c |   41
-
  1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 3467462..691265f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1100,7 +1100,7 @@ static void atmel_serial_pm(struct uart_port
*port, unsigned int state,
   * Enable the peripheral clock for this serial port.
   * This is called on uart_open() or a resume event.
   */
-clk_enable(atmel_port->clk);
+clk_prepare_enable(atmel_port->clk);


Do we need to check return code here?

We could, but the pm function will not reflect the failure (void return).
In addition, the clk_prepare_enable should not fail because it already
succeed in the probe
function (no memory allocation, simple bit masking in clk-peripheral
implem).

I can add it add this check (and avoid the interrrupt activation if it
fails) if you want.


No, it seems okay like that. Thanks for the precision.

Bye,







  /* re-enable interrupts if we disabled some on suspend */
  UART_PUT_IER(port, atmel_port->backup_imr);
@@ -1114,7 +1114,7 @@ static void atmel_serial_pm(struct uart_port
*port, unsigned int state,
   * Disable the peripheral clock for this serial port.
   * This is called on uart_close() or a suspend event.
   */
-clk_disable(atmel_port->clk);
+clk_disable_unprepare(atmel_port->clk);
  break;
  default:
  printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
@@ -1458,9 +1458,10 @@ static void atmel_of_init_port(struct
atmel_uart_port *atmel_port,
  /*
   * Configure the port from the platform device resource info.
   */
-static void atmel_init_port(struct atmel_uart_port *atmel_port,
+static int atmel_init_port(struct atmel_uart_port *atmel_port,
struct platform_device *pdev)
  {
+int ret;
  struct uart_port *port = _port->uart;
  struct atmel_uart_data *pdata = pdev->dev.platform_data;

@@ -1496,9 +1497,19 @@ static void atmel_init_port(struct
atmel_uart_port *atmel_port,
  /* for console, the clock could already be configured */
  if (!atmel_port->clk) {
  atmel_port->clk = clk_get(>dev, "usart");
-clk_enable(atmel_port->clk);
+if (IS_ERR(atmel_port->clk)) {
+ret = PTR_ERR(atmel_port->clk);
+atmel_port->clk = NULL;
+return ret;
+}
+ret = clk_prepare_enable(atmel_port->clk);
+if (ret) {
+clk_put(atmel_port->clk);
+atmel_port->clk = NULL;
+return ret;
+}
  port->uartclk = clk_get_rate(atmel_port->clk);
-clk_disable(atmel_port->clk);
+clk_disable_unprepare(atmel_port->clk);
  /* only enable clock when USART is in use */
  }

@@ -1511,6 +1522,8 @@ static void atmel_init_port(struct
atmel_uart_port *atmel_port,
  } else {
  atmel_port->tx_done_mask = ATMEL_US_TXRDY;
  }
+
+return 0;
  }

  struct platform_device *atmel_default_console_device;/* the
serial console device */
@@ -1601,6 +1614,7 @@ static void __init
atmel_console_get_options(struct uart_port *port, int *baud,

  static int __init atmel_console_setup(struct console *co, char
*options)
  {
+int ret;
  struct uart_port *port = _ports[co->index].uart;
  int baud = 115200;
  int bits = 8;
@@ -1612,7 +1626,9 @@ static int __init atmel_console_setup(struct
console *co, char *options)
  return -ENODEV;
  }

-clk_enable(atmel_ports[co->index].clk);
+ret = clk_prepare_enable(atmel_ports[co->index].clk);
+if (ret)
+return ret;

  UART_PUT_IDR(port, -1);
  UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
@@ -1645,6 +1661,7 @@ static struct console atmel_console = {
   */
  static int __init atmel_console_init(void)
  {
+int ret;
  if (atmel_default_console_device) {
  struct atmel_uart_data *pdata =
atmel_default_console_device->dev.platform_data;
@@ -1655,7 +1672,9 @@ static int __init atmel_console_init(void)
  port->uart.line = id;

  add_preferred_console(ATMEL_DEVICENAME, id, NULL);
-atmel_init_port(port, atmel_default_console_device);
+ret = atmel_init_port(port, atmel_default_console_device);
+if (ret)
+return ret;
  register_console(_console);
  }

@@ -1786,7 +1805,9 @@ static int atmel_serial_probe(struct
platform_device *pdev)
  port->backup_imr = 0;
  port->uart.line = ret;

-atmel_init_port(port, pdev);

Re: [PATCH 4/8] tty: atmel_serial: prepare clk before calling enable

2013-06-20 Thread boris brezillon

On 20/06/2013 09:48, Nicolas Ferre wrote:

On 19/06/2013 13:17, Boris BREZILLON :

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON 


Acked-by: Nicolas Ferre 

(one comment though)


---
  drivers/tty/serial/atmel_serial.c |   41 
-

  1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c

index 3467462..691265f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1100,7 +1100,7 @@ static void atmel_serial_pm(struct uart_port 
*port, unsigned int state,

   * Enable the peripheral clock for this serial port.
   * This is called on uart_open() or a resume event.
   */
-clk_enable(atmel_port->clk);
+clk_prepare_enable(atmel_port->clk);


Do we need to check return code here?

We could, but the pm function will not reflect the failure (void return).
In addition, the clk_prepare_enable should not fail because it already 
succeed in the probe
function (no memory allocation, simple bit masking in clk-peripheral 
implem).


I can add it add this check (and avoid the interrrupt activation if it 
fails) if you want.






  /* re-enable interrupts if we disabled some on suspend */
  UART_PUT_IER(port, atmel_port->backup_imr);
@@ -1114,7 +1114,7 @@ static void atmel_serial_pm(struct uart_port 
*port, unsigned int state,

   * Disable the peripheral clock for this serial port.
   * This is called on uart_close() or a suspend event.
   */
-clk_disable(atmel_port->clk);
+clk_disable_unprepare(atmel_port->clk);
  break;
  default:
  printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
@@ -1458,9 +1458,10 @@ static void atmel_of_init_port(struct 
atmel_uart_port *atmel_port,

  /*
   * Configure the port from the platform device resource info.
   */
-static void atmel_init_port(struct atmel_uart_port *atmel_port,
+static int atmel_init_port(struct atmel_uart_port *atmel_port,
struct platform_device *pdev)
  {
+int ret;
  struct uart_port *port = _port->uart;
  struct atmel_uart_data *pdata = pdev->dev.platform_data;

@@ -1496,9 +1497,19 @@ static void atmel_init_port(struct 
atmel_uart_port *atmel_port,

  /* for console, the clock could already be configured */
  if (!atmel_port->clk) {
  atmel_port->clk = clk_get(>dev, "usart");
-clk_enable(atmel_port->clk);
+if (IS_ERR(atmel_port->clk)) {
+ret = PTR_ERR(atmel_port->clk);
+atmel_port->clk = NULL;
+return ret;
+}
+ret = clk_prepare_enable(atmel_port->clk);
+if (ret) {
+clk_put(atmel_port->clk);
+atmel_port->clk = NULL;
+return ret;
+}
  port->uartclk = clk_get_rate(atmel_port->clk);
-clk_disable(atmel_port->clk);
+clk_disable_unprepare(atmel_port->clk);
  /* only enable clock when USART is in use */
  }

@@ -1511,6 +1522,8 @@ static void atmel_init_port(struct 
atmel_uart_port *atmel_port,

  } else {
  atmel_port->tx_done_mask = ATMEL_US_TXRDY;
  }
+
+return 0;
  }

  struct platform_device *atmel_default_console_device;/* the 
serial console device */
@@ -1601,6 +1614,7 @@ static void __init 
atmel_console_get_options(struct uart_port *port, int *baud,


  static int __init atmel_console_setup(struct console *co, char 
*options)

  {
+int ret;
  struct uart_port *port = _ports[co->index].uart;
  int baud = 115200;
  int bits = 8;
@@ -1612,7 +1626,9 @@ static int __init atmel_console_setup(struct 
console *co, char *options)

  return -ENODEV;
  }

-clk_enable(atmel_ports[co->index].clk);
+ret = clk_prepare_enable(atmel_ports[co->index].clk);
+if (ret)
+return ret;

  UART_PUT_IDR(port, -1);
  UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
@@ -1645,6 +1661,7 @@ static struct console atmel_console = {
   */
  static int __init atmel_console_init(void)
  {
+int ret;
  if (atmel_default_console_device) {
  struct atmel_uart_data *pdata =
atmel_default_console_device->dev.platform_data;
@@ -1655,7 +1672,9 @@ static int __init atmel_console_init(void)
  port->uart.line = id;

  add_preferred_console(ATMEL_DEVICENAME, id, NULL);
-atmel_init_port(port, atmel_default_console_device);
+ret = atmel_init_port(port, atmel_default_console_device);
+if (ret)
+return ret;
  register_console(_console);
  }

@@ -1786,7 +1805,9 @@ static int atmel_serial_probe(struct 
platform_device *pdev)

  port->backup_imr = 0;
  port->uart.line = ret;

-atmel_init_port(port, pdev);
+ret = atmel_init_port(port, pdev);
+if (ret)
+goto err;

   

Re: [PATCH 4/8] tty: atmel_serial: prepare clk before calling enable

2013-06-20 Thread Nicolas Ferre

On 19/06/2013 13:17, Boris BREZILLON :

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON 


Acked-by: Nicolas Ferre 

(one comment though)


---
  drivers/tty/serial/atmel_serial.c |   41 -
  1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index 3467462..691265f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1100,7 +1100,7 @@ static void atmel_serial_pm(struct uart_port *port, 
unsigned int state,
 * Enable the peripheral clock for this serial port.
 * This is called on uart_open() or a resume event.
 */
-   clk_enable(atmel_port->clk);
+   clk_prepare_enable(atmel_port->clk);


Do we need to check return code here?



/* re-enable interrupts if we disabled some on suspend */
UART_PUT_IER(port, atmel_port->backup_imr);
@@ -1114,7 +1114,7 @@ static void atmel_serial_pm(struct uart_port *port, 
unsigned int state,
 * Disable the peripheral clock for this serial port.
 * This is called on uart_close() or a suspend event.
 */
-   clk_disable(atmel_port->clk);
+   clk_disable_unprepare(atmel_port->clk);
break;
default:
printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
@@ -1458,9 +1458,10 @@ static void atmel_of_init_port(struct atmel_uart_port 
*atmel_port,
  /*
   * Configure the port from the platform device resource info.
   */
-static void atmel_init_port(struct atmel_uart_port *atmel_port,
+static int atmel_init_port(struct atmel_uart_port *atmel_port,
  struct platform_device *pdev)
  {
+   int ret;
struct uart_port *port = _port->uart;
struct atmel_uart_data *pdata = pdev->dev.platform_data;

@@ -1496,9 +1497,19 @@ static void atmel_init_port(struct atmel_uart_port 
*atmel_port,
/* for console, the clock could already be configured */
if (!atmel_port->clk) {
atmel_port->clk = clk_get(>dev, "usart");
-   clk_enable(atmel_port->clk);
+   if (IS_ERR(atmel_port->clk)) {
+   ret = PTR_ERR(atmel_port->clk);
+   atmel_port->clk = NULL;
+   return ret;
+   }
+   ret = clk_prepare_enable(atmel_port->clk);
+   if (ret) {
+   clk_put(atmel_port->clk);
+   atmel_port->clk = NULL;
+   return ret;
+   }
port->uartclk = clk_get_rate(atmel_port->clk);
-   clk_disable(atmel_port->clk);
+   clk_disable_unprepare(atmel_port->clk);
/* only enable clock when USART is in use */
}

@@ -1511,6 +1522,8 @@ static void atmel_init_port(struct atmel_uart_port 
*atmel_port,
} else {
atmel_port->tx_done_mask = ATMEL_US_TXRDY;
}
+
+   return 0;
  }

  struct platform_device *atmel_default_console_device; /* the serial console 
device */
@@ -1601,6 +1614,7 @@ static void __init atmel_console_get_options(struct 
uart_port *port, int *baud,

  static int __init atmel_console_setup(struct console *co, char *options)
  {
+   int ret;
struct uart_port *port = _ports[co->index].uart;
int baud = 115200;
int bits = 8;
@@ -1612,7 +1626,9 @@ static int __init atmel_console_setup(struct console *co, 
char *options)
return -ENODEV;
}

-   clk_enable(atmel_ports[co->index].clk);
+   ret = clk_prepare_enable(atmel_ports[co->index].clk);
+   if (ret)
+   return ret;

UART_PUT_IDR(port, -1);
UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
@@ -1645,6 +1661,7 @@ static struct console atmel_console = {
   */
  static int __init atmel_console_init(void)
  {
+   int ret;
if (atmel_default_console_device) {
struct atmel_uart_data *pdata =
atmel_default_console_device->dev.platform_data;
@@ -1655,7 +1672,9 @@ static int __init atmel_console_init(void)
port->uart.line = id;

add_preferred_console(ATMEL_DEVICENAME, id, NULL);
-   atmel_init_port(port, atmel_default_console_device);
+   ret = atmel_init_port(port, atmel_default_console_device);
+   if (ret)
+   return ret;
register_console(_console);
}

@@ -1786,7 +1805,9 @@ static int atmel_serial_probe(struct platform_device 
*pdev)
port->backup_imr = 0;
port->uart.line = ret;

-   atmel_init_port(port, pdev);
+   ret = atmel_init_port(port, pdev);
+   if (ret)
+   

Re: [PATCH 4/8] tty: atmel_serial: prepare clk before calling enable

2013-06-20 Thread Nicolas Ferre

On 19/06/2013 13:17, Boris BREZILLON :

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com


Acked-by: Nicolas Ferre nicolas.fe...@atmel.com

(one comment though)


---
  drivers/tty/serial/atmel_serial.c |   41 -
  1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index 3467462..691265f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1100,7 +1100,7 @@ static void atmel_serial_pm(struct uart_port *port, 
unsigned int state,
 * Enable the peripheral clock for this serial port.
 * This is called on uart_open() or a resume event.
 */
-   clk_enable(atmel_port-clk);
+   clk_prepare_enable(atmel_port-clk);


Do we need to check return code here?



/* re-enable interrupts if we disabled some on suspend */
UART_PUT_IER(port, atmel_port-backup_imr);
@@ -1114,7 +1114,7 @@ static void atmel_serial_pm(struct uart_port *port, 
unsigned int state,
 * Disable the peripheral clock for this serial port.
 * This is called on uart_close() or a suspend event.
 */
-   clk_disable(atmel_port-clk);
+   clk_disable_unprepare(atmel_port-clk);
break;
default:
printk(KERN_ERR atmel_serial: unknown pm %d\n, state);
@@ -1458,9 +1458,10 @@ static void atmel_of_init_port(struct atmel_uart_port 
*atmel_port,
  /*
   * Configure the port from the platform device resource info.
   */
-static void atmel_init_port(struct atmel_uart_port *atmel_port,
+static int atmel_init_port(struct atmel_uart_port *atmel_port,
  struct platform_device *pdev)
  {
+   int ret;
struct uart_port *port = atmel_port-uart;
struct atmel_uart_data *pdata = pdev-dev.platform_data;

@@ -1496,9 +1497,19 @@ static void atmel_init_port(struct atmel_uart_port 
*atmel_port,
/* for console, the clock could already be configured */
if (!atmel_port-clk) {
atmel_port-clk = clk_get(pdev-dev, usart);
-   clk_enable(atmel_port-clk);
+   if (IS_ERR(atmel_port-clk)) {
+   ret = PTR_ERR(atmel_port-clk);
+   atmel_port-clk = NULL;
+   return ret;
+   }
+   ret = clk_prepare_enable(atmel_port-clk);
+   if (ret) {
+   clk_put(atmel_port-clk);
+   atmel_port-clk = NULL;
+   return ret;
+   }
port-uartclk = clk_get_rate(atmel_port-clk);
-   clk_disable(atmel_port-clk);
+   clk_disable_unprepare(atmel_port-clk);
/* only enable clock when USART is in use */
}

@@ -1511,6 +1522,8 @@ static void atmel_init_port(struct atmel_uart_port 
*atmel_port,
} else {
atmel_port-tx_done_mask = ATMEL_US_TXRDY;
}
+
+   return 0;
  }

  struct platform_device *atmel_default_console_device; /* the serial console 
device */
@@ -1601,6 +1614,7 @@ static void __init atmel_console_get_options(struct 
uart_port *port, int *baud,

  static int __init atmel_console_setup(struct console *co, char *options)
  {
+   int ret;
struct uart_port *port = atmel_ports[co-index].uart;
int baud = 115200;
int bits = 8;
@@ -1612,7 +1626,9 @@ static int __init atmel_console_setup(struct console *co, 
char *options)
return -ENODEV;
}

-   clk_enable(atmel_ports[co-index].clk);
+   ret = clk_prepare_enable(atmel_ports[co-index].clk);
+   if (ret)
+   return ret;

UART_PUT_IDR(port, -1);
UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
@@ -1645,6 +1661,7 @@ static struct console atmel_console = {
   */
  static int __init atmel_console_init(void)
  {
+   int ret;
if (atmel_default_console_device) {
struct atmel_uart_data *pdata =
atmel_default_console_device-dev.platform_data;
@@ -1655,7 +1672,9 @@ static int __init atmel_console_init(void)
port-uart.line = id;

add_preferred_console(ATMEL_DEVICENAME, id, NULL);
-   atmel_init_port(port, atmel_default_console_device);
+   ret = atmel_init_port(port, atmel_default_console_device);
+   if (ret)
+   return ret;
register_console(atmel_console);
}

@@ -1786,7 +1805,9 @@ static int atmel_serial_probe(struct platform_device 
*pdev)
port-backup_imr = 0;
port-uart.line = ret;

-   atmel_init_port(port, pdev);
+   ret = atmel_init_port(port, pdev);
+  

Re: [PATCH 4/8] tty: atmel_serial: prepare clk before calling enable

2013-06-20 Thread boris brezillon

On 20/06/2013 09:48, Nicolas Ferre wrote:

On 19/06/2013 13:17, Boris BREZILLON :

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com


Acked-by: Nicolas Ferre nicolas.fe...@atmel.com

(one comment though)


---
  drivers/tty/serial/atmel_serial.c |   41 
-

  1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c

index 3467462..691265f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1100,7 +1100,7 @@ static void atmel_serial_pm(struct uart_port 
*port, unsigned int state,

   * Enable the peripheral clock for this serial port.
   * This is called on uart_open() or a resume event.
   */
-clk_enable(atmel_port-clk);
+clk_prepare_enable(atmel_port-clk);


Do we need to check return code here?

We could, but the pm function will not reflect the failure (void return).
In addition, the clk_prepare_enable should not fail because it already 
succeed in the probe
function (no memory allocation, simple bit masking in clk-peripheral 
implem).


I can add it add this check (and avoid the interrrupt activation if it 
fails) if you want.






  /* re-enable interrupts if we disabled some on suspend */
  UART_PUT_IER(port, atmel_port-backup_imr);
@@ -1114,7 +1114,7 @@ static void atmel_serial_pm(struct uart_port 
*port, unsigned int state,

   * Disable the peripheral clock for this serial port.
   * This is called on uart_close() or a suspend event.
   */
-clk_disable(atmel_port-clk);
+clk_disable_unprepare(atmel_port-clk);
  break;
  default:
  printk(KERN_ERR atmel_serial: unknown pm %d\n, state);
@@ -1458,9 +1458,10 @@ static void atmel_of_init_port(struct 
atmel_uart_port *atmel_port,

  /*
   * Configure the port from the platform device resource info.
   */
-static void atmel_init_port(struct atmel_uart_port *atmel_port,
+static int atmel_init_port(struct atmel_uart_port *atmel_port,
struct platform_device *pdev)
  {
+int ret;
  struct uart_port *port = atmel_port-uart;
  struct atmel_uart_data *pdata = pdev-dev.platform_data;

@@ -1496,9 +1497,19 @@ static void atmel_init_port(struct 
atmel_uart_port *atmel_port,

  /* for console, the clock could already be configured */
  if (!atmel_port-clk) {
  atmel_port-clk = clk_get(pdev-dev, usart);
-clk_enable(atmel_port-clk);
+if (IS_ERR(atmel_port-clk)) {
+ret = PTR_ERR(atmel_port-clk);
+atmel_port-clk = NULL;
+return ret;
+}
+ret = clk_prepare_enable(atmel_port-clk);
+if (ret) {
+clk_put(atmel_port-clk);
+atmel_port-clk = NULL;
+return ret;
+}
  port-uartclk = clk_get_rate(atmel_port-clk);
-clk_disable(atmel_port-clk);
+clk_disable_unprepare(atmel_port-clk);
  /* only enable clock when USART is in use */
  }

@@ -1511,6 +1522,8 @@ static void atmel_init_port(struct 
atmel_uart_port *atmel_port,

  } else {
  atmel_port-tx_done_mask = ATMEL_US_TXRDY;
  }
+
+return 0;
  }

  struct platform_device *atmel_default_console_device;/* the 
serial console device */
@@ -1601,6 +1614,7 @@ static void __init 
atmel_console_get_options(struct uart_port *port, int *baud,


  static int __init atmel_console_setup(struct console *co, char 
*options)

  {
+int ret;
  struct uart_port *port = atmel_ports[co-index].uart;
  int baud = 115200;
  int bits = 8;
@@ -1612,7 +1626,9 @@ static int __init atmel_console_setup(struct 
console *co, char *options)

  return -ENODEV;
  }

-clk_enable(atmel_ports[co-index].clk);
+ret = clk_prepare_enable(atmel_ports[co-index].clk);
+if (ret)
+return ret;

  UART_PUT_IDR(port, -1);
  UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
@@ -1645,6 +1661,7 @@ static struct console atmel_console = {
   */
  static int __init atmel_console_init(void)
  {
+int ret;
  if (atmel_default_console_device) {
  struct atmel_uart_data *pdata =
atmel_default_console_device-dev.platform_data;
@@ -1655,7 +1672,9 @@ static int __init atmel_console_init(void)
  port-uart.line = id;

  add_preferred_console(ATMEL_DEVICENAME, id, NULL);
-atmel_init_port(port, atmel_default_console_device);
+ret = atmel_init_port(port, atmel_default_console_device);
+if (ret)
+return ret;
  register_console(atmel_console);
  }

@@ -1786,7 +1805,9 @@ static int atmel_serial_probe(struct 
platform_device *pdev)

  port-backup_imr = 0;
  port-uart.line = ret;

-atmel_init_port(port, pdev);
+ret = atmel_init_port(port, pdev);
+   

Re: [PATCH 4/8] tty: atmel_serial: prepare clk before calling enable

2013-06-20 Thread Nicolas Ferre

On 20/06/2013 10:06, boris brezillon :

On 20/06/2013 09:48, Nicolas Ferre wrote:

On 19/06/2013 13:17, Boris BREZILLON :

Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.

Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com


Acked-by: Nicolas Ferre nicolas.fe...@atmel.com

(one comment though)


---
  drivers/tty/serial/atmel_serial.c |   41
-
  1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 3467462..691265f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1100,7 +1100,7 @@ static void atmel_serial_pm(struct uart_port
*port, unsigned int state,
   * Enable the peripheral clock for this serial port.
   * This is called on uart_open() or a resume event.
   */
-clk_enable(atmel_port-clk);
+clk_prepare_enable(atmel_port-clk);


Do we need to check return code here?

We could, but the pm function will not reflect the failure (void return).
In addition, the clk_prepare_enable should not fail because it already
succeed in the probe
function (no memory allocation, simple bit masking in clk-peripheral
implem).

I can add it add this check (and avoid the interrrupt activation if it
fails) if you want.


No, it seems okay like that. Thanks for the precision.

Bye,







  /* re-enable interrupts if we disabled some on suspend */
  UART_PUT_IER(port, atmel_port-backup_imr);
@@ -1114,7 +1114,7 @@ static void atmel_serial_pm(struct uart_port
*port, unsigned int state,
   * Disable the peripheral clock for this serial port.
   * This is called on uart_close() or a suspend event.
   */
-clk_disable(atmel_port-clk);
+clk_disable_unprepare(atmel_port-clk);
  break;
  default:
  printk(KERN_ERR atmel_serial: unknown pm %d\n, state);
@@ -1458,9 +1458,10 @@ static void atmel_of_init_port(struct
atmel_uart_port *atmel_port,
  /*
   * Configure the port from the platform device resource info.
   */
-static void atmel_init_port(struct atmel_uart_port *atmel_port,
+static int atmel_init_port(struct atmel_uart_port *atmel_port,
struct platform_device *pdev)
  {
+int ret;
  struct uart_port *port = atmel_port-uart;
  struct atmel_uart_data *pdata = pdev-dev.platform_data;

@@ -1496,9 +1497,19 @@ static void atmel_init_port(struct
atmel_uart_port *atmel_port,
  /* for console, the clock could already be configured */
  if (!atmel_port-clk) {
  atmel_port-clk = clk_get(pdev-dev, usart);
-clk_enable(atmel_port-clk);
+if (IS_ERR(atmel_port-clk)) {
+ret = PTR_ERR(atmel_port-clk);
+atmel_port-clk = NULL;
+return ret;
+}
+ret = clk_prepare_enable(atmel_port-clk);
+if (ret) {
+clk_put(atmel_port-clk);
+atmel_port-clk = NULL;
+return ret;
+}
  port-uartclk = clk_get_rate(atmel_port-clk);
-clk_disable(atmel_port-clk);
+clk_disable_unprepare(atmel_port-clk);
  /* only enable clock when USART is in use */
  }

@@ -1511,6 +1522,8 @@ static void atmel_init_port(struct
atmel_uart_port *atmel_port,
  } else {
  atmel_port-tx_done_mask = ATMEL_US_TXRDY;
  }
+
+return 0;
  }

  struct platform_device *atmel_default_console_device;/* the
serial console device */
@@ -1601,6 +1614,7 @@ static void __init
atmel_console_get_options(struct uart_port *port, int *baud,

  static int __init atmel_console_setup(struct console *co, char
*options)
  {
+int ret;
  struct uart_port *port = atmel_ports[co-index].uart;
  int baud = 115200;
  int bits = 8;
@@ -1612,7 +1626,9 @@ static int __init atmel_console_setup(struct
console *co, char *options)
  return -ENODEV;
  }

-clk_enable(atmel_ports[co-index].clk);
+ret = clk_prepare_enable(atmel_ports[co-index].clk);
+if (ret)
+return ret;

  UART_PUT_IDR(port, -1);
  UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
@@ -1645,6 +1661,7 @@ static struct console atmel_console = {
   */
  static int __init atmel_console_init(void)
  {
+int ret;
  if (atmel_default_console_device) {
  struct atmel_uart_data *pdata =
atmel_default_console_device-dev.platform_data;
@@ -1655,7 +1672,9 @@ static int __init atmel_console_init(void)
  port-uart.line = id;

  add_preferred_console(ATMEL_DEVICENAME, id, NULL);
-atmel_init_port(port, atmel_default_console_device);
+ret = atmel_init_port(port, atmel_default_console_device);
+if (ret)
+return ret;
  register_console(atmel_console);
  }

@@ -1786,7 +1805,9 @@ static int atmel_serial_probe(struct
platform_device *pdev)
  port-backup_imr = 0;
  port-uart.line = ret;