Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-29 Thread Frans Klaver
On Wed, Sep 10, 2014 at 09:30:01PM +0200, Sebastian Andrzej Siewior wrote:
 + /*
 +  * We enable TRIG_GRANU for RX and TX and additionaly we set
 +  * SCR_TX_EMPTY bit. The result is the following:
 +  * - RX_TRIGGER amount of bytes in the FIFO will cause an interrupt.
 +  * - less than RX_TRIGGER number of bytes will also cause an interrupt
 +  *   once the UART decides that there no new bytes arriving.
 +  * - Once THRE is enabled, the interrupt will be fired once the FIFO is
 +  *   empty - the trigger level is ignored here.
 +  *
 +  * Once DMA is enabled:
 +  * - UART will assert the TX DMA line once there is room for TX_TRIGGER
 +  *   bytes in the TX FIFO. On each assert the DMA engine will move
 +  *   TX_TRIGGER bytes into the FIFO.
 +  * - UART will assert the RX DMA line once there are RX_TRIGGER bytes in
 +  *   the FIFO and move RX_TRIGGER bytes.
 +  * This is because treshold and trigger values are the same.

threshold

 + /*
 +  * It claims to be 16C750 compatible however it is a little different.
 +  * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
 +  * have) is enabled via EFR instead of MCR. The type is set here 8250
 +  * just to get things going. UNKNOWN does not work for a few reasons and
 +  * we don't need our own type since we don't use 8250's set_termios()
 +  * and our bugs are handeld via the bug member.

handled

 +  */
 + up.port.type = PORT_8250;
 + up.port.iotype = UPIO_MEM;
 + up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
 + UPF_HARD_FLOW;
 + up.port.private_data = priv;
 +
 + up.port.regshift = 2;
 + up.port.fifosize = 64;
 + up.tx_loadsz = 64;
 + up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
 +#ifdef CONFIG_PM_RUNTIME
 + /*
 +  * PM_RUNTIME is mostly transparent. However to do it right we need to a

need to _do_ a ...?

 +  * TX empty interrupt before we can put the device to auto idle. So if
 +  * PM_RUNTIME is not enabled we don't add that flag and can spare that
 +  * one extra interrupt in the TX path.
 +  */

snip

 +config SERIAL_8250_OMAP
 + tristate Support for OMAP internal UART (8250 based driver)
 + depends on SERIAL_8250  ARCH_OMAP2PLUS
 + help
 +   If you have a machine based on an Texas Instruments OMAP CPU you
 +   can enable its onboard serial ports by enabling this option.
 +
 +   This driver is in early stage and uses ttyS instead of ttyO.
 +

I just wondered if this driver should be marked experimental?


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


Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-29 Thread Sebastian Andrzej Siewior
* Frans Klaver | 2014-09-29 11:38:23 [+0200]:

threshold
fixed

 +/*
 + * It claims to be 16C750 compatible however it is a little different.
 + * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
 + * have) is enabled via EFR instead of MCR. The type is set here 8250
 + * just to get things going. UNKNOWN does not work for a few reasons and
 + * we don't need our own type since we don't use 8250's set_termios()
 + * and our bugs are handeld via the bug member.

handled
replaced that last line with 
or pm callback.

since there no bugs member anymore.


 + */
 +up.port.type = PORT_8250;
 +up.port.iotype = UPIO_MEM;
 +up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
 +UPF_HARD_FLOW;
 +up.port.private_data = priv;
 +
 +up.port.regshift = 2;
 +up.port.fifosize = 64;
 +up.tx_loadsz = 64;
 +up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
 +#ifdef CONFIG_PM_RUNTIME
 +/*
 + * PM_RUNTIME is mostly transparent. However to do it right we need to a

need to _do_ a ...?
I think dropping that 'to' should fix it.

 + * TX empty interrupt before we can put the device to auto idle. So if
 + * PM_RUNTIME is not enabled we don't add that flag and can spare that
 + * one extra interrupt in the TX path.
 + */

snip

 +config SERIAL_8250_OMAP
 +tristate Support for OMAP internal UART (8250 based driver)
 +depends on SERIAL_8250  ARCH_OMAP2PLUS
 +help
 +  If you have a machine based on an Texas Instruments OMAP CPU you
 +  can enable its onboard serial ports by enabling this option.
 +
 +  This driver is in early stage and uses ttyS instead of ttyO.
 +

I just wondered if this driver should be marked experimental?
What did you have in mind? CONFIG_EXPERIMENTAL is gone. After all that
debuging that I had in the meantime I was thinking about dropping that
early stage.

Thanks,
Frans

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


Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-29 Thread Frans Klaver
On Mon, Sep 29, 2014 at 3:27 PM, Sebastian Andrzej Siewior
bige...@linutronix.de wrote:
 * Frans Klaver | 2014-09-29 11:38:23 [+0200]:

threshold
 fixed

 +/*
 + * It claims to be 16C750 compatible however it is a little different.
 + * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
 + * have) is enabled via EFR instead of MCR. The type is set here 8250
 + * just to get things going. UNKNOWN does not work for a few reasons 
 and
 + * we don't need our own type since we don't use 8250's set_termios()
 + * and our bugs are handeld via the bug member.

handled
 replaced that last line with
 or pm callback.

 since there no bugs member anymore.


 + */
 +up.port.type = PORT_8250;
 +up.port.iotype = UPIO_MEM;
 +up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
 +UPF_HARD_FLOW;
 +up.port.private_data = priv;
 +
 +up.port.regshift = 2;
 +up.port.fifosize = 64;
 +up.tx_loadsz = 64;
 +up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
 +#ifdef CONFIG_PM_RUNTIME
 +/*
 + * PM_RUNTIME is mostly transparent. However to do it right we need to 
 a

need to _do_ a ...?
 I think dropping that 'to' should fix it.

Yup.


 + * TX empty interrupt before we can put the device to auto idle. So if
 + * PM_RUNTIME is not enabled we don't add that flag and can spare that
 + * one extra interrupt in the TX path.
 + */

snip

 +config SERIAL_8250_OMAP
 +tristate Support for OMAP internal UART (8250 based driver)
 +depends on SERIAL_8250  ARCH_OMAP2PLUS
 +help
 +  If you have a machine based on an Texas Instruments OMAP CPU you
 +  can enable its onboard serial ports by enabling this option.
 +
 +  This driver is in early stage and uses ttyS instead of ttyO.
 +

I just wondered if this driver should be marked experimental?
 What did you have in mind? CONFIG_EXPERIMENTAL is gone. After all that
 debuging that I had in the meantime I was thinking about dropping that
 early stage.

That was the other option. I'm good with that. Also, I never noticed
CONFIG_EXPERIMENTAL being gone, so that's down the drain ;).


Thanks,
Frans

 Sebastian

 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-16 Thread Sebastian Andrzej Siewior
On 09/11/2014 01:57 PM, Peter Hurley wrote:
 Hi Sebastian,

Hi Peter,

 Nice work. Minor comments within.

Thanks.

 After this is merged, it may be worth investigating how to use Yoshihiro's
 newly-added 8250-based tunable RX trigger interface for omap.

We need to overwrite the FCR callback. First because we can support
trigger levels 1…64 and it it split across two registers and second
because a change here results also in different DMA attributes.

 +++ b/drivers/tty/serial/8250/8250_omap.c
 @@ -0,0 +1,911 @@
 +/*
 + * 8250-core based driver for the OMAP internal UART
 + *
 + *  Copyright (C) 2014 Sebastian Andrzej Siewior
 
 + * based on omap-serial.c, Copyright (C) 2010 Texas Instruments.
 
 or something like that, since this is (partly) based on omap-serial.c

of course.

 + *
 + */
 +
…
 +/*
 + * Ask the core to calculate the divisor for us.
 + */
 +baud = uart_get_baud_rate(port, termios, old,
 +port-uartclk / 16 / 0x,
 +port-uartclk / 13);
 +omap_8250_get_divisor(port, baud, priv);
 +
 +/*
 + * Ok, we're now changing the port state. Do it with
 + * interrupts disabled.
 + */
 +pm_runtime_get_sync(port-dev);
 +spin_lock_irqsave(port-lock, flags);
^^^
 spin_lock_irq(port-lock);
 
 The serial core calls the -set_termios() method with interrupts enabled.

Okay, this could work.

 +
 +/*
 + * Update the per-port timeout.
 + */
 +uart_update_timeout(port, termios-c_cflag, baud);
 +
 +up-port.read_status_mask = UART_LSR_OE | UART_LSR_THRE | UART_LSR_DR;
 +if (termios-c_iflag  INPCK)
 +up-port.read_status_mask |= UART_LSR_FE | UART_LSR_PE;
 +if (termios-c_iflag  (BRKINT | PARMRK))
 ^
   IGNBRK |
 
 Otherwise, the read_status_mask will mask out the BI condition, so
 uart_insert_char() will send '\0' byte as TTY_NORMAL.
 
 The 8250 and omap RX path differed so the omap driver didn't need this
 change, whereas the 8250 driver does.

Updated.

 +up-port.read_status_mask |= UART_LSR_BI;
 +
 +/*
…
 +
 +priv-efr = 0;
 +if (termios-c_cflag  CRTSCTS  up-port.flags  UPF_HARD_FLOW) {
 +/* Enable AUTORTS and AUTOCTS */
 +priv-efr |= UART_EFR_CTS | UART_EFR_RTS;
 +
 +/* Ensure MCR RTS is asserted */
 +up-mcr |= UART_MCR_RTS;
 +}
 +
 +if (up-port.flags  UPF_SOFT_FLOW) {
 
 I'm aware that this is basically from the omap driver but can someone clear
 up if omap hardware can actually do UPF_HARD_FLOW and UPF_SOFT_FLOW
 simultaneously? The datasheets that I've looked at say no.

The one that I have here for am335x says:
The UART module can use hardware or software flow control to manage
transmission and reception.
So yes, you are right about this. I changed this to do UPF_HARD_FLOW if
possible + else UPF_SOFT_FLOW.

 Regards,
 Peter Hurley

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


Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-11 Thread Peter Hurley
Hi Sebastian,

Nice work. Minor comments within.

On 09/10/2014 03:30 PM, Sebastian Andrzej Siewior wrote:
 This patch provides a 8250-core based UART driver for the internal OMAP
 UART. The long term goal is to provide the same functionality as the
 current OMAP uart driver and DMA support.
 I tried to merge omap-serial code together with the 8250-core code.
 There should should be hardly a noticable difference. The trigger levels
 are different compared to omap-serial:
 - omap serial
   TX: Interrupt comes after TX FIFO has room for 16 bytes.
   TX of 4096 bytes in one go results in 256 interrupts
 
   RX: Interrupt comes after there is on byte in the FIFO.
   RX of 4096 bytes results in 4096 interrupts.
 
 - this driver
   TX: Interrupt comes once the TX FIFO is empty.
   TX of 4096 bytes results in 65 interrupts. That means there will
   be gaps on the line while the driver reloads the FIFO.
 
   RX: Interrupt comes once there are 48 bytes in the FIFO or less over
   longer time frame. We have
   1 / 11520 * 10^3 * 16 = 1.38… ms
   1.38ms to react and purge the FIFO on 115200,8N1. Since the other
   driver fired after each byte it had ~5.47ms time to react. This
   _may_ cause problems if one relies on no missing bytes and has no
   flow control. On the other hand we get only 85 interrupts for the
   same amount of data.

After this is merged, it may be worth investigating how to use Yoshihiro's
newly-added 8250-based tunable RX trigger interface for omap.

 It has been only tested as console UART on am335x-evm, dra7-evm and
 beagle bone. I also did some longer raw-transfers to meassure the load.
 
 The device name is ttyS based instead of ttyO. If a ttyO based node name
 is required please ask udev for it. If both driver are activated (this
 and omap-serial) then this serial driver will take control over the
 device due to the link order
 
 v8…v9:
   - less on a file seems to hang the am335x after a while. I
 believe I introduce this bug a while ago since I can reproduce
 this prior to v8. Fixed by redoing the omap8250_restore_regs()
 v7…v8:
   - redo the register write. There is now one function for that
 which is used from set_termios() and runtime-resume.
   - drop PORT_OMAP_16750 and move the setup to the omap file. We
 have our own set termios function anyway (Heikki Krogerus)
   - use MEM instead of MEM32. TRM of AM/DM37x says that 32bit
 access on THR might result in data abort. We only need 32bit
 access in the errata function which is before we use 8250's
 read function so it doesn't matter.
 v4…v7:
   - change trigger levels after some tests with raw transfers.
 v3…v4:
   - drop RS485 support
   - wire up -throttle / -unthrottle
 v2…v3:
   - wire up startup  shutdown for wakeup-irq handling.
   - RS485 handling (well the core does).
 
 v1…v2:
   - added runtime PM. Could somebody could please double check
 this?
   - added omap_8250_set_termios()
 
 Reviewed-by: Tony Lindgren t...@atomide.com
 Tested-by: Tony Lindgren t...@atomide.com
 Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
 ---
  drivers/tty/serial/8250/8250_omap.c | 911 
 
  drivers/tty/serial/8250/Kconfig |   9 +
  drivers/tty/serial/8250/Makefile|   1 +
  3 files changed, 921 insertions(+)
  create mode 100644 drivers/tty/serial/8250/8250_omap.c
 
 diff --git a/drivers/tty/serial/8250/8250_omap.c 
 b/drivers/tty/serial/8250/8250_omap.c
 new file mode 100644
 index ..2a187b00ed0a
 --- /dev/null
 +++ b/drivers/tty/serial/8250/8250_omap.c
 @@ -0,0 +1,911 @@
 +/*
 + * 8250-core based driver for the OMAP internal UART
 + *
 + *  Copyright (C) 2014 Sebastian Andrzej Siewior

+ * based on omap-serial.c, Copyright (C) 2010 Texas Instruments.

or something like that, since this is (partly) based on omap-serial.c

 + *
 + */
 +
 +#include linux/device.h
 +#include linux/io.h
 +#include linux/module.h
 +#include linux/serial_8250.h
 +#include linux/serial_core.h
 +#include linux/serial_reg.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/of.h
 +#include linux/of_gpio.h
 +#include linux/of_irq.h
 +#include linux/delay.h
 +#include linux/pm_runtime.h
 +#include linux/console.h
 +#include linux/pm_qos.h
 +
 +#include 8250.h
 +
 +#define DEFAULT_CLK_SPEED4800
 +
 +#define UART_ERRATA_i202_MDR1_ACCESS (1  0)
 +#define OMAP_UART_WER_HAS_TX_WAKEUP  (1  1)
 +
 +#define OMAP_UART_FCR_RX_TRIG6
 +#define OMAP_UART_FCR_TX_TRIG4
 +
 +/* SCR register bitmasks */
 +#define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK(1  7)
 +#define OMAP_UART_SCR_TX_TRIG_GRANU1_MASK(1  6)
 +#define OMAP_UART_SCR_TX_EMPTY   (1  3)
 +#define OMAP_UART_SCR_DMAMODE_MASK   (3  1)
 +#define OMAP_UART_SCR_DMAMODE_1  (1  1)
 +#define