[PATCH v2 0/4] LiteUART serial driver

[PATCH v2 0/4] LiteUART serial driver

[Mateusz Holenko]

This patchset introduces support for LiteUART
- serial device from LiteX SoC builder
(https://github.com/enjoy-digital/litex).

In the following patchset I will add
a new mor1kx-based (OpenRISC) platform that
uses this device.

Later I plan to extend this platform by
adding support for more devices from LiteX suite.

Changes in v2:
- binding description rewritten to a yaml schema file
- added litex.h header with common register access functions

Filip Kokosinski (3):
  dt-bindings: vendor: add vendor prefix for LiteX
  dt-bindings: serial: document LiteUART bindings
  drivers/tty/serial: add LiteUART driver

Mateusz Holenko (1):
  litex: add common LiteX header

 .../bindings/serial/litex,liteuart.yaml       |  38 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   8 +
 drivers/tty/serial/Kconfig                    |  30 ++
 drivers/tty/serial/Makefile                   |   1 +
 drivers/tty/serial/liteuart.c                 | 373 ++++++++++++++++++
 include/linux/litex.h                         |  59 +++
 include/uapi/linux/serial_core.h              |   3 +
 8 files changed, 514 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/litex,liteuart.yaml
 create mode 100644 drivers/tty/serial/liteuart.c
 create mode 100644 include/linux/litex.h

-- 
2.23.0

[PATCH v2 1/4] dt-bindings: vendor: add vendor prefix for LiteX

[Mateusz Holenko]

From: Filip Kokosinski <fkokosinski@internships.antmicro.com>

Add vendor prefix for LiteX SoC builder.

Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---
No changes in v2.

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 967e78c5ec0a..dae98f826290 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -537,6 +537,8 @@ patternProperties:
     description: Linux-specific binding
   "^linx,.*":
     description: Linx Technologies
+  "^litex,.*":
+    description: LiteX SoC builder
   "^lltc,.*":
     description: Linear Technology Corporation
   "^logicpd,.*":
-- 
2.23.0

Re: [PATCH v2 1/4] dt-bindings: vendor: add vendor prefix for LiteX

[Rob Herring]

On Wed, 23 Oct 2019 11:46:54 +0200, Mateusz Holenko wrote:
> From: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> 
> Add vendor prefix for LiteX SoC builder.
> 
> Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> No changes in v2.
> 
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v2 2/4] litex: add common LiteX header

[Mateusz Holenko]

It provides helper CSR access functions used by all
LiteX drivers.

Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---
This commit has been introduced in v2 of the patchset.

 MAINTAINERS           |  6 +++++
 include/linux/litex.h | 59 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 include/linux/litex.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 296de2b51c83..eaa51209bfb2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9493,6 +9493,12 @@ F:	Documentation/misc-devices/lis3lv02d.rst
 F:	drivers/misc/lis3lv02d/
 F:	drivers/platform/x86/hp_accel.c
 
+LITEX PLATFORM
+M:	Karol Gugala <kgugala@antmicro.com>
+M:	Mateusz Holenko <mholenko@antmicro.com>
+S:	Maintained
+F:	include/linux/litex.h
+
 LIVE PATCHING
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
 M:	Jiri Kosina <jikos@kernel.org>
diff --git a/include/linux/litex.h b/include/linux/litex.h
new file mode 100644
index 000000000000..e793d2d7c881
--- /dev/null
+++ b/include/linux/litex.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common LiteX header providing
+ * helper functions for accessing CSRs.
+ *
+ * Copyright (C) 2019 Antmicro <www.antmicro.com>
+ */
+
+#ifndef _LINUX_LITEX_H
+#define _LINUX_LITEX_H
+
+#include <linux/io.h>
+#include <linux/types.h>
+#include <linux/compiler_types.h>
+
+#define LITEX_REG_SIZE             0x4
+#define LITEX_SUBREG_SIZE          0x1
+#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)
+
+#ifdef __LITTLE_ENDIAN
+# define LITEX_READ_REG(addr)                  ioread32(addr)
+# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
+# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
+# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
+#else
+# define LITEX_READ_REG(addr)                  ioread32be(addr)
+# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
+# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
+# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
+#endif
+
+/* Helper functions for manipulating LiteX registers */
+
+static inline void litex_set_reg(void __iomem *reg, u32 reg_size, u32 val)
+{
+	u32 shifted_data, shift, i;
+
+	for (i = 0; i < reg_size; ++i) {
+		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
+		shifted_data = val >> shift;
+		LITEX_WRITE_REG(shifted_data, reg + (LITEX_REG_SIZE * i));
+	}
+}
+
+static inline u32 litex_get_reg(void __iomem *reg, u32 reg_size)
+{
+	u32 shifted_data, shift, i;
+	u32 result = 0;
+
+	for (i = 0; i < reg_size; ++i) {
+		shifted_data = LITEX_READ_REG(reg + (LITEX_REG_SIZE * i));
+		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
+		result |= (shifted_data << shift);
+	}
+
+	return result;
+}
+
+#endif /* _LINUX_LITEX_H */
-- 
2.23.0

Re: [PATCH v2 2/4] litex: add common LiteX header

[Rob Herring]

On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> It provides helper CSR access functions used by all
> LiteX drivers.
> 
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> This commit has been introduced in v2 of the patchset.
> 
>  MAINTAINERS           |  6 +++++
>  include/linux/litex.h | 59 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100644 include/linux/litex.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 296de2b51c83..eaa51209bfb2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9493,6 +9493,12 @@ F:	Documentation/misc-devices/lis3lv02d.rst
>  F:	drivers/misc/lis3lv02d/
>  F:	drivers/platform/x86/hp_accel.c
>  
> +LITEX PLATFORM
> +M:	Karol Gugala <kgugala@antmicro.com>
> +M:	Mateusz Holenko <mholenko@antmicro.com>
> +S:	Maintained
> +F:	include/linux/litex.h
> +
>  LIVE PATCHING
>  M:	Josh Poimboeuf <jpoimboe@redhat.com>
>  M:	Jiri Kosina <jikos@kernel.org>
> diff --git a/include/linux/litex.h b/include/linux/litex.h
> new file mode 100644
> index 000000000000..e793d2d7c881
> --- /dev/null
> +++ b/include/linux/litex.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Common LiteX header providing
> + * helper functions for accessing CSRs.
> + *
> + * Copyright (C) 2019 Antmicro <www.antmicro.com>
> + */
> +
> +#ifndef _LINUX_LITEX_H
> +#define _LINUX_LITEX_H
> +
> +#include <linux/io.h>
> +#include <linux/types.h>
> +#include <linux/compiler_types.h>
> +
> +#define LITEX_REG_SIZE             0x4
> +#define LITEX_SUBREG_SIZE          0x1
> +#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)
> +
> +#ifdef __LITTLE_ENDIAN
> +# define LITEX_READ_REG(addr)                  ioread32(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> +#else
> +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)

Defining custom accessors makes it harder for others to understand 
the code. The __raw_readl/writel accessors are native endian, so use 
those. One difference though is they don't have a memory barrier, but 
based on the below functions, you may want to do your own barrier 
anyways. And if DMA is not involved you don't need the barriers either.

The _OFF variants don't add anything. LITEX_READ_REG(addr + off) is just 
as easy to read if not easier than LITEX_READ_REG_OFF(addr, off).

> +#endif
> +
> +/* Helper functions for manipulating LiteX registers */
> +
> +static inline void litex_set_reg(void __iomem *reg, u32 reg_size, u32 val)
> +{
> +	u32 shifted_data, shift, i;
> +
> +	for (i = 0; i < reg_size; ++i) {
> +		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> +		shifted_data = val >> shift;
> +		LITEX_WRITE_REG(shifted_data, reg + (LITEX_REG_SIZE * i));
> +	}
> +}

