Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
Hi Masahiro, On 20 September 2014 01:18, Masahiro YAMADA wrote: > Hi Simon, > > > 2014-09-20 1:30 GMT+09:00 Simon Glass : >>> >>> It looks like CONFIG_DM_SERIAL depends on CONFIG_OF_CONTROL. >>> >>> >>> UniPhier SoCs do not support device tree control. >>> Is the driver model serial still available? >>> What will happen if gd->fdt_blob is not set? >>> >> >> Please this patch. >> >> http://patchwork.ozlabs.org/patch/390433/ >> >> I haven't got to a pull request yet for DM, but will do this in the next >> few days. >> > > > Can I postpone the driver-model conversion in the next phase? > > My first priority is to include the core support of Panasonic SoCs in > the 2014.10 release. > > We do not have a month before that. > I do not want to be aggressive now. > > First, I want the current well-tested code to be upstreamed and see > where I stand. > And then I can move forward to the next development. > > (Once I have a stable code in the mainline, > I can consult "git-bisect" whenever something is broken in the future > development.) That seems fine to me. It is good to do things in stages. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
Hi Simon, 2014-09-20 1:30 GMT+09:00 Simon Glass : >> >> It looks like CONFIG_DM_SERIAL depends on CONFIG_OF_CONTROL. >> >> >> UniPhier SoCs do not support device tree control. >> Is the driver model serial still available? >> What will happen if gd->fdt_blob is not set? >> > > Please this patch. > > http://patchwork.ozlabs.org/patch/390433/ > > I haven't got to a pull request yet for DM, but will do this in the next > few days. > Can I postpone the driver-model conversion in the next phase? My first priority is to include the core support of Panasonic SoCs in the 2014.10 release. We do not have a month before that. I do not want to be aggressive now. First, I want the current well-tested code to be upstreamed and see where I stand. And then I can move forward to the next development. (Once I have a stable code in the mainline, I can consult "git-bisect" whenever something is broken in the future development.) -- Best Regards Masahiro Yamada ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
HI Masahiro,
On 19 September 2014 06:15, Masahiro Yamada
wrote:
> Hi Simon,
>
>
>
> On Fri, 5 Sep 2014 10:41:54 -0600
> Simon Glass wrote:
> > Do you think we could use driver model instead? We have the serial
> > infrastructure in place and I will likely merge it next week.
> >
> > It moves the \r\n logic to a higher level.
> >
> > It also removes the need for all the horrible #define stuff you have
> > here to deal with multiple serial ports.
>
>
>
>
> I am seeing serial_find_console_or_panic() func
> in drivers/serial/serial-uclass.c
>
>
> static void serial_find_console_or_panic(void)
> {
> int node;
>
> /* Check for a chosen console */
> node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
> if (node < 0)
> node = fdtdec_get_alias_node(gd->fdt_blob, "console");
> if (!uclass_get_device_by_of_offset(UCLASS_SERIAL, node, &cur_dev))
> return;
>
> /*
> * If the console is not marked to be bound before relocation, bind
> * it anyway.
> */
> if (node > 0 &&
> !lists_bind_fdt(gd->dm_root, gd->fdt_blob, node, &cur_dev)) {
> if (!device_probe(cur_dev))
> return;
> cur_dev = NULL;
> }
>
>
>
>
>
> It looks like CONFIG_DM_SERIAL depends on CONFIG_OF_CONTROL.
>
>
> UniPhier SoCs do not support device tree control.
> Is the driver model serial still available?
> What will happen if gd->fdt_blob is not set?
>
Please this patch.
http://patchwork.ozlabs.org/patch/390433/
I haven't got to a pull request yet for DM, but will do this in the next
few days.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
Hi Simon,
On Fri, 5 Sep 2014 10:41:54 -0600
Simon Glass wrote:
> Do you think we could use driver model instead? We have the serial
> infrastructure in place and I will likely merge it next week.
>
> It moves the \r\n logic to a higher level.
>
> It also removes the need for all the horrible #define stuff you have
> here to deal with multiple serial ports.
I am seeing serial_find_console_or_panic() func
in drivers/serial/serial-uclass.c
static void serial_find_console_or_panic(void)
{
int node;
/* Check for a chosen console */
node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
if (node < 0)
node = fdtdec_get_alias_node(gd->fdt_blob, "console");
if (!uclass_get_device_by_of_offset(UCLASS_SERIAL, node, &cur_dev))
return;
/*
* If the console is not marked to be bound before relocation, bind
* it anyway.
*/
if (node > 0 &&
!lists_bind_fdt(gd->dm_root, gd->fdt_blob, node, &cur_dev)) {
if (!device_probe(cur_dev))
return;
cur_dev = NULL;
}
It looks like CONFIG_DM_SERIAL depends on CONFIG_OF_CONTROL.
UniPhier SoCs do not support device tree control.
Is the driver model serial still available?
What will happen if gd->fdt_blob is not set?
Best Regards
Masahiro Yamada
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
Hi Simon, 2014-09-06 1:41 GMT+09:00 Simon Glass : >> Maybe we can move "\n -> \r\n" logic >> to the upper layer and allow users to enable/disable it >> with a CONFIG_ option. > > Do you think we could use driver model instead? We have the serial > infrastructure in place and I will likely merge it next week. OK. I haven't checked it yet, but I will next week. > It moves the \r\n logic to a higher level. > > It also removes the need for all the horrible #define stuff you have > here to deal with multiple serial ports. > > Does your board have SPL? If so, does it use serial in SPL? Yes, all the UniPhier boards have SPL support. The serial in SPL is not currently supported, but I am planning to add it in the next phase. -- Best Regards Masahiro Yamada ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
Hi Masahiro,
On 5 September 2014 06:03, Masahiro Yamada wrote:
> Hi Marek,
>
>
>
> On Fri, 5 Sep 2014 12:35:18 +0200
> Marek Vasut wrote:
>
>> On Friday, September 05, 2014 at 07:50:19 AM, Masahiro Yamada wrote:
>> > The driver for on-chip UART used on Panasonic UniPhier platform.
>> >
>> > Signed-off-by: Masahiro Yamada
>>
>> [...]
>>
>> Hi!
>>
>> > +static void uniphier_serial_putc(struct uniphier_serial *port, const char
>> > c) +{
>> > + if (c == '\n')
>> > + uniphier_serial_putc(port, '\r');
>>
>> Just curious, but what is the concensus about inserting \r upon \n ?
>> Shouldn't
>> this be something that the "upper layers" do consistently ? I recall seeing
>> this
>> in some drivers and not seeing this in the others, so I wonder why this is
>> like
>> so ...
>
>
> This converts "\n" to "\r\n".
>
> Without this conversion, CarriageReturn is not provided,
> which means the cursor goes to the next line, but
> column position does not change.
>
>
> For example,
>
> printf("Hello\nWorld\n");
>
> will be displayed on (at least my) terminal emulator like this:
>
>
> Hello
> World
>
>
> With the conversion code, it will be displaye as follows:
>
> Hello
> World
>
>
>
> Perhaps the behavior might depend on
> which therminal emulator you are using.
> (also depend on the preference
> how LF and CR are handled.)
>
>
>
> Maybe we can move "\n -> \r\n" logic
> to the upper layer and allow users to enable/disable it
> with a CONFIG_ option.
Do you think we could use driver model instead? We have the serial
infrastructure in place and I will likely merge it next week.
It moves the \r\n logic to a higher level.
It also removes the need for all the horrible #define stuff you have
here to deal with multiple serial ports.
Does your board have SPL? If so, does it use serial in SPL?
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
On Friday, September 05, 2014 at 02:03:38 PM, Masahiro Yamada wrote:
> Hi Marek,
>
>
>
> On Fri, 5 Sep 2014 12:35:18 +0200
>
> Marek Vasut wrote:
> > On Friday, September 05, 2014 at 07:50:19 AM, Masahiro Yamada wrote:
> > > The driver for on-chip UART used on Panasonic UniPhier platform.
> > >
> > > Signed-off-by: Masahiro Yamada
> >
> > [...]
> >
> > Hi!
> >
> > > +static void uniphier_serial_putc(struct uniphier_serial *port, const
> > > char c) +{
> > > + if (c == '\n')
> > > + uniphier_serial_putc(port, '\r');
> >
> > Just curious, but what is the concensus about inserting \r upon \n ?
> > Shouldn't this be something that the "upper layers" do consistently ? I
> > recall seeing this in some drivers and not seeing this in the others, so
> > I wonder why this is like so ...
>
> This converts "\n" to "\r\n".
Apologies, you're right. This is what I meant.
> Without this conversion, CarriageReturn is not provided,
> which means the cursor goes to the next line, but
> column position does not change.
>
>
> For example,
>
> printf("Hello\nWorld\n");
>
> will be displayed on (at least my) terminal emulator like this:
>
>
> Hello
> World
>
>
> With the conversion code, it will be displaye as follows:
>
> Hello
> World
>
> Perhaps the behavior might depend on
> which therminal emulator you are using.
> (also depend on the preference
> how LF and CR are handled.)
I use minicom . You do have a point that it might be it.
> Maybe we can move "\n -> \r\n" logic
> to the upper layer and allow users to enable/disable it
> with a CONFIG_ option.
Either that or make it even run-time configurable, esp. if this depends on the
users' terminal setting.
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
Hi Marek,
On Fri, 5 Sep 2014 12:35:18 +0200
Marek Vasut wrote:
> On Friday, September 05, 2014 at 07:50:19 AM, Masahiro Yamada wrote:
> > The driver for on-chip UART used on Panasonic UniPhier platform.
> >
> > Signed-off-by: Masahiro Yamada
>
> [...]
>
> Hi!
>
> > +static void uniphier_serial_putc(struct uniphier_serial *port, const char
> > c) +{
> > + if (c == '\n')
> > + uniphier_serial_putc(port, '\r');
>
> Just curious, but what is the concensus about inserting \r upon \n ?
> Shouldn't
> this be something that the "upper layers" do consistently ? I recall seeing
> this
> in some drivers and not seeing this in the others, so I wonder why this is
> like
> so ...
This converts "\n" to "\r\n".
Without this conversion, CarriageReturn is not provided,
which means the cursor goes to the next line, but
column position does not change.
For example,
printf("Hello\nWorld\n");
will be displayed on (at least my) terminal emulator like this:
Hello
World
With the conversion code, it will be displaye as follows:
Hello
World
Perhaps the behavior might depend on
which therminal emulator you are using.
(also depend on the preference
how LF and CR are handled.)
Maybe we can move "\n -> \r\n" logic
to the upper layer and allow users to enable/disable it
with a CONFIG_ option.
Best Regards
Masahiro Yamada
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
On Friday, September 05, 2014 at 07:50:19 AM, Masahiro Yamada wrote:
> The driver for on-chip UART used on Panasonic UniPhier platform.
>
> Signed-off-by: Masahiro Yamada
[...]
Hi!
> +static void uniphier_serial_putc(struct uniphier_serial *port, const char
> c) +{
> + if (c == '\n')
> + uniphier_serial_putc(port, '\r');
Just curious, but what is the concensus about inserting \r upon \n ? Shouldn't
this be something that the "upper layers" do consistently ? I recall seeing
this
in some drivers and not seeing this in the others, so I wonder why this is like
so ...
> + while (!(readb(&port->lsr) & UART_LSR_THRE))
> + ;
> +
> + writeb(c, &port->thr);
> +}
[...]
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v4 2/6] serial: add UniPhier serial driver
The driver for on-chip UART used on Panasonic UniPhier platform.
Signed-off-by: Masahiro Yamada
---
Changes in v4: None
Changes in v3: None
Changes in v2:
- Use "const unsigned int mode_x_div = 16"
instead of "#define MODE_X_DIV 16"
- Use DIV_ROUND_CLOSEST() macro to compute the divisor
drivers/serial/Makefile | 1 +
drivers/serial/serial.c | 2 +
drivers/serial/serial_uniphier.c | 204 +++
3 files changed, 207 insertions(+)
create mode 100644 drivers/serial/serial_uniphier.c
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 571c18f..385b2f9 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_BFIN_SERIAL) += serial_bfin.o
obj-$(CONFIG_FSL_LPUART) += serial_lpuart.o
obj-$(CONFIG_MXS_AUART) += mxs_auart.o
obj-$(CONFIG_ARC_SERIAL) += serial_arc.o
+obj-$(CONFIG_UNIPHIER_SERIAL) += serial_uniphier.o
ifndef CONFIG_SPL_BUILD
obj-$(CONFIG_USB_TTY) += usbtty.o
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index d2eb752..d32673e 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -157,6 +157,7 @@ serial_initfunc(sh_serial_initialize);
serial_initfunc(arm_dcc_initialize);
serial_initfunc(mxs_auart_initialize);
serial_initfunc(arc_serial_initialize);
+serial_initfunc(uniphier_serial_initialize);
/**
* serial_register() - Register serial driver with serial driver core
@@ -250,6 +251,7 @@ void serial_initialize(void)
arm_dcc_initialize();
mxs_auart_initialize();
arc_serial_initialize();
+ uniphier_serial_initialize();
serial_assign(default_serial_console()->name);
}
diff --git a/drivers/serial/serial_uniphier.c b/drivers/serial/serial_uniphier.c
new file mode 100644
index 000..f8c9d92
--- /dev/null
+++ b/drivers/serial/serial_uniphier.c
@@ -0,0 +1,204 @@
+/*
+ * Copyright (C) 2012-2014 Panasonic Corporation
+ * Author: Masahiro Yamada
+ *
+ * Based on serial_ns16550.c
+ * (C) Copyright 2000
+ * Rob Taylor, Flying Pig Systems. r...@flyingpig.com.
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include
+#include
+
+#define UART_REG(x)\
+ u8 x; \
+ u8 postpad_##x[3];
+
+/*
+ * Note: Register map is slightly different from that of 16550.
+ */
+struct uniphier_serial {
+ UART_REG(rbr); /* 0x00 */
+ UART_REG(ier); /* 0x04 */
+ UART_REG(iir); /* 0x08 */
+ UART_REG(fcr); /* 0x0c */
+ u8 mcr; /* 0x10 */
+ u8 lcr;
+ u16 __postpad;
+ UART_REG(lsr); /* 0x14 */
+ UART_REG(msr); /* 0x18 */
+ u32 __none1;
+ u32 __none2;
+ u16 dlr;
+ u16 __postpad2;
+};
+
+#define thr rbr
+
+/*
+ * These are the definitions for the Line Control Register
+ */
+#define UART_LCR_WLS_8 0x03/* 8 bit character length */
+
+/*
+ * These are the definitions for the Line Status Register
+ */
+#define UART_LSR_DR0x01/* Data ready */
+#define UART_LSR_THRE 0x20/* Xmit holding register empty */
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static void uniphier_serial_init(struct uniphier_serial *port)
+{
+ const unsigned int mode_x_div = 16;
+ unsigned int divisor;
+
+ writeb(UART_LCR_WLS_8, &port->lcr);
+
+ divisor = DIV_ROUND_CLOSEST(CONFIG_SYS_UNIPHIER_UART_CLK,
+ mode_x_div * gd->baudrate);
+
+ writew(divisor, &port->dlr);
+}
+
+static void uniphier_serial_setbrg(struct uniphier_serial *port)
+{
+ uniphier_serial_init(port);
+}
+
+static int uniphier_serial_tstc(struct uniphier_serial *port)
+{
+ return (readb(&port->lsr) & UART_LSR_DR) != 0;
+}
+
+static int uniphier_serial_getc(struct uniphier_serial *port)
+{
+ while (!uniphier_serial_tstc(port))
+ ;
+
+ return readb(&port->rbr);
+}
+
+static void uniphier_serial_putc(struct uniphier_serial *port, const char c)
+{
+ if (c == '\n')
+ uniphier_serial_putc(port, '\r');
+
+ while (!(readb(&port->lsr) & UART_LSR_THRE))
+ ;
+
+ writeb(c, &port->thr);
+}
+
+static struct uniphier_serial *serial_ports[4] = {
+#ifdef CONFIG_SYS_UNIPHIER_SERIAL_BASE0
+ (struct uniphier_serial *)CONFIG_SYS_UNIPHIER_SERIAL_BASE0,
+#else
+ NULL,
+#endif
+#ifdef CONFIG_SYS_UNIPHIER_SERIAL_BASE1
+ (struct uniphier_serial *)CONFIG_SYS_UNIPHIER_SERIAL_BASE1,
+#else
+ NULL,
+#endif
+#ifdef CONFIG_SYS_UNIPHIER_SERIAL_BASE2
+ (struct uniphier_serial *)CONFIG_SYS_UNIPHIER_SERIAL_BASE2,
+#else
+ NULL,
+#endif
+#ifdef CONFIG_SYS_UNIPHIER_SERIAL_BASE3
+ (struct uniphier_serial *)CONFIG_SYS_UNIPHIER_SERIAL_BASE3,
+#else
+ NULL,
+#endif
+};
+
+/* Multi serial device functions */
+#define DECLARE_ESERIAL_FUNCTIONS(port) \
+ static int es
