[PATCH 0/2] HSCIF tweaks support

[PATCH 0/2] HSCIF tweaks support

[Ulrich Hecht]

Hi!

These patches enable tweaking of the HSCIF RX FIFO timeout and sampling
point via sysfs. I have put them in one series because they have textual
dependencies and implement similar functionality.

"serial: sh-sci: Support for variable HSCIF hardware RX timeout" includes
minor touch-ups suggested in Geert's review of the standalone version.

CU
Uli


Ulrich Hecht (2):
  serial: sh-sci: Support for variable HSCIF hardware RX timeout
  serial: sh-sci: Support for HSCIF RX sampling point adjustment

 drivers/tty/serial/sh-sci.c | 114 +++++++++++++++++++++++++++++++++++++-------
 drivers/tty/serial/sh-sci.h |   7 +++
 2 files changed, 104 insertions(+), 17 deletions(-)

-- 
2.7.4

[PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout

[Ulrich Hecht]

HSCIF has facilities that allow changing the timeout after which an RX
interrupt is triggered even if the FIFO is not filled. This patch allows
changing the default (15 bits of silence) using the existing sysfs
attribute "rx_fifo_timeout".

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 52 ++++++++++++++++++++++++++++++++-------------
 drivers/tty/serial/sh-sci.h |  3 +++
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 784dd42..41bf910 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -152,6 +152,7 @@ struct sci_port {
 	int				rx_trigger;
 	struct timer_list		rx_fifo_timer;
 	int				rx_fifo_timeout;
+	u16				hscif_tot;
 
 	bool has_rtscts;
 	bool autorts;
@@ -1107,8 +1108,14 @@ static ssize_t rx_fifo_timeout_show(struct device *dev,
 {
 	struct uart_port *port = dev_get_drvdata(dev);
 	struct sci_port *sci = to_sci_port(port);
+	int v;
 
-	return sprintf(buf, "%d\n", sci->rx_fifo_timeout);
+	if (port->type == PORT_HSCIF)
+		v = sci->hscif_tot >> HSSCR_TOT_SHIFT;
+	else
+		v = sci->rx_fifo_timeout;
+
+	return sprintf(buf, "%d\n", v);
 }
 
 static ssize_t rx_fifo_timeout_store(struct device *dev,
@@ -1124,11 +1131,19 @@ static ssize_t rx_fifo_timeout_store(struct device *dev,
 	ret = kstrtol(buf, 0, &r);
 	if (ret)
 		return ret;
-	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);
+
+	if (port->type == PORT_HSCIF) {
+		if (r < 0 || r > 3)
+			return -EINVAL;
+		sci->hscif_tot = r << HSSCR_TOT_SHIFT;
+	} else {
+		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);
+	}
+
 	return count;
 }
 
@@ -2037,9 +2052,13 @@ static void sci_shutdown(struct uart_port *port)
 	spin_lock_irqsave(&port->lock, flags);
 	sci_stop_rx(port);
 	sci_stop_tx(port);
-	/* Stop RX and TX, disable related interrupts, keep clock source */
+	/*
+	 * Stop RX and TX, disable related interrupts, keep clock source
+	 * and HSCIF TOT bits
+	 */
 	scr = serial_port_in(port, SCSCR);
-	serial_port_out(port, SCSCR, scr & (SCSCR_CKE1 | SCSCR_CKE0));
+	serial_port_out(port, SCSCR, scr &
+			(SCSCR_CKE1 | SCSCR_CKE0 | s->hscif_tot));
 	spin_unlock_irqrestore(&port->lock, flags);
 
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
@@ -2186,7 +2205,7 @@ static void sci_reset(struct uart_port *port)
 	unsigned int status;
 	struct sci_port *s = to_sci_port(port);
 
-	serial_port_out(port, SCSCR, 0x00);	/* TE=0, RE=0, CKE1=0 */
+	serial_port_out(port, SCSCR, s->hscif_tot);	/* TE=0, RE=0, CKE1=0 */
 
 	reg = sci_getreg(port, SCFCR);
 	if (reg->size)
@@ -2356,7 +2375,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		dev_dbg(port->dev,
 			 "SCR 0x%x SMR 0x%x BRR %u CKS 0x%x DL %u SRR %u\n",
 			 scr_val, smr_val, brr, sccks, dl, srr);
-		serial_port_out(port, SCSCR, scr_val);
+		serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 		serial_port_out(port, SCSMR, smr_val);
 		serial_port_out(port, SCBRR, brr);
 		if (sci_getreg(port, HSSRR)->size)
@@ -2370,7 +2389,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		smr_val |= serial_port_in(port, SCSMR) &
 			   (SCSMR_CKEDG | SCSMR_SRC_MASK | SCSMR_CKS);
 		dev_dbg(port->dev, "SCR 0x%x SMR 0x%x\n", scr_val, smr_val);
-		serial_port_out(port, SCSCR, scr_val);
+		serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 		serial_port_out(port, SCSMR, smr_val);
 	}
 
@@ -2407,7 +2426,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	scr_val |= SCSCR_RE | SCSCR_TE |
 		   (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0));
 	dev_dbg(port->dev, "SCSCR 0x%x\n", scr_val);