The problem with this is it hides whether you need to do any locking. 
Normally, you would assume a register access is atomic when it's not 
here. You could add locking in this function, but then you're doing 
locking even when not needed.

It doesn't look like you actually use this in your series, so maybe 
remove until you do.

> +
> +static inline u32 litex_get_reg(void __iomem *reg, u32 reg_size)
> +{
> +	u32 shifted_data, shift, i;
> +	u32 result = 0;
> +
> +	for (i = 0; i < reg_size; ++i) {
> +		shifted_data = LITEX_READ_REG(reg + (LITEX_REG_SIZE * i));
> +		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> +		result |= (shifted_data << shift);
> +	}
> +
> +	return result;
> +}
> +
> +#endif /* _LINUX_LITEX_H */
> -- 
> 2.23.0

Re: [PATCH v2 2/4] litex: add common LiteX header

[Mateusz Holenko]

sob., 26 paź 2019 o 02:03 Rob Herring <robh@kernel.org> napisał(a):
>
> On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> > It provides helper CSR access functions used by all
> > LiteX drivers.
> >
> > Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> > ---
> > This commit has been introduced in v2 of the patchset.
> >
> >  MAINTAINERS           |  6 +++++
> >  include/linux/litex.h | 59 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+)
> >  create mode 100644 include/linux/litex.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 296de2b51c83..eaa51209bfb2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9493,6 +9493,12 @@ F:     Documentation/misc-devices/lis3lv02d.rst
> >  F:   drivers/misc/lis3lv02d/
> >  F:   drivers/platform/x86/hp_accel.c
> >
> > +LITEX PLATFORM
> > +M:   Karol Gugala <kgugala@antmicro.com>
> > +M:   Mateusz Holenko <mholenko@antmicro.com>
> > +S:   Maintained
> > +F:   include/linux/litex.h
> > +
> >  LIVE PATCHING
> >  M:   Josh Poimboeuf <jpoimboe@redhat.com>
> >  M:   Jiri Kosina <jikos@kernel.org>
> > diff --git a/include/linux/litex.h b/include/linux/litex.h
> > new file mode 100644
> > index 000000000000..e793d2d7c881
> > --- /dev/null
> > +++ b/include/linux/litex.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Common LiteX header providing
> > + * helper functions for accessing CSRs.
> > + *
> > + * Copyright (C) 2019 Antmicro <www.antmicro.com>
> > + */
> > +
> > +#ifndef _LINUX_LITEX_H
> > +#define _LINUX_LITEX_H
> > +
> > +#include <linux/io.h>
> > +#include <linux/types.h>
> > +#include <linux/compiler_types.h>
> > +
> > +#define LITEX_REG_SIZE             0x4
> > +#define LITEX_SUBREG_SIZE          0x1
> > +#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)
> > +
> > +#ifdef __LITTLE_ENDIAN
> > +# define LITEX_READ_REG(addr)                  ioread32(addr)
> > +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> > +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> > +#else
> > +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> > +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> > +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
>
> Defining custom accessors makes it harder for others to understand
> the code. The __raw_readl/writel accessors are native endian, so use
> those. One difference though is they don't have a memory barrier, but
> based on the below functions, you may want to do your own barrier
> anyways. And if DMA is not involved you don't need the barriers either.

LiteX CSRs are always little endian (even when combined with a big endian CPU),
so using just __raw_readl/writel won't work in all cases. This is the reason why
we proposed a custom accessors defined based on host CPU endianness.
Would adding a comment explaining this be enough or do you have other ideas?

> The _OFF variants don't add anything. LITEX_READ_REG(addr + off) is just
> as easy to read if not easier than LITEX_READ_REG_OFF(addr, off).

I agree, LITEX_READ_REG/LITEX_WRITE_REG is enough.

> > +#endif
> > +
> > +/* Helper functions for manipulating LiteX registers */
> > +
> > +static inline void litex_set_reg(void __iomem *reg, u32 reg_size, u32 val)
> > +{
> > +     u32 shifted_data, shift, i;
> > +
> > +     for (i = 0; i < reg_size; ++i) {
> > +             shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> > +             shifted_data = val >> shift;
> > +             LITEX_WRITE_REG(shifted_data, reg + (LITEX_REG_SIZE * i));
> > +     }
> > +}
>
> The problem with this is it hides whether you need to do any locking.
> Normally, you would assume a register access is atomic when it's not
> here. You could add locking in this function, but then you're doing
> locking even when not needed.
>
> It doesn't look like you actually use this in your series, so maybe
> remove until you do.

That's right. I added those functions in advance, having in mind
further drivers,
but maybe it will be better to drop them for now and re-introduce later.

> > +
> > +static inline u32 litex_get_reg(void __iomem *reg, u32 reg_size)
> > +{
> > +     u32 shifted_data, shift, i;
> > +     u32 result = 0;
> > +
> > +     for (i = 0; i < reg_size; ++i) {
> > +             shifted_data = LITEX_READ_REG(reg + (LITEX_REG_SIZE * i));
> > +             shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> > +             result |= (shifted_data << shift);
> > +     }
> > +
> > +     return result;
> > +}
> > +
> > +#endif /* _LINUX_LITEX_H */
> > --
> > 2.23.0



--
Mateusz Holenko
Antmicro Ltd | www.antmicro.com
Roosevelta 22, 60-829 Poznan, Poland

Re: [PATCH v2 2/4] litex: add common LiteX header

[Gabriel L. Somlo]

> It provides helper CSR access functions used by all
> LiteX drivers.
> 
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> This commit has been introduced in v2 of the patchset.
> 
>  MAINTAINERS           |  6 +++++
>  include/linux/litex.h | 59 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100644 include/linux/litex.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 296de2b51c83..eaa51209bfb2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9493,6 +9493,12 @@ F:	Documentation/misc-devices/lis3lv02d.rst
>  F:	drivers/misc/lis3lv02d/
>  F:	drivers/platform/x86/hp_accel.c
>  
> +LITEX PLATFORM
> +M:	Karol Gugala <kgugala@antmicro.com>
> +M:	Mateusz Holenko <mholenko@antmicro.com>
> +S:	Maintained
> +F:	include/linux/litex.h
> +
>  LIVE PATCHING
>  M:	Josh Poimboeuf <jpoimboe@redhat.com>
>  M:	Jiri Kosina <jikos@kernel.org>
> diff --git a/include/linux/litex.h b/include/linux/litex.h
> new file mode 100644
> index 000000000000..e793d2d7c881
> --- /dev/null
> +++ b/include/linux/litex.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Common LiteX header providing
> + * helper functions for accessing CSRs.
> + *
> + * Copyright (C) 2019 Antmicro <www.antmicro.com>
> + */
> +
> +#ifndef _LINUX_LITEX_H
> +#define _LINUX_LITEX_H
> +
> +#include <linux/io.h>
> +#include <linux/types.h>
> +#include <linux/compiler_types.h>
> +
> +#define LITEX_REG_SIZE             0x4
> +#define LITEX_SUBREG_SIZE          0x1
> +#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)

Please be aware that LiteX supports "LITEX_SUBREG_SIZE" of up to 4,
and might go even as far as 8 in the near future. Hard coding this
here might be short sighted.

A good alternative might be to have "litex_subreg_size" or
"mmio-dw-bytes" passed in via a DT property of the SoC node, as
illustrated here:

https://github.com/litex-hub/linux-on-litex-vexriscv/pull/60

Thanks,
--Gabriel

