[PATCH v3 0/5] LiteX SoC controller and LiteUART serial driver

[PATCH v3 0/5] LiteX SoC controller and LiteUART serial driver

[Mateusz Holenko]

This patchset introduces support for LiteX SoC Controller
and 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 v3:
- added Acked-by and Reviewed-by tags
- introduced LiteX SoC Controller driver
- removed endianness detection (handled now by LiteX SoC Controller driver)
- modified litex.h header
- DTS aliases for LiteUART made optional
- renamed SERIAL_LITEUART_NR_PORTS to SERIAL_LITEUART_MAX_PORTS
- changed PORT_LITEUART from 122 to 123

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

Pawel Czarnecki (2):
  dt-bindings: soc: document LiteX SoC Controller bindings
  drivers/soc/litex: add LiteX SoC Controller driver

 .../bindings/serial/litex,liteuart.yaml       |  38 ++
 .../soc/litex/litex,soc_controller.yaml       |  46 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   9 +
 drivers/soc/Kconfig                           |   1 +
 drivers/soc/Makefile                          |   1 +
 drivers/soc/litex/Kconfig                     |  14 +
 drivers/soc/litex/Makefile                    |   3 +
 drivers/soc/litex/litex_soc_ctrl.c            | 233 ++++++++++
 drivers/tty/serial/Kconfig                    |  32 +-
 drivers/tty/serial/Makefile                   |   1 +
 drivers/tty/serial/liteuart.c                 | 411 ++++++++++++++++++
 include/linux/litex.h                         |  45 ++
 include/uapi/linux/serial_core.h              |   3 +
 14 files changed, 838 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/serial/litex,liteuart.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
 create mode 100644 drivers/soc/litex/Kconfig
 create mode 100644 drivers/soc/litex/Makefile
 create mode 100644 drivers/soc/litex/litex_soc_ctrl.c
 create mode 100644 drivers/tty/serial/liteuart.c
 create mode 100644 include/linux/litex.h

-- 
2.25.0

[PATCH v3 1/5] dt-bindings: vendor: add vendor prefix for LiteX

[Mateusz Holenko]

From: Filip Kokosinski <fkokosinski@antmicro.com>

Add vendor prefix for LiteX SoC builder.

Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Notes:
    Changes in v3:
    - added Acked-by tag
    
    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 9e67944bec9c..d9107f0fed2e 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -549,6 +549,8 @@ patternProperties:
     description: Linux-specific binding
   "^linx,.*":
     description: Linx Technologies
+  "^litex,.*":
+    description: LiteX SoC builder
   "^lltc,.*":
     description: Linear Technology Corporation
   "^logicpd,.*":
-- 
2.25.0

[PATCH v3 2/5] dt-bindings: soc: document LiteX SoC Controller bindings

[Mateusz Holenko]

From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>

Add documentation for LiteX SoC Controller bindings.

Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---

Notes:
    This commit has been introduced in v3 of the patchset.

 .../soc/litex/litex,soc_controller.yaml       | 46 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml

diff --git a/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml b/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
new file mode 100644
index 000000000000..039894265319
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
@@ -0,0 +1,46 @@
+PDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/litex/litex,soc_controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LiteX SoC Controller driver
+
+description: |
+  This is the SoC Controller driver for the LiteX SoC Builder.
+  It's purpose is to verify LiteX CSR (Control&Status Register) access
+  operations and provide function for other drivers to read/write CSRs
+  and to check if those accessors are ready to use.
+
+maintainers:
+  - Karol Gugala <kgugala@antmicro.com>
+  - Mateusz Holenko <mholenko@antmicro.com>
+
+properties:
+  compatible:
+    const: litex,soc_controller
+
+  reg:
+    description: Base address and length of the register space
+
+  status:
+    description: |
+      disables or enables node
+
+    const: "okay"
+
+required:
+  - compatible
+  - reg
+  - status
+
+examples:
+  - |
+
+  soc_ctrl0: soc_controller@f0000000 {
+			compatible = "litex,soc_controller";
+			reg = <0x0 0xf0000000 0x0 0xC>;
+			status = "okay";
+  };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 4beb8dc4c7eb..ec925c081dd2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9725,6 +9725,12 @@ L:	kunit-dev@googlegroups.com
 S:	Maintained
 F:	lib/list-test.c
 
+LITEX PLATFORM
+M:	Karol Gugala <kgugala@antmicro.com>
+M:	Mateusz Holenko <mholenko@antmicro.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/*/litex,*.yaml
+
 LIVE PATCHING
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
 M:	Jiri Kosina <jikos@kernel.org>
-- 
2.25.0

[PATCH v3 3/5] drivers/soc/litex: add LiteX SoC Controller driver

[Mateusz Holenko]

From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>

This commit adds driver for the FPGA-based LiteX SoC
Controller from LiteX SoC builder.

Co-developed-by: Mateusz Holenko <mholenko@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
---

Notes:
    This commit has been introduced in v3 of the patchset.
    
    It includes a simplified version of common 'litex.h'
    header introduced in v2 of the patchset.

 MAINTAINERS                        |   2 +
 drivers/soc/Kconfig                |   1 +
 drivers/soc/Makefile               |   1 +
 drivers/soc/litex/Kconfig          |  14 ++
 drivers/soc/litex/Makefile         |   3 +
 drivers/soc/litex/litex_soc_ctrl.c | 233 +++++++++++++++++++++++++++++
 include/linux/litex.h              |  45 ++++++
 7 files changed, 299 insertions(+)
 create mode 100644 drivers/soc/litex/Kconfig
 create mode 100644 drivers/soc/litex/Makefile
 create mode 100644 drivers/soc/litex/litex_soc_ctrl.c
 create mode 100644 include/linux/litex.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ec925c081dd2..22a67514ace3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9730,6 +9730,8 @@ M:	Karol Gugala <kgugala@antmicro.com>
 M:	Mateusz Holenko <mholenko@antmicro.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/*/litex,*.yaml
+F:	drivers/soc/litex/litex_soc_ctrl.c
+F:	include/linux/litex.h
 
 LIVE PATCHING
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 1778f8c62861..78add2a163be 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/fsl/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
+source "drivers/soc/litex/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
 source "drivers/soc/qcom/Kconfig"
 source "drivers/soc/renesas/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 8b49d782a1ab..fd016b51cddd 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_ARCH_GEMINI)	+= gemini/
 obj-$(CONFIG_ARCH_MXC)		+= imx/
 obj-$(CONFIG_ARCH_IXP4XX)	+= ixp4xx/
 obj-$(CONFIG_SOC_XWAY)		+= lantiq/
+obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
 obj-y				+= mediatek/
 obj-y				+= amlogic/
 obj-y				+= qcom/
diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
new file mode 100644
index 000000000000..22c78cda0b83
--- /dev/null
+++ b/drivers/soc/litex/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License_Identifier: GPL-2.0
+
+menu "Enable LiteX SoC Builder specific drivers"
+
+config LITEX_SOC_CONTROLLER
+	tristate "Enable LiteX SoC Controller driver"
+	help
+	This option enables the SoC Controller Driver which verifies
+	LiteX CSR access and provides common litex_get_reg/litex_set_reg
+	accessors.
+	All drivers that use functions from litex.h must depend on
+	LITEX_SOC_CONTROLLER
+
+endmenu
diff --git a/drivers/soc/litex/Makefile b/drivers/soc/litex/Makefile
new file mode 100644
index 000000000000..98ff7325b1c0
--- /dev/null
+++ b/drivers/soc/litex/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License_Identifier: GPL-2.0
+
+obj-$(CONFIG_LITEX_SOC_CONTROLLER)	+= litex_soc_ctrl.o
diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
new file mode 100644
index 000000000000..b810782da3a5
--- /dev/null
+++ b/drivers/soc/litex/litex_soc_ctrl.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LiteX SoC Controller Driver
+ *
+ * Copyright (C) 2020 Antmicro
+ *
+ */
+
+#include <linux/litex.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+
+/*
+ * The parameters below are true for LiteX SoC
+ * configured for 8-bit CSR Bus, 32-bit aligned.
+ *
+ * Supporting other configurations will require
+ * extending the logic in this header.
+ */
+#define LITEX_REG_SIZE             0x4
+#define LITEX_SUBREG_SIZE          0x1
+#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)
+
+static DEFINE_SPINLOCK(csr_lock);
+
+static inline unsigned long read_pointer_with_barrier(
+	const volatile void __iomem *addr)
+{
+	unsigned long val;
+
+	__io_br();
+	val = *(const volatile unsigned long __force *)addr;
+	__io_ar();
+	return val;
+}
+
+static inline void write_pointer_with_barrier(
+	volatile void __iomem *addr, unsigned long val)
+{
+	__io_br();
+	*(volatile unsigned long __force *)addr = val;
+	__io_ar();
+}
+
+/*
+ * LiteX SoC Generator, depending on the configuration,
+ * can split a single logical CSR (Control & Status Register)
+ * into a series of consecutive physical registers.
+ *
+ * For example, in the configuration with 8-bit CSR Bus,
+ * 32-bit aligned (the default one for 32-bit CPUs) a 32-bit
+ * logical CSR will be generated as four 32-bit physical registers,
+ * each one containing one byte of meaningful data.
+ *
+ * For details see: https://github.com/enjoy-digital/litex/issues/314
+ *
+ * The purpose of `litex_set_reg`/`litex_get_reg` is to implement
+ * the logic of writing to/reading from the LiteX CSR in a single
+ * place that can be then reused by all LiteX drivers.
+ */
+void litex_set_reg(
+	void __iomem *reg, unsigned long reg_size, unsigned long val)
+{
+	unsigned long shifted_data, shift, i;
+	unsigned long flags;
+
+	spin_lock_irqsave(&csr_lock, flags);
+
+	for (i = 0; i < reg_size; ++i) {
+		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
+		shifted_data = val >> shift;
+		write_pointer_with_barrier(
+			reg + (LITEX_REG_SIZE * i), shifted_data);
+	}
+
+	spin_unlock_irqrestore(&csr_lock, flags);
+}
+
+unsigned long litex_get_reg(
+	void __iomem *reg, unsigned long reg_size)
+{
+	unsigned long shifted_data, shift, i;
+	unsigned long result = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&csr_lock, flags);
+
+	for (i = 0; i < reg_size; ++i) {
+		shifted_data = read_pointer_with_barrier(
+			reg + (LITEX_REG_SIZE * i));
+		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
+		result |= (shifted_data << shift);
+	}
+
+	spin_unlock_irqrestore(&csr_lock, flags);
+
+	return result;
+}
+
+static int accessors_ok;
+
+/*
+ * Check if accessors are safe to be used by other drivers
+ * returns true if yes - false if not
+ */
+int litex_check_accessors(void)
+{
+	return accessors_ok;
+}
+
+#define SCRATCH_REG_OFF         0x04
+#define SCRATCH_REG_SIZE        4
+#define SCRATCH_REG_VALUE       0x12345678
+#define SCRATCH_TEST_VALUE      0xdeadbeef
+
+/*
+ * Check LiteX CSR read/write access
+ *
+ * This function reads and writes a scratch register in order
+ * to verify if CSR access works.
+ *
+ * In case any problems are detected, the driver should panic
+ * and not set `accessors_ok` flag. As a result no other
+ * LiteX driver should access CSR bus.
+ *
+ * Access to the LiteX CSR is, by design, done in CPU native
+ * endianness. The driver should not dynamically configure
+ * access functions when the endianness mismatch is detected.
+ * Such situation indicates problems in the soft SoC design
+ * and should be solved at the LiteX generator level,
+ * not in the software.
+ */
+static int litex_check_csr_access(void __iomem *reg_addr)
+{
+	unsigned long reg;
+
+	reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
+
+	if (reg != SCRATCH_REG_VALUE) {
+		panic("Scratch register read error! Expected: 0x%x but got: 0x%lx",
+			SCRATCH_REG_VALUE, reg);
+		return -EINVAL;
+	}
+
+	litex_set_reg(reg_addr + SCRATCH_REG_OFF,
+		SCRATCH_REG_SIZE, SCRATCH_TEST_VALUE);
+	reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
+
+	if (reg != SCRATCH_TEST_VALUE) {
+		panic("Scratch register write error! Expected: 0x%x but got: 0x%lx",
+			SCRATCH_TEST_VALUE, reg);
+		return -EINVAL;
+	}
+
+	/* restore original value of the SCRATCH register */
+	litex_set_reg(reg_addr + SCRATCH_REG_OFF,
+		SCRATCH_REG_SIZE, SCRATCH_REG_VALUE);
+
+	/* Set flag for other drivers */
+	accessors_ok = 1;
+	pr_info("LiteX SoC Controller driver initialized");
+
+	return 0;
+}
+
+struct litex_soc_ctrl_device {
+	void __iomem *base;
+};
+
+static const struct of_device_id litex_soc_ctrl_of_match[] = {
+	{.compatible = "litex,soc_controller"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, litex_soc_ctrl_of_match);
+
+static int litex_soc_ctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev;
+	struct device_node *node;
+	const struct of_device_id *id;
+	struct litex_soc_ctrl_device *soc_ctrl_dev;
+	struct resource *res;
+
+	dev = &pdev->dev;
+	node = dev->of_node;
+	if (!node)
+		return -ENODEV;
+
+	id = of_match_node(litex_soc_ctrl_of_match, node);
+	if (!id)
+		return -ENODEV;
+
+	soc_ctrl_dev = devm_kzalloc(dev, sizeof(*soc_ctrl_dev), GFP_KERNEL);
+	if (IS_ERR_OR_NULL(soc_ctrl_dev))
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (IS_ERR_OR_NULL(res))
+		return -EBUSY;
+
+	soc_ctrl_dev->base = devm_of_iomap(dev, node, 0, &res->end);
+	if (IS_ERR_OR_NULL(soc_ctrl_dev->base))
+		return -EIO;
+
+	return litex_check_csr_access(soc_ctrl_dev->base);
+}
+
+static int litex_soc_ctrl_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver litex_soc_ctrl_driver = {
+	.driver = {
+		.name = "litex-soc-controller",
+		.of_match_table = of_match_ptr(litex_soc_ctrl_of_match)
+	},
+	.probe = litex_soc_ctrl_probe,
+	.remove = litex_soc_ctrl_remove
+};
+
+module_platform_driver(litex_soc_ctrl_driver);
+MODULE_DESCRIPTION("LiteX SoC Controller driver");
+MODULE_AUTHOR("Antmicro <www.antmicro.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/litex.h b/include/linux/litex.h
new file mode 100644
index 000000000000..44ec415abc85
--- /dev/null
+++ b/include/linux/litex.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common LiteX header providing
+ * helper functions for accessing CSRs.
+ *
+ * Implementation of the functions is provided by
+ * the LiteX SoC Controller driver.
+ *
+ * Copyright (C) 2019 - 2020 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>
+
+/*
+ * litex_check_accessors is a function implemented in
+ * drivers/soc/litex/litex_soc_controller.c
+ * checking if the common LiteX CSR accessors
+ * are safe to be used by the drivers;
+ * returns true (1) if yes - false (0) if not
+ *
+ * Important: All drivers that use litex_set_reg/litex_get_reg
+ * functions should make sure that LiteX SoC Controller driver
+ * has verified LiteX CSRs read and write operations before
+ * issuing any read/writes to the LiteX peripherals.
+ *
+ * Exemplary snippet that can be used at the beginning
+ * of the driver's probe() function to ensure that LiteX
+ * SoC Controller driver is properely initialized:
+ *
+ * if (!litex_check_accessors())
+ *     return -EPROBE_DEFER;
+ */
+int litex_check_accessors(void);
+
+void litex_set_reg(void __iomem *reg, unsigned long reg_sz, unsigned long val);
+
+unsigned long litex_get_reg(void __iomem *reg, unsigned long reg_sz);
+
+
+#endif /* _LINUX_LITEX_H */
-- 
2.25.0

[PATCH v3 4/5] dt-bindings: serial: document LiteUART bindings

[Mateusz Holenko]

From: Filip Kokosinski <fkokosinski@antmicro.com>

Add documentation for LiteUART devicetree bindings.

Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Notes:
    Changes in v3:
    - added Reviewed-by tag
    - patch number changed from 3 to 4
    - removed changes in MAINTAINERS file (moved to patch #2)
    
    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 +++++++++++++++++++
 1 file changed, 38 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>;
+    };
-- 
2.25.0

[PATCH v3 5/5] drivers/tty/serial: add LiteUART driver

[Mateusz Holenko]

From: Filip Kokosinski <fkokosinski@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@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---

Notes:
    Changes in v3:
    - aliases made optional
    - used litex_get_reg/litex_set_reg functions instead of macros
    - SERIAL_LITEUART_NR_PORTS renamed to SERIAL_LITEUART_MAX_PORTS
    - PORT_LITEUART changed from 122 to 123
    - added dependency on LITEX_SOC_CONTROLLER
    - patch number changed from 4 to 5
    
    No changes in v2.

 MAINTAINERS                      |   1 +
 drivers/tty/serial/Kconfig       |  32 ++-
 drivers/tty/serial/Makefile      |   1 +
 drivers/tty/serial/liteuart.c    | 411 +++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |   3 +
 5 files changed, 447 insertions(+), 1 deletion(-)
 create mode 100644 drivers/tty/serial/liteuart.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 22a67514ace3..9b294f083640 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9732,6 +9732,7 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/*/litex,*.yaml
 F:	drivers/soc/litex/litex_soc_ctrl.c
 F:	include/linux/litex.h
+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 52eaac21ff9f..577c088b9feb 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -529,7 +529,7 @@ config SERIAL_IMX_CONSOLE
 
 config SERIAL_UARTLITE
 	tristate "Xilinx uartlite serial port support"
-	depends on HAS_IOMEM
+	depends on HAS_IOMEM && LITEX_SOC_CONTROLLER
 	select SERIAL_CORE
 	help
 	  Say Y here if you want to use the Xilinx uartlite serial controller.
@@ -1572,6 +1572,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_MAX_PORTS
+	int "Maximum number of LiteUART ports"
+	depends on SERIAL_LITEUART
+	default "1"
+	help
+	  Set this to the maximum 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 d056ee6cca33..9f8ba419ff3b 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..184ecb9f51f3
--- /dev/null
+++ b/drivers/tty/serial/liteuart.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LiteUART serial controller (LiteX) Driver
+ *
+ * Copyright (C) 2019 - 2020 Antmicro <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"
+
+/*
+ * CSRs definitions
+ * (base address offsets + width)
+ *
+ * The definitions below are true for
+ * LiteX SoC configured for
+ * 8-bit CSR Bus, 32-bit aligned.
+ *
+ * Supporting other configurations
+ * might require new definitions
+ * or a more generic way of indexing
+ * the LiteX CSRs.
+ *
+ * For more details on how CSRs
+ * are defined and handled in LiteX,
+ * see comments in the LiteX SoC Driver:
+ * drivers/soc/litex/litex_soc_ctrl.c
+ */
+#define OFF_RXTX	0x00
+#define SIZE_RXTX	1
+#define OFF_TXFULL	0x04
+#define SIZE_TXFULL	1
+#define OFF_RXEMPTY	0x08
+#define SIZE_RXEMPTY	1
+#define OFF_EV_STATUS	0x0c
+#define SIZE_EV_STATUS	1
+#define OFF_EV_PENDING	0x10
+#define SIZE_EV_PENDING	1
+#define OFF_EV_ENABLE	0x14
+#define SIZE_EV_ENABLE	1
+
+/* 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_MAX_PORTS];
+static DECLARE_BITMAP(liteuart_ports_in_use, CONFIG_SERIAL_LITEUART_MAX_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_MAX_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 long status;
+
+	while ((status = !litex_get_reg(membase + OFF_RXEMPTY,
+			SIZE_RXEMPTY)) == 1) {
+		ch = litex_get_reg(membase + OFF_RXTX, SIZE_RXTX);
+		port->icount.rx++;
+
+		/* necessary for RXEMPTY to refresh its value */
+		litex_set_reg(membase + OFF_EV_PENDING,
+			SIZE_EV_PENDING, EV_TX | EV_RX);
+
+		/* 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_get_reg(port->membase + OFF_TXFULL, SIZE_TXFULL))
+		cpu_relax();
+
+	litex_set_reg(port->membase + OFF_RXTX, SIZE_RXTX, ch);
+}
+
+static unsigned int liteuart_tx_empty(struct uart_port *port)
+{
+	/* not really tx empty, just checking if tx is not full */
+	if (!litex_get_reg(port->membase + OFF_TXFULL, SIZE_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_set_reg(port->membase + OFF_RXTX,
+			SIZE_RXTX, port->x_char);
+		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_set_reg(port->membase + OFF_EV_ENABLE, SIZE_EV_ENABLE, 0);
+
+	/* 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;
+
+	if (!litex_check_accessors())
+		return -EPROBE_DEFER;
+
+	/* no device tree */
+	if (!np)
+		return -ENODEV;
+
+	/* look for aliases; auto-enumerate for free index if not found */
+	dev_id = of_alias_get_id(np, "serial");
+	if (dev_id < 0)
+		dev_id = find_first_zero_bit(liteuart_ports_in_use,
+					     CONFIG_SERIAL_LITEUART_MAX_PORTS);
+
+	if (dev_id >= CONFIG_SERIAL_LITEUART_MAX_PORTS)
+		return -ENODEV;
+
+	if (test_and_set_bit(dev_id, liteuart_ports_in_use))
+		return -EBUSY;
+
+	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 <www.antmicro.com>");
+MODULE_DESCRIPTION("LiteUART serial driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 8ec3dd742ea4..449b8fe9273c 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -293,4 +293,7 @@
 /* Freescale LINFlexD UART */
 #define PORT_LINFLEXUART	122
 
+/* LiteUART */
+#define PORT_LITEUART	123
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
-- 
2.25.0

Re: [PATCH v3 2/5] dt-bindings: soc: document LiteX SoC Controller bindings

[Maxime Ripard]

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

Hi Mateusz,

On Tue, Feb 25, 2020 at 09:46:45AM +0100, Mateusz Holenko wrote:
> From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
>
> Add documentation for LiteX SoC Controller bindings.
>
> Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
>
> Notes:
>     This commit has been introduced in v3 of the patchset.
>
>  .../soc/litex/litex,soc_controller.yaml       | 46 +++++++++++++++++++
>  MAINTAINERS                                   |  6 +++
>  2 files changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml b/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> new file mode 100644
> index 000000000000..039894265319
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> @@ -0,0 +1,46 @@
> +PDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/litex/litex,soc_controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LiteX SoC Controller driver
> +
> +description: |
> +  This is the SoC Controller driver for the LiteX SoC Builder.
> +  It's purpose is to verify LiteX CSR (Control&Status Register) access
> +  operations and provide function for other drivers to read/write CSRs
> +  and to check if those accessors are ready to use.
> +
> +maintainers:
> +  - Karol Gugala <kgugala@antmicro.com>
> +  - Mateusz Holenko <mholenko@antmicro.com>
> +
> +properties:
> +  compatible:
> +    const: litex,soc_controller

Usually compatible will use dash as separators, not underscores.

> +  reg:
> +    description: Base address and length of the register space

This is usually removed since it's what's expected from the property
anyway. However, what you should really test for in the number of
address/size couples being set, and you can do that using maxItems: 1

> +  status:
> +    description: |
> +      disables or enables node
> +
> +    const: "okay"

This is added automatically by the tooling, so you can leave it out.

> +required:
> +  - compatible
> +  - reg
> +  - status

And in general, status is not required. Leaving status out is
equivalent to status = "okay"

> +examples:
> +  - |
> +
> +  soc_ctrl0: soc_controller@f0000000 {
> +			compatible = "litex,soc_controller";
> +			reg = <0x0 0xf0000000 0x0 0xC>;
> +			status = "okay";
> +  };

The indentation looks weird here?

Maxime

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

Re: [PATCH v3 3/5] drivers/soc/litex: add LiteX SoC Controller driver

[Maxime Ripard]

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

On Tue, Feb 25, 2020 at 09:46:57AM +0100, Mateusz Holenko wrote:
> From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
>
> This commit adds driver for the FPGA-based LiteX SoC
> Controller from LiteX SoC builder.
>
> Co-developed-by: Mateusz Holenko <mholenko@antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> ---
>
> Notes:
>     This commit has been introduced in v3 of the patchset.
>
>     It includes a simplified version of common 'litex.h'
>     header introduced in v2 of the patchset.
>
>  MAINTAINERS                        |   2 +
>  drivers/soc/Kconfig                |   1 +
>  drivers/soc/Makefile               |   1 +
>  drivers/soc/litex/Kconfig          |  14 ++
>  drivers/soc/litex/Makefile         |   3 +
>  drivers/soc/litex/litex_soc_ctrl.c | 233 +++++++++++++++++++++++++++++
>  include/linux/litex.h              |  45 ++++++
>  7 files changed, 299 insertions(+)
>  create mode 100644 drivers/soc/litex/Kconfig
>  create mode 100644 drivers/soc/litex/Makefile
>  create mode 100644 drivers/soc/litex/litex_soc_ctrl.c
>  create mode 100644 include/linux/litex.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ec925c081dd2..22a67514ace3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9730,6 +9730,8 @@ M:	Karol Gugala <kgugala@antmicro.com>
>  M:	Mateusz Holenko <mholenko@antmicro.com>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/*/litex,*.yaml
> +F:	drivers/soc/litex/litex_soc_ctrl.c
> +F:	include/linux/litex.h
>
>  LIVE PATCHING
>  M:	Josh Poimboeuf <jpoimboe@redhat.com>
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 1778f8c62861..78add2a163be 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig"
>  source "drivers/soc/fsl/Kconfig"
>  source "drivers/soc/imx/Kconfig"
>  source "drivers/soc/ixp4xx/Kconfig"
> +source "drivers/soc/litex/Kconfig"
>  source "drivers/soc/mediatek/Kconfig"
>  source "drivers/soc/qcom/Kconfig"
>  source "drivers/soc/renesas/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 8b49d782a1ab..fd016b51cddd 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_ARCH_GEMINI)	+= gemini/
>  obj-$(CONFIG_ARCH_MXC)		+= imx/
>  obj-$(CONFIG_ARCH_IXP4XX)	+= ixp4xx/
>  obj-$(CONFIG_SOC_XWAY)		+= lantiq/
> +obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
>  obj-y				+= mediatek/
>  obj-y				+= amlogic/
>  obj-y				+= qcom/
> diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
> new file mode 100644
> index 000000000000..22c78cda0b83
> --- /dev/null
> +++ b/drivers/soc/litex/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License_Identifier: GPL-2.0
> +
> +menu "Enable LiteX SoC Builder specific drivers"
> +
> +config LITEX_SOC_CONTROLLER
> +	tristate "Enable LiteX SoC Controller driver"
> +	help
> +	This option enables the SoC Controller Driver which verifies
> +	LiteX CSR access and provides common litex_get_reg/litex_set_reg
> +	accessors.
> +	All drivers that use functions from litex.h must depend on
> +	LITEX_SOC_CONTROLLER
> +
> +endmenu
> diff --git a/drivers/soc/litex/Makefile b/drivers/soc/litex/Makefile
> new file mode 100644
> index 000000000000..98ff7325b1c0
> --- /dev/null
> +++ b/drivers/soc/litex/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License_Identifier: GPL-2.0
> +
> +obj-$(CONFIG_LITEX_SOC_CONTROLLER)	+= litex_soc_ctrl.o
> diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
> new file mode 100644
> index 000000000000..b810782da3a5
> --- /dev/null
> +++ b/drivers/soc/litex/litex_soc_ctrl.c
> @@ -0,0 +1,233 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LiteX SoC Controller Driver
> + *
> + * Copyright (C) 2020 Antmicro
> + *
> + */
> +
> +#include <linux/litex.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +
> +/*
> + * The parameters below are true for LiteX SoC
> + * configured for 8-bit CSR Bus, 32-bit aligned.
> + *
> + * Supporting other configurations will require
> + * extending the logic in this header.
> + */
> +#define LITEX_REG_SIZE             0x4
> +#define LITEX_SUBREG_SIZE          0x1
> +#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)
> +
> +static DEFINE_SPINLOCK(csr_lock);
> +
> +static inline unsigned long read_pointer_with_barrier(
> +	const volatile void __iomem *addr)
> +{
> +	unsigned long val;
> +
> +	__io_br();
> +	val = *(const volatile unsigned long __force *)addr;
> +	__io_ar();
> +	return val;
> +}
> +
> +static inline void write_pointer_with_barrier(
> +	volatile void __iomem *addr, unsigned long val)
> +{
> +	__io_br();
> +	*(volatile unsigned long __force *)addr = val;
> +	__io_ar();
> +}

What's wrong with readl/writel ?

> +/*
> + * LiteX SoC Generator, depending on the configuration,
> + * can split a single logical CSR (Control & Status Register)
> + * into a series of consecutive physical registers.
> + *
> + * For example, in the configuration with 8-bit CSR Bus,
> + * 32-bit aligned (the default one for 32-bit CPUs) a 32-bit
> + * logical CSR will be generated as four 32-bit physical registers,
> + * each one containing one byte of meaningful data.
> + *
> + * For details see: https://github.com/enjoy-digital/litex/issues/314
> + *
> + * The purpose of `litex_set_reg`/`litex_get_reg` is to implement
> + * the logic of writing to/reading from the LiteX CSR in a single
> + * place that can be then reused by all LiteX drivers.
> + */
> +void litex_set_reg(
> +	void __iomem *reg, unsigned long reg_size, unsigned long val)
> +{
> +	unsigned long shifted_data, shift, i;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&csr_lock, flags);
> +
> +	for (i = 0; i < reg_size; ++i) {
> +		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> +		shifted_data = val >> shift;
> +		write_pointer_with_barrier(
> +			reg + (LITEX_REG_SIZE * i), shifted_data);
> +	}
> +
> +	spin_unlock_irqrestore(&csr_lock, flags);
> +}
> +
> +unsigned long litex_get_reg(
> +	void __iomem *reg, unsigned long reg_size)
> +{
> +	unsigned long shifted_data, shift, i;
> +	unsigned long result = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&csr_lock, flags);
> +
> +	for (i = 0; i < reg_size; ++i) {
> +		shifted_data = read_pointer_with_barrier(
> +			reg + (LITEX_REG_SIZE * i));
> +		shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> +		result |= (shifted_data << shift);
> +	}
> +
> +	spin_unlock_irqrestore(&csr_lock, flags);
> +
> +	return result;
> +}

I'm not sure what's supposed to be in that register, but usually it
will either be abstracted away through a framework since letting each
and every driver poke into the same register is not really ideal, or
if you really have to do it, using a syscon.

> +static int accessors_ok;
> +
> +/*
> + * Check if accessors are safe to be used by other drivers
> + * returns true if yes - false if not
> + */
> +int litex_check_accessors(void)
> +{
> +	return accessors_ok;
> +}
> +
> +#define SCRATCH_REG_OFF         0x04
> +#define SCRATCH_REG_SIZE        4
> +#define SCRATCH_REG_VALUE       0x12345678
> +#define SCRATCH_TEST_VALUE      0xdeadbeef
> +
> +/*
> + * Check LiteX CSR read/write access
> + *
> + * This function reads and writes a scratch register in order
> + * to verify if CSR access works.
> + *
> + * In case any problems are detected, the driver should panic
> + * and not set `accessors_ok` flag. As a result no other
> + * LiteX driver should access CSR bus.
> + *
> + * Access to the LiteX CSR is, by design, done in CPU native
> + * endianness. The driver should not dynamically configure
> + * access functions when the endianness mismatch is detected.
> + * Such situation indicates problems in the soft SoC design
> + * and should be solved at the LiteX generator level,
> + * not in the software.

I guess that's why you don't use readl/writel. Then
__raw_readl/__raw_writel?

> + */
> +static int litex_check_csr_access(void __iomem *reg_addr)
> +{
> +	unsigned long reg;
> +
> +	reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
> +
> +	if (reg != SCRATCH_REG_VALUE) {
> +		panic("Scratch register read error! Expected: 0x%x but got: 0x%lx",
> +			SCRATCH_REG_VALUE, reg);
> +		return -EINVAL;
> +	}
> +
> +	litex_set_reg(reg_addr + SCRATCH_REG_OFF,
> +		SCRATCH_REG_SIZE, SCRATCH_TEST_VALUE);
> +	reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
> +
> +	if (reg != SCRATCH_TEST_VALUE) {
> +		panic("Scratch register write error! Expected: 0x%x but got: 0x%lx",
> +			SCRATCH_TEST_VALUE, reg);
> +		return -EINVAL;
> +	}
> +
> +	/* restore original value of the SCRATCH register */
> +	litex_set_reg(reg_addr + SCRATCH_REG_OFF,
> +		SCRATCH_REG_SIZE, SCRATCH_REG_VALUE);
> +
> +	/* Set flag for other drivers */
> +	accessors_ok = 1;
> +	pr_info("LiteX SoC Controller driver initialized");
> +
> +	return 0;
> +}
> +
> +struct litex_soc_ctrl_device {
> +	void __iomem *base;
> +};
> +
> +static const struct of_device_id litex_soc_ctrl_of_match[] = {
> +	{.compatible = "litex,soc_controller"},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, litex_soc_ctrl_of_match);
> +
> +static int litex_soc_ctrl_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct device_node *node;
> +	const struct of_device_id *id;
> +	struct litex_soc_ctrl_device *soc_ctrl_dev;
> +	struct resource *res;
> +
> +	dev = &pdev->dev;
> +	node = dev->of_node;
> +	if (!node)
> +		return -ENODEV;
> +
> +	id = of_match_node(litex_soc_ctrl_of_match, node);
> +	if (!id)
> +		return -ENODEV;

That's pretty much guaranteed.

> +	soc_ctrl_dev = devm_kzalloc(dev, sizeof(*soc_ctrl_dev), GFP_KERNEL);
> +	if (IS_ERR_OR_NULL(soc_ctrl_dev))
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (IS_ERR_OR_NULL(res))
> +		return -EBUSY;
> +
> +	soc_ctrl_dev->base = devm_of_iomap(dev, node, 0, &res->end);
> +	if (IS_ERR_OR_NULL(soc_ctrl_dev->base))
> +		return -EIO;

devm_platform_ioremap_resource ?

> +	return litex_check_csr_access(soc_ctrl_dev->base);
> +}
> +
> +static int litex_soc_ctrl_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}