-	serial_port_out(port, SCSCR, scr_val);
+	serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 	if ((srr + 1 == 5) &&
 	    (port->type == PORT_SCIFA || port->type == PORT_SCIFB)) {
 		/*
@@ -2773,6 +2792,7 @@ static int sci_init_single(struct platform_device *dev,
 	}
 
 	sci_port->rx_fifo_timeout = 0;
+	sci_port->hscif_tot = 0;
 
 	/* SCIFA on sh7723 and sh7724 need a custom sampling rate that doesn't
 	 * match the SoC datasheet, this should be investigated. Let platform
@@ -2860,7 +2880,7 @@ static void serial_console_write(struct console *co, const char *s,
 	ctrl_temp = SCSCR_RE | SCSCR_TE |
 		    (sci_port->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)) |
 		    (ctrl & (SCSCR_CKE1 | SCSCR_CKE0));
-	serial_port_out(port, SCSCR, ctrl_temp);
+	serial_port_out(port, SCSCR, ctrl_temp | sci_port->hscif_tot);
 
 	uart_console_write(port, s, count, serial_console_putchar);
 
@@ -2988,7 +3008,8 @@ static int sci_remove(struct platform_device *dev)
 		sysfs_remove_file(&dev->dev.kobj,
 				  &dev_attr_rx_fifo_trigger.attr);
 	}
-	if (port->port.type == PORT_SCIFA || port->port.type == PORT_SCIFB) {
+	if (port->port.type == PORT_SCIFA || port->port.type == PORT_SCIFB ||
+	    port->port.type == PORT_HSCIF) {
 		sysfs_remove_file(&dev->dev.kobj,
 				  &dev_attr_rx_fifo_timeout.attr);
 	}
@@ -3173,7 +3194,8 @@ static int sci_probe(struct platform_device *dev)
 		if (ret)
 			return ret;
 	}
-	if (sp->port.type == PORT_SCIFA || sp->port.type ==  PORT_SCIFB) {
+	if (sp->port.type == PORT_SCIFA || sp->port.type == PORT_SCIFB ||
+	    sp->port.type == PORT_HSCIF) {
 		ret = sysfs_create_file(&dev->dev.kobj,
 				&dev_attr_rx_fifo_timeout.attr);
 		if (ret) {
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 971b2ab..2b708cc 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -62,6 +62,9 @@ enum {
 #define SCSCR_TDRQE	BIT(15)	/* Tx Data Transfer Request Enable */
 #define SCSCR_RDRQE	BIT(14)	/* Rx Data Transfer Request Enable */
 
+/* Serial Control Register, HSCIF-only bits */
+#define HSSCR_TOT_SHIFT	14
+
 /* SCxSR (Serial Status Register) on SCI */
 #define SCI_TDRE	BIT(7)	/* Transmit Data Register Empty */
 #define SCI_RDRF	BIT(6)	/* Receive Data Register Full */
-- 
2.7.4

Re: [PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout

[Geert Uytterhoeven]

Hi Uli,

On Fri, Sep 29, 2017 at 3:08 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> HSCIF has facilities that allow changing the timeout after which an RX
> interrupt is triggered even if the FIFO is not filled. This patch allows
> changing the default (15 bits of silence) using the existing sysfs
> attribute "rx_fifo_timeout".

Thanks for the update!

> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On Fri, Sep 29, 2017 at 03:08:53PM +0200, Ulrich Hecht wrote:
> HSCIF has facilities that allow changing the timeout after which an RX
> interrupt is triggered even if the FIFO is not filled. This patch allows
> changing the default (15 bits of silence) using the existing sysfs
> attribute "rx_fifo_timeout".
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Can't this patch be resent independently of the other? With Geert's tag
added?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout

[Geert Uytterhoeven]

Hi Wolfram,

On Wed, Nov 15, 2017 at 4:06 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Fri, Sep 29, 2017 at 03:08:53PM +0200, Ulrich Hecht wrote:
>> HSCIF has facilities that allow changing the timeout after which an RX
>> interrupt is triggered even if the FIFO is not filled. This patch allows
>> changing the default (15 bits of silence) using the existing sysfs
>> attribute "rx_fifo_timeout".
>>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>
> Can't this patch be resent independently of the other? With Geert's tag
> added?

No need to resend, see commit fa2abb03637a5528 ("serial: sh-sci: Support
for variable HSCIF hardware RX timeout") in today's upstream.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 157 bytes --]


> No need to resend, see commit fa2abb03637a5528 ("serial: sh-sci: Support
> for variable HSCIF hardware RX timeout") in today's upstream.

In deed. Nice!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/2] serial: sh-sci: Support for HSCIF RX sampling point adjustment

[Ulrich Hecht]

HSCIF has facilities that allow moving the RX sampling point by between
-8 and 7 sampling cycles (one sampling cycles equals 1/15 of a bit
by default) to improve the error margin in case of slightly mismatched
bit rates between sender and receiver.

This patch allows changing the default (0, meaning center) using the
sysfs attribute "rx_sampling_point".

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 62 +++++++++++++++++++++++++++++++++++++++++++--
 drivers/tty/serial/sh-sci.h |  4 +++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 41bf910..924a393 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -153,6 +153,7 @@ struct sci_port {
 	struct timer_list		rx_fifo_timer;
 	int				rx_fifo_timeout;
 	u16				hscif_tot;
+	u16				hscif_srhp;
 
 	bool has_rtscts;
 	bool autorts;
@@ -992,6 +993,42 @@ static int sci_handle_breaks(struct uart_port *port)
 	return copied;
 }
 
+static ssize_t rx_sampling_point_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->hscif_srhp);
+}
+
+static ssize_t rx_sampling_point_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);
+	int ret;
+	long r;
+
+	ret = kstrtol(buf, 0, &r);
+	if (ret)
+		return ret;
+
+	if (r < -8 || r > 7)
+		return -EINVAL;
+
+	sci->hscif_srhp = r;
+
+	return count;
+}
+
+static DEVICE_ATTR(rx_sampling_point, 0644,
+		   rx_sampling_point_show,
+		   rx_sampling_point_store);
+
 static int scif_set_rtrg(struct uart_port *port, int rx_trig)
 {
 	unsigned int bits;
@@ -2378,8 +2415,6 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 		serial_port_out(port, SCSMR, smr_val);
 		serial_port_out(port, SCBRR, brr);
-		if (sci_getreg(port, HSSRR)->size)
-			serial_port_out(port, HSSRR, srr | HSCIF_SRE);
 
 		/* Wait one bit interval */
 		udelay((1000000 + (baud - 1)) / baud);
@@ -2393,6 +2428,18 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_port_out(port, SCSMR, smr_val);
 	}
 