> +
> +#ifdef __LITTLE_ENDIAN
> +# define LITEX_READ_REG(addr)                  ioread32(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> +#else
> +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
> +#endif
> +
> +/* Helper functions for manipulating LiteX registers */
> +
> +static inline void litex_set_reg(void __iomem *reg, u32 reg_size, u32 val)
> +{
> +	u32 shifted_data, shift, i;
> +
> +	for (i = 0; i < reg_size; ++i) {
> +		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> +		shifted_data = val >> shift;
> +		LITEX_WRITE_REG(shifted_data, reg + (LITEX_REG_SIZE * i));
> +	}
> +}
> +
> +static inline u32 litex_get_reg(void __iomem *reg, u32 reg_size)
> +{
> +	u32 shifted_data, shift, i;
> +	u32 result = 0;
> +
> +	for (i = 0; i < reg_size; ++i) {
> +		shifted_data = LITEX_READ_REG(reg + (LITEX_REG_SIZE * i));
> +		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> +		result |= (shifted_data << shift);
> +	}
> +
> +	return result;
> +}
> +
> +#endif /* _LINUX_LITEX_H */

Re: [PATCH v2 2/4] litex: add common LiteX header

[Greg Kroah-Hartman]

On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> +#ifdef __LITTLE_ENDIAN
> +# define LITEX_READ_REG(addr)                  ioread32(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> +#else
> +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
> +#endif

I just noticed this.

Ick, this is not good.  You will run into problems in the future with
this, I can guarantee it.  What about systems where the CPU is one
endian and the hardware in the other?  It will happen trust us.

Make these real functions (inline is nice) and pass in the pointer to
the device so you can test for it and call the correct function based on
the cpu/hardware type.

And what about bitfields?  What endian are they for your
system/hardware?

Almost no kernel code should EVER be testing __LITTLE_ENDIAN, don't add
to it as it is not a good idea.

thanks,

greg k-h

Re: [PATCH v2 2/4] litex: add common LiteX header

[Mateusz Holenko]

śr., 20 lis 2019 o 20:26 Greg Kroah-Hartman
<gregkh@linuxfoundation.org> napisał(a):
>
> On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> > +#ifdef __LITTLE_ENDIAN
> > +# define LITEX_READ_REG(addr)                  ioread32(addr)
> > +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> > +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> > +#else
> > +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> > +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> > +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
> > +#endif
>
> I just noticed this.
>
> Ick, this is not good.  You will run into problems in the future with
> this, I can guarantee it.  What about systems where the CPU is one
> endian and the hardware in the other?  It will happen trust us.

As mentioned in the previous comment, LiteX CSRs are guaranteed to be
always little-endian - this includes configurations with both
big-endian and little-endian CPUs.

The aim of including the ifdef section was exactly to target situation
where endianness is different for CPU and devices. As such this
approach *should* work.

> Make these real functions (inline is nice) and pass in the pointer to
> the device so you can test for it and call the correct function based on
> the cpu/hardware type.
>
> And what about bitfields?  What endian are they for your
> system/hardware?
>
> Almost no kernel code should EVER be testing __LITTLE_ENDIAN, don't add
> to it as it is not a good idea.

If I understand correctly, you suggest to replace compile-time
ifdefing with probing the endianness in the runtime (by reading some
register that should return a known value, say 1, and testing how bits
are arranged). This is a good idea, as it protects against breaking an
always-little-endian property of LiteX CSRs in the future.

I'll include this in the next version of the patchset.

> thanks,
>
> greg k-h

Thanks for your comments!

-- 
Mateusz Holenko
Antmicro Ltd | www.antmicro.com
Roosevelta 22, 60-829 Poznan, Poland

Re: [PATCH v2 2/4] litex: add common LiteX header

[Greg Kroah-Hartman]

On Tue, Nov 26, 2019 at 10:02:18AM +0100, Mateusz Holenko wrote:
> śr., 20 lis 2019 o 20:26 Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> napisał(a):
> >
> > On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> > > +#ifdef __LITTLE_ENDIAN
> > > +# define LITEX_READ_REG(addr)                  ioread32(addr)
> > > +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> > > +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> > > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> > > +#else
> > > +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> > > +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> > > +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> > > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
> > > +#endif
> >
> > I just noticed this.
> >
> > Ick, this is not good.  You will run into problems in the future with
> > this, I can guarantee it.  What about systems where the CPU is one
> > endian and the hardware in the other?  It will happen trust us.
> 
> As mentioned in the previous comment, LiteX CSRs are guaranteed to be
> always little-endian - this includes configurations with both
> big-endian and little-endian CPUs.

What enforces that guarantee?

> The aim of including the ifdef section was exactly to target situation
> where endianness is different for CPU and devices. As such this
> approach *should* work.

"should" :)

We have seen it happen all the time that some hardware team hooks this
up backwards, no matter what the "spec" required.  So be careful here.

good luck!

greg k-h

Re: [PATCH v2 2/4] litex: add common LiteX header

[Mateusz Holenko]

wt., 26 lis 2019 o 10:19 Greg Kroah-Hartman
<gregkh@linuxfoundation.org> napisał(a):
>
> On Tue, Nov 26, 2019 at 10:02:18AM +0100, Mateusz Holenko wrote:
> > śr., 20 lis 2019 o 20:26 Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> napisał(a):
> > >
> > > On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> > > > +#ifdef __LITTLE_ENDIAN
> > > > +# define LITEX_READ_REG(addr)                  ioread32(addr)
> > > > +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> > > > +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> > > > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> > > > +#else
> > > > +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> > > > +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> > > > +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> > > > +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
> > > > +#endif
> > >
> > > I just noticed this.
> > >
> > > Ick, this is not good.  You will run into problems in the future with
> > > this, I can guarantee it.  What about systems where the CPU is one
> > > endian and the hardware in the other?  It will happen trust us.
> >
> > As mentioned in the previous comment, LiteX CSRs are guaranteed to be
> > always little-endian - this includes configurations with both
> > big-endian and little-endian CPUs.
>
> What enforces that guarantee?

liteuart is na IP core that comes as a part of a design generated and
configured by LiteX SoC builder
(https://github.com/enjoy-digital/litex). Current implementation of
LiteX generates systems in such a way that CSRs in peripherals are
little-endian regardless of a softcore CPU used (available options
cover both little-endian and big-endian CPU cores). Liteuart is a part
of the LiteX project and is probably not usable outside of it.

I was digging through the code and documentation to verify that the
always-little-endian-CSR behaviour is "guaranteed", but - to my
surprise - couldn't find anything. It seems to be simply an
implementation detail that might (maybe?) change in the future.

In this context your suggestion about probing things dynamically seems
to be even more relevant.

> > The aim of including the ifdef section was exactly to target situation
> > where endianness is different for CPU and devices. As such this
> > approach *should* work.
>
> "should" :)
>
> We have seen it happen all the time that some hardware team hooks this
> up backwards, no matter what the "spec" required.  So be careful here.
>
> good luck!
>
> greg k-h



-- 
Mateusz Holenko
Antmicro Ltd | www.antmicro.com
Roosevelta 22, 60-829 Poznan, Poland

[PATCH v2 3/4] dt-bindings: serial: document LiteUART bindings

[Mateusz Holenko]

From: Filip Kokosinski <fkokosinski@internships.antmicro.com>

Add documentation for LiteUART devicetree bindings.

Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---
Changes in v2:
- binding description rewritten to a yaml schema file
- added interrupt line
- fixed unit address
- patch number changed from 2 to 3

 .../bindings/serial/litex,liteuart.yaml       | 38 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/litex,liteuart.yaml