You can leave the remove hook out entirely if you don't have anything
in it.

Maxime

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

Re: [PATCH v3 5/5] drivers/tty/serial: add LiteUART driver

[Mateusz Holenko]

On Tue, Feb 25, 2020 at 9:47 AM Mateusz Holenko <mholenko@antmicro.com> wrote:
>
> From: Filip Kokosinski <fkokosinski@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@antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
>
> Notes:
>     Changes in v3:
>     - aliases made optional
>     - used litex_get_reg/litex_set_reg functions instead of macros
>     - SERIAL_LITEUART_NR_PORTS renamed to SERIAL_LITEUART_MAX_PORTS
>     - PORT_LITEUART changed from 122 to 123
>     - added dependency on LITEX_SOC_CONTROLLER
>     - patch number changed from 4 to 5
>
>     No changes in v2.
>
>  MAINTAINERS                      |   1 +
>  drivers/tty/serial/Kconfig       |  32 ++-
>  drivers/tty/serial/Makefile      |   1 +
>  drivers/tty/serial/liteuart.c    | 411 +++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |   3 +
>  5 files changed, 447 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/tty/serial/liteuart.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 22a67514ace3..9b294f083640 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9732,6 +9732,7 @@ S:        Maintained
>  F:     Documentation/devicetree/bindings/*/litex,*.yaml
>  F:     drivers/soc/litex/litex_soc_ctrl.c
>  F:     include/linux/litex.h
> +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 52eaac21ff9f..577c088b9feb 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -529,7 +529,7 @@ config SERIAL_IMX_CONSOLE
>
>  config SERIAL_UARTLITE
>         tristate "Xilinx uartlite serial port support"
> -       depends on HAS_IOMEM
> +       depends on HAS_IOMEM && LITEX_SOC_CONTROLLER

Just noticed this - it's wrong. We don't want to change Xilinx UARTLITE config.
It's SERIAL_LITEUART that should be dependent on LITEX_SOC_CONTROLLER.

>         select SERIAL_CORE
>         help
>           Say Y here if you want to use the Xilinx uartlite serial controller.
> @@ -1572,6 +1572,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

This should also depend on LITEX_SOC_CONTROLLER.

> +       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_MAX_PORTS
> +       int "Maximum number of LiteUART ports"
> +       depends on SERIAL_LITEUART
> +       default "1"
> +       help
> +         Set this to the maximum 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 d056ee6cca33..9f8ba419ff3b 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..184ecb9f51f3
> --- /dev/null
> +++ b/drivers/tty/serial/liteuart.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LiteUART serial controller (LiteX) Driver
> + *
> + * Copyright (C) 2019 - 2020 Antmicro <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"
> +
> +/*
> + * CSRs definitions
> + * (base address offsets + width)
> + *
> + * The definitions below are true for
> + * LiteX SoC configured for
> + * 8-bit CSR Bus, 32-bit aligned.
> + *
> + * Supporting other configurations
> + * might require new definitions
> + * or a more generic way of indexing
> + * the LiteX CSRs.
> + *
> + * For more details on how CSRs
> + * are defined and handled in LiteX,
> + * see comments in the LiteX SoC Driver:
> + * drivers/soc/litex/litex_soc_ctrl.c
> + */
> +#define OFF_RXTX       0x00
> +#define SIZE_RXTX      1
> +#define OFF_TXFULL     0x04
> +#define SIZE_TXFULL    1
> +#define OFF_RXEMPTY    0x08
> +#define SIZE_RXEMPTY   1
> +#define OFF_EV_STATUS  0x0c
> +#define SIZE_EV_STATUS 1
> +#define OFF_EV_PENDING 0x10
> +#define SIZE_EV_PENDING        1
> +#define OFF_EV_ENABLE  0x14
> +#define SIZE_EV_ENABLE 1
> +
> +/* 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_MAX_PORTS];
> +static DECLARE_BITMAP(liteuart_ports_in_use, CONFIG_SERIAL_LITEUART_MAX_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_MAX_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 long status;
> +
> +       while ((status = !litex_get_reg(membase + OFF_RXEMPTY,
> +                       SIZE_RXEMPTY)) == 1) {
> +               ch = litex_get_reg(membase + OFF_RXTX, SIZE_RXTX);
> +               port->icount.rx++;
> +
> +               /* necessary for RXEMPTY to refresh its value */
> +               litex_set_reg(membase + OFF_EV_PENDING,
> +                       SIZE_EV_PENDING, EV_TX | EV_RX);
> +
> +               /* 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_get_reg(port->membase + OFF_TXFULL, SIZE_TXFULL))
> +               cpu_relax();
> +
> +       litex_set_reg(port->membase + OFF_RXTX, SIZE_RXTX, ch);
> +}
> +
> +static unsigned int liteuart_tx_empty(struct uart_port *port)
> +{
> +       /* not really tx empty, just checking if tx is not full */
> +       if (!litex_get_reg(port->membase + OFF_TXFULL, SIZE_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_set_reg(port->membase + OFF_RXTX,
> +                       SIZE_RXTX, port->x_char);
> +               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_set_reg(port->membase + OFF_EV_ENABLE, SIZE_EV_ENABLE, 0);
> +
> +       /* 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;
> +
> +       if (!litex_check_accessors())
> +               return -EPROBE_DEFER;
> +
> +       /* no device tree */
> +       if (!np)
> +               return -ENODEV;
> +
> +       /* look for aliases; auto-enumerate for free index if not found */
> +       dev_id = of_alias_get_id(np, "serial");
> +       if (dev_id < 0)
> +               dev_id = find_first_zero_bit(liteuart_ports_in_use,
> +                                            CONFIG_SERIAL_LITEUART_MAX_PORTS);
> +
> +       if (dev_id >= CONFIG_SERIAL_LITEUART_MAX_PORTS)
> +               return -ENODEV;
> +
> +       if (test_and_set_bit(dev_id, liteuart_ports_in_use))
> +               return -EBUSY;
> +
> +       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 <www.antmicro.com>");
> +MODULE_DESCRIPTION("LiteUART serial driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 8ec3dd742ea4..449b8fe9273c 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -293,4 +293,7 @@
>  /* Freescale LINFlexD UART */
>  #define PORT_LINFLEXUART       122
>
> +/* LiteUART */
> +#define PORT_LITEUART  123
> +
>  #endif /* _UAPILINUX_SERIAL_CORE_H */
> --
> 2.25.0
>


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

Re: [PATCH v3 3/5] drivers/soc/litex: add LiteX SoC Controller driver

[Randy Dunlap]

On 2/25/20 12:46 AM, Mateusz Holenko wrote:
> diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
> new file mode 100644
> index 000000000000..22c78cda0b83
> --- /dev/null
> +++ b/drivers/soc/litex/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License_Identifier: GPL-2.0
> +
> +menu "Enable LiteX SoC Builder specific drivers"
> +
> +config LITEX_SOC_CONTROLLER
> +	tristate "Enable LiteX SoC Controller driver"
> +	help
> +	This option enables the SoC Controller Driver which verifies
> +	LiteX CSR access and provides common litex_get_reg/litex_set_reg
> +	accessors.
> +	All drivers that use functions from litex.h must depend on
> +	LITEX_SOC_CONTROLLER

Hi,
Please indent the help text with 2 additional spaces, as explained in the
coding-style.rst file:

10) Kconfig configuration files
-------------------------------

For all of the Kconfig* configuration files throughout the source tree,
the indentation is somewhat different.  Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces.  Example::

  config AUDIT
	bool "Auditing support"
	depends on NET
	help
	  Enable auditing infrastructure that can be used with another
	  kernel subsystem, such as SELinux (which requires this for
	  logging of avc messages output).  Does not do system-call
	  auditing without CONFIG_AUDITSYSCALL.

> +
> +endmenu

and then end the last line of the help text with a period ('.').

thanks.

-- 
~Randy

Re: [PATCH v3 2/5] dt-bindings: soc: document LiteX SoC Controller bindings

[Rob Herring]

On Tue, 25 Feb 2020 09:46:45 +0100, Mateusz Holenko wrote:
> From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> 
> Add documentation for LiteX SoC Controller bindings.
> 
> Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> 
> Notes:
>     This commit has been introduced in v3 of the patchset.
> 
>  .../soc/litex/litex,soc_controller.yaml       | 46 +++++++++++++++++++
>  MAINTAINERS                                   |  6 +++
>  2 files changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

warning: no schema found in file: Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml: ignoring, error parsing file
Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
Traceback (most recent call last):
  File "/usr/local/bin/dt-doc-validate", line 35, in check_doc
    testtree = dtschema.load(filename, line_number=line_number, duplicate_keys=False)
  File "/usr/local/lib/python3.6/dist-packages/dtschema/lib.py", line 513, in load
    return yaml.load(f.read())
  File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/main.py", line 343, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 718, in _ruamel_yaml.CParser.get_single_node
ruamel.yaml.composer.ComposerError: expected a single document in the stream
  in "<unicode string>", line 1, column 1
but found another document
  in "<unicode string>", line 2, column 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/dt-doc-validate", line 74, in <module>
    ret = check_doc(args.yamldt)
  File "/usr/local/bin/dt-doc-validate", line 40, in check_doc
    print(filename + ":", exc.path[-1], exc.message)
AttributeError: 'ComposerError' object has no attribute 'path'
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/soc/litex/litex,soc_controller.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/soc/litex/litex,soc_controller.example.dts] Error 1
Makefile:1263: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1243930
Please check and re-submit.

Re: [PATCH v3 3/5] drivers/soc/litex: add LiteX SoC Controller driver

[Mateusz Holenko]

On Tue, Feb 25, 2020 at 5:16 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 2/25/20 12:46 AM, Mateusz Holenko wrote:
> > diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
> > new file mode 100644
> > index 000000000000..22c78cda0b83
> > --- /dev/null
> > +++ b/drivers/soc/litex/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License_Identifier: GPL-2.0
> > +
> > +menu "Enable LiteX SoC Builder specific drivers"
> > +
> > +config LITEX_SOC_CONTROLLER
> > +     tristate "Enable LiteX SoC Controller driver"
> > +     help
> > +     This option enables the SoC Controller Driver which verifies
> > +     LiteX CSR access and provides common litex_get_reg/litex_set_reg
> > +     accessors.
> > +     All drivers that use functions from litex.h must depend on
> > +     LITEX_SOC_CONTROLLER
>
> Hi,
> Please indent the help text with 2 additional spaces, as explained in the
> coding-style.rst file:
>
> 10) Kconfig configuration files
> -------------------------------
>
> For all of the Kconfig* configuration files throughout the source tree,
> the indentation is somewhat different.  Lines under a ``config`` definition
> are indented with one tab, while help text is indented an additional two
> spaces.  Example::
>
>   config AUDIT
>         bool "Auditing support"
>         depends on NET
>         help
>           Enable auditing infrastructure that can be used with another
>           kernel subsystem, such as SELinux (which requires this for
>           logging of avc messages output).  Does not do system-call
>           auditing without CONFIG_AUDITSYSCALL.
>
> > +
> > +endmenu
>
> and then end the last line of the help text with a period ('.').
>
> thanks.
>
> --
> ~Randy
>