+	if (sci_getreg(port, HSSRR)->size) {
+		u16 hssrr = srr | HSCIF_SRE;
+
+		if (s->hscif_srhp) {
+			u16 srhp = (s->hscif_srhp << HSCIF_SRHP_SHIFT) &
+				   HSCIF_SRHP_MASK;
+
+			hssrr |= HSCIF_SRDE | srhp;
+		}
+		serial_port_out(port, HSSRR, hssrr);
+	}
+
 	sci_init_pins(port, termios->c_cflag);
 
 	port->status &= ~UPSTAT_AUTOCTS;
@@ -2793,6 +2840,7 @@ static int sci_init_single(struct platform_device *dev,
 
 	sci_port->rx_fifo_timeout = 0;
 	sci_port->hscif_tot = 0;
+	sci_port->hscif_shrp = 0;
 
 	/* SCIFA on sh7723 and sh7724 need a custom sampling rate that doesn't
 	 * match the SoC datasheet, this should be investigated. Let platform
@@ -3013,6 +3061,10 @@ static int sci_remove(struct platform_device *dev)
 		sysfs_remove_file(&dev->dev.kobj,
 				  &dev_attr_rx_fifo_timeout.attr);
 	}
+	if (port->port.type == PORT_HSCIF) {
+		sysfs_remove_file(&dev->dev.kobj,
+				  &dev_attr_rx_sampling_point.attr);
+	}
 
 	return 0;
 }
@@ -3206,6 +3258,12 @@ static int sci_probe(struct platform_device *dev)
 			return ret;
 		}
 	}
+	if (sp->port.type == PORT_HSCIF) {
+		ret = sysfs_create_file(&dev->dev.kobj,
+				&dev_attr_rx_sampling_point.attr);
+		if (ret)
+			return ret;
+	}
 
 #ifdef CONFIG_SH_STANDARD_BIOS
 	sh_bios_gdb_detach();
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 2b708cc..80958b2 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -129,6 +129,10 @@ enum {
 
 /* HSSRR HSCIF */
 #define HSCIF_SRE	BIT(15)	/* Sampling Rate Register Enable */
+#define HSCIF_SRDE	BIT(14)	/* Sampling Point Register Enable */
+
+#define HSCIF_SRHP_SHIFT	8
+#define HSCIF_SRHP_MASK		0x0f00
 
 /* SCPCR (Serial Port Control Register), SCIFA/SCIFB only */
 #define SCPCR_RTSC	BIT(4)	/* Serial Port RTS# Pin / Output Pin */
-- 
2.7.4

Re: [PATCH 2/2] serial: sh-sci: Support for HSCIF RX sampling point adjustment

[Geert Uytterhoeven]

Hi Uli,

On Fri, Sep 29, 2017 at 3:08 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> HSCIF has facilities that allow moving the RX sampling point by between
> -8 and 7 sampling cycles (one sampling cycles equals 1/15 of a bit
> by default) to improve the error margin in case of slightly mismatched
> bit rates between sender and receiver.
>
> This patch allows changing the default (0, meaning center) using the
> sysfs attribute "rx_sampling_point".
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Thanks for your patch!

Have you noticed any improvements by fiddling with the sampling point?

While this patch is useful as an investigation tool due to the sysfs
interface, I don't think it should be applied as-is.

I guess the goal is to select automatically the optimal sampling point,
based on requested bit rate and available clock input rates, right?

Or perhaps you want to keep the sysfs interface, to accommodate a sender
that uses an off bit rate, unbeknownst to Linux (and unsupported by Linux,
as software can request standard bit rates only) and thus needs user
configuration?
If that is the case, perhaps the sysfs interface should not be provided the
raw sampling point offset, but the actual bit rate used by the other side,
so the driver can calculate the offset?

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -153,6 +153,7 @@ struct sci_port {
>         struct timer_list               rx_fifo_timer;
>         int                             rx_fifo_timeout;
>         u16                             hscif_tot;
> +       u16                             hscif_srhp;

s16, or even s8?

>
>         bool has_rtscts;
>         bool autorts;
> @@ -992,6 +993,42 @@ static int sci_handle_breaks(struct uart_port *port)
>         return copied;
>  }
>
> +static ssize_t rx_sampling_point_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->hscif_srhp);

... else it will not output negative numbers here.

> +}
> +
> +static ssize_t rx_sampling_point_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);
> +       int ret;
> +       long r;

s8 or just int?

> +
> +       ret = kstrtol(buf, 0, &r);

kstrtos8() or kstrtoint()?

> +       if (ret)
> +               return ret;
> +
> +       if (r < -8 || r > 7)
> +               return -EINVAL;
> +
> +       sci->hscif_srhp = r;
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR(rx_sampling_point, 0644,
> +                  rx_sampling_point_show,
> +                  rx_sampling_point_store);
> +
>  static int scif_set_rtrg(struct uart_port *port, int rx_trig)
>  {
>         unsigned int bits;
> @@ -2378,8 +2415,6 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
>                 serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
>                 serial_port_out(port, SCSMR, smr_val);
>                 serial_port_out(port, SCBRR, brr);
> -               if (sci_getreg(port, HSSRR)->size)
> -                       serial_port_out(port, HSSRR, srr | HSCIF_SRE);

Why have you moved the setting of HSSRR?
AFAIK all bit rate settings should be completed before the one bit delay below.

>                 /* Wait one bit interval */
>                 udelay((1000000 + (baud - 1)) / baud);
> @@ -2393,6 +2428,18 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
>                 serial_port_out(port, SCSMR, smr_val);
>         }
>
> +       if (sci_getreg(port, HSSRR)->size) {
> +               u16 hssrr = srr | HSCIF_SRE;
> +
> +               if (s->hscif_srhp) {
> +                       u16 srhp = (s->hscif_srhp << HSCIF_SRHP_SHIFT) &
> +                                  HSCIF_SRHP_MASK;
> +
> +                       hssrr |= HSCIF_SRDE | srhp;
> +               }
> +               serial_port_out(port, HSSRR, hssrr);
> +       }
> +
>         sci_init_pins(port, termios->c_cflag);
>
>         port->status &= ~UPSTAT_AUTOCTS;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] serial: sh-sci: Support for HSCIF RX sampling point adjustment

