Hi Simon,

Hi Akshay

...

On Thu, Mar 21, 2013 at 11:33 PM, Akshay Saraswat akshay.s@samsung.com wrote:

This patch enables the uart tx/rx fifo. Now that fifo is enabled, the uart read/write functions are modfied to check the UFSTAT register for fifo status instead of UTRSTAT (as required with fifo's enabled). Tested by booting linux kernel. Before enabling tx/rx fifo "Uncompressing linux" message is garbled and after enabling it is proper.

Is this because Linux enables the FIFOs?

Anyway this seems fine to me, but I have a question below.

...

Signed-off-by: Alim Akhtar alim.akhtar@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com

---

drivers/serial/serial_s5p.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 3c41242..e65125c 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -30,6 +30,10 @@

DECLARE_GLOBAL_DATA_PTR;

+#define RX_FIFO_COUNT_MASK 0xff +#define RX_FIFO_FULL_MASK (1 << 8) +#define TX_FIFO_FULL_MASK (1 << 24)

static inline struct s5p_uart *s5p_get_base_uart(int dev_index) { u32 offset = dev_index * sizeof(struct s5p_uart); @@ -87,8 +91,8 @@ int serial_init_dev(const int dev_index) { struct s5p_uart *const uart = s5p_get_base_uart(dev_index);

•   /* reset and enable FIFOs, set triggers to the maximum */
  

•   writel(0, &uart->ufcon);
  

•   /* enable FIFOs */
  

•   writel(0x1, &uart->ufcon);
  
  

It is odd that you seem to be saying that the old code did not enable FIFOs, but this code does? Or should you update your comment?

Actually old comment was inapt because we were resetting FIFO's but not enabling FIFO. As per manual, for enabling FIFO we need to set LSB of UFCON register, which was missing. But now we are setting this bit and it works as expected. This could be verified by noticing that before applying this patch "Uncompressing linux" message printed garbled/not readable on console but after applying this patch characters appear well in place. Please suggest if this comment needs edit.

...

   writel(0, &uart->umcon);
   /* 8N1 */
   writel(0x3, &uart->ulcon);

@@ -130,7 +134,8 @@ int serial_getc_dev(const int dev_index) struct s5p_uart *const uart = s5p_get_base_uart(dev_index);

   /* wait for character to arrive */

•   while (!(readl(&uart->utrstat) & 0x1)) {
  

•   while (!(readl(&uart->ufstat) & (RX_FIFO_COUNT_MASK |
  

•                                    RX_FIFO_FULL_MASK))) {
           if (serial_err_check(dev_index, 0))
                   return 0;
   }
  

@@ -146,7 +151,7 @@ void serial_putc_dev(const char c, const int dev_index) struct s5p_uart *const uart = s5p_get_base_uart(dev_index);

   /* wait for room in the tx FIFO */

•   while (!(readl(&uart->utrstat) & 0x2)) {
  

•   while ((readl(&uart->ufstat) & TX_FIFO_FULL_MASK)) {
           if (serial_err_check(dev_index, 1))
                   return;
   }
  

-- 1.8.0

Regards,

Simon

Thanks & Regards, Akshay Saraswat