Skip to contentSkip to conversation

serial: ns16550: add setconfig support

PATCH In review Simon Goldschmidt 4 participants 10 messages Nov 2, 2018 – Nov 17, 2018

lore
Share & tools
lore

Simon Goldschmidt

author
#0 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

 drivers/serial/ns16550.c | 43 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)
drivers/serial/ns16550.c +41 -2
index f9041aa626..88b312d0be 100644
@@ -148,10 +148,13 @@ int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate)
 
 static void NS16550_setbrg(NS16550_t com_port, int baud_divisor)
 {
-	serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);
+	/* to keep serial format, read lcr before writing BKSE */
+	int lcr_val = serial_in(&com_port->lcr) & ~UART_LCR_BKSE;
+
+	serial_out(UART_LCR_BKSE | lcr_val, &com_port->lcr);
 	serial_out(baud_divisor & 0xff, &com_port->dll);
 	serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);
-	serial_out(UART_LCRVAL, &com_port->lcr);
+	serial_out(lcr_val, &com_port->lcr);
 }
 
 void NS16550_init(NS16550_t com_port, int baud_divisor)

@@ -181,6 +184,8 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 
 	serial_out(UART_MCRVAL, &com_port->mcr);
 	serial_out(ns16550_getfcr(com_port), &com_port->fcr);
+	/* initialize serial config to 8N1 before writing baudrate */
+	serial_out(UART_LCRVAL, &com_port->lcr);
 	if (baud_divisor != -1)
 		NS16550_setbrg(com_port, baud_divisor);
 #if defined(CONFIG_ARCH_OMAP2PLUS) || defined(CONFIG_SOC_DA8XX) || \

@@ -334,6 +339,39 @@ static int ns16550_serial_setbrg(struct udevice *dev, int baudrate)
 	return 0;
 }
 