Thanks for the comments!

I'll fix the documentation and resubmit the patch after addressing all
other remarks.

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

Re: [PATCH v3 2/5] dt-bindings: soc: document LiteX SoC Controller bindings

[Mateusz Holenko]

Hi Maxime,

On Tue, Feb 25, 2020 at 10:11 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Mateusz,
>
> On Tue, Feb 25, 2020 at 09:46:45AM +0100, Mateusz Holenko wrote:
> > From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> >
> > Add documentation for LiteX SoC Controller bindings.
> >
> > Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> > Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> > ---
> >
> > Notes:
> >     This commit has been introduced in v3 of the patchset.
> >
> >  .../soc/litex/litex,soc_controller.yaml       | 46 +++++++++++++++++++
> >  MAINTAINERS                                   |  6 +++
> >  2 files changed, 52 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml b/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> > new file mode 100644
> > index 000000000000..039894265319
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> > @@ -0,0 +1,46 @@
> > +PDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/soc/litex/litex,soc_controller.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: LiteX SoC Controller driver
> > +
> > +description: |
> > +  This is the SoC Controller driver for the LiteX SoC Builder.
> > +  It's purpose is to verify LiteX CSR (Control&Status Register) access
> > +  operations and provide function for other drivers to read/write CSRs
> > +  and to check if those accessors are ready to use.
> > +
> > +maintainers:
> > +  - Karol Gugala <kgugala@antmicro.com>
> > +  - Mateusz Holenko <mholenko@antmicro.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: litex,soc_controller
>
> Usually compatible will use dash as separators, not underscores.
>
> > +  reg:
> > +    description: Base address and length of the register space
>
> This is usually removed since it's what's expected from the property
> anyway. However, what you should really test for in the number of
> address/size couples being set, and you can do that using maxItems: 1
>
> > +  status:
> > +    description: |
> > +      disables or enables node
> > +
> > +    const: "okay"
>
> This is added automatically by the tooling, so you can leave it out.
>
> > +required:
> > +  - compatible
> > +  - reg
> > +  - status
>
> And in general, status is not required. Leaving status out is
> equivalent to status = "okay"
>
> > +examples:
> > +  - |
> > +
> > +  soc_ctrl0: soc_controller@f0000000 {
> > +                     compatible = "litex,soc_controller";
> > +                     reg = <0x0 0xf0000000 0x0 0xC>;
> > +                     status = "okay";
> > +  };
>
> The indentation looks weird here?
>
> Maxime