diff --git a/Documentation/devicetree/bindings/serial/litex,liteuart.yaml b/Documentation/devicetree/bindings/serial/litex,liteuart.yaml
new file mode 100644
index 000000000000..87bf846b170a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/litex,liteuart.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/litex,liteuart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LiteUART serial controller
+
+maintainers:
+  - Karol Gugala <kgugala@antmicro.com>
+  - Mateusz Holenko <mholenko@antmicro.com>
+
+description: |
+  LiteUART serial controller is a part of LiteX FPGA SoC builder. It supports
+  multiple CPU architectures, currently including e.g. OpenRISC and RISC-V.
+
+properties:
+  compatible:
+    const: litex,liteuart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    uart0: serial@e0001800 {
+      compatible = "litex,liteuart";
+      reg = <0xe0001800 0x100>;
+      interrupts = <2>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index eaa51209bfb2..1dc783c9edb7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9498,6 +9498,7 @@ M:	Karol Gugala <kgugala@antmicro.com>
 M:	Mateusz Holenko <mholenko@antmicro.com>
 S:	Maintained
 F:	include/linux/litex.h
+F:	Documentation/devicetree/bindings/*/litex,*.yaml
 
 LIVE PATCHING
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
-- 
2.23.0

Re: [PATCH v2 3/4] dt-bindings: serial: document LiteUART bindings

[Rob Herring]

On Wed, 23 Oct 2019 11:47:14 +0200, Mateusz Holenko wrote:
> From: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> 
> Add documentation for LiteUART devicetree bindings.
> 
> Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> Changes in v2:
> - binding description rewritten to a yaml schema file
> - added interrupt line
> - fixed unit address
> - patch number changed from 2 to 3
> 
>  .../bindings/serial/litex,liteuart.yaml       | 38 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/litex,liteuart.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v2 4/4] drivers/tty/serial: add LiteUART driver

[Mateusz Holenko]

From: Filip Kokosinski <fkokosinski@internships.antmicro.com>

This commit adds driver for the FPGA-based LiteUART serial controller
from LiteX SoC builder.

The current implementation supports LiteUART configured
for 32 bit data width and 8 bit CSR bus width.

It does not support IRQ.

Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---
Changes in v2:
- used register access functions from newly introduced litex.h
- patch number changed from 3 to 4

 MAINTAINERS                      |   1 +
 drivers/tty/serial/Kconfig       |  30 +++
 drivers/tty/serial/Makefile      |   1 +
 drivers/tty/serial/liteuart.c    | 373 +++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |   3 +
 5 files changed, 408 insertions(+)
 create mode 100644 drivers/tty/serial/liteuart.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1dc783c9edb7..c24a37833e78 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9499,6 +9499,7 @@ M:	Mateusz Holenko <mholenko@antmicro.com>
 S:	Maintained
 F:	include/linux/litex.h
 F:	Documentation/devicetree/bindings/*/litex,*.yaml
+F:	drivers/tty/serial/liteuart.c
 
 LIVE PATCHING
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4789b5d62f63..b01fe12a1411 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1571,6 +1571,36 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
 	  receives all kernel messages and warnings and which allows logins in
 	  single user mode).
 
+config SERIAL_LITEUART
+	tristate "LiteUART serial port support"
+	depends on HAS_IOMEM
+	depends on OF
+	select SERIAL_CORE
+	help
+	  This driver is for the FPGA-based LiteUART serial controller from LiteX
+	  SoC builder.
+
+	  Say 'Y' here if you wish to use the LiteUART serial controller.
+	  Otherwise, say 'N'.
+
+config SERIAL_LITEUART_NR_PORTS
+	int "Number of LiteUART ports"
+	depends on SERIAL_LITEUART
+	default "1"
+	help
+	  Set this to the number of serial ports you want the driver
+	  to support.
+
+config SERIAL_LITEUART_CONSOLE
+	bool "LiteUART serial port console support"
+	depends on SERIAL_LITEUART=y
+	select SERIAL_CORE_CONSOLE
+	help
+	  Say 'Y' here if you wish to use the FPGA-based LiteUART serial controller
+	  from LiteX SoC builder as the system console (the system console is the
+	  device which receives all kernel messages and warnings and which allows
+	  logins in single user mode). Otherwise, say 'N'.
+
 endmenu
 
 config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 863f47056539..c8d7e2046284 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
 obj-$(CONFIG_SERIAL_RDA)	+= rda-uart.o
 obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o
 obj-$(CONFIG_SERIAL_SIFIVE)	+= sifive.o
+obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
new file mode 100644
index 000000000000..e142f78df57a
--- /dev/null
+++ b/drivers/tty/serial/liteuart.c
@@ -0,0 +1,373 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LiteUART serial controller (LiteX) Driver
+ *
+ * Copyright (C) 2019 Antmicro Ltd <www.antmicro.com>
+ */
+
+#include <linux/console.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/timer.h>
+#include <linux/tty_flip.h>
+#include <linux/litex.h>
+
+/* module-related defines */
+#define DRIVER_NAME	"liteuart"
+#define DRIVER_MAJOR	0
+#define DRIVER_MINOR	0
+#define DEV_NAME	"ttyLXU"
+
+/* base address offsets */
+#define OFF_RXTX	0x00
+#define OFF_TXFULL	0x04
+#define OFF_RXEMPTY	0x08
+#define OFF_EV_STATUS	0x0c
+#define OFF_EV_PENDING	0x10
+#define OFF_EV_ENABLE	0x14
+
+/* events */
+#define EV_TX	0x1
+#define EV_RX	0x2
+
+struct liteuart_port {
+	struct uart_port port;
+	struct timer_list timer;
+};
+
+#define to_liteuart_port(port)	container_of(port, struct liteuart_port, port)
+
+static struct liteuart_port liteuart_ports[CONFIG_SERIAL_LITEUART_NR_PORTS];
+
+#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+static struct console liteuart_console;
+#endif
+
+static struct uart_driver liteuart_driver = {
+	.owner = THIS_MODULE,
+	.driver_name = DRIVER_NAME,
+	.dev_name = DEV_NAME,
+	.major = DRIVER_MAJOR,
+	.minor = DRIVER_MINOR,
+	.nr = CONFIG_SERIAL_LITEUART_NR_PORTS,
+#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+	.cons = &liteuart_console,
+#endif
+};
+
+static void liteuart_timer(struct timer_list *t)
+{
+	struct liteuart_port *uart = from_timer(uart, t, timer);
+	struct uart_port *port = &uart->port;
+	unsigned char __iomem *membase = port->membase;
+	unsigned int flg = TTY_NORMAL;
+	int ch;
+	unsigned int status;
+
+	while ((status = !LITEX_READ_REG_OFF(membase, OFF_RXEMPTY)) == 1) {
+		ch = LITEX_READ_REG_OFF(membase, OFF_RXTX);
+		port->icount.rx++;
+
+		/* necessary for RXEMPTY to refresh its value */
+		LITEX_WRITE_REG_OFF(EV_TX | EV_RX, membase, OFF_EV_PENDING);
+
+		/* no overflow bits in status */
+		if (!(uart_handle_sysrq_char(port, ch)))
+			uart_insert_char(port, status, 0, ch, flg);
+
+		tty_flip_buffer_push(&port->state->port);
+	}
+
+	mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
+}
+
+static void liteuart_putchar(struct uart_port *port, int ch)
+{
+	while (LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
+		cpu_relax();
+
+	LITEX_WRITE_REG_OFF(ch, port->membase, OFF_RXTX);
+}
+
+static unsigned int liteuart_tx_empty(struct uart_port *port)
+{
+	/* not really tx empty, just checking if tx is not full */
+	if (!LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
+		return TIOCSER_TEMT;
+
+	return 0;
+}
+
+static void liteuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* modem control register is not present in LiteUART */
+}
+
+static unsigned int liteuart_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+}
+
+static void liteuart_stop_tx(struct uart_port *port)
+{
+}
+
+static void liteuart_start_tx(struct uart_port *port)
+{
+	struct circ_buf *xmit = &port->state->xmit;
+	unsigned char ch;
+
+	if (unlikely(port->x_char)) {
+		LITEX_WRITE_REG_OFF(port->x_char, port->membase, OFF_RXTX);
+		port->icount.tx++;
+		port->x_char = 0;
+	} else if (!uart_circ_empty(xmit)) {
+		while (xmit->head != xmit->tail) {
+			ch = xmit->buf[xmit->tail];
+			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+			port->icount.tx++;
+			liteuart_putchar(port, ch);
+		}
+	}
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
+}
+
+static void liteuart_stop_rx(struct uart_port *port)
+{
+	struct liteuart_port *uart = to_liteuart_port(port);
+
+	/* just delete timer */
+	del_timer(&uart->timer);
+}
+
+static void liteuart_break_ctl(struct uart_port *port, int break_state)
+{
+	/* LiteUART doesn't support sending break signal */
+}
+
+static int liteuart_startup(struct uart_port *port)
+{
+	struct liteuart_port *uart = to_liteuart_port(port);
+
+	/* disable events */
+	LITEX_WRITE_REG_OFF(0, port->membase, OFF_EV_ENABLE);
+
+	/* prepare timer for polling */
+	timer_setup(&uart->timer, liteuart_timer, 0);
+	mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
+
+	return 0;
+}
+
+static void liteuart_shutdown(struct uart_port *port)
+{
+}
+
+static void liteuart_set_termios(struct uart_port *port, struct ktermios *new,
+				 struct ktermios *old)
+{
+	unsigned int baud;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	/* update baudrate */
+	baud = uart_get_baud_rate(port, new, old, 0, 460800);
+	uart_update_timeout(port, new->c_cflag, baud);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static const char *liteuart_type(struct uart_port *port)
+{
+	return (port->type == PORT_LITEUART) ? DRIVER_NAME : NULL;
+}
+
+static void liteuart_release_port(struct uart_port *port)
+{
+}
+
+static int liteuart_request_port(struct uart_port *port)
+{
+	return 0;
+}
+
+static void liteuart_config_port(struct uart_port *port, int flags)
+{
+	if (flags & UART_CONFIG_TYPE)
+		port->type = PORT_LITEUART;
+}
+
+static int liteuart_verify_port(struct uart_port *port,
+				struct serial_struct *ser)
+{
+	if (port->type != PORT_UNKNOWN && ser->type != PORT_LITEUART)
+		return -EINVAL;
+
+	return 0;
+}
+
+static const struct uart_ops liteuart_ops = {
+	.tx_empty	= liteuart_tx_empty,
+	.set_mctrl	= liteuart_set_mctrl,
+	.get_mctrl	= liteuart_get_mctrl,
+	.stop_tx	= liteuart_stop_tx,
+	.start_tx	= liteuart_start_tx,
+	.stop_rx	= liteuart_stop_rx,
+	.break_ctl	= liteuart_break_ctl,
+	.startup	= liteuart_startup,
+	.shutdown	= liteuart_shutdown,
+	.set_termios	= liteuart_set_termios,
+	.type		= liteuart_type,
+	.release_port	= liteuart_release_port,
+	.request_port	= liteuart_request_port,
+	.config_port	= liteuart_config_port,
+	.verify_port	= liteuart_verify_port,
+};
+
+static int liteuart_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct liteuart_port *uart;
+	struct uart_port *port;
+	int dev_id;
+
+	/* no device tree */
+	if (!np)
+		return -ENODEV;
+
+	dev_id = of_alias_get_id(np, "serial");
+	if (dev_id < 0 || dev_id >= CONFIG_SERIAL_LITEUART_NR_PORTS)
+		return -EINVAL;
+
+	uart = &liteuart_ports[dev_id];
+	port = &uart->port;
+
+	/* get {map,mem}base */
+	port->mapbase = platform_get_resource(pdev, IORESOURCE_MEM, 0)->start;
+	port->membase = of_iomap(np, 0);
+	if (!port->membase)
+		return -ENXIO;
+
+	/* values not from device tree */
+	port->dev = &pdev->dev;
+	port->iotype = UPIO_MEM;
+	port->flags = UPF_BOOT_AUTOCONF;
+	port->ops = &liteuart_ops;
+	port->regshift = 2;
+	port->fifosize = 16;
+	port->iobase = 1;
+	port->type = PORT_UNKNOWN;
+	port->line = dev_id;
+
+	return uart_add_one_port(&liteuart_driver,
+				 &liteuart_ports[dev_id].port);
+}
+
+static int liteuart_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id liteuart_of_match[] = {
+	{ .compatible = "litex,liteuart" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, liteuart_of_match);
+
+static struct platform_driver liteuart_platform_driver = {
+	.probe = liteuart_probe,
+	.remove = liteuart_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = of_match_ptr(liteuart_of_match),
+	},
+};
+
+#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+
+static void liteuart_console_write(struct console *co, const char *s,
+	unsigned int count)
+{
+	struct uart_port *port = &liteuart_ports[co->index].port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	uart_console_write(port, s, count, liteuart_putchar);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int liteuart_console_setup(struct console *co, char *options)
+{
+	struct uart_port *port;
+	int baud = 115200;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	port = &liteuart_ports[co->index].port;
+	if (!port->membase)
+		return -ENODEV;
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct console liteuart_console = {
+	.name = DRIVER_NAME,
+	.write = liteuart_console_write,
+	.device = uart_console_device,
+	.setup = liteuart_console_setup,
+	.flags = CON_PRINTBUFFER,
+	.index = -1,
+	.data = &liteuart_driver,
+};
+
+static int __init liteuart_console_init(void)
+{
+	register_console(&liteuart_console);
+
+	return 0;
+}
+
+console_initcall(liteuart_console_init);
+#endif /* CONFIG_SERIAL_LITEUART_CONSOLE */
+
+static int __init liteuart_init(void)
+{
+	int res;
+
+	res = uart_register_driver(&liteuart_driver);
+	if (res)
+		return res;
+
+	res = platform_driver_register(&liteuart_platform_driver);
+	if (res) {
+		uart_unregister_driver(&liteuart_driver);
+		return res;
+	}
+
+	return 0;
+}
+
+static void __exit liteuart_exit(void)
+{
+	platform_driver_unregister(&liteuart_platform_driver);
+	uart_unregister_driver(&liteuart_driver);
+}
+
+module_init(liteuart_init);
+module_exit(liteuart_exit);
+
+MODULE_AUTHOR("Antmicro Ltd <www.antmicro.com>");
+MODULE_DESCRIPTION("LiteUART serial driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 0f4f87a6fd54..8fe6b0bf8014 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -293,4 +293,7 @@
 /* Freescale Linflex UART */
 #define PORT_LINFLEXUART	121
 
+/* LiteUART */
+#define PORT_LITEUART	122
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
-- 
2.23.0

Re: [PATCH v2 4/4] drivers/tty/serial: add LiteUART driver

[Rob Herring]

On Wed, Oct 23, 2019 at 11:47:23AM +0200, Mateusz Holenko wrote:
> From: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> 
> This commit adds driver for the FPGA-based LiteUART serial controller
> from LiteX SoC builder.
> 
> The current implementation supports LiteUART configured
> for 32 bit data width and 8 bit CSR bus width.
> 
> It does not support IRQ.
> 
> Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> Changes in v2:
> - used register access functions from newly introduced litex.h
> - patch number changed from 3 to 4
> 
>  MAINTAINERS                      |   1 +
>  drivers/tty/serial/Kconfig       |  30 +++
>  drivers/tty/serial/Makefile      |   1 +
>  drivers/tty/serial/liteuart.c    | 373 +++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |   3 +
>  5 files changed, 408 insertions(+)
>  create mode 100644 drivers/tty/serial/liteuart.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1dc783c9edb7..c24a37833e78 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9499,6 +9499,7 @@ M:	Mateusz Holenko <mholenko@antmicro.com>
>  S:	Maintained
>  F:	include/linux/litex.h
>  F:	Documentation/devicetree/bindings/*/litex,*.yaml
> +F:	drivers/tty/serial/liteuart.c
>  
>  LIVE PATCHING
>  M:	Josh Poimboeuf <jpoimboe@redhat.com>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 4789b5d62f63..b01fe12a1411 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1571,6 +1571,36 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
>  	  receives all kernel messages and warnings and which allows logins in
>  	  single user mode).
>  
> +config SERIAL_LITEUART
> +	tristate "LiteUART serial port support"
> +	depends on HAS_IOMEM
> +	depends on OF
> +	select SERIAL_CORE
> +	help
> +	  This driver is for the FPGA-based LiteUART serial controller from LiteX
> +	  SoC builder.
> +
> +	  Say 'Y' here if you wish to use the LiteUART serial controller.
> +	  Otherwise, say 'N'.
> +
> +config SERIAL_LITEUART_NR_PORTS
> +	int "Number of LiteUART ports"
> +	depends on SERIAL_LITEUART
> +	default "1"
> +	help
> +	  Set this to the number of serial ports you want the driver
> +	  to support.

It's better if the driver supports a dynamic number of ports.

> +
> +config SERIAL_LITEUART_CONSOLE
> +	bool "LiteUART serial port console support"
> +	depends on SERIAL_LITEUART=y
> +	select SERIAL_CORE_CONSOLE
> +	help
> +	  Say 'Y' here if you wish to use the FPGA-based LiteUART serial controller
> +	  from LiteX SoC builder as the system console (the system console is the
> +	  device which receives all kernel messages and warnings and which allows
> +	  logins in single user mode). Otherwise, say 'N'.
> +
>  endmenu
>  
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 863f47056539..c8d7e2046284 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
>  obj-$(CONFIG_SERIAL_RDA)	+= rda-uart.o
>  obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o
>  obj-$(CONFIG_SERIAL_SIFIVE)	+= sifive.o
> +obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
>  
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> new file mode 100644
> index 000000000000..e142f78df57a
> --- /dev/null
> +++ b/drivers/tty/serial/liteuart.c
> @@ -0,0 +1,373 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LiteUART serial controller (LiteX) Driver
> + *
> + * Copyright (C) 2019 Antmicro Ltd <www.antmicro.com>
> + */
> +
> +#include <linux/console.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/serial.h>
> +#include <linux/serial_core.h>
> +#include <linux/timer.h>
> +#include <linux/tty_flip.h>
> +#include <linux/litex.h>
> +
> +/* module-related defines */
> +#define DRIVER_NAME	"liteuart"
> +#define DRIVER_MAJOR	0
> +#define DRIVER_MINOR	0
> +#define DEV_NAME	"ttyLXU"
> +
> +/* base address offsets */
> +#define OFF_RXTX	0x00
> +#define OFF_TXFULL	0x04
> +#define OFF_RXEMPTY	0x08
> +#define OFF_EV_STATUS	0x0c
> +#define OFF_EV_PENDING	0x10
> +#define OFF_EV_ENABLE	0x14
> +
> +/* events */
> +#define EV_TX	0x1
> +#define EV_RX	0x2
> +
> +struct liteuart_port {
> +	struct uart_port port;
> +	struct timer_list timer;
> +};
> +
> +#define to_liteuart_port(port)	container_of(port, struct liteuart_port, port)
> +
> +static struct liteuart_port liteuart_ports[CONFIG_SERIAL_LITEUART_NR_PORTS];
> +
> +#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
> +static struct console liteuart_console;
> +#endif
> +
> +static struct uart_driver liteuart_driver = {
> +	.owner = THIS_MODULE,
> +	.driver_name = DRIVER_NAME,
> +	.dev_name = DEV_NAME,
> +	.major = DRIVER_MAJOR,
> +	.minor = DRIVER_MINOR,
> +	.nr = CONFIG_SERIAL_LITEUART_NR_PORTS,
> +#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
> +	.cons = &liteuart_console,
> +#endif
> +};
> +
> +static void liteuart_timer(struct timer_list *t)
> +{
> +	struct liteuart_port *uart = from_timer(uart, t, timer);
> +	struct uart_port *port = &uart->port;
> +	unsigned char __iomem *membase = port->membase;
> +	unsigned int flg = TTY_NORMAL;
> +	int ch;
> +	unsigned int status;
> +
> +	while ((status = !LITEX_READ_REG_OFF(membase, OFF_RXEMPTY)) == 1) {
> +		ch = LITEX_READ_REG_OFF(membase, OFF_RXTX);
> +		port->icount.rx++;
> +
> +		/* necessary for RXEMPTY to refresh its value */
> +		LITEX_WRITE_REG_OFF(EV_TX | EV_RX, membase, OFF_EV_PENDING);
> +
> +		/* no overflow bits in status */
> +		if (!(uart_handle_sysrq_char(port, ch)))
> +			uart_insert_char(port, status, 0, ch, flg);
> +
> +		tty_flip_buffer_push(&port->state->port);
> +	}
> +
> +	mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> +}
> +
> +static void liteuart_putchar(struct uart_port *port, int ch)
> +{
> +	while (LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
> +		cpu_relax();
> +
> +	LITEX_WRITE_REG_OFF(ch, port->membase, OFF_RXTX);
> +}
> +
> +static unsigned int liteuart_tx_empty(struct uart_port *port)
> +{
> +	/* not really tx empty, just checking if tx is not full */
> +	if (!LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
> +		return TIOCSER_TEMT;
> +
> +	return 0;
> +}
> +
> +static void liteuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +	/* modem control register is not present in LiteUART */
> +}
> +
> +static unsigned int liteuart_get_mctrl(struct uart_port *port)
> +{
> +	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +}
> +
> +static void liteuart_stop_tx(struct uart_port *port)
> +{
> +}
> +
> +static void liteuart_start_tx(struct uart_port *port)
> +{
> +	struct circ_buf *xmit = &port->state->xmit;
> +	unsigned char ch;
> +
> +	if (unlikely(port->x_char)) {
> +		LITEX_WRITE_REG_OFF(port->x_char, port->membase, OFF_RXTX);
> +		port->icount.tx++;
> +		port->x_char = 0;
> +	} else if (!uart_circ_empty(xmit)) {
> +		while (xmit->head != xmit->tail) {
> +			ch = xmit->buf[xmit->tail];
> +			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> +			port->icount.tx++;
> +			liteuart_putchar(port, ch);
> +		}
> +	}
> +
> +	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +		uart_write_wakeup(port);
> +}
> +
> +static void liteuart_stop_rx(struct uart_port *port)
> +{
> +	struct liteuart_port *uart = to_liteuart_port(port);
> +
> +	/* just delete timer */
> +	del_timer(&uart->timer);
> +}
> +
> +static void liteuart_break_ctl(struct uart_port *port, int break_state)
> +{
> +	/* LiteUART doesn't support sending break signal */
> +}
> +
> +static int liteuart_startup(struct uart_port *port)
> +{
> +	struct liteuart_port *uart = to_liteuart_port(port);
> +
> +	/* disable events */
> +	LITEX_WRITE_REG_OFF(0, port->membase, OFF_EV_ENABLE);
> +
> +	/* prepare timer for polling */
> +	timer_setup(&uart->timer, liteuart_timer, 0);
> +	mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> +
> +	return 0;
> +}
> +
> +static void liteuart_shutdown(struct uart_port *port)
> +{
> +}
> +
> +static void liteuart_set_termios(struct uart_port *port, struct ktermios *new,
> +				 struct ktermios *old)
> +{
> +	unsigned int baud;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	/* update baudrate */
> +	baud = uart_get_baud_rate(port, new, old, 0, 460800);
> +	uart_update_timeout(port, new->c_cflag, baud);
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static const char *liteuart_type(struct uart_port *port)
> +{
> +	return (port->type == PORT_LITEUART) ? DRIVER_NAME : NULL;
> +}
> +
> +static void liteuart_release_port(struct uart_port *port)
> +{
> +}
> +
> +static int liteuart_request_port(struct uart_port *port)
> +{
> +	return 0;
> +}
> +
> +static void liteuart_config_port(struct uart_port *port, int flags)
> +{
> +	if (flags & UART_CONFIG_TYPE)
> +		port->type = PORT_LITEUART;
> +}
> +
> +static int liteuart_verify_port(struct uart_port *port,
> +				struct serial_struct *ser)
> +{
> +	if (port->type != PORT_UNKNOWN && ser->type != PORT_LITEUART)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct uart_ops liteuart_ops = {
> +	.tx_empty	= liteuart_tx_empty,
> +	.set_mctrl	= liteuart_set_mctrl,
> +	.get_mctrl	= liteuart_get_mctrl,
> +	.stop_tx	= liteuart_stop_tx,
> +	.start_tx	= liteuart_start_tx,
> +	.stop_rx	= liteuart_stop_rx,
> +	.break_ctl	= liteuart_break_ctl,
> +	.startup	= liteuart_startup,
> +	.shutdown	= liteuart_shutdown,
> +	.set_termios	= liteuart_set_termios,
> +	.type		= liteuart_type,
> +	.release_port	= liteuart_release_port,
> +	.request_port	= liteuart_request_port,
> +	.config_port	= liteuart_config_port,
> +	.verify_port	= liteuart_verify_port,
> +};
> +
> +static int liteuart_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct liteuart_port *uart;
> +	struct uart_port *port;
> +	int dev_id;
> +
> +	/* no device tree */
> +	if (!np)
> +		return -ENODEV;
> +
> +	dev_id = of_alias_get_id(np, "serial");
> +	if (dev_id < 0 || dev_id >= CONFIG_SERIAL_LITEUART_NR_PORTS)
> +		return -EINVAL;

aliases should be optional. There's a helper to get a free index without 
an alias you can use.

Re: [PATCH v2 4/4] drivers/tty/serial: add LiteUART driver

[Mateusz Holenko]

sob., 26 paź 2019 o 02:13 Rob Herring <robh@kernel.org> napisał(a):
>
> On Wed, Oct 23, 2019 at 11:47:23AM +0200, Mateusz Holenko wrote:
> > From: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> >
> > This commit adds driver for the FPGA-based LiteUART serial controller
> > from LiteX SoC builder.
> >
> > The current implementation supports LiteUART configured
> > for 32 bit data width and 8 bit CSR bus width.
> >
> > It does not support IRQ.
> >
> > Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> > Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> > ---
> > Changes in v2:
> > - used register access functions from newly introduced litex.h
> > - patch number changed from 3 to 4
> >
> >  MAINTAINERS                      |   1 +
> >  drivers/tty/serial/Kconfig       |  30 +++
> >  drivers/tty/serial/Makefile      |   1 +
> >  drivers/tty/serial/liteuart.c    | 373 +++++++++++++++++++++++++++++++
> >  include/uapi/linux/serial_core.h |   3 +
> >  5 files changed, 408 insertions(+)
> >  create mode 100644 drivers/tty/serial/liteuart.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1dc783c9edb7..c24a37833e78 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9499,6 +9499,7 @@ M:      Mateusz Holenko <mholenko@antmicro.com>
> >  S:   Maintained
> >  F:   include/linux/litex.h
> >  F:   Documentation/devicetree/bindings/*/litex,*.yaml
> > +F:   drivers/tty/serial/liteuart.c
> >
> >  LIVE PATCHING
> >  M:   Josh Poimboeuf <jpoimboe@redhat.com>
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 4789b5d62f63..b01fe12a1411 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -1571,6 +1571,36 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
> >         receives all kernel messages and warnings and which allows logins in
> >         single user mode).
> >
> > +config SERIAL_LITEUART
> > +     tristate "LiteUART serial port support"
> > +     depends on HAS_IOMEM
> > +     depends on OF
> > +     select SERIAL_CORE
> > +     help
> > +       This driver is for the FPGA-based LiteUART serial controller from LiteX
> > +       SoC builder.
> > +
> > +       Say 'Y' here if you wish to use the LiteUART serial controller.
> > +       Otherwise, say 'N'.
> > +
> > +config SERIAL_LITEUART_NR_PORTS
> > +     int "Number of LiteUART ports"
> > +     depends on SERIAL_LITEUART
> > +     default "1"
> > +     help
> > +       Set this to the number of serial ports you want the driver
> > +       to support.
>
> It's better if the driver supports a dynamic number of ports.

Do you mean that the number of ports should be taken from the device
tree when booting the kernel?
This is exactly how it works right now. The SERIAL_LITEUART_NR_PORTS just
defines the maximum number of supported ports.
I can rename the config to SERIAL_LITEUART_MAX_NR_PORTS to make it
clearer, what do you think?

> > +
> > +config SERIAL_LITEUART_CONSOLE
> > +     bool "LiteUART serial port console support"
> > +     depends on SERIAL_LITEUART=y
> > +     select SERIAL_CORE_CONSOLE
> > +     help
> > +       Say 'Y' here if you wish to use the FPGA-based LiteUART serial controller
> > +       from LiteX SoC builder as the system console (the system console is the
> > +       device which receives all kernel messages and warnings and which allows
> > +       logins in single user mode). Otherwise, say 'N'.
> > +
> >  endmenu
> >
> >  config SERIAL_MCTRL_GPIO
> > diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> > index 863f47056539..c8d7e2046284 100644
> > --- a/drivers/tty/serial/Makefile
> > +++ b/drivers/tty/serial/Makefile
> > @@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_OWL)    += owl-uart.o
> >  obj-$(CONFIG_SERIAL_RDA)     += rda-uart.o
> >  obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o
> >  obj-$(CONFIG_SERIAL_SIFIVE)  += sifive.o
> > +obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
> >
> >  # GPIOLIB helpers for modem control lines
> >  obj-$(CONFIG_SERIAL_MCTRL_GPIO)      += serial_mctrl_gpio.o
> > diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> > new file mode 100644
> > index 000000000000..e142f78df57a
> > --- /dev/null
> > +++ b/drivers/tty/serial/liteuart.c
> > @@ -0,0 +1,373 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * LiteUART serial controller (LiteX) Driver
> > + *
> > + * Copyright (C) 2019 Antmicro Ltd <www.antmicro.com>
> > + */
> > +
> > +#include <linux/console.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/serial.h>
> > +#include <linux/serial_core.h>
> > +#include <linux/timer.h>
> > +#include <linux/tty_flip.h>
> > +#include <linux/litex.h>
> > +
> > +/* module-related defines */
> > +#define DRIVER_NAME  "liteuart"
> > +#define DRIVER_MAJOR 0
> > +#define DRIVER_MINOR 0
> > +#define DEV_NAME     "ttyLXU"
> > +
> > +/* base address offsets */
> > +#define OFF_RXTX     0x00
> > +#define OFF_TXFULL   0x04
> > +#define OFF_RXEMPTY  0x08
> > +#define OFF_EV_STATUS        0x0c
> > +#define OFF_EV_PENDING       0x10
> > +#define OFF_EV_ENABLE        0x14
> > +
> > +/* events */
> > +#define EV_TX        0x1
> > +#define EV_RX        0x2
> > +
> > +struct liteuart_port {
> > +     struct uart_port port;
> > +     struct timer_list timer;
> > +};
> > +
> > +#define to_liteuart_port(port)       container_of(port, struct liteuart_port, port)
> > +
> > +static struct liteuart_port liteuart_ports[CONFIG_SERIAL_LITEUART_NR_PORTS];
> > +
> > +#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
> > +static struct console liteuart_console;
> > +#endif
> > +
> > +static struct uart_driver liteuart_driver = {
> > +     .owner = THIS_MODULE,
> > +     .driver_name = DRIVER_NAME,
> > +     .dev_name = DEV_NAME,
> > +     .major = DRIVER_MAJOR,
> > +     .minor = DRIVER_MINOR,
> > +     .nr = CONFIG_SERIAL_LITEUART_NR_PORTS,
> > +#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
> > +     .cons = &liteuart_console,
> > +#endif
> > +};
> > +
> > +static void liteuart_timer(struct timer_list *t)
> > +{
> > +     struct liteuart_port *uart = from_timer(uart, t, timer);
> > +     struct uart_port *port = &uart->port;
> > +     unsigned char __iomem *membase = port->membase;
> > +     unsigned int flg = TTY_NORMAL;
> > +     int ch;
> > +     unsigned int status;
> > +
> > +     while ((status = !LITEX_READ_REG_OFF(membase, OFF_RXEMPTY)) == 1) {
> > +             ch = LITEX_READ_REG_OFF(membase, OFF_RXTX);
> > +             port->icount.rx++;
> > +
> > +             /* necessary for RXEMPTY to refresh its value */
> > +             LITEX_WRITE_REG_OFF(EV_TX | EV_RX, membase, OFF_EV_PENDING);
> > +
> > +             /* no overflow bits in status */
> > +             if (!(uart_handle_sysrq_char(port, ch)))
> > +                     uart_insert_char(port, status, 0, ch, flg);
> > +
> > +             tty_flip_buffer_push(&port->state->port);
> > +     }
> > +
> > +     mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> > +}
> > +
> > +static void liteuart_putchar(struct uart_port *port, int ch)
> > +{
> > +     while (LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
> > +             cpu_relax();
> > +
> > +     LITEX_WRITE_REG_OFF(ch, port->membase, OFF_RXTX);
> > +}
> > +
> > +static unsigned int liteuart_tx_empty(struct uart_port *port)
> > +{
> > +     /* not really tx empty, just checking if tx is not full */
> > +     if (!LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
> > +             return TIOCSER_TEMT;
> > +
> > +     return 0;
> > +}
> > +
> > +static void liteuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> > +{
> > +     /* modem control register is not present in LiteUART */
> > +}
> > +
> > +static unsigned int liteuart_get_mctrl(struct uart_port *port)
> > +{
> > +     return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> > +}
> > +
> > +static void liteuart_stop_tx(struct uart_port *port)
> > +{
> > +}
> > +
> > +static void liteuart_start_tx(struct uart_port *port)
> > +{
> > +     struct circ_buf *xmit = &port->state->xmit;
> > +     unsigned char ch;
> > +
> > +     if (unlikely(port->x_char)) {
> > +             LITEX_WRITE_REG_OFF(port->x_char, port->membase, OFF_RXTX);
> > +             port->icount.tx++;
> > +             port->x_char = 0;
> > +     } else if (!uart_circ_empty(xmit)) {
> > +             while (xmit->head != xmit->tail) {
> > +                     ch = xmit->buf[xmit->tail];
> > +                     xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> > +                     port->icount.tx++;
> > +                     liteuart_putchar(port, ch);
> > +             }
> > +     }
> > +
> > +     if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> > +             uart_write_wakeup(port);
> > +}
> > +
> > +static void liteuart_stop_rx(struct uart_port *port)
> > +{
> > +     struct liteuart_port *uart = to_liteuart_port(port);
> > +
> > +     /* just delete timer */
> > +     del_timer(&uart->timer);
> > +}
> > +
> > +static void liteuart_break_ctl(struct uart_port *port, int break_state)
> > +{
> > +     /* LiteUART doesn't support sending break signal */
> > +}
> > +
> > +static int liteuart_startup(struct uart_port *port)
> > +{
> > +     struct liteuart_port *uart = to_liteuart_port(port);
> > +
> > +     /* disable events */
> > +     LITEX_WRITE_REG_OFF(0, port->membase, OFF_EV_ENABLE);
> > +
> > +     /* prepare timer for polling */
> > +     timer_setup(&uart->timer, liteuart_timer, 0);
> > +     mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> > +
> > +     return 0;
> > +}
> > +
> > +static void liteuart_shutdown(struct uart_port *port)
> > +{
> > +}
> > +
> > +static void liteuart_set_termios(struct uart_port *port, struct ktermios *new,
> > +                              struct ktermios *old)
> > +{
> > +     unsigned int baud;
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&port->lock, flags);
> > +
> > +     /* update baudrate */
> > +     baud = uart_get_baud_rate(port, new, old, 0, 460800);
> > +     uart_update_timeout(port, new->c_cflag, baud);
> > +
> > +     spin_unlock_irqrestore(&port->lock, flags);
> > +}
> > +
> > +static const char *liteuart_type(struct uart_port *port)
> > +{
> > +     return (port->type == PORT_LITEUART) ? DRIVER_NAME : NULL;
> > +}
> > +
> > +static void liteuart_release_port(struct uart_port *port)
> > +{
> > +}
> > +
> > +static int liteuart_request_port(struct uart_port *port)
> > +{
> > +     return 0;
> > +}
> > +
> > +static void liteuart_config_port(struct uart_port *port, int flags)
> > +{
> > +     if (flags & UART_CONFIG_TYPE)
> > +             port->type = PORT_LITEUART;
> > +}
> > +
> > +static int liteuart_verify_port(struct uart_port *port,
> > +                             struct serial_struct *ser)
> > +{
> > +     if (port->type != PORT_UNKNOWN && ser->type != PORT_LITEUART)
> > +             return -EINVAL;
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct uart_ops liteuart_ops = {
> > +     .tx_empty       = liteuart_tx_empty,
> > +     .set_mctrl      = liteuart_set_mctrl,
> > +     .get_mctrl      = liteuart_get_mctrl,
> > +     .stop_tx        = liteuart_stop_tx,
> > +     .start_tx       = liteuart_start_tx,
> > +     .stop_rx        = liteuart_stop_rx,
> > +     .break_ctl      = liteuart_break_ctl,
> > +     .startup        = liteuart_startup,
> > +     .shutdown       = liteuart_shutdown,
> > +     .set_termios    = liteuart_set_termios,
> > +     .type           = liteuart_type,
> > +     .release_port   = liteuart_release_port,
> > +     .request_port   = liteuart_request_port,
> > +     .config_port    = liteuart_config_port,
> > +     .verify_port    = liteuart_verify_port,
> > +};
> > +
> > +static int liteuart_probe(struct platform_device *pdev)
> > +{
> > +     struct device_node *np = pdev->dev.of_node;
> > +     struct liteuart_port *uart;
> > +     struct uart_port *port;
> > +     int dev_id;
> > +
> > +     /* no device tree */
> > +     if (!np)
> > +             return -ENODEV;
> > +
> > +     dev_id = of_alias_get_id(np, "serial");
> > +     if (dev_id < 0 || dev_id >= CONFIG_SERIAL_LITEUART_NR_PORTS)
> > +             return -EINVAL;
>
> aliases should be optional. There's a helper to get a free index without
> an alias you can use.

Sure, we will fix it in v4.


-- 
Mateusz Holenko
Antmicro Ltd | www.antmicro.com
Roosevelta 22, 60-829 Poznan, Poland