+static int ns16550_serial_setconfig(struct udevice *dev, uint serial_config)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+	int lcr_val = UART_LCR_WLS_8;
+	uint parity = SERIAL_GET_PARITY(serial_config);
+	uint bits = SERIAL_GET_BITS(serial_config);
+	uint stop = SERIAL_GET_STOP(serial_config);
+
+	/*
+	 * only parity config is implemented, check if other serial settings
+	 * are the default one.
+	 */
+	if (bits != SERIAL_8_BITS || stop != SERIAL_ONE_STOP)
+		return -ENOTSUPP; /* not supported in driver*/
+
+	switch (parity) {
+	case SERIAL_PAR_NONE:
+		/* no bits to add */
+		break;
+	case SERIAL_PAR_ODD:
+		lcr_val |= UART_LCR_PEN;
+		break;
+	case SERIAL_PAR_EVEN:
+		lcr_val |= UART_LCR_PEN | UART_LCR_EPS;
+		break;
+	default:
+		return -ENOTSUPP; /* not supported in driver*/
+	}
+
+	serial_out(lcr_val, &com_port->lcr);
+	return 0;
+}
+
 int ns16550_serial_probe(struct udevice *dev)
 {
 	struct NS16550 *const com_port = dev_get_priv(dev);

@@ -440,6 +478,7 @@ const struct dm_serial_ops ns16550_serial_ops = {
 	.pending = ns16550_serial_pending,
 	.getc = ns16550_serial_getc,
 	.setbrg = ns16550_serial_setbrg,
+	.setconfig = ns16550_serial_setconfig
 };
 
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
Signature · 1 line
2.17.1
#1 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Quoted 8 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

 drivers/serial/ns16550.c | 43 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

author
#2 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Quoted 7 lines · Simon Glass
Quoted 5 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

Quoted 6 lines
Quoted 3 lines
  drivers/serial/ns16550.c | 43 ++++++++++++++++++++++++++++++++++++++--
  1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

Hmm, I'm a bit lost there. I haven't really used the tests so far and grep'ing through test/ does not show any reference to ns16550 either (where I could easily add a test).

Did you mean adding a test for this specific driver (ns16550) or for the call to 'setconfig' in general? (Because I thought Patrice had already done that.)

Simon

author
#3 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Am Sa., 3. Nov. 2018, 08:56 hat Simon Goldschmidt < [email protected]> geschrieben:

Quoted 15 lines · Simon Glass
Quoted 7 lines
Quoted 5 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

Quoted 6 lines
Quoted 3 lines
  drivers/serial/ns16550.c | 43 ++++++++++++++++++++++++++++++++++++++--
  1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

Quoted 8 lines

Hmm, I'm a bit lost there. I haven't really used the tests so far and grep'ing through test/ does not show any reference to ns16550 either (where I could easily add a test).

Did you mean adding a test for this specific driver (ns16550) or for the call to 'setconfig' in general? (Because I thought Patrice had already done that.)

Simon, any input on this? Does your comment somehow hold back this from being pushed?

Simon

Quoted 1 line
#4 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Quoted 22 lines · Simon Goldschmidt

Am Sa., 3. Nov. 2018, 08:56 hat Simon Goldschmidt < simon.k.r.goldschmidt at gmail.com> geschrieben:

Quoted 19 lines · Simon Glass
Quoted 7 lines
Quoted 5 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

Quoted 6 lines
Quoted 3 lines
  drivers/serial/ns16550.c | 43 ++++++++++++++++++++++++++++++++++++++--
  1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

Hmm, I'm a bit lost there. I haven't really used the tests so far and grep'ing through test/ does not show any reference to ns16550 either (where I could easily add a test).

Quoted 7 lines
Quoted 4 lines

Did you mean adding a test for this specific driver (ns16550) or for the call to 'setconfig' in general? (Because I thought Patrice had already done that.)

Simon, any input on this? Does your comment somehow hold back this from being pushed?

I'm testing this now, btw.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181116/b43f52f5/attachment.sig>
author
#5 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Am Fr., 16. Nov. 2018, 18:09 hat Tom Rini <[email protected]> geschrieben:

Quoted 26 lines · Simon Goldschmidt
Quoted 12 lines

Am Sa., 3. Nov. 2018, 08:56 hat Simon Goldschmidt < simon.k.r.goldschmidt at gmail.com> geschrieben:

Quoted 9 lines · Simon Glass
Quoted 8 lines
Quoted 6 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

  drivers/serial/ns16550.c | 43

++++++++++++++++++++++++++++++++++++++--

Quoted 11 lines
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
  1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

Hmm, I'm a bit lost there. I haven't really used the tests so far and grep'ing through test/ does not show any reference to ns16550 either (where I could easily add a test).

Did you mean adding a test for this specific driver (ns16550) or for

the

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines

call to 'setconfig' in general? (Because I thought Patrice had already done that.)

Simon, any input on this? Does your comment somehow hold back this from being pushed?

I'm testing this now, btw.

Oh, cool, thanks for the info. I tested this on socfpga gen5 only, but the registers should be portable...

Simon

#6 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

In reply to Simon Goldschmidt

Hi,

Quoted 25 lines · Simon Goldschmidt

Am Sa., 3. Nov. 2018, 08:56 hat Simon Goldschmidt
<[email protected]> geschrieben:

Quoted 21 lines · Simon Glass
Quoted 7 lines
Quoted 5 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

Quoted 7 lines
Quoted 4 lines
  drivers/serial/ns16550.c | 43
++++++++++++++++++++++++++++++++++++++--
  1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

Hmm, I'm a bit lost there. I haven't really used the tests so far and grep'ing through test/ does not show any reference to ns16550 either (where I could easily add a test).

Quoted 7 lines
Quoted 3 lines

Did you mean adding a test for this specific driver (ns16550) or for the call to 'setconfig' in general? (Because I thought Patrice had already done that.)

Simon, any input on this? Does your comment somehow hold back this from being pushed?

No, I added my review tag.

It looks like serial_setconfig() is written without a struct udevice parameter. I think that should be fixed, since new functionality should use DM.

Regards,
Simon

author
#7 lore raw

PATCH 1/1 serial: ns16550: add setconfig support

Am Fr., 16. Nov. 2018, 18:25 hat Simon Glass <[email protected]> geschrieben:

Quoted 34 lines

Hi,

Quoted 30 lines · Simon Goldschmidt

Am Sa., 3. Nov. 2018, 08:56 hat Simon Goldschmidt
<[email protected]> geschrieben:

Quoted 24 lines · Simon Glass
Quoted 7 lines
Quoted 5 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>

---

Quoted 7 lines
Quoted 4 lines
  drivers/serial/ns16550.c | 43
++++++++++++++++++++++++++++++++++++++--
  1 file changed, 41 insertions(+), 2 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Ideally we should call this from sandbox (or a test) somewhere.

Hmm, I'm a bit lost there. I haven't really used the tests so far and grep'ing through test/ does not show any reference to ns16550 either (where I could easily add a test).

Did you mean adding a test for this specific driver (ns16550) or for the call to 'setconfig' in general? (Because I thought Patrice had already done that.)

Quoted 9 lines
Quoted 2 lines

Simon, any input on this? Does your comment somehow hold back this from being pushed?

No, I added my review tag.

It looks like serial_setconfig() is written without a struct udevice parameter. I think that should be fixed, since new functionality should use DM.

Right, that would be good. I just saw we're calling the operation by directly accessing the struct...

I would change and test that but I'm still having trouble in getting the tests to run...

Simon

#9 lore raw
In reply to Simon Goldschmidt
Quoted 5 lines · Simon Goldschmidt

Add possibility to update the serial parity used.

Signed-off-by Simon Goldschmidt <[email protected]>
Reviewed-by Simon Glass <[email protected]>
Reviewed-by Hannes Schmelzer <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181117/43376045/attachment.sig>