Thanks for all the comments!

I'll fix this file and resubmit the whole patchset after addressing
other remarks.

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

Re: [PATCH v3 2/5] dt-bindings: soc: document LiteX SoC Controller bindings

[Mateusz Holenko]

On Tue, Feb 25, 2020 at 5:57 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, 25 Feb 2020 09:46:45 +0100, Mateusz Holenko wrote:
> > From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> >
> > Add documentation for LiteX SoC Controller bindings.
> >
> > Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> > Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> > ---
> >
> > Notes:
> >     This commit has been introduced in v3 of the patchset.
> >
> >  .../soc/litex/litex,soc_controller.yaml       | 46 +++++++++++++++++++
> >  MAINTAINERS                                   |  6 +++
> >  2 files changed, 52 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> warning: no schema found in file: Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/litex/litex,soc_controller.yaml: ignoring, error parsing file
> Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
> Traceback (most recent call last):
>   File "/usr/local/bin/dt-doc-validate", line 35, in check_doc
>     testtree = dtschema.load(filename, line_number=line_number, duplicate_keys=False)
>   File "/usr/local/lib/python3.6/dist-packages/dtschema/lib.py", line 513, in load
>     return yaml.load(f.read())
>   File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/main.py", line 343, in load
>     return constructor.get_single_data()
>   File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
>     node = self.composer.get_single_node()
>   File "_ruamel_yaml.pyx", line 718, in _ruamel_yaml.CParser.get_single_node
> ruamel.yaml.composer.ComposerError: expected a single document in the stream
>   in "<unicode string>", line 1, column 1
> but found another document
>   in "<unicode string>", line 2, column 1
>
> During handling of the above exception, another exception occurred:
>
> Traceback (most recent call last):
>   File "/usr/local/bin/dt-doc-validate", line 74, in <module>
>     ret = check_doc(args.yamldt)
>   File "/usr/local/bin/dt-doc-validate", line 40, in check_doc
>     print(filename + ":", exc.path[-1], exc.message)
> AttributeError: 'ComposerError' object has no attribute 'path'
> Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/soc/litex/litex,soc_controller.example.dts' failed
> make[1]: *** [Documentation/devicetree/bindings/soc/litex/litex,soc_controller.example.dts] Error 1
> Makefile:1263: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
>
> See https://patchwork.ozlabs.org/patch/1243930
> Please check and re-submit.

Thanks for spotting this.

It looks like the first line of the document is broken - it should be
a comment with a license descriptor, but the initial characters are
missing.

I'll fix this and resubmit the whole patchset after addressing all
other remarks.


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

Re: [PATCH v3 3/5] drivers/soc/litex: add LiteX SoC Controller driver

[Mateusz Holenko]

Hi Maxime,

On Tue, Feb 25, 2020 at 10:25 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Tue, Feb 25, 2020 at 09:46:57AM +0100, Mateusz Holenko wrote:
> > From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> >
> > This commit adds driver for the FPGA-based LiteX SoC
> > Controller from LiteX SoC builder.
> >
> > Co-developed-by: Mateusz Holenko <mholenko@antmicro.com>
> > Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> > Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
> > ---
> >
> > Notes:
> >     This commit has been introduced in v3 of the patchset.
> >
> >     It includes a simplified version of common 'litex.h'
> >     header introduced in v2 of the patchset.
> >
> >  MAINTAINERS                        |   2 +
> >  drivers/soc/Kconfig                |   1 +
> >  drivers/soc/Makefile               |   1 +
> >  drivers/soc/litex/Kconfig          |  14 ++
> >  drivers/soc/litex/Makefile         |   3 +
> >  drivers/soc/litex/litex_soc_ctrl.c | 233 +++++++++++++++++++++++++++++
> >  include/linux/litex.h              |  45 ++++++
> >  7 files changed, 299 insertions(+)
> >  create mode 100644 drivers/soc/litex/Kconfig
> >  create mode 100644 drivers/soc/litex/Makefile
> >  create mode 100644 drivers/soc/litex/litex_soc_ctrl.c
> >  create mode 100644 include/linux/litex.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ec925c081dd2..22a67514ace3 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9730,6 +9730,8 @@ M:      Karol Gugala <kgugala@antmicro.com>
> >  M:   Mateusz Holenko <mholenko@antmicro.com>
> >  S:   Maintained
> >  F:   Documentation/devicetree/bindings/*/litex,*.yaml
> > +F:   drivers/soc/litex/litex_soc_ctrl.c
> > +F:   include/linux/litex.h
> >
> >  LIVE PATCHING
> >  M:   Josh Poimboeuf <jpoimboe@redhat.com>
> > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> > index 1778f8c62861..78add2a163be 100644
> > --- a/drivers/soc/Kconfig
> > +++ b/drivers/soc/Kconfig
> > @@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig"
> >  source "drivers/soc/fsl/Kconfig"
> >  source "drivers/soc/imx/Kconfig"
> >  source "drivers/soc/ixp4xx/Kconfig"
> > +source "drivers/soc/litex/Kconfig"
> >  source "drivers/soc/mediatek/Kconfig"
> >  source "drivers/soc/qcom/Kconfig"
> >  source "drivers/soc/renesas/Kconfig"
> > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> > index 8b49d782a1ab..fd016b51cddd 100644
> > --- a/drivers/soc/Makefile
> > +++ b/drivers/soc/Makefile
> > @@ -14,6 +14,7 @@ obj-$(CONFIG_ARCH_GEMINI)   += gemini/
> >  obj-$(CONFIG_ARCH_MXC)               += imx/
> >  obj-$(CONFIG_ARCH_IXP4XX)    += ixp4xx/
> >  obj-$(CONFIG_SOC_XWAY)               += lantiq/
> > +obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
> >  obj-y                                += mediatek/
> >  obj-y                                += amlogic/
> >  obj-y                                += qcom/
> > diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
> > new file mode 100644
> > index 000000000000..22c78cda0b83
> > --- /dev/null
> > +++ b/drivers/soc/litex/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License_Identifier: GPL-2.0
> > +
> > +menu "Enable LiteX SoC Builder specific drivers"
> > +
> > +config LITEX_SOC_CONTROLLER
> > +     tristate "Enable LiteX SoC Controller driver"
> > +     help
> > +     This option enables the SoC Controller Driver which verifies
> > +     LiteX CSR access and provides common litex_get_reg/litex_set_reg
> > +     accessors.
> > +     All drivers that use functions from litex.h must depend on
> > +     LITEX_SOC_CONTROLLER
> > +
> > +endmenu
> > diff --git a/drivers/soc/litex/Makefile b/drivers/soc/litex/Makefile
> > new file mode 100644
> > index 000000000000..98ff7325b1c0
> > --- /dev/null
> > +++ b/drivers/soc/litex/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License_Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_LITEX_SOC_CONTROLLER)   += litex_soc_ctrl.o
> > diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
> > new file mode 100644
> > index 000000000000..b810782da3a5
> > --- /dev/null
> > +++ b/drivers/soc/litex/litex_soc_ctrl.c
> > @@ -0,0 +1,233 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * LiteX SoC Controller Driver
> > + *
> > + * Copyright (C) 2020 Antmicro
> > + *
> > + */
> > +
> > +#include <linux/litex.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/printk.h>
> > +#include <linux/module.h>
> > +#include <linux/errno.h>
> > +#include <linux/io.h>
> > +
> > +/*
> > + * The parameters below are true for LiteX SoC
> > + * configured for 8-bit CSR Bus, 32-bit aligned.
> > + *
> > + * Supporting other configurations will require
> > + * extending the logic in this header.
> > + */
> > +#define LITEX_REG_SIZE             0x4
> > +#define LITEX_SUBREG_SIZE          0x1
> > +#define LITEX_SUBREG_SIZE_BIT      (LITEX_SUBREG_SIZE * 8)
> > +
> > +static DEFINE_SPINLOCK(csr_lock);
> > +
> > +static inline unsigned long read_pointer_with_barrier(
> > +     const volatile void __iomem *addr)
> > +{
> > +     unsigned long val;
> > +
> > +     __io_br();
> > +     val = *(const volatile unsigned long __force *)addr;
> > +     __io_ar();
> > +     return val;
> > +}
> > +
> > +static inline void write_pointer_with_barrier(
> > +     volatile void __iomem *addr, unsigned long val)
> > +{
> > +     __io_br();
> > +     *(volatile unsigned long __force *)addr = val;
> > +     __io_ar();
> > +}
>
> What's wrong with readl/writel ?
>

readl/writel use a fixed endianness. What I want to do here is
to access bytes in the CPU native ordering - there is a comment
below explaining how CSRs are handled in LiteX.

> > +/*
> > + * LiteX SoC Generator, depending on the configuration,
> > + * can split a single logical CSR (Control & Status Register)
> > + * into a series of consecutive physical registers.
> > + *
> > + * For example, in the configuration with 8-bit CSR Bus,
> > + * 32-bit aligned (the default one for 32-bit CPUs) a 32-bit
> > + * logical CSR will be generated as four 32-bit physical registers,
> > + * each one containing one byte of meaningful data.
> > + *
> > + * For details see: https://github.com/enjoy-digital/litex/issues/314
> > + *
> > + * The purpose of `litex_set_reg`/`litex_get_reg` is to implement
> > + * the logic of writing to/reading from the LiteX CSR in a single
> > + * place that can be then reused by all LiteX drivers.
> > + */
> > +void litex_set_reg(
> > +     void __iomem *reg, unsigned long reg_size, unsigned long val)
> > +{
> > +     unsigned long shifted_data, shift, i;
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&csr_lock, flags);
> > +
> > +     for (i = 0; i < reg_size; ++i) {
> > +             shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> > +             shifted_data = val >> shift;
> > +             write_pointer_with_barrier(
> > +                     reg + (LITEX_REG_SIZE * i), shifted_data);
> > +     }
> > +
> > +     spin_unlock_irqrestore(&csr_lock, flags);
> > +}
> > +
> > +unsigned long litex_get_reg(
> > +     void __iomem *reg, unsigned long reg_size)
> > +{
> > +     unsigned long shifted_data, shift, i;
> > +     unsigned long result = 0;
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&csr_lock, flags);
> > +
> > +     for (i = 0; i < reg_size; ++i) {
> > +             shifted_data = read_pointer_with_barrier(
> > +                     reg + (LITEX_REG_SIZE * i));
> > +             shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> > +             result |= (shifted_data << shift);
> > +     }
> > +
> > +     spin_unlock_irqrestore(&csr_lock, flags);
> > +
> > +     return result;
> > +}
>
> I'm not sure what's supposed to be in that register, but usually it
> will either be abstracted away through a framework since letting each
> and every driver poke into the same register is not really ideal, or
> if you really have to do it, using a syscon.

Those litex_set_reg/ltex_get_reg are generic functions allowing to access
any CSR generated by LiteX (addressed with reg argument). Each peripheral
has its own set of CSRs and each driver should access only CSRs from
a single peripheral.

The reason for having a common accessor is to have the logic of doing
a single CSR write (implemented internally as many writes) and a single
CSR read (implemented internally as many reads) just in one place
to avoid code duplication.

> > +static int accessors_ok;
> > +
> > +/*
> > + * Check if accessors are safe to be used by other drivers
> > + * returns true if yes - false if not
> > + */
> > +int litex_check_accessors(void)
> > +{
> > +     return accessors_ok;
> > +}
> > +
> > +#define SCRATCH_REG_OFF         0x04
> > +#define SCRATCH_REG_SIZE        4
> > +#define SCRATCH_REG_VALUE       0x12345678
> > +#define SCRATCH_TEST_VALUE      0xdeadbeef
> > +
> > +/*
> > + * Check LiteX CSR read/write access
> > + *
> > + * This function reads and writes a scratch register in order
> > + * to verify if CSR access works.
> > + *
> > + * In case any problems are detected, the driver should panic
> > + * and not set `accessors_ok` flag. As a result no other
> > + * LiteX driver should access CSR bus.
> > + *
> > + * Access to the LiteX CSR is, by design, done in CPU native
> > + * endianness. The driver should not dynamically configure
> > + * access functions when the endianness mismatch is detected.
> > + * Such situation indicates problems in the soft SoC design
> > + * and should be solved at the LiteX generator level,
> > + * not in the software.
>
> I guess that's why you don't use readl/writel. Then
> __raw_readl/__raw_writel?

Do __raw_readl/__raw_writel provide memory barriers? I want to make sure
that CSR's subregisters are read in a specific order.

> > + */
> > +static int litex_check_csr_access(void __iomem *reg_addr)
> > +{
> > +     unsigned long reg;
> > +
> > +     reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
> > +
> > +     if (reg != SCRATCH_REG_VALUE) {
> > +             panic("Scratch register read error! Expected: 0x%x but got: 0x%lx",
> > +                     SCRATCH_REG_VALUE, reg);
> > +             return -EINVAL;
> > +     }
> > +
> > +     litex_set_reg(reg_addr + SCRATCH_REG_OFF,
> > +             SCRATCH_REG_SIZE, SCRATCH_TEST_VALUE);
> > +     reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
> > +
> > +     if (reg != SCRATCH_TEST_VALUE) {
> > +             panic("Scratch register write error! Expected: 0x%x but got: 0x%lx",
> > +                     SCRATCH_TEST_VALUE, reg);
> > +             return -EINVAL;
> > +     }
> > +
> > +     /* restore original value of the SCRATCH register */
> > +     litex_set_reg(reg_addr + SCRATCH_REG_OFF,
> > +             SCRATCH_REG_SIZE, SCRATCH_REG_VALUE);
> > +
> > +     /* Set flag for other drivers */
> > +     accessors_ok = 1;
> > +     pr_info("LiteX SoC Controller driver initialized");
> > +
> > +     return 0;
> > +}
> > +
> > +struct litex_soc_ctrl_device {
> > +     void __iomem *base;
> > +};
> > +
> > +static const struct of_device_id litex_soc_ctrl_of_match[] = {
> > +     {.compatible = "litex,soc_controller"},
> > +     {},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, litex_soc_ctrl_of_match);
> > +
> > +static int litex_soc_ctrl_probe(struct platform_device *pdev)
> > +{
> > +     struct device *dev;
> > +     struct device_node *node;
> > +     const struct of_device_id *id;
> > +     struct litex_soc_ctrl_device *soc_ctrl_dev;
> > +     struct resource *res;
> > +
> > +     dev = &pdev->dev;
> > +     node = dev->of_node;
> > +     if (!node)
> > +             return -ENODEV;
> > +
> > +     id = of_match_node(litex_soc_ctrl_of_match, node);
> > +     if (!id)
> > +             return -ENODEV;
>
> That's pretty much guaranteed.

You are right, I'll remove this fragment.

> > +     soc_ctrl_dev = devm_kzalloc(dev, sizeof(*soc_ctrl_dev), GFP_KERNEL);
> > +     if (IS_ERR_OR_NULL(soc_ctrl_dev))
> > +             return -ENOMEM;
> > +
> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (IS_ERR_OR_NULL(res))
> > +             return -EBUSY;
> > +
> > +     soc_ctrl_dev->base = devm_of_iomap(dev, node, 0, &res->end);
> > +     if (IS_ERR_OR_NULL(soc_ctrl_dev->base))
> > +             return -EIO;
>
> devm_platform_ioremap_resource ?

I'll simplify this fragment by using devm_platform_ioremap_resource as
suggested.

> > +     return litex_check_csr_access(soc_ctrl_dev->base);
> > +}
> > +
> > +static int litex_soc_ctrl_remove(struct platform_device *pdev)
> > +{
> > +     return 0;
> > +}
>
> You can leave the remove hook out entirely if you don't have anything
> in it.

Right.

> Maxime

Thanks for the comments!

I'll refactor the code to address your suggestions.



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