Hi Ulrich,

Thank you for the patch.

On Friday 03 Feb 2017 11:38:19 Ulrich Hecht wrote:
> Allows tuning of the RX FIFO fill threshold and timeout. (The latter is
> only applicable to SCIFA and SCIFB).
> 
> Signed-off-by: Ulrich Hecht <[email protected]>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> ---
>  drivers/tty/serial/sh-sci.c | 87 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 4a165ed..f95a56c 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1055,6 +1055,66 @@ static void rx_fifo_timer_fn(unsigned long arg)
>       scif_set_rtrg(port, 1);
>  }
> 
> +static ssize_t rx_trigger_show(struct device *dev,
> +                            struct device_attribute *attr,
> +                            char *buf)
> +{
> +     struct uart_port *port = dev_get_drvdata(dev);
> +     struct sci_port *sci = to_sci_port(port);
> +
> +     return sprintf(buf, "%d\n", sci->rx_trigger);
> +}
> +
> +static ssize_t rx_trigger_store(struct device *dev,
> +                             struct device_attribute *attr,
> +                             const char *buf,
> +                             size_t count)
> +{
> +     struct uart_port *port = dev_get_drvdata(dev);
> +     struct sci_port *sci = to_sci_port(port);
> +     long r;
> +
> +     if (kstrtol(buf, 0, &r) == -EINVAL)
> +             return -EINVAL;
> +     sci->rx_trigger = scif_set_rtrg(port, r);
> +     scif_set_rtrg(port, 1);
> +     return count;
> +}
> +
> +static DEVICE_ATTR(rx_fifo_trigger, 0644, rx_trigger_show,
> rx_trigger_store); +
> +static ssize_t rx_fifo_timeout_show(struct device *dev,
> +                            struct device_attribute *attr,
> +                            char *buf)
> +{
> +     struct uart_port *port = dev_get_drvdata(dev);
> +     struct sci_port *sci = to_sci_port(port);
> +
> +     return sprintf(buf, "%d\n", sci->rx_fifo_timeout);
> +}
> +
> +static ssize_t rx_fifo_timeout_store(struct device *dev,
> +                             struct device_attribute *attr,
> +                             const char *buf,
> +                             size_t count)
> +{
> +     struct uart_port *port = dev_get_drvdata(dev);
> +     struct sci_port *sci = to_sci_port(port);
> +     long r;
> +
> +     if (kstrtol(buf, 0, &r) == -EINVAL)
> +             return -EINVAL;
> +     sci->rx_fifo_timeout = r;
> +     scif_set_rtrg(port, 1);
> +     if (r > 0)
> +             setup_timer(&sci->rx_fifo_timer, rx_fifo_timer_fn,
> +                         (unsigned long)sci);

Where's the locking ?

> +     return count;
> +}
> +
> +static DEVICE_ATTR(rx_fifo_timeout, 0644, rx_fifo_timeout_show,
> rx_fifo_timeout_store); +
> +
>  #ifdef CONFIG_SERIAL_SH_SCI_DMA
>  static void sci_dma_tx_complete(void *arg)
>  {
> @@ -2886,6 +2946,15 @@ static int sci_remove(struct platform_device *dev)
> 
>       sci_cleanup_single(port);
> 
> +     if (port->port.fifosize > 1) {
> +             sysfs_remove_file(&dev->dev.kobj,
> +                               &dev_attr_rx_fifo_trigger.attr);
> +     }
> +     if (port->port.type == PORT_SCIFA || port->port.type == PORT_SCIFB) {
> +             sysfs_remove_file(&dev->dev.kobj,
> +                               &dev_attr_rx_fifo_timeout.attr);
> +     }

No need for curly braces.

> +
>       return 0;
>  }
> 
> @@ -3051,6 +3120,24 @@ static int sci_probe(struct platform_device *dev)
>       if (ret)
>               return ret;
> 
> +     if (sp->port.fifosize > 1) {
> +             ret = sysfs_create_file(&dev->dev.kobj,
> +                             &dev_attr_rx_fifo_trigger.attr);

You should use device_create_file for device attributes. Even better would be 
to create an attribute group if possible.

> +             if (ret)
> +                     return ret;
> +     }
> +     if (sp->port.type == PORT_SCIFA || sp->port.type ==  PORT_SCIFB) {
> +             ret = sysfs_create_file(&dev->dev.kobj,
> +                             &dev_attr_rx_fifo_timeout.attr);
> +             if (ret) {
> +                     if (sp->port.fifosize > 1) {
> +                             sysfs_remove_file(&dev->dev.kobj,
> +                                     &dev_attr_rx_fifo_trigger.attr);
> +                     }
> +                     return ret;
> +             }
> +     }
> +
>  #ifdef CONFIG_SH_STANDARD_BIOS
>       sh_bios_gdb_detach();
>  #endif

-- 
Regards,

Laurent Pinchart

Reply via email to