[Greg KH]

On Fri, Sep 29, 2017 at 03:08:54PM +0200, Ulrich Hecht wrote:
> HSCIF has facilities that allow moving the RX sampling point by between
> -8 and 7 sampling cycles (one sampling cycles equals 1/15 of a bit
> by default) to improve the error margin in case of slightly mismatched
> bit rates between sender and receiver.
> 
> This patch allows changing the default (0, meaning center) using the
> sysfs attribute "rx_sampling_point".
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/tty/serial/sh-sci.c | 62 +++++++++++++++++++++++++++++++++++++++++++--
>  drivers/tty/serial/sh-sci.h |  4 +++
>  2 files changed, 64 insertions(+), 2 deletions(-)

Adding random sysfs files for random serial ports isn't good, especially
if you do not document them in Documentation/ABI.

I don't want to see this, but if you write sysfs files in the future,
here's some review comments:

> +static DEVICE_ATTR(rx_sampling_point, 0644,
> +		   rx_sampling_point_show,
> +		   rx_sampling_point_store);

DEVICE_ATTR_RW()

> +	if (port->port.type == PORT_HSCIF) {
> +		sysfs_remove_file(&dev->dev.kobj,
> +				  &dev_attr_rx_sampling_point.attr);

No driver code should ever call sysfs functions, this should be either:
	device_create_file()
or properly use an attribute group.

thanks,

greg k-h

Re: [PATCH 2/2] serial: sh-sci: Support for HSCIF RX sampling point adjustment

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Fri, Sep 29, 2017 at 03:08:54PM +0200, Ulrich Hecht wrote:
> HSCIF has facilities that allow moving the RX sampling point by between
> -8 and 7 sampling cycles (one sampling cycles equals 1/15 of a bit
> by default) to improve the error margin in case of slightly mismatched
> bit rates between sender and receiver.
> 
> This patch allows changing the default (0, meaning center) using the
> sysfs attribute "rx_sampling_point".
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Wasn't there a conclusion in a chat meeting we had how to move this
forward? How much effort would be implementing that?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]