[PATCH v3 0/5] soc: samsung: Add USI driver

[PATCH v3 0/5] soc: samsung: Add USI driver

[Sam Protsenko]

USIv2 IP-core provides selectable serial protocol (UART, SPI or
High-Speed I2C); only one can be chosen at a time. This series
implements USIv2 driver, which allows one to select particular USI
function in device tree, and also performs USI block initialization.

With that driver implemented, it's not needed to do USI initialization
in protocol drivers anymore, so that code is removed from the serial
driver.

Because USI driver is tristate (can be built as a module), serial driver
was reworked so it's possible to use its console part as a module too.
This way we can load serial driver module from user space and still have
serial console functional.

Design features:
  - "reg" property contains USI registers start address (0xc0 offset);
    it's used in the driver to access USI_CON and USI_OPTION registers.
    This way all USI initialization (reset, HWACG, etc) can be done in
    USIv2 driver separately, rather than duplicating that code over
    UART/SPI/I2C drivers
  - System Register (system controller node) and its SW_CONF register
    offset are provided in "samsung,sysreg" property; it's used to
    select USI function (protocol to be used)
  - USI function is specified in "samsung,mode" property; integer value
    is used to simplify parsing
  - there is "samsung,clkreq-on" bool property, which makes driver
    disable HWACG control (needed for UART to work properly)
  - PCLK and IPCLK clocks are both provided to USI node; apparently both
    need to be enabled to access USI registers
  - protocol nodes are embedded (as a child nodes) in USI node; it
    allows correct init order, and reflects HW properly
  - USI driver is a tristate: can be also useful from Android GKI
    requirements point of view
  - driver functions are implemented with further development in mind:
    - we might want to add some DebugFs interface later
    - some functions might need to be revealed to serial drivers with
      EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
      instance
    - another USI revisions could be added (like USIv1)

Changes in v3:
  - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
  - Used clk_bulk API instead of handling each clock separately
  - Spell check fixes and coding style fixes
  - Improved dt-bindings doc

Changes in v2:
  - Renamed all 'usi_v2' wording to just 'usi' everywhere
  - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
    drivers
  - Added patch: "tty: serial: samsung: Fix console registration from
    module"
  - Combined dt-bindings doc and dt-bindings header patches
  - Reworked USI driver to be ready for USIv1 addition
  - Improved dt-bindings
  - Added USI_V2_NONE mode value

Sam Protsenko (5):
  dt-bindings: soc: samsung: Add Exynos USI bindings
  soc: samsung: Add USI driver
  tty: serial: samsung: Remove USI initialization
  tty: serial: samsung: Enable console as module
  tty: serial: samsung: Fix console registration from module

 .../bindings/soc/samsung/exynos-usi.yaml      | 159 ++++++++++
 drivers/soc/samsung/Kconfig                   |  14 +
 drivers/soc/samsung/Makefile                  |   2 +
 drivers/soc/samsung/exynos-usi.c              | 285 ++++++++++++++++++
 drivers/tty/serial/Kconfig                    |   2 +-
 drivers/tty/serial/samsung_tty.c              |  78 ++---
 include/dt-bindings/soc/samsung,exynos-usi.h  |  17 ++
 include/linux/serial_s3c.h                    |   9 -
 8 files changed, 518 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
 create mode 100644 drivers/soc/samsung/exynos-usi.c
 create mode 100644 include/dt-bindings/soc/samsung,exynos-usi.h

-- 
2.30.2

[PATCH v3 1/5] dt-bindings: soc: samsung: Add Exynos USI bindings

[Sam Protsenko]

Add constants for choosing USIv2 configuration mode in device tree.
Those are further used in USI driver to figure out which value to write
into SW_CONF register. Also document USIv2 IP-core bindings.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v3:
  - Removed usi0 "status" property from the example
  - Made child nodes pattern allow only serial/spi/i2c node names
  - Moved all USIv2 specific properties under corresponding "if:"
  - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
  - Related: submitted separate patch removing warning on
    "make dt_binding_check" for USI bindings

Changes in v2:
  - Combined dt-bindings doc and dt-bindings header patches
  - Added i2c node to example in bindings doc
  - Added mentioning of shared internal circuits
  - Added USI_V2_NONE value to bindings header

 .../bindings/soc/samsung/exynos-usi.yaml      | 159 ++++++++++++++++++
 include/dt-bindings/soc/samsung,exynos-usi.h  |  17 ++
 2 files changed, 176 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
 create mode 100644 include/dt-bindings/soc/samsung,exynos-usi.h

diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
new file mode 100644
index 000000000000..0af4821fae5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
@@ -0,0 +1,159 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/samsung/exynos-usi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung's Exynos USI (Universal Serial Interface) binding
+
+maintainers:
+  - Sam Protsenko <semen.protsenko@linaro.org>
+  - Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
+
+description: |
+  USI IP-core provides selectable serial protocol (UART, SPI or High-Speed I2C).
+  USI shares almost all internal circuits within each protocol, so only one
+  protocol can be chosen at a time. USI is modeled as a node with zero or more
+  child nodes, each representing a serial sub-node device. The mode setting
+  selects which particular function will be used.
+
+  Refer to next bindings documentation for information on protocol subnodes that
+  can exist under USI node:
+
+  [1] Documentation/devicetree/bindings/serial/samsung_uart.yaml
+  [2] Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
+  [3] Documentation/devicetree/bindings/spi/spi-samsung.txt
+
+properties:
+  $nodename:
+    pattern: "^usi@[0-9a-f]+$"
+
+  compatible:
+    enum:
+      - samsung,exynos850-usi   # for USIv2 (Exynos850, ExynosAutoV9)
+
+  reg: true
+
+  clocks: true
+
+  clock-names: true
+
+  ranges: true
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  samsung,sysreg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Should be phandle/offset pair. The phandle to System Register syscon node
+      (for the same domain where this USI controller resides) and the offset
+      of SW_CONF register for this USI controller.
+
+  samsung,mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Selects USI function (which serial protocol to use). Refer to
+      <include/dt-bindings/soc/samsung,exynos-usi.h> for valid USI mode values.
+
+  samsung,clkreq-on:
+    type: boolean
+    description:
+      Enable this property if underlying protocol requires the clock to be
+      continuously provided without automatic gating. As suggested by SoC
+      manual, it should be set in case of SPI/I2C slave, UART Rx and I2C
+      multi-master mode. Usually this property is needed if USI mode is set
+      to "UART".
+
+      This property is optional.
+
+patternProperties:
+  # All other properties should be child nodes
+  "^(serial|spi|i2c)@[0-9a-f]+$":
+    type: object
+    description: Child node describing underlying USI serial protocol
+
+required:
+  - compatible
+  - ranges
+  - "#address-cells"
+  - "#size-cells"
+  - samsung,sysreg
+  - samsung,mode
+
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - samsung,exynos850-usi
+
+then:
+  properties:
+    reg:
+      maxItems: 1
+
+    clocks:
+      items:
+        - description: Bus (APB) clock
+        - description: Operating clock for UART/SPI/I2C protocol
+
+    clock-names:
+      items:
+        - const: pclk
+        - const: ipclk
+
+  required:
+    - reg
+    - clocks
+    - clock-names
+
+else:
+  properties:
+    reg: false
+    clocks: false
+    clock-names: false
+    samsung,clkreq-on: false
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/soc/samsung,exynos-usi.h>
+
+    usi0: usi@138200c0 {
+        compatible = "samsung,exynos850-usi";
+        reg = <0x138200c0 0x20>;
+        samsung,sysreg = <&sysreg_peri 0x1010>;
+        samsung,mode = <USI_V2_UART>;
+        samsung,clkreq-on; /* needed for UART mode */
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+        clocks = <&cmu_peri 32>, <&cmu_peri 31>;
+        clock-names = "pclk", "ipclk";
+
+        serial_0: serial@13820000 {
+            compatible = "samsung,exynos850-uart";
+            reg = <0x13820000 0xc0>;
+            interrupts = <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cmu_peri 32>, <&cmu_peri 31>;
+            clock-names = "uart", "clk_uart_baud0";
+            status = "disabled";
+        };
+
+        hsi2c_0: i2c@13820000 {
+            compatible = "samsung,exynosautov9-hsi2c";
+            reg = <0x13820000 0xc0>;
+            interrupts = <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            clocks = <&cmu_peri 32>, <&cmu_peri 31>;
+            clock-names = "hsi2c_pclk", "hsi2c";
+            status = "disabled";
+        };
+    };
diff --git a/include/dt-bindings/soc/samsung,exynos-usi.h b/include/dt-bindings/soc/samsung,exynos-usi.h
new file mode 100644
index 000000000000..a01af169d249
--- /dev/null
+++ b/include/dt-bindings/soc/samsung,exynos-usi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2021 Linaro Ltd.
+ * Author: Sam Protsenko <semen.protsenko@linaro.org>
+ *
+ * Device Tree bindings for Samsung Exynos USI (Universal Serial Interface).
+ */
+
+#ifndef __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H
+#define __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H
+
+#define USI_V2_NONE		0
+#define USI_V2_UART		1
+#define USI_V2_SPI		2
+#define USI_V2_I2C		3
+
+#endif /* __DT_BINDINGS_SAMSUNG_EXYNOS_USI_H */
-- 
2.30.2

Re: [PATCH v3 1/5] dt-bindings: soc: samsung: Add Exynos USI bindings

[Rob Herring]

On Sat, 04 Dec 2021 21:57:53 +0200, Sam Protsenko wrote:
> Add constants for choosing USIv2 configuration mode in device tree.
> Those are further used in USI driver to figure out which value to write
> into SW_CONF register. Also document USIv2 IP-core bindings.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v3:
>   - Removed usi0 "status" property from the example
>   - Made child nodes pattern allow only serial/spi/i2c node names
>   - Moved all USIv2 specific properties under corresponding "if:"
>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
>   - Related: submitted separate patch removing warning on
>     "make dt_binding_check" for USI bindings
> 
> Changes in v2:
>   - Combined dt-bindings doc and dt-bindings header patches
>   - Added i2c node to example in bindings doc
>   - Added mentioning of shared internal circuits
>   - Added USI_V2_NONE value to bindings header
> 
>  .../bindings/soc/samsung/exynos-usi.yaml      | 159 ++++++++++++++++++
>  include/dt-bindings/soc/samsung,exynos-usi.h  |  17 ++
>  2 files changed, 176 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml
>  create mode 100644 include/dt-bindings/soc/samsung,exynos-usi.h
> 

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

Re: (subset) [PATCH v3 1/5] dt-bindings: soc: samsung: Add Exynos USI bindings

[Krzysztof Kozlowski]

On Sat, 4 Dec 2021 21:57:53 +0200, Sam Protsenko wrote:
> Add constants for choosing USIv2 configuration mode in device tree.
> Those are further used in USI driver to figure out which value to write
> into SW_CONF register. Also document USIv2 IP-core bindings.
> 
> 

Applied, thanks!

[1/5] dt-bindings: soc: samsung: Add Exynos USI bindings
      commit: e522ae91b8ff7bf89d22d9322308aba1a6326996

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

[PATCH v3 2/5] soc: samsung: Add USI driver

[Sam Protsenko]

USIv2 IP-core is found on modern ARM64 Exynos SoCs (like Exynos850) and
provides selectable serial protocol (one of: UART, SPI, I2C). USIv2
registers usually reside in the same register map as a particular
underlying protocol it implements, but have some particular offset. E.g.
on Exynos850 the USI_UART has 0x13820000 base address, where UART
registers have 0x00..0x40 offsets, and USI registers have 0xc0..0xdc
offsets. Desired protocol can be chosen via SW_CONF register from System
Register block of the same domain as USI.

Before starting to use a particular protocol, USIv2 must be configured
properly:
  1. Select protocol to be used via System Register
  2. Clear "reset" flag in USI_CON
  3. Configure HWACG behavior (e.g. for UART Rx the HWACG must be
     disabled, so that the IP clock is not gated automatically); this is
     done using USI_OPTION register
  4. Keep both USI clocks (PCLK and IPCLK) running during USI registers
     modification

This driver implements the above behavior. Of course, USIv2 driver
should be probed before UART/I2C/SPI drivers. It can be achieved by
embedding UART/I2C/SPI nodes inside of the USI node (in Device Tree);
driver then walks underlying nodes and instantiates those. Driver also
handles USI configuration on PM resume, as register contents can be lost
during CPU suspend.

This driver is designed with different USI versions in mind. So it
should be relatively easy to add new USI revisions to it later.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v3:
  - Fixed spell check issues in driver code and commit message
  - Removed comma after {} sentinel
  - Used clk_bulk API instead of handling each clock separately
  - Use __maybe_resume instead of guarding resume function with
    #ifdef CONFIG_PM_SLEEP
  - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
  - Style fixes

Changes in v2:
  - Replaced arch_initcall() with module_platform_driver()
  - Reworked the whole driver for the easy adoption of other USI
    revisions
  - Added "mode" validation right after reading it from device tree
  - Handled new USI_V2_NONE value

 drivers/soc/samsung/Kconfig      |  14 ++
 drivers/soc/samsung/Makefile     |   2 +
 drivers/soc/samsung/exynos-usi.c | 285 +++++++++++++++++++++++++++++++
 3 files changed, 301 insertions(+)
 create mode 100644 drivers/soc/samsung/exynos-usi.c

diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
index e2cedef1e8d1..a9f8b224322e 100644
--- a/drivers/soc/samsung/Kconfig
+++ b/drivers/soc/samsung/Kconfig
@@ -23,6 +23,20 @@ config EXYNOS_CHIPID
 	  Support for Samsung Exynos SoC ChipID and Adaptive Supply Voltage.
 	  This driver can also be built as module (exynos_chipid).
 
+config EXYNOS_USI
+	tristate "Exynos USI (Universal Serial Interface) driver"
+	default ARCH_EXYNOS && ARM64
+	depends on ARCH_EXYNOS || COMPILE_TEST
+	select MFD_SYSCON
+	help
+	  Enable support for USI block. USI (Universal Serial Interface) is an
+	  IP-core found in modern Samsung Exynos SoCs, like Exynos850 and
+	  ExynosAutoV0. USI block can be configured to provide one of the
+	  following serial protocols: UART, SPI or High Speed I2C.
+
+	  This driver allows one to configure USI for desired protocol, which
+	  is usually done in USI node in Device Tree.
+
 config EXYNOS_PMU
 	bool "Exynos PMU controller driver" if COMPILE_TEST
 	depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST)
diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
index 2ae4bea804cf..9f59d1905ab0 100644
--- a/drivers/soc/samsung/Makefile
+++ b/drivers/soc/samsung/Makefile
@@ -4,6 +4,8 @@ obj-$(CONFIG_EXYNOS_ASV_ARM)	+= exynos5422-asv.o
 obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos_chipid.o
 exynos_chipid-y			+= exynos-chipid.o exynos-asv.o
 
+obj-$(CONFIG_EXYNOS_USI)	+= exynos-usi.o
+
 obj-$(CONFIG_EXYNOS_PMU)	+= exynos-pmu.o
 
 obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)	+= exynos3250-pmu.o exynos4-pmu.o \
diff --git a/drivers/soc/samsung/exynos-usi.c b/drivers/soc/samsung/exynos-usi.c
new file mode 100644
index 000000000000..114352695ac2
--- /dev/null
+++ b/drivers/soc/samsung/exynos-usi.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 Linaro Ltd.
+ * Author: Sam Protsenko <semen.protsenko@linaro.org>
+ *
+ * Samsung Exynos USI driver (Universal Serial Interface).
+ */
+
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/soc/samsung,exynos-usi.h>
+
+/* USIv2: System Register: SW_CONF register bits */
+#define USI_V2_SW_CONF_NONE	0x0
+#define USI_V2_SW_CONF_UART	BIT(0)
+#define USI_V2_SW_CONF_SPI	BIT(1)
+#define USI_V2_SW_CONF_I2C	BIT(2)
+#define USI_V2_SW_CONF_MASK	(USI_V2_SW_CONF_UART | USI_V2_SW_CONF_SPI | \
+				 USI_V2_SW_CONF_I2C)
+
+/* USIv2: USI register offsets */
+#define USI_CON			0x04
+#define USI_OPTION		0x08
+
+/* USIv2: USI register bits */
+#define USI_CON_RESET		BIT(0)
+#define USI_OPTION_CLKREQ_ON	BIT(1)
+#define USI_OPTION_CLKSTOP_ON	BIT(2)
+
+enum exynos_usi_ver {
+	USI_VER2 = 2,
+};
+
+struct exynos_usi_variant {
+	enum exynos_usi_ver ver;	/* USI IP-core version */
+	unsigned int sw_conf_mask;	/* SW_CONF mask for all protocols */
+	size_t min_mode;		/* first index in exynos_usi_modes[] */
+	size_t max_mode;		/* last index in exynos_usi_modes[] */
+	size_t num_clks;		/* number of clocks to assert */
+	const char * const *clk_names;	/* clock names to assert */
+};
+
+struct exynos_usi {
+	struct device *dev;
+	void __iomem *regs;		/* USI register map */
+	struct clk_bulk_data *clks;	/* USI clocks */
+
+	size_t mode;			/* current USI SW_CONF mode index */
+	bool clkreq_on;			/* always provide clock to IP */
+
+	/* System Register */
+	struct regmap *sysreg;		/* System Register map */
+	unsigned int sw_conf;		/* SW_CONF register offset in sysreg */
+
+	const struct exynos_usi_variant *data;
+};
+
+struct exynos_usi_mode {
+	const char *name;		/* mode name */
+	unsigned int val;		/* mode register value */
+};
+
+static const struct exynos_usi_mode exynos_usi_modes[] = {
+	[USI_V2_NONE] =	{ .name = "none", .val = USI_V2_SW_CONF_NONE },
+	[USI_V2_UART] =	{ .name = "uart", .val = USI_V2_SW_CONF_UART },
+	[USI_V2_SPI] =	{ .name = "spi",  .val = USI_V2_SW_CONF_SPI },
+	[USI_V2_I2C] =	{ .name = "i2c",  .val = USI_V2_SW_CONF_I2C },
+};
+
+static const char * const exynos850_usi_clk_names[] = { "pclk", "ipclk" };
+static const struct exynos_usi_variant exynos850_usi_data = {
+	.ver		= USI_VER2,
+	.sw_conf_mask	= USI_V2_SW_CONF_MASK,
+	.min_mode	= USI_V2_NONE,
+	.max_mode	= USI_V2_I2C,
+	.num_clks	= ARRAY_SIZE(exynos850_usi_clk_names),
+	.clk_names	= exynos850_usi_clk_names,
+};
+
+static const struct of_device_id exynos_usi_dt_match[] = {
+	{
+		.compatible = "samsung,exynos850-usi",
+		.data = &exynos850_usi_data,
+	},
+	{ } /* sentinel */
+};
+MODULE_DEVICE_TABLE(of, exynos_usi_dt_match);
+
+/**
+ * exynos_usi_set_sw_conf - Set USI block configuration mode
+ * @usi: USI driver object
+ * @mode: Mode index
+ *
+ * Select underlying serial protocol (UART/SPI/I2C) in USI IP-core.
+ *
+ * Return: 0 on success, or negative error code on failure.
+ */
+static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode)
+{
+	unsigned int val;
+	int ret;
+
+	if (mode < usi->data->min_mode || mode > usi->data->max_mode)
+		return -EINVAL;
+
+	val = exynos_usi_modes[mode].val;
+	ret = regmap_update_bits(usi->sysreg, usi->sw_conf,
+				 usi->data->sw_conf_mask, val);
+	if (ret)
+		return ret;
+
+	usi->mode = mode;
+	dev_dbg(usi->dev, "protocol: %s\n", exynos_usi_modes[usi->mode].name);
+
+	return 0;
+}
+
+/**
+ * exynos_usi_enable - Initialize USI block
+ * @usi: USI driver object
+ *
+ * USI IP-core start state is "reset" (on startup and after CPU resume). This
+ * routine enables the USI block by clearing the reset flag. It also configures
+ * HWACG behavior (needed e.g. for UART Rx). It should be performed before
+ * underlying protocol becomes functional.
+ *
+ * Return: 0 on success, or negative error code on failure.
+ */
+static int exynos_usi_enable(const struct exynos_usi *usi)
+{
+	u32 val;
+	int ret;
+
+	ret = clk_bulk_prepare_enable(usi->data->num_clks, usi->clks);
+	if (ret)
+		return ret;
+
+	/* Enable USI block */
+	val = readl(usi->regs + USI_CON);
+	val &= ~USI_CON_RESET;
+	writel(val, usi->regs + USI_CON);
+	udelay(1);
+
+	/* Continuously provide the clock to USI IP w/o gating */
+	if (usi->clkreq_on) {
+		val = readl(usi->regs + USI_OPTION);
+		val &= ~USI_OPTION_CLKSTOP_ON;
+		val |= USI_OPTION_CLKREQ_ON;
+		writel(val, usi->regs + USI_OPTION);
+	}
+
+	clk_bulk_disable_unprepare(usi->data->num_clks, usi->clks);
+
+	return ret;
+}
+
+static int exynos_usi_configure(struct exynos_usi *usi)
+{
+	int ret;
+
+	ret = exynos_usi_set_sw_conf(usi, usi->mode);
+	if (ret)
+		return ret;
+
+	if (usi->data->ver == USI_VER2)
+		return exynos_usi_enable(usi);
+
+	return 0;
+}
+
+static int exynos_usi_parse_dt(struct device_node *np, struct exynos_usi *usi)
+{
+	int ret;
+	u32 mode;
+
+	ret = of_property_read_u32(np, "samsung,mode", &mode);
+	if (ret)
+		return ret;
+	if (mode < usi->data->min_mode || mode > usi->data->max_mode)
+		return -EINVAL;
+	usi->mode = mode;
+
+	usi->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
+	if (IS_ERR(usi->sysreg))
+		return PTR_ERR(usi->sysreg);
+
+	ret = of_property_read_u32_index(np, "samsung,sysreg", 1,
+					 &usi->sw_conf);
+	if (ret)
+		return ret;
+
+	usi->clkreq_on = of_property_read_bool(np, "samsung,clkreq-on");
+
+	return 0;
+}
+
+static int exynos_usi_get_clocks(struct exynos_usi *usi)
+{
+	const size_t num = usi->data->num_clks;
+	struct device *dev = usi->dev;
+	size_t i;
+
+	if (num == 0)
+		return 0;
+
+	usi->clks = devm_kcalloc(dev, num, sizeof(*usi->clks), GFP_KERNEL);
+	if (!usi->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < num; ++i)
+		usi->clks[i].id = usi->data->clk_names[i];
+
+	return devm_clk_bulk_get(dev, num, usi->clks);
+}
+
+static int exynos_usi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct exynos_usi *usi;
+	int ret;
+
+	usi = devm_kzalloc(dev, sizeof(*usi), GFP_KERNEL);
+	if (!usi)
+		return -ENOMEM;
+
+	usi->dev = dev;
+	platform_set_drvdata(pdev, usi);
+
+	usi->data = of_device_get_match_data(dev);
+	if (!usi->data)
+		return -EINVAL;
+
+	ret = exynos_usi_parse_dt(np, usi);
+	if (ret)
+		return ret;
+
+	ret = exynos_usi_get_clocks(usi);
+	if (ret)
+		return ret;
+
+	if (usi->data->ver == USI_VER2) {
+		usi->regs = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(usi->regs))
+			return PTR_ERR(usi->regs);
+	}
+
+	ret = exynos_usi_configure(usi);
+	if (ret)
+		return ret;
+
+	/* Make it possible to embed protocol nodes into USI np */
+	return of_platform_populate(np, NULL, NULL, dev);
+}
+
+static int __maybe_unused exynos_usi_resume_noirq(struct device *dev)
+{
+	struct exynos_usi *usi = dev_get_drvdata(dev);
+
+	return exynos_usi_configure(usi);
+}
+
+static const struct dev_pm_ops exynos_usi_pm = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, exynos_usi_resume_noirq)
+};
+
+static struct platform_driver exynos_usi_driver = {
+	.driver = {
+		.name		= "exynos-usi",
+		.pm		= &exynos_usi_pm,
+		.of_match_table	= exynos_usi_dt_match,
+	},
+	.probe = exynos_usi_probe,
+};
+module_platform_driver(exynos_usi_driver);
+
+MODULE_DESCRIPTION("Samsung USI driver");
+MODULE_AUTHOR("Sam Protsenko <semen.protsenko@linaro.org>");
+MODULE_LICENSE("GPL");
-- 
2.30.2

Re: (subset) [PATCH v3 2/5] soc: samsung: Add USI driver

[Krzysztof Kozlowski]

On Sat, 4 Dec 2021 21:57:54 +0200, Sam Protsenko wrote:
> USIv2 IP-core is found on modern ARM64 Exynos SoCs (like Exynos850) and
> provides selectable serial protocol (one of: UART, SPI, I2C). USIv2
> registers usually reside in the same register map as a particular
> underlying protocol it implements, but have some particular offset. E.g.
> on Exynos850 the USI_UART has 0x13820000 base address, where UART
> registers have 0x00..0x40 offsets, and USI registers have 0xc0..0xdc
> offsets. Desired protocol can be chosen via SW_CONF register from System
> Register block of the same domain as USI.
> 
> [...]

Applied, thanks!

[2/5] soc: samsung: Add USI driver
      commit: cc4fcd60ac51d99ef75fa80f14ac6202f46c3dfc

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

[PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Sam Protsenko]

USI control is now extracted to the dedicated USI driver. Remove USI
related code from serial driver to avoid conflicts and code duplication.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v3:
  - Spell check fixes in commit message

Changes in v2:
  - (none)

 drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
 include/linux/serial_s3c.h       |  9 --------
 2 files changed, 4 insertions(+), 41 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index ca084c10d0bb..f986a9253dc8 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -65,7 +65,6 @@ enum s3c24xx_port_type {
 struct s3c24xx_uart_info {
 	char			*name;
 	enum s3c24xx_port_type	type;
-	bool			has_usi;
 	unsigned int		port_type;
 	unsigned int		fifosize;
 	unsigned long		rx_fifomask;
@@ -1357,28 +1356,6 @@ static int apple_s5l_serial_startup(struct uart_port *port)
 	return ret;
 }
 
-static void exynos_usi_init(struct uart_port *port)
-{
-	struct s3c24xx_uart_port *ourport = to_ourport(port);
-	struct s3c24xx_uart_info *info = ourport->info;
-	unsigned int val;
-
-	if (!info->has_usi)
-		return;
-
-	/* Clear the software reset of USI block (it's set at startup) */
-	val = rd_regl(port, USI_CON);
-	val &= ~USI_CON_RESET_MASK;
-	wr_regl(port, USI_CON, val);
-	udelay(1);
-
-	/* Continuously provide the clock to USI IP w/o gating (for Rx mode) */
-	val = rd_regl(port, USI_OPTION);
-	val &= ~USI_OPTION_HWACG_MASK;
-	val |= USI_OPTION_HWACG_CLKREQ_ON;
-	wr_regl(port, USI_OPTION, val);
-}
-
 /* power power management control */
 
 static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
@@ -1405,8 +1382,6 @@ static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
 
 		if (!IS_ERR(ourport->baudclk))
 			clk_prepare_enable(ourport->baudclk);
-
-		exynos_usi_init(port);
 		break;
 	default:
 		dev_err(port->dev, "s3c24xx_serial: unknown pm %d\n", level);
@@ -2130,8 +2105,6 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
 	if (ret)
 		pr_warn("uart: failed to enable baudclk\n");
 
-	exynos_usi_init(port);
-
 	/* Keep all interrupts masked and cleared */
 	switch (ourport->info->type) {
 	case TYPE_S3C6400:
@@ -2780,11 +2753,10 @@ static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = {
 #endif
 
 #if defined(CONFIG_ARCH_EXYNOS)
-#define EXYNOS_COMMON_SERIAL_DRV_DATA(_has_usi)			\
+#define EXYNOS_COMMON_SERIAL_DRV_DATA()				\
 	.info = &(struct s3c24xx_uart_info) {			\
 		.name		= "Samsung Exynos UART",	\
 		.type		= TYPE_S3C6400,			\
-		.has_usi	= _has_usi,			\
 		.port_type	= PORT_S3C6400,			\
 		.has_divslot	= 1,				\
 		.rx_fifomask	= S5PV210_UFSTAT_RXMASK,	\
@@ -2805,17 +2777,17 @@ static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = {
 	}							\
 
 static struct s3c24xx_serial_drv_data exynos4210_serial_drv_data = {
-	EXYNOS_COMMON_SERIAL_DRV_DATA(false),
+	EXYNOS_COMMON_SERIAL_DRV_DATA(),
 	.fifosize = { 256, 64, 16, 16 },
 };
 
 static struct s3c24xx_serial_drv_data exynos5433_serial_drv_data = {
-	EXYNOS_COMMON_SERIAL_DRV_DATA(false),
+	EXYNOS_COMMON_SERIAL_DRV_DATA(),
 	.fifosize = { 64, 256, 16, 256 },
 };
 
 static struct s3c24xx_serial_drv_data exynos850_serial_drv_data = {
-	EXYNOS_COMMON_SERIAL_DRV_DATA(true),
+	EXYNOS_COMMON_SERIAL_DRV_DATA(),
 	.fifosize = { 256, 64, 64, 64 },
 };
 
diff --git a/include/linux/serial_s3c.h b/include/linux/serial_s3c.h
index cf0de4a86640..f6c3323fc4c5 100644
--- a/include/linux/serial_s3c.h
+++ b/include/linux/serial_s3c.h
@@ -27,15 +27,6 @@
 #define S3C2410_UERSTAT	  (0x14)
 #define S3C2410_UFSTAT	  (0x18)
 #define S3C2410_UMSTAT	  (0x1C)
-#define USI_CON		  (0xC4)
-#define USI_OPTION	  (0xC8)
-
-#define USI_CON_RESET			(1<<0)
-#define USI_CON_RESET_MASK		(1<<0)
-
-#define USI_OPTION_HWACG_CLKREQ_ON	(1<<1)
-#define USI_OPTION_HWACG_CLKSTOP_ON	(1<<2)
-#define USI_OPTION_HWACG_MASK		(3<<1)
 
 #define S3C2410_LCON_CFGMASK	  ((0xF<<3)|(0x3))
 
-- 
2.30.2

Re: [PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Krzysztof Kozlowski]

On 04/12/2021 20:57, Sam Protsenko wrote:
> USI control is now extracted to the dedicated USI driver. Remove USI
> related code from serial driver to avoid conflicts and code duplication.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v3:
>   - Spell check fixes in commit message
> 
> Changes in v2:
>   - (none)
> 
>  drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
>  include/linux/serial_s3c.h       |  9 --------
>  2 files changed, 4 insertions(+), 41 deletions(-)
> 

Hi Sam,

Does this patch depend on USI driver? In cover letter you did not
mention any dependency, so this can go via Greg's tree, right?

Best regards,
Krzysztof

Re: [PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Sam Protsenko]

On Mon, 13 Dec 2021 at 13:00, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 04/12/2021 20:57, Sam Protsenko wrote:
> > USI control is now extracted to the dedicated USI driver. Remove USI
> > related code from serial driver to avoid conflicts and code duplication.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> > Changes in v3:
> >   - Spell check fixes in commit message
> >
> > Changes in v2:
> >   - (none)
> >
> >  drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
> >  include/linux/serial_s3c.h       |  9 --------
> >  2 files changed, 4 insertions(+), 41 deletions(-)
> >
>
> Hi Sam,
>
> Does this patch depend on USI driver? In cover letter you did not
> mention any dependency, so this can go via Greg's tree, right?
>

Hi Krzysztof,

TL;DR: Serial patches from this series don't really depend on USI
driver and can go via Greg's tree.

I'd say ideally those should be applied right after USI driver
patches. Because otherwise we'd have double initialization of USI
block (from USI driver and from serial driver), which wasn't tested
and may lead to not functional USI (though I think it should be fine).
That's why I decided to keep those in one single patch series. But if
we don't care about some short period of not tested behavior -- then
those can go via Greg's tree.

> Best regards,
> Krzysztof

Re: [PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Krzysztof Kozlowski]

On 13/12/2021 12:31, Sam Protsenko wrote:
> On Mon, 13 Dec 2021 at 13:00, Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
>>
>> On 04/12/2021 20:57, Sam Protsenko wrote:
>>> USI control is now extracted to the dedicated USI driver. Remove USI
>>> related code from serial driver to avoid conflicts and code duplication.
>>>
>>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>>> ---
>>> Changes in v3:
>>>   - Spell check fixes in commit message
>>>
>>> Changes in v2:
>>>   - (none)
>>>
>>>  drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
>>>  include/linux/serial_s3c.h       |  9 --------
>>>  2 files changed, 4 insertions(+), 41 deletions(-)
>>>
>>
>> Hi Sam,
>>
>> Does this patch depend on USI driver? In cover letter you did not
>> mention any dependency, so this can go via Greg's tree, right?
>>
> 
> Hi Krzysztof,
> 
> TL;DR: Serial patches from this series don't really depend on USI
> driver and can go via Greg's tree.
> 
> I'd say ideally those should be applied right after USI driver
> patches. Because otherwise we'd have double initialization of USI
> block (from USI driver and from serial driver), which wasn't tested
> and may lead to not functional USI (though I think it should be fine).
> That's why I decided to keep those in one single patch series. But if
> we don't care about some short period of not tested behavior -- then
> those can go via Greg's tree.

I think it is fine considering that only one board will be affected and
it will be already non-bisectable due to DTS/driver changes going
separate branches. More important is to get USI and serial driver
changes in the same kernel release, so in v5.17.


Best regards,
Krzysztof

Re: [PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Krzysztof Kozlowski]

On 04/12/2021 20:57, Sam Protsenko wrote:
> USI control is now extracted to the dedicated USI driver. Remove USI
> related code from serial driver to avoid conflicts and code duplication.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v3:
>   - Spell check fixes in commit message
> 
> Changes in v2:
>   - (none)
> 
>  drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
>  include/linux/serial_s3c.h       |  9 --------
>  2 files changed, 4 insertions(+), 41 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Greg,
If you are fine with the changes, please take the serial driver changes
via your tree.

Best regards,
Krzysztof

Re: [PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Sam Protsenko]

On Mon, 13 Dec 2021 at 13:35, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 04/12/2021 20:57, Sam Protsenko wrote:
> > USI control is now extracted to the dedicated USI driver. Remove USI
> > related code from serial driver to avoid conflicts and code duplication.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> > Changes in v3:
> >   - Spell check fixes in commit message
> >
> > Changes in v2:
> >   - (none)
> >
> >  drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
> >  include/linux/serial_s3c.h       |  9 --------
> >  2 files changed, 4 insertions(+), 41 deletions(-)
> >
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>
> Greg,
> If you are fine with the changes, please take the serial driver changes
> via your tree.
>

Hi Greg,

If it's ok with you, can you please apply patches 3, 4 and 5 from this
series? If it's possible, would be nice to see those in v5.17.

Thanks!

> Best regards,
> Krzysztof

Re: [PATCH v3 3/5] tty: serial: samsung: Remove USI initialization

[Greg Kroah-Hartman]

On Mon, Dec 20, 2021 at 05:21:16PM +0200, Sam Protsenko wrote:
> On Mon, 13 Dec 2021 at 13:35, Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
> >
> > On 04/12/2021 20:57, Sam Protsenko wrote:
> > > USI control is now extracted to the dedicated USI driver. Remove USI
> > > related code from serial driver to avoid conflicts and code duplication.
> > >
> > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > > ---
> > > Changes in v3:
> > >   - Spell check fixes in commit message
> > >
> > > Changes in v2:
> > >   - (none)
> > >
> > >  drivers/tty/serial/samsung_tty.c | 36 ++++----------------------------
> > >  include/linux/serial_s3c.h       |  9 --------
> > >  2 files changed, 4 insertions(+), 41 deletions(-)
> > >
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >
> > Greg,
> > If you are fine with the changes, please take the serial driver changes
> > via your tree.
> >
> 
> Hi Greg,
> 
> If it's ok with you, can you please apply patches 3, 4 and 5 from this
> series? If it's possible, would be nice to see those in v5.17.

All now queued up, thanks.

greg k-h

[PATCH v3 4/5] tty: serial: samsung: Enable console as module

[Sam Protsenko]

Enable serial driver to be built as a module. To do so, init the console
support on driver/module load instead of using console_initcall().

Inspired by commit 87a0b9f98ac5 ("tty: serial: meson: enable console as
module").

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v3:
  - Removed the USI mentioning from commit message

Changes in v2:
  - Added error path handling in samsung_serial_init()
  - Added console unregister in samsung_serial_exit()

 drivers/tty/serial/Kconfig       |  2 +-
 drivers/tty/serial/samsung_tty.c | 36 ++++++++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index fc543ac97c13..0e5ccb25bdb1 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -263,7 +263,7 @@ config SERIAL_SAMSUNG_UARTS
 
 config SERIAL_SAMSUNG_CONSOLE
 	bool "Support for console on Samsung SoC serial port"
-	depends on SERIAL_SAMSUNG=y
+	depends on SERIAL_SAMSUNG
 	select SERIAL_CORE_CONSOLE
 	select SERIAL_EARLYCON
 	help
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index f986a9253dc8..61ccb359620a 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -1715,15 +1715,21 @@ s3c24xx_serial_verify_port(struct uart_port *port, struct serial_struct *ser)
 
 static struct console s3c24xx_serial_console;
 
-static int __init s3c24xx_serial_console_init(void)
+static void __init s3c24xx_serial_register_console(void)
 {
 	register_console(&s3c24xx_serial_console);
-	return 0;
 }
-console_initcall(s3c24xx_serial_console_init);
+
+static void s3c24xx_serial_unregister_console(void)
+{
+	if (s3c24xx_serial_console.flags & CON_ENABLED)
+		unregister_console(&s3c24xx_serial_console);
+}
 
 #define S3C24XX_SERIAL_CONSOLE &s3c24xx_serial_console
 #else
+static inline void s3c24xx_serial_register_console(void) { }
+static inline void s3c24xx_serial_unregister_console(void) { }
 #define S3C24XX_SERIAL_CONSOLE NULL
 #endif
 
@@ -2898,7 +2904,29 @@ static struct platform_driver samsung_serial_driver = {
 	},
 };
 
-module_platform_driver(samsung_serial_driver);
+static int __init samsung_serial_init(void)
+{
+	int ret;
+
+	s3c24xx_serial_register_console();
+
+	ret = platform_driver_register(&samsung_serial_driver);
+	if (ret) {
+		s3c24xx_serial_unregister_console();
+		return ret;
+	}
+
+	return 0;
+}
+
+static void __exit samsung_serial_exit(void)
+{
+	platform_driver_unregister(&samsung_serial_driver);
+	s3c24xx_serial_unregister_console();
+}
+
+module_init(samsung_serial_init);
+module_exit(samsung_serial_exit);
 
 #ifdef CONFIG_SERIAL_SAMSUNG_CONSOLE
 /*
-- 
2.30.2

Re: [PATCH v3 4/5] tty: serial: samsung: Enable console as module

[Krzysztof Kozlowski]

On 04/12/2021 20:57, Sam Protsenko wrote:
> Enable serial driver to be built as a module. To do so, init the console
> support on driver/module load instead of using console_initcall().
> 
> Inspired by commit 87a0b9f98ac5 ("tty: serial: meson: enable console as
> module").
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v3:
>   - Removed the USI mentioning from commit message
> 
> Changes in v2:
>   - Added error path handling in samsung_serial_init()
>   - Added console unregister in samsung_serial_exit()
> 
>  drivers/tty/serial/Kconfig       |  2 +-
>  drivers/tty/serial/samsung_tty.c | 36 ++++++++++++++++++++++++++++----
>  2 files changed, 33 insertions(+), 5 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

[PATCH v3 5/5] tty: serial: samsung: Fix console registration from module

[Sam Protsenko]

On modern Exynos SoCs (like Exynos850) the UART can be implemented as a
part of USI IP-core. In such case, USI driver is used to initialize USI
registers, and it also calls of_platform_populate() to instantiate all
sub-nodes (e.g. serial node) of USI node. When serial driver is
built-in, but USI driver is a module, and CONFIG_SERIAL_SAMSUNG_CONSOLE
is enabled, next call chain will happen when loading USI module from
user space:

    usi_init
      v
    usi_probe
      v
    of_platform_populate
      v
    s3c24xx_serial_probe
      v
    uart_add_one_port
      v
    uart_configure_port
      v
    register_console
      v
    try_enable_new_console
      v
    s3c24xx_serial_console_setup

But because the serial driver is built-in, and
s3c24xx_serial_console_setup() is marked with __init keyword, that
symbol will discarded and long gone by that time already, causing failed
paging request.

That happens during the next config combination:

    EXYNOS_USI=m
    SERIAL_SAMSUNG=y
    SERIAL_SAMSUNG_CONSOLE=y

That config should be completely possible, so rather than limiting
SERIAL_SAMSUNG choice to "m" only when USI=m, remove __init keyword for
all affected functions.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v3:
  - (none)

Changes in v2:
  - This patch is added in v2

 drivers/tty/serial/samsung_tty.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 61ccb359620a..d002a4e48ed9 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -2500,7 +2500,8 @@ s3c24xx_serial_console_write(struct console *co, const char *s,
 	uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar);
 }
 
-static void __init
+/* Shouldn't be __init, as it can be instantiated from other module */
+static void
 s3c24xx_serial_get_options(struct uart_port *port, int *baud,
 			   int *parity, int *bits)
 {
@@ -2563,7 +2564,8 @@ s3c24xx_serial_get_options(struct uart_port *port, int *baud,
 	}
 }
 
-static int __init
+/* Shouldn't be __init, as it can be instantiated from other module */
+static int
 s3c24xx_serial_console_setup(struct console *co, char *options)
 {
 	struct uart_port *port;
-- 
2.30.2

Re: [PATCH v3 5/5] tty: serial: samsung: Fix console registration from module

[Krzysztof Kozlowski]

On 04/12/2021 20:57, Sam Protsenko wrote:
> On modern Exynos SoCs (like Exynos850) the UART can be implemented as a
> part of USI IP-core. In such case, USI driver is used to initialize USI
> registers, and it also calls of_platform_populate() to instantiate all
> sub-nodes (e.g. serial node) of USI node. When serial driver is
> built-in, but USI driver is a module, and CONFIG_SERIAL_SAMSUNG_CONSOLE
> is enabled, next call chain will happen when loading USI module from
> user space:
> 
>     usi_init
>       v
>     usi_probe
>       v
>     of_platform_populate
>       v
>     s3c24xx_serial_probe
>       v
>     uart_add_one_port
>       v
>     uart_configure_port
>       v
>     register_console
>       v
>     try_enable_new_console
>       v
>     s3c24xx_serial_console_setup
> 
> But because the serial driver is built-in, and
> s3c24xx_serial_console_setup() is marked with __init keyword, that
> symbol will discarded and long gone by that time already, causing failed
> paging request.
> 
> That happens during the next config combination:
> 
>     EXYNOS_USI=m
>     SERIAL_SAMSUNG=y
>     SERIAL_SAMSUNG_CONSOLE=y
> 
> That config should be completely possible, so rather than limiting
> SERIAL_SAMSUNG choice to "m" only when USI=m, remove __init keyword for
> all affected functions.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v3:
>   - (none)
> 
> Changes in v2:
>   - This patch is added in v2
> 
>  drivers/tty/serial/samsung_tty.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

Re: [PATCH v3 0/5] soc: samsung: Add USI driver

[Krzysztof Kozlowski]

On 04/12/2021 20:57, Sam Protsenko wrote:
> USIv2 IP-core provides selectable serial protocol (UART, SPI or
> High-Speed I2C); only one can be chosen at a time. This series
> implements USIv2 driver, which allows one to select particular USI
> function in device tree, and also performs USI block initialization.
> 
> With that driver implemented, it's not needed to do USI initialization
> in protocol drivers anymore, so that code is removed from the serial
> driver.
> 
> Because USI driver is tristate (can be built as a module), serial driver
> was reworked so it's possible to use its console part as a module too.
> This way we can load serial driver module from user space and still have
> serial console functional.
> 
> Design features:
>   - "reg" property contains USI registers start address (0xc0 offset);
>     it's used in the driver to access USI_CON and USI_OPTION registers.
>     This way all USI initialization (reset, HWACG, etc) can be done in
>     USIv2 driver separately, rather than duplicating that code over
>     UART/SPI/I2C drivers
>   - System Register (system controller node) and its SW_CONF register
>     offset are provided in "samsung,sysreg" property; it's used to
>     select USI function (protocol to be used)
>   - USI function is specified in "samsung,mode" property; integer value
>     is used to simplify parsing
>   - there is "samsung,clkreq-on" bool property, which makes driver
>     disable HWACG control (needed for UART to work properly)
>   - PCLK and IPCLK clocks are both provided to USI node; apparently both
>     need to be enabled to access USI registers
>   - protocol nodes are embedded (as a child nodes) in USI node; it
>     allows correct init order, and reflects HW properly
>   - USI driver is a tristate: can be also useful from Android GKI
>     requirements point of view
>   - driver functions are implemented with further development in mind:
>     - we might want to add some DebugFs interface later
>     - some functions might need to be revealed to serial drivers with
>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
>       instance
>     - another USI revisions could be added (like USIv1)
> 
> Changes in v3:
>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
>   - Used clk_bulk API instead of handling each clock separately
>   - Spell check fixes and coding style fixes
>   - Improved dt-bindings doc
> 
> Changes in v2:
>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
>     drivers
>   - Added patch: "tty: serial: samsung: Fix console registration from
>     module"
>   - Combined dt-bindings doc and dt-bindings header patches
>   - Reworked USI driver to be ready for USIv1 addition
>   - Improved dt-bindings
>   - Added USI_V2_NONE mode value
> 
> Sam Protsenko (5):
>   dt-bindings: soc: samsung: Add Exynos USI bindings
>   soc: samsung: Add USI driver
>   tty: serial: samsung: Remove USI initialization
>   tty: serial: samsung: Enable console as module
>   tty: serial: samsung: Fix console registration from module
> 

All this looks good to me. The serial driver changes should come
together with this one (usi driver is now a dependency for them). If I
am correct, mention this please in future cover letter (if there is such).

I will still need DTSI changes for Exynos Auto v9 and confirmation that
is not being used downstream and breaking DTB ABI is okay. Because this
will be a non-bisctable and also a DTB ABI break.

Best regards,
Krzysztof

Re: [PATCH v3 0/5] soc: samsung: Add USI driver

[Krzysztof Kozlowski]

On 05/12/2021 17:24, Krzysztof Kozlowski wrote:
> On 04/12/2021 20:57, Sam Protsenko wrote:
>> USIv2 IP-core provides selectable serial protocol (UART, SPI or
>> High-Speed I2C); only one can be chosen at a time. This series
>> implements USIv2 driver, which allows one to select particular USI
>> function in device tree, and also performs USI block initialization.
>>
>> With that driver implemented, it's not needed to do USI initialization
>> in protocol drivers anymore, so that code is removed from the serial
>> driver.
>>
>> Because USI driver is tristate (can be built as a module), serial driver
>> was reworked so it's possible to use its console part as a module too.
>> This way we can load serial driver module from user space and still have
>> serial console functional.
>>
>> Design features:
>>   - "reg" property contains USI registers start address (0xc0 offset);
>>     it's used in the driver to access USI_CON and USI_OPTION registers.
>>     This way all USI initialization (reset, HWACG, etc) can be done in
>>     USIv2 driver separately, rather than duplicating that code over
>>     UART/SPI/I2C drivers
>>   - System Register (system controller node) and its SW_CONF register
>>     offset are provided in "samsung,sysreg" property; it's used to
>>     select USI function (protocol to be used)
>>   - USI function is specified in "samsung,mode" property; integer value
>>     is used to simplify parsing
>>   - there is "samsung,clkreq-on" bool property, which makes driver
>>     disable HWACG control (needed for UART to work properly)
>>   - PCLK and IPCLK clocks are both provided to USI node; apparently both
>>     need to be enabled to access USI registers
>>   - protocol nodes are embedded (as a child nodes) in USI node; it
>>     allows correct init order, and reflects HW properly
>>   - USI driver is a tristate: can be also useful from Android GKI
>>     requirements point of view
>>   - driver functions are implemented with further development in mind:
>>     - we might want to add some DebugFs interface later
>>     - some functions might need to be revealed to serial drivers with
>>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
>>       instance
>>     - another USI revisions could be added (like USIv1)
>>
>> Changes in v3:
>>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
>>   - Used clk_bulk API instead of handling each clock separately
>>   - Spell check fixes and coding style fixes
>>   - Improved dt-bindings doc
>>
>> Changes in v2:
>>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
>>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
>>     drivers
>>   - Added patch: "tty: serial: samsung: Fix console registration from
>>     module"
>>   - Combined dt-bindings doc and dt-bindings header patches
>>   - Reworked USI driver to be ready for USIv1 addition
>>   - Improved dt-bindings
>>   - Added USI_V2_NONE mode value
>>
>> Sam Protsenko (5):
>>   dt-bindings: soc: samsung: Add Exynos USI bindings
>>   soc: samsung: Add USI driver
>>   tty: serial: samsung: Remove USI initialization
>>   tty: serial: samsung: Enable console as module
>>   tty: serial: samsung: Fix console registration from module
>>
> 
> All this looks good to me. The serial driver changes should come
> together with this one (usi driver is now a dependency for them). If I
> am correct, mention this please in future cover letter (if there is such).
> 
> I will still need DTSI changes for Exynos Auto v9 and confirmation that
> is not being used downstream and breaking DTB ABI is okay. Because this
> will be a non-bisctable and also a DTB ABI break.

+CC Arnd and Olof,

Dear Arnd and Olof,

The patchset discussed here reworks recently added USI code to Samsung
Exynos UART driver in a non-bisectable and ABI-breaking way. The
existing code in serial driver was added in v5.15-rc1, however first
user - Exyons Auto v9 - appeared in v5.16-rc1.

The bisectability and ABI break will affect only newly upstreamed
Samsung Exynos SoC, so for now only Exynos Auto v9.

The early code has some drawbacks and limitations which came up now when
we want to extend the USI code to support more blocks (I2C, SPI) and
devices (including older Exynos chipsets). Therefore I am planning to
make an ABI break of this features because:
1. The code was added recently (v5.15-rc1) and users even later (v5.16-rc1).
2. Even though code was merged, I consider it still development phase.
Kernel development goes very fast and we do not defer patches waiting
for perfect solution.
3. There are no known out-of-tree users of this because this is fairly
new. For this I am waiting for confirmation from Chanho (and/or other
Samsung folks). I don't expect there are out of tree users because any
mobile or automotive product will take Samsung vendor sources and recent
kernels are not a base for Samsung vendor kernels.
4. Keeping it backwards compatible will be a pain.

The alternative is to keep old code in Samsung Serial driver and try to
detect the case when driver is probed in old way (with old DTB). This
won't be trivial, because there will be no properties change, but
devicetree hierarchy change like:

from:
soc@0 {
  serial@0xabcd0123 {
    ...
  }
}

into:
soc@0 {
  usi@0x12345678 {
    serial@0xabcd0123 {
      ...
    }
  }
}


Are you okay with such planned ABI break?

Best regards,
Krzysztof

RE: [PATCH v3 0/5] soc: samsung: Add USI driver

[Chanho Park]

Hi,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: Tuesday, December 7, 2021 3:24 AM
> To: Arnd Bergmann <arnd@arndb.de>; Olof Johansson <olof@lixom.net>
> Cc: Jaewon Kim <jaewon02.kim@samsung.com>; Chanho Park
> <chanho61.park@samsung.com>; David Virag <virag.david003@gmail.com>;
> Youngmin Nam <youngmin.nam@samsung.com>; Sam Protsenko
> <semen.protsenko@linaro.org>; devicetree@vger.kernel.org; linux-
> serial@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Greg Kroah-
> Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; linux-
> samsung-soc@vger.kernel.org; Rob Herring <robh+dt@kernel.org>
> Subject: Re: [PATCH v3 0/5] soc: samsung: Add USI driver
> 
> On 05/12/2021 17:24, Krzysztof Kozlowski wrote:
> > On 04/12/2021 20:57, Sam Protsenko wrote:
> >> USIv2 IP-core provides selectable serial protocol (UART, SPI or
> >> High-Speed I2C); only one can be chosen at a time. This series
> >> implements USIv2 driver, which allows one to select particular USI
> >> function in device tree, and also performs USI block initialization.
> >>
> >> With that driver implemented, it's not needed to do USI
> >> initialization in protocol drivers anymore, so that code is removed
> >> from the serial driver.
> >>
> >> Because USI driver is tristate (can be built as a module), serial
> >> driver was reworked so it's possible to use its console part as a
> module too.
> >> This way we can load serial driver module from user space and still
> >> have serial console functional.
> >>
> >> Design features:
> >>   - "reg" property contains USI registers start address (0xc0 offset);
> >>     it's used in the driver to access USI_CON and USI_OPTION registers.
> >>     This way all USI initialization (reset, HWACG, etc) can be done in
> >>     USIv2 driver separately, rather than duplicating that code over
> >>     UART/SPI/I2C drivers
> >>   - System Register (system controller node) and its SW_CONF register
> >>     offset are provided in "samsung,sysreg" property; it's used to
> >>     select USI function (protocol to be used)
> >>   - USI function is specified in "samsung,mode" property; integer value
> >>     is used to simplify parsing
> >>   - there is "samsung,clkreq-on" bool property, which makes driver
> >>     disable HWACG control (needed for UART to work properly)
> >>   - PCLK and IPCLK clocks are both provided to USI node; apparently
> both
> >>     need to be enabled to access USI registers
> >>   - protocol nodes are embedded (as a child nodes) in USI node; it
> >>     allows correct init order, and reflects HW properly
> >>   - USI driver is a tristate: can be also useful from Android GKI
> >>     requirements point of view
> >>   - driver functions are implemented with further development in mind:
> >>     - we might want to add some DebugFs interface later
> >>     - some functions might need to be revealed to serial drivers with
> >>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
> >>       instance
> >>     - another USI revisions could be added (like USIv1)
> >>
> >> Changes in v3:
> >>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-
> usi
> >>   - Used clk_bulk API instead of handling each clock separately
> >>   - Spell check fixes and coding style fixes
> >>   - Improved dt-bindings doc
> >>
> >> Changes in v2:
> >>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
> >>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
> >>     drivers
> >>   - Added patch: "tty: serial: samsung: Fix console registration from
> >>     module"
> >>   - Combined dt-bindings doc and dt-bindings header patches
> >>   - Reworked USI driver to be ready for USIv1 addition
> >>   - Improved dt-bindings
> >>   - Added USI_V2_NONE mode value
> >>
> >> Sam Protsenko (5):
> >>   dt-bindings: soc: samsung: Add Exynos USI bindings
> >>   soc: samsung: Add USI driver
> >>   tty: serial: samsung: Remove USI initialization
> >>   tty: serial: samsung: Enable console as module
> >>   tty: serial: samsung: Fix console registration from module
> >>
> >
> > All this looks good to me. The serial driver changes should come
> > together with this one (usi driver is now a dependency for them). If I
> > am correct, mention this please in future cover letter (if there is
> such).
> >
> > I will still need DTSI changes for Exynos Auto v9 and confirmation
> > that is not being used downstream and breaking DTB ABI is okay.
> > Because this will be a non-bisctable and also a DTB ABI break.
> 
> +CC Arnd and Olof,
> 
> Dear Arnd and Olof,
> 
> The patchset discussed here reworks recently added USI code to Samsung
> Exynos UART driver in a non-bisectable and ABI-breaking way. The existing
> code in serial driver was added in v5.15-rc1, however first user - Exyons
> Auto v9 - appeared in v5.16-rc1.
> 
> The bisectability and ABI break will affect only newly upstreamed Samsung
> Exynos SoC, so for now only Exynos Auto v9.
> 
> The early code has some drawbacks and limitations which came up now when
> we want to extend the USI code to support more blocks (I2C, SPI) and
> devices (including older Exynos chipsets). Therefore I am planning to make
> an ABI break of this features because:
> 1. The code was added recently (v5.15-rc1) and users even later (v5.16-
> rc1).
> 2. Even though code was merged, I consider it still development phase.
> Kernel development goes very fast and we do not defer patches waiting for
> perfect solution.
> 3. There are no known out-of-tree users of this because this is fairly new.
> For this I am waiting for confirmation from Chanho (and/or other Samsung
> folks). I don't expect there are out of tree users because any mobile or
> automotive product will take Samsung vendor sources and recent kernels are
> not a base for Samsung vendor kernels.

This will be not an issue in my case. Please apply below patch with this series.
https://lore.kernel.org/linux-samsung-soc/20211208003946.139423-1-chanho61.park@samsung.com/T/#u

Best Regards,
Chanho Park

RE: [PATCH v3 0/5] soc: samsung: Add USI driver

[Chanho Park]

Hi,

> -----Original Message-----
> From: Sam Protsenko <semen.protsenko@linaro.org>
> Sent: Sunday, December 5, 2021 4:58 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>; Rob Herring
> <robh+dt@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jaewon Kim <jaewon02.kim@samsung.com>; Chanho Park
> <chanho61.park@samsung.com>; David Virag <virag.david003@gmail.com>;
> Youngmin Nam <youngmin.nam@samsung.com>; devicetree@vger.kernel.org;
> linux-serial@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> Subject: [PATCH v3 0/5] soc: samsung: Add USI driver
> 
> USIv2 IP-core provides selectable serial protocol (UART, SPI or High-Speed
> I2C); only one can be chosen at a time. This series implements USIv2
> driver, which allows one to select particular USI function in device tree,
> and also performs USI block initialization.
> 
> With that driver implemented, it's not needed to do USI initialization in
> protocol drivers anymore, so that code is removed from the serial driver.
> 
> Because USI driver is tristate (can be built as a module), serial driver
> was reworked so it's possible to use its console part as a module too.
> This way we can load serial driver module from user space and still have
> serial console functional.
> 
> Design features:
>   - "reg" property contains USI registers start address (0xc0 offset);
>     it's used in the driver to access USI_CON and USI_OPTION registers.
>     This way all USI initialization (reset, HWACG, etc) can be done in
>     USIv2 driver separately, rather than duplicating that code over
>     UART/SPI/I2C drivers
>   - System Register (system controller node) and its SW_CONF register
>     offset are provided in "samsung,sysreg" property; it's used to
>     select USI function (protocol to be used)
>   - USI function is specified in "samsung,mode" property; integer value
>     is used to simplify parsing
>   - there is "samsung,clkreq-on" bool property, which makes driver
>     disable HWACG control (needed for UART to work properly)
>   - PCLK and IPCLK clocks are both provided to USI node; apparently both
>     need to be enabled to access USI registers
>   - protocol nodes are embedded (as a child nodes) in USI node; it
>     allows correct init order, and reflects HW properly
>   - USI driver is a tristate: can be also useful from Android GKI
>     requirements point of view
>   - driver functions are implemented with further development in mind:
>     - we might want to add some DebugFs interface later
>     - some functions might need to be revealed to serial drivers with
>       EXPORT_SYMBOL(), and provide somehow pointer to needed USI driver
>       instance
>     - another USI revisions could be added (like USIv1)
> 
> Changes in v3:
>   - Renamed compatible from samsung,exynos-usi-v2 to samsung,exynos850-usi
>   - Used clk_bulk API instead of handling each clock separately
>   - Spell check fixes and coding style fixes
>   - Improved dt-bindings doc
> 
> Changes in v2:
>   - Renamed all 'usi_v2' wording to just 'usi' everywhere
>   - Removed patches adding dependency on EXYNOS_USI for UART/I2C/SPI
>     drivers
>   - Added patch: "tty: serial: samsung: Fix console registration from
>     module"
>   - Combined dt-bindings doc and dt-bindings header patches
>   - Reworked USI driver to be ready for USIv1 addition
>   - Improved dt-bindings
>   - Added USI_V2_NONE mode value
> 
> Sam Protsenko (5):
>   dt-bindings: soc: samsung: Add Exynos USI bindings
>   soc: samsung: Add USI driver
>   tty: serial: samsung: Remove USI initialization
>   tty: serial: samsung: Enable console as module
>   tty: serial: samsung: Fix console registration from module

Tested-by: Chanho Park <chanho61.park@samsung.com> with below patch.
https://lore.kernel.org/linux-samsung-soc/20211208003946.139423-1-chanho61.p
ark@samsung.com/T/#u

Thanks.

Best Regards,
Chanho Park