On Mon, Oct 10, 2016 at 01:52:53PM +0300, Andy Shevchenko wrote:
> Provide an interface to 8250 driver to set DMA mode. It will be effective
> for the transfers through the port next time it will be opened.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> Documentation/ABI/testing/sysfs-tty | 11 ++++
> drivers/tty/serial/8250/8250_port.c | 110 +++++++++++++++++++++++++++++++++++-
> 2 files changed, 119 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-tty b/Documentation/ABI/testing/sysfs-tty
> index 9eb3c2b..1dd68c9 100644
> --- a/Documentation/ABI/testing/sysfs-tty
> +++ b/Documentation/ABI/testing/sysfs-tty
> @@ -154,3 +154,14 @@ Description:
> device specification. For example, when user sets 7bytes on
> 16550A, which has 1/4/8/14 bytes trigger, the RX trigger is
> automatically changed to 4 bytes.
> +
> +What: /sys/class/tty/ttyS0/skip_rxdma
> +What: /sys/class/tty/ttyS0/skip_txdma
> +Date: September 2016
> +Contact: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the current state of DMA use per channel or enforces PIO
> + mode. Users can show this value regardless of opening the
> + serial device file or not, though setting change is allowed
> + only on closed device. The change will be effective as of next
> + port opening.
Oh that's going to get confusing, why can't this change if the port is
open?
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 2cc4196..88bda6c 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2948,14 +2948,120 @@ static DEVICE_ATTR(rx_trig_bytes, S_IRUSR | S_IWUSR | S_IRGRP,
> serial8250_get_attr_rx_trig_bytes,
> serial8250_set_attr_rx_trig_bytes);
>
> +static ssize_t serial8250_get_attr_skip_rxdma(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct tty_port *port = dev_get_drvdata(dev);
> + struct uart_state *state = container_of(port, struct uart_state, port);
> + struct uart_port *uport = state->uart_port;
> + bool skip_rxdma;
> +
> + mutex_lock(&port->mutex);
> + skip_rxdma = !!(uport->quirks & UPQ_NO_DMA_RX);
> + mutex_unlock(&port->mutex);
> +
> + return snprintf(buf, PAGE_SIZE, "%c\n", skip_rxdma ? 'Y' : 'N');
it's sysfs, if you ever have to "count" the size of the buffer you are
using, you are doing it wrong. Just use sprintf please.
> +}
> +
> +static ssize_t serial8250_set_attr_skip_rxdma(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct tty_port *port = dev_get_drvdata(dev);
> + struct uart_state *state = container_of(port, struct uart_state, port);
> + struct uart_port *uport = state->uart_port;
> + struct uart_8250_port *up = up_to_u8250p(uport);
> + bool skip_rxdma;
> + int ret;
> +
> + if (!up->dma)
> + return -ENODEV;
> +
> + ret = kstrtobool(buf, &skip_rxdma);
> + if (ret < 0)
> + return ret;
> +
> + mutex_lock(&port->mutex);
> + if (up->dma->in_use)
> + count = -EBUSY;
> + else {
> + if (skip_rxdma)
> + uport->quirks |= UPQ_NO_DMA_RX;
> + else
> + uport->quirks &= ~UPQ_NO_DMA_RX;
> + }
> + mutex_unlock(&port->mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(skip_rxdma, S_IRUSR | S_IWUSR | S_IRGRP,
> + serial8250_get_attr_skip_rxdma,
> + serial8250_set_attr_skip_rxdma);
DEVICE_ATTR_RW()?
> +
> +static ssize_t serial8250_get_attr_skip_txdma(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct tty_port *port = dev_get_drvdata(dev);
> + struct uart_state *state = container_of(port, struct uart_state, port);
> + struct uart_port *uport = state->uart_port;
> + bool skip_txdma;
> +
> + mutex_lock(&port->mutex);
> + skip_txdma = !!(uport->quirks & UPQ_NO_DMA_TX);
> + mutex_unlock(&port->mutex);
> +
> + return snprintf(buf, PAGE_SIZE, "%c\n", skip_txdma ? 'Y' : 'N');
same here, no snprintf.
> +}
> +
> +static ssize_t serial8250_set_attr_skip_txdma(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct tty_port *port = dev_get_drvdata(dev);
> + struct uart_state *state = container_of(port, struct uart_state, port);
> + struct uart_port *uport = state->uart_port;
> + struct uart_8250_port *up = up_to_u8250p(uport);
> + bool skip_txdma;
> + int ret;
> +
> + if (!up->dma)
> + return -ENODEV;
> +
> + ret = kstrtobool(buf, &skip_txdma);
> + if (ret < 0)
> + return ret;
> +
> + mutex_lock(&port->mutex);
> + if (up->dma->in_use)
> + count = -EBUSY;
> + else {
> + if (skip_txdma)
> + uport->quirks |= UPQ_NO_DMA_TX;
> + else
> + uport->quirks &= ~UPQ_NO_DMA_TX;
> + }
> + mutex_unlock(&port->mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(skip_txdma, S_IRUSR | S_IWUSR | S_IRGRP,
> + serial8250_get_attr_skip_txdma,
> + serial8250_set_attr_skip_txdma);
DEVICE_ATTR_RW()?
And really, I don't like this, how is anyone going to know to set these
values?
Why can't it just be "automatic"?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
