Re: mpc52xx_uart and rs485

2008-09-26 Thread Wolfram Sang
Hello,

On Wed, Sep 24, 2008 at 08:20:16AM -0600, Grant Likely wrote:

  Hi Wolfgang and others, 
  below is patch for mpc52xx_usrt.c which add software rs485 support to 
  this driver.
  It has worked for me and if you think it is good enough it might be 
  useful for someone else.
 
 Thanks for the patch.  I've made a number of comments below and it
 appears that your mail client has damaged the patch.  Can you please
 respin after addressing the comments.  When you repost, please include a
 technical description of what you have done and why.

And please add linux-serial as cc then. I recently read some criticism
about software RS485 (http://article.gmane.org/gmane.linux.serial/2573).
I don't know if these apply to this patch, but it would be nice to have
it sorted out IMHO.

All the best,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: mpc52xx_uart and rs485

2008-09-24 Thread Grant Likely
On Thu, Aug 14, 2008 at 11:23:49AM +0200, Sinisa Denic wrote:
 From bf050adbc092043c1ba6e43373563bb761bb2e40 Mon Sep 17 00:00:00 2001
 From: Sinisa Denic [EMAIL PROTECTED]
 Date: Wed, 13 Aug 2008 15:57:52 +0200
 Subject: [PATCH] mpc52xx_uart RS485 support added
 
 Hi Wolfgang and others, 
 below is patch for mpc52xx_usrt.c which add software rs485 support to 
 this driver.
 It has worked for me and if you think it is good enough it might be 
 useful for someone else.

Thanks for the patch.  I've made a number of comments below and it
appears that your mail client has damaged the patch.  Can you please
respin after addressing the comments.  When you repost, please include a
technical description of what you have done and why.

Thanks,
g.

 ---
  drivers/serial/mpc52xx_uart.c |   47 +
 +---
  1 files changed, 39 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
 index 7a3625f..38b2751 100644
 --- a/drivers/serial/mpc52xx_uart.c
 +++ b/drivers/serial/mpc52xx_uart.c
 @@ -62,7 +62,7 @@
   * by the bootloader or in the platform init code.
   */
  
 -#undef DEBUG
 +//#undef DEBUG

Unrelated changes; please remove.

  
  #include linux/device.h
  #include linux/module.h
 @@ -142,11 +142,13 @@ struct psc_ops {
   int (*rx_rdy)(struct uart_port *port);
   int (*tx_rdy)(struct uart_port *port);
   int (*tx_empty)(struct uart_port *port);
 + int (*tx_empty_enabled)(struct uart_port *port);
   void(*stop_rx)(struct uart_port *port);
   void(*start_tx)(struct uart_port *port);
   void(*stop_tx)(struct uart_port *port);
   void(*rx_clr_irq)(struct uart_port *port);
   void(*tx_clr_irq)(struct uart_port *port);
 + void(*tx_empty_clr_irq)(struct uart_port *port);
   void(*write_char)(struct uart_port *port, unsigned 
 char c);
   unsigned char   (*read_char)(struct uart_port *port);
   void(*cw_disable_ints)(struct uart_port *port);
 @@ -169,7 +171,7 @@ static void mpc52xx_psc_fifo_init(struct uart_port 
 *port)
   out_8(fifo-tfcntl, 0x07);
   out_be16(fifo-tfalarm, 0x80);
  
 - port-read_status_mask |= MPC52xx_PSC_IMR_RXRDY | 
 MPC52xx_PSC_IMR_TXRDY;
 + port-read_status_mask |= MPC52xx_PSC_IMR_RXRDY | 
 MPC52xx_PSC_IMR_TXRDY | MPC52xx_PSC_IMR_TXEMP; 

Patch was line wrapped here by your mailer

   out_be16(psc-mpc52xx_psc_imr, port-read_status_mask);
  }
  
 @@ -200,6 +202,13 @@ static int mpc52xx_psc_tx_rdy(struct uart_port *port)
MPC52xx_PSC_IMR_TXRDY;
  }
  
 +static int mpc52xx_psc_tx_empty_enabled(struct uart_port *port)
 +{
 + return in_be16(PSC(port)-mpc52xx_psc_isr)
 +  port-read_status_mask
 +  MPC52xx_PSC_IMR_TXEMP;
 +}
 +
  static int mpc52xx_psc_tx_empty(struct uart_port *port)
  {
   return in_be16(PSC(port)-mpc52xx_psc_status)
 @@ -209,6 +218,7 @@ static int mpc52xx_psc_tx_empty(struct uart_port 
 *port)
  static void mpc52xx_psc_start_tx(struct uart_port *port)
  {
   port-read_status_mask |= MPC52xx_PSC_IMR_TXRDY;
 + port-read_status_mask |= MPC52xx_PSC_IMR_TXEMP;

You're enabling the TXEMP interrupts unconditionally, regardless of
whether or not rs485 is used.  This adds performance impact to
non-rs485 users.

   out_be16(PSC(port)-mpc52xx_psc_imr, port-read_status_mask);
  }
  
 @@ -232,6 +242,11 @@ static void mpc52xx_psc_tx_clr_irq(struct uart_port 
 *port)
  {
  }
  
 +static void mpc52xx_psc_tx_empty_clr_irq(struct uart_port *port)
 +{
 + port-read_status_mask = ~MPC52xx_PSC_IMR_TXEMP;
 + out_be16(PSC(port)-mpc52xx_psc_imr, port-read_status_mask);
 +}
  static void mpc52xx_psc_write_char(struct uart_port *port, unsigned char 
 c)
  {
   out_8(PSC(port)-mpc52xx_psc_buffer_8, c);
 @@ -275,11 +290,13 @@ static struct psc_ops mpc52xx_psc_ops = {
   .rx_rdy = mpc52xx_psc_rx_rdy,
   .tx_rdy = mpc52xx_psc_tx_rdy,
   .tx_empty = mpc52xx_psc_tx_empty,
 + .tx_empty_enabled = mpc52xx_psc_tx_empty_enabled,
   .stop_rx = mpc52xx_psc_stop_rx,
   .start_tx = mpc52xx_psc_start_tx,
   .stop_tx = mpc52xx_psc_stop_tx,
   .rx_clr_irq = mpc52xx_psc_rx_clr_irq,
   .tx_clr_irq = mpc52xx_psc_tx_clr_irq,
 + .tx_empty_clr_irq = mpc52xx_psc_tx_empty_clr_irq,
   .write_char = mpc52xx_psc_write_char,
   .read_char = mpc52xx_psc_read_char,
   .cw_disable_ints = mpc52xx_psc_cw_disable_ints,
 @@ -583,7 +600,7 @@ mpc52xx_uart_set_termios(struct uart_port *port, 
 struct ktermios *new,
  
  
   mr2 = 0;
 -
 + 

Unrelated whitespace change; please omit from patch

   if (new-c_cflag  CSTOPB)
   mr2 |= MPC52xx_PSC_MODE_TWO_STOP;
   else
 @@ -591,7 +608,6 @@ mpc52xx_uart_set_termios(struct uart_port *port, 
 struct ktermios *new,
   MPC52xx_PSC_MODE_ONE_STOP_5_BITS :