[PATCH v2 0/6] can: mcan: Add MCAN support for FSD SoC

[PATCH v2 0/6] can: mcan: Add MCAN support for FSD SoC

[Vivek Yadav]

Add support for MCAN instances present on the FSD platform.
Also add support for handling error correction code (ECC) for MCAN message
RAM.

Sriranjani P (2):
  dt-bindings: Document the SYSREG specific compatibles found on FSD SoC
  arm64: dts: fsd: add sysreg device node

Vivek Yadav (4):
  dt-bindings: can: mcan: Add ECC functionality to message ram
  arm64: dts: fsd: Add MCAN device node
  can: m_can: Add ECC functionality for message RAM
  arm64: dts: fsd: Add support for error correction code for message RAM

 .../devicetree/bindings/arm/tesla-sysreg.yaml | 50 +++++++++++
 .../bindings/net/can/bosch,m_can.yaml         | 31 +++++++
 MAINTAINERS                                   |  1 +
 arch/arm64/boot/dts/tesla/fsd-evb.dts         | 16 ++++
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi    | 28 +++++++
 arch/arm64/boot/dts/tesla/fsd.dtsi            | 82 +++++++++++++++++++
 drivers/net/can/m_can/m_can.c                 | 48 ++++++++++-
 drivers/net/can/m_can/m_can.h                 | 17 ++++
 drivers/net/can/m_can/m_can_platform.c        | 76 ++++++++++++++++-
 9 files changed, 343 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/tesla-sysreg.yaml

-- 
2.17.1

[PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Vivek Yadav]

From: Sriranjani P <sriranjani.p@samsung.com>

Describe the compatible properties for SYSREG controllers found on
FSD SoC.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
---
 .../devicetree/bindings/arm/tesla-sysreg.yaml | 50 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/tesla-sysreg.yaml

diff --git a/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
new file mode 100644
index 000000000000..bbcc6dd75918
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/tesla-sysreg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tesla Full Self-Driving platform's system registers
+
+maintainers:
+  - Alim Akhtar <alim.akhtar@samsung.com>
+
+description: |
+  This is a system control registers block, providing multiple low level
+  platform functions like board detection and identification, software
+  interrupt generation.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - tesla,sysreg_fsys0
+              - tesla,sysreg_peric
+          - const: syscon
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      sysreg_fsys0: system-controller@15030000 {
+            compatible = "tesla,sysreg_fsys0", "syscon";
+            reg = <0x0 0x15030000 0x0 0x1000>;
+      };
+
+      sysreg_peric: system-controller@14030000 {
+            compatible = "tesla,sysreg_peric", "syscon";
+            reg = <0x0 0x14030000 0x0 0x1000>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a198da986146..56995e7d63ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2943,6 +2943,7 @@ M:	linux-fsd@tesla.com
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 L:	linux-samsung-soc@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
 F:	arch/arm64/boot/dts/tesla*
 
 ARM/TETON BGA MACHINE SUPPORT
-- 
2.17.1

[PATCH v2 2/6] dt-bindings: can: mcan: Add ECC functionality to message ram

[Vivek Yadav]

Whenever the data is transferred or stored on message ram, there are
inherent risks of it being lost or corruption known as single-bit errors.

ECC constantly scans data as it is processed to the message ram, using a
method known as parity checking and raise the error signals for corruption.

Add error correction code config property to enable/disable the
error correction code (ECC) functionality for Message RAM used to create
valid ECC checksums.

Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
---
 .../bindings/net/can/bosch,m_can.yaml         | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
index 26aa0830eea1..91dc458ec33f 100644
--- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
+++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
@@ -50,6 +50,12 @@ properties:
       - const: hclk
       - const: cclk
 
+  tesla,mram-ecc-cfg:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Handle to system control region that contains the ECC INIT register
+      and register offset to the ECC INIT register.
+
   bosch,mram-cfg:
     description: |
       Message RAM configuration data.
@@ -149,4 +155,29 @@ examples:
       };
     };
 
+  # Example 2: m_can on the FSD SoC
+  - |
+    #include <dt-bindings/clock/fsd-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      can@14088000 {
+        compatible = "bosch,m_can";
+        reg = <0x0 0x14088000 0x0 0x0200>,
+              <0x0 0x14080000 0x0 0x8000>;
+        reg-names = "m_can", "message_ram";
+        interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "int0", "int1";
+        pinctrl-names = "default";
+        pinctrl-0 = <&m_can0_bus>;
+        clocks = <&clock_peric PERIC_MCAN0_IPCLKPORT_PCLK>,
+                 <&clock_peric PERIC_MCAN0_IPCLKPORT_CCLK>;
+        clock-names = "hclk", "cclk";
+        tesla,mram-ecc-cfg = <&sysreg_peric 0x708>;
+        bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+      };
+    };
 ...
-- 
2.17.1

[PATCH v2 3/6] arm64: dts: fsd: add sysreg device node

[Vivek Yadav]

From: Sriranjani P <sriranjani.p@samsung.com>

Add SYSREG controller device node, which is available in PERIC and FSYS0
block of FSD SoC.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index f35bc5a288c2..3d8ebbfc27f4 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -518,6 +518,16 @@
 				"dout_cmu_fsys1_shared0div4";
 		};
 
+		sysreg_peric: system-controller@14030000 {
+			compatible = "tesla,sysreg_peric", "syscon";
+			reg = <0x0 0x14030000 0x0 0x1000>;
+		};
+
+		sysreg_fsys0: system-controller@15030000 {
+			compatible = "tesla,sysreg_fsys0", "syscon";
+			reg = <0x0 0x15030000 0x0 0x1000>;
+		};
+
 		mdma0: dma-controller@10100000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x0 0x10100000 0x0 0x1000>;
-- 
2.17.1

[PATCH v2 4/6] arm64: dts: fsd: Add MCAN device node

[Vivek Yadav]

Add MCAN device node and enable the same for FSD platform.
This also adds the required pin configuration for the same.

Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd-evb.dts      | 16 +++++
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 28 +++++++++
 arch/arm64/boot/dts/tesla/fsd.dtsi         | 68 ++++++++++++++++++++++
 3 files changed, 112 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index 1db6ddf03f01..af3862e9fe3b 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -34,6 +34,22 @@
 	clock-frequency = <24000000>;
 };
 
+&m_can0 {
+	status = "okay";
+};
+
+&m_can1 {
+	status = "okay";
+};
+
+&m_can2 {
+	status = "okay";
+};
+
+&m_can3 {
+	status = "okay";
+};
+
 &serial_0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
index d0abb9aa0e9e..bb5289ebfef3 100644
--- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
@@ -339,6 +339,34 @@
 		samsung,pin-pud = <FSD_PIN_PULL_UP>;
 		samsung,pin-drv = <FSD_PIN_DRV_LV1>;
 	};
+
+	m_can0_bus: m-can0-bus-pins {
+		samsung,pins = "gpd0-0", "gpd0-1";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	m_can1_bus: m-can1-bus-pins {
+		samsung,pins = "gpd0-2", "gpd0-3";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	m_can2_bus: m-can2-bus-pins {
+		samsung,pins = "gpd0-4", "gpd0-5";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	m_can3_bus: m-can3-bus-pins {
+		samsung,pins = "gpd0-6", "gpd0-7";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
 };
 
 &pinctrl_pmu {
diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index 3d8ebbfc27f4..154fd3fc5895 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -765,6 +765,74 @@
 			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		m_can0: can@14088000 {
+			compatible = "bosch,m_can";
+			reg = <0x0 0x14088000 0x0 0x0200>,
+				<0x0 0x14080000 0x0 0x8000>;
+			reg-names = "m_can", "message_ram";
+			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "int0", "int1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&m_can0_bus>;
+			clocks = <&clock_peric PERIC_MCAN0_IPCLKPORT_PCLK>,
+				<&clock_peric PERIC_MCAN0_IPCLKPORT_CCLK>;
+			clock-names = "hclk", "cclk";
+			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+			status = "disabled";
+		};
+
+		m_can1: can@14098000 {
+			compatible = "bosch,m_can";
+			reg = <0x0 0x14098000 0x0 0x0200>,
+				<0x0 0x14090000 0x0 0x8000>;
+			reg-names = "m_can", "message_ram";
+			interrupts = <GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "int0", "int1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&m_can1_bus>;
+			clocks = <&clock_peric PERIC_MCAN1_IPCLKPORT_PCLK>,
+				<&clock_peric PERIC_MCAN1_IPCLKPORT_CCLK>;
+			clock-names = "hclk", "cclk";
+			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+			status = "disabled";
+		};
+
+		m_can2: can@140a8000 {
+			compatible = "bosch,m_can";
+			reg = <0x0 0x140a8000 0x0 0x0200>,
+				<0x0 0x140a0000 0x0 0x8000>;
+			reg-names = "m_can", "message_ram";
+			interrupts = <GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "int0", "int1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&m_can2_bus>;
+			clocks = <&clock_peric PERIC_MCAN2_IPCLKPORT_PCLK>,
+				<&clock_peric PERIC_MCAN2_IPCLKPORT_CCLK>;
+			clock-names = "hclk", "cclk";
+			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+			status = "disabled";
+		};
+
+		m_can3: can@140b8000 {
+			compatible = "bosch,m_can";
+			reg = <0x0 0x140b8000 0x0 0x0200>,
+				<0x0 0x140b0000 0x0 0x8000>;
+			reg-names = "m_can", "message_ram";
+			interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "int0", "int1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&m_can3_bus>;
+			clocks = <&clock_peric PERIC_MCAN3_IPCLKPORT_PCLK>,
+				<&clock_peric PERIC_MCAN3_IPCLKPORT_CCLK>;
+			clock-names = "hclk", "cclk";
+			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+			status = "disabled";
+		};
+
 		spi_0: spi@14140000 {
 			compatible = "tesla,fsd-spi";
 			reg = <0x0 0x14140000 0x0 0x100>;
-- 
2.17.1

[PATCH v2 5/6] can: m_can: Add ECC functionality for message RAM

[Vivek Yadav]

Whenever MCAN Buffers and FIFOs are stored on message ram, there are
inherent risks of corruption known as single-bit errors.

Enable error correction code (ECC) data integrity check for Message RAM
to create valid ECC checksums.

ECC uses a respective number of bits, which are added to each word as a
parity and that will raise the error signal on the corruption in the
Interrupt Register(IR).

This indicates either bit error detected and Corrected(BEC) or No bit
error detected when reading from Message RAM.

Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
---
 drivers/net/can/m_can/m_can.c          | 48 +++++++++++++++-
 drivers/net/can/m_can/m_can.h          | 17 ++++++
 drivers/net/can/m_can/m_can_platform.c | 76 ++++++++++++++++++++++++--
 3 files changed, 135 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a776cab1a5a4..ddff615ccad4 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -307,6 +307,14 @@ enum m_can_reg {
 #define TX_EVENT_MM_MASK	GENMASK(31, 24)
 #define TX_EVENT_TXTS_MASK	GENMASK(15, 0)
 
+/* ECC Config Bits */
+#define MCAN_ECC_CFG_VALID    BIT(5)
+#define MCAN_ECC_ENABLE       BIT(3)
+#define MCAN_ECC_INIT_ENABLE  BIT(1)
+#define MCAN_ECC_INIT_DONE    BIT(0)
+#define MCAN_ECC_REG_MASK     GENMASK(5, 0)
+#define MCAN_ECC_INIT_TIMEOUT 100
+
 /* The ID and DLC registers are adjacent in M_CAN FIFO memory,
  * and we can save a (potentially slow) bus round trip by combining
  * reads and writes to them.
@@ -1516,9 +1524,9 @@ static int m_can_dev_setup(struct m_can_classdev *cdev)
 	}
 
 	if (cdev->ops->init)
-		cdev->ops->init(cdev);
+		err = cdev->ops->init(cdev);
 
-	return 0;
+	return err;
 }
 
 static void m_can_stop(struct net_device *dev)
@@ -1535,6 +1543,39 @@ static void m_can_stop(struct net_device *dev)
 	cdev->can.state = CAN_STATE_STOPPED;
 }
 
+int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, bool enable)
+{
+	struct  m_can_ecc_regmap *ecc_cfg = &cdev->ecc_cfg_sys;
+	int val, ret;
+
+	val = FIELD_PREP(MCAN_ECC_REG_MASK, MCAN_ECC_ENABLE |
+			 MCAN_ECC_CFG_VALID | MCAN_ECC_INIT_ENABLE);
+	regmap_clear_bits(ecc_cfg->syscon, ecc_cfg->reg, val);
+
+	if (enable) {
+		val = FIELD_PREP(MCAN_ECC_REG_MASK, MCAN_ECC_ENABLE |
+				 MCAN_ECC_INIT_ENABLE);
+		regmap_set_bits(ecc_cfg->syscon, ecc_cfg->reg, val);
+	}
+
+	/* after enable or disable, valid flag need to be set*/
+	val = FIELD_PREP(MCAN_ECC_REG_MASK, MCAN_ECC_CFG_VALID);
+	regmap_set_bits(ecc_cfg->syscon, ecc_cfg->reg, val);
+
+	if (enable) {
+		/* Poll for completion */
+		ret = regmap_read_poll_timeout(ecc_cfg->syscon, ecc_cfg->reg,
+					       val,
+					       (val & MCAN_ECC_INIT_DONE), 5,
+					       MCAN_ECC_INIT_TIMEOUT);
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int m_can_close(struct net_device *dev)
 {
 	struct m_can_classdev *cdev = netdev_priv(dev);
@@ -1557,6 +1598,9 @@ static int m_can_close(struct net_device *dev)
 	if (cdev->is_peripheral)
 		can_rx_offload_disable(&cdev->offload);
 
+	if (cdev->ops->deinit)
+		cdev->ops->deinit(cdev);
+
 	close_candev(dev);
 
 	phy_power_off(cdev->transceiver);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 401410022823..9821b135a2be 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -19,6 +19,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/of.h>
@@ -26,6 +27,7 @@
 #include <linux/phy/phy.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 
@@ -52,12 +54,23 @@ enum m_can_mram_cfg {
 	MRAM_CFG_NUM,
 };
 
+enum m_can_ecc_cfg {
+	ECC_DISABLE = 0,
+	ECC_ENABLE,
+};
+
 /* address offset and element number for each FIFO/Buffer in the Message RAM */
 struct mram_cfg {
 	u16 off;
 	u8  num;
 };
 
+struct m_can_ecc_regmap {
+	struct regmap *syscon;  /* for mram ecc ctrl. reg. access */
+	unsigned int reg;       /* register index within syscon */
+	u8 ecc_cfg_flag;
+};
+
 struct m_can_classdev;
 struct m_can_ops {
 	/* Device specific call backs */
@@ -68,6 +81,7 @@ struct m_can_ops {
 	int (*write_fifo)(struct m_can_classdev *cdev, int addr_offset,
 			  const void *val, size_t val_count);
 	int (*init)(struct m_can_classdev *cdev);
+	int (*deinit)(struct m_can_classdev *cdev);
 };
 
 struct m_can_classdev {
@@ -92,7 +106,9 @@ struct m_can_classdev {
 	int pm_clock_support;
 	int is_peripheral;
 
+	struct m_can_ecc_regmap ecc_cfg_sys; /* ecc config via syscon regmap */
 	struct mram_cfg mcfg[MRAM_CFG_NUM];
+	u8 mram_cfg_flag;
 };
 
 struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, int sizeof_priv);
@@ -104,4 +120,5 @@ int m_can_init_ram(struct m_can_classdev *priv);
 
 int m_can_class_suspend(struct device *dev);
 int m_can_class_resume(struct device *dev);
+int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, bool enable);
 #endif	/* _CAN_M_H_ */
diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index b5a5bedb3116..1281214a3f43 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -67,11 +67,83 @@ static int iomap_write_fifo(struct m_can_classdev *cdev, int offset,
 	return 0;
 }
 
+static int m_can_plat_init(struct m_can_classdev *cdev)
+{
+	struct  m_can_ecc_regmap *ecc_cfg = &cdev->ecc_cfg_sys;
+	struct device_node *np = cdev->dev->of_node;
+	int ret = 0;
+
+	if (cdev->mram_cfg_flag != ECC_ENABLE) {
+		/* Initialize mcan message ram */
+		ret = m_can_init_ram(cdev);
+
+		if (ret)
+			return ret;
+
+		cdev->mram_cfg_flag = ECC_ENABLE;
+	}
+
+	if (ecc_cfg->ecc_cfg_flag != ECC_ENABLE) {
+		/* configure error code check for mram */
+		if (!ecc_cfg->syscon) {
+			ecc_cfg->syscon =
+			syscon_regmap_lookup_by_phandle_args(np,
+							     "tesla,mram-ecc-cfg"
+							     , 1,
+							     &ecc_cfg->reg);
+		}
+
+		if (IS_ERR(ecc_cfg->syscon)) {
+			dev_err(cdev->dev, "couldn't get the syscon reg!\n");
+			goto ecc_failed;
+		}
+
+		if (!ecc_cfg->reg) {
+			dev_err(cdev->dev,
+				"couldn't get the ecc init reg. offset!\n");
+			goto ecc_failed;
+		}
+
+		/* Enable error code check functionality for message ram */
+		if (m_can_config_mram_ecc_check(cdev, ECC_ENABLE))
+			goto ecc_failed;
+
+		ecc_cfg->ecc_cfg_flag = ECC_ENABLE;
+	}
+
+	return 0;
+
+ecc_failed:
+	dev_err(cdev->dev, "Message ram ecc enable config failed\n");
+
+	return 0;
+}
+
+static int m_can_plat_deinit(struct m_can_classdev *cdev)
+{
+	struct  m_can_ecc_regmap *ecc_cfg = &cdev->ecc_cfg_sys;
+
+	if (ecc_cfg->ecc_cfg_flag == ECC_ENABLE) {
+		/* Disable error code check functionality for message ram */
+		if (m_can_config_mram_ecc_check(cdev, ECC_DISABLE)) {
+			dev_err(cdev->dev,
+				"Message ram ecc disable config failed\n");
+			return 0;
+		}
+
+		ecc_cfg->ecc_cfg_flag = ECC_DISABLE;
+	}
+
+	return 0;
+}
+
 static struct m_can_ops m_can_plat_ops = {
 	.read_reg = iomap_read_reg,
 	.write_reg = iomap_write_reg,
 	.write_fifo = iomap_write_fifo,
 	.read_fifo = iomap_read_fifo,
+	.init = m_can_plat_init,
+	.deinit = m_can_plat_deinit,
 };
 
 static int m_can_plat_probe(struct platform_device *pdev)
@@ -140,10 +212,6 @@ static int m_can_plat_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mcan_class);
 
-	ret = m_can_init_ram(mcan_class);
-	if (ret)
-		goto probe_fail;
-
 	pm_runtime_enable(mcan_class->dev);
 	ret = m_can_class_register(mcan_class);
 	if (ret)
-- 
2.17.1

[PATCH v2 6/6] arm64: dts: fsd: Add support for error correction code for message RAM

[Vivek Yadav]

Add mram-ecc-cfg property which indicates the error correction code config
and enable the same for FSD platform.

In FSD, error correction code (ECC) is configured via PERIC SYSREG
registers.

Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index 154fd3fc5895..6483bbf521e5 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -778,6 +778,7 @@
 			clocks = <&clock_peric PERIC_MCAN0_IPCLKPORT_PCLK>,
 				<&clock_peric PERIC_MCAN0_IPCLKPORT_CCLK>;
 			clock-names = "hclk", "cclk";
+			tesla,mram-ecc-cfg = <&sysreg_peric 0x700>;
 			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
 			status = "disabled";
 		};
@@ -795,6 +796,7 @@
 			clocks = <&clock_peric PERIC_MCAN1_IPCLKPORT_PCLK>,
 				<&clock_peric PERIC_MCAN1_IPCLKPORT_CCLK>;
 			clock-names = "hclk", "cclk";
+			tesla,mram-ecc-cfg = <&sysreg_peric 0x704>;
 			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
 			status = "disabled";
 		};
@@ -812,6 +814,7 @@
 			clocks = <&clock_peric PERIC_MCAN2_IPCLKPORT_PCLK>,
 				<&clock_peric PERIC_MCAN2_IPCLKPORT_CCLK>;
 			clock-names = "hclk", "cclk";
+			tesla,mram-ecc-cfg = <&sysreg_peric 0x708>;
 			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
 			status = "disabled";
 		};
@@ -829,6 +832,7 @@
 			clocks = <&clock_peric PERIC_MCAN3_IPCLKPORT_PCLK>,
 				<&clock_peric PERIC_MCAN3_IPCLKPORT_CCLK>;
 			clock-names = "hclk", "cclk";
+			tesla,mram-ecc-cfg = <&sysreg_peric 0x70c>;
 			bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
 			status = "disabled";
 		};
-- 
2.17.1

Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Krzysztof Kozlowski]

On 09/11/2022 11:09, Vivek Yadav wrote:
> From: Sriranjani P <sriranjani.p@samsung.com>
> 

Use subject prefixes matching the subsystem (git log --oneline -- ...).

> Describe the compatible properties for SYSREG controllers found on
> FSD SoC.

This is ARM SoC patch, split it from the patchset.

> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>

Drop the Cc list from commit log. It's not helpful.

> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> ---
>  .../devicetree/bindings/arm/tesla-sysreg.yaml | 50 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> new file mode 100644
> index 000000000000..bbcc6dd75918
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml

arm is only for top level stuff. This goes to soc under tesla or samsung
directory.

> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/tesla-sysreg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Tesla Full Self-Driving platform's system registers
> +
> +maintainers:
> +  - Alim Akhtar <alim.akhtar@samsung.com>
> +
> +description: |
> +  This is a system control registers block, providing multiple low level
> +  platform functions like board detection and identification, software
> +  interrupt generation.
> +
> +properties:
> +  compatible:
> +    oneOf:

No need for oneOf.

> +      - items:
> +          - enum:
> +              - tesla,sysreg_fsys0
> +              - tesla,sysreg_peric

From where did you get underscores in compatibles?

> +          - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      sysreg_fsys0: system-controller@15030000 {
> +            compatible = "tesla,sysreg_fsys0", "syscon";

Use 4 spaces for example indentation.

> +            reg = <0x0 0x15030000 0x0 0x1000>;
> +      };
> +
> +      sysreg_peric: system-controller@14030000 {
> +            compatible = "tesla,sysreg_peric", "syscon";
> +            reg = <0x0 0x14030000 0x0 0x1000>;
> +      };

One example is enough, they are the same.
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a198da986146..56995e7d63ad 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2943,6 +2943,7 @@ M:	linux-fsd@tesla.com
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  L:	linux-samsung-soc@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
>  F:	arch/arm64/boot/dts/tesla*
>  
>  ARM/TETON BGA MACHINE SUPPORT

Best regards,
Krzysztof

Re: [PATCH v2 2/6] dt-bindings: can: mcan: Add ECC functionality to message ram

[Krzysztof Kozlowski]

On 09/11/2022 11:09, Vivek Yadav wrote:
> Whenever the data is transferred or stored on message ram, there are
> inherent risks of it being lost or corruption known as single-bit errors.
> 
> ECC constantly scans data as it is processed to the message ram, using a
> method known as parity checking and raise the error signals for corruption.
> 
> Add error correction code config property to enable/disable the
> error correction code (ECC) functionality for Message RAM used to create
> valid ECC checksums.
> 
> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> ---
>  .../bindings/net/can/bosch,m_can.yaml         | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> index 26aa0830eea1..91dc458ec33f 100644
> --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
> @@ -50,6 +50,12 @@ properties:
>        - const: hclk
>        - const: cclk
>  
> +  tesla,mram-ecc-cfg:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Handle to system control region that contains the ECC INIT register
> +      and register offset to the ECC INIT register.

That's not way to describe syscon phandle. Property name is ok. For the
rest look at:
https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42

Anyway, this looks like SoC-specific hack, so it does not really fit to
the driver. You have to think of something generic.


Best regards,
Krzysztof

Re: [PATCH v2 3/6] arm64: dts: fsd: add sysreg device node

[Krzysztof Kozlowski]

On 09/11/2022 11:09, Vivek Yadav wrote:
> From: Sriranjani P <sriranjani.p@samsung.com>
> 
> Add SYSREG controller device node, which is available in PERIC and FSYS0
> block of FSD SoC.
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>

Drop Cc list from commit msgs. Instead use get_maintainers.pl.

> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> index f35bc5a288c2..3d8ebbfc27f4 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -518,6 +518,16 @@
>  				"dout_cmu_fsys1_shared0div4";
>  		};
>  
> +		sysreg_peric: system-controller@14030000 {
> +			compatible = "tesla,sysreg_peric", "syscon";
> +			reg = <0x0 0x14030000 0x0 0x1000>;

Put it next  to the other system-controller node (and ordered by unit
address against it).

Best regards,
Krzysztof

Re: [PATCH v2 3/6] arm64: dts: fsd: add sysreg device node

[Sam Protsenko]

Hi Vivek,

On Wed, 9 Nov 2022 at 11:54, Vivek Yadav <vivek.2311@samsung.com> wrote:
>
> From: Sriranjani P <sriranjani.p@samsung.com>
>
> Add SYSREG controller device node, which is available in PERIC and FSYS0
> block of FSD SoC.
>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> index f35bc5a288c2..3d8ebbfc27f4 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -518,6 +518,16 @@
>                                 "dout_cmu_fsys1_shared0div4";
>                 };
>
> +               sysreg_peric: system-controller@14030000 {
> +                       compatible = "tesla,sysreg_peric", "syscon";
> +                       reg = <0x0 0x14030000 0x0 0x1000>;

Probably not related to this particular patch, but does the "reg"
really have to have those extra 0x0s? Why it can't be just:

    reg = <0x14030000 0x1000>;

That comment applies to the whole dts/dtsi. Looks like #address-cells
or #size-cells are bigger than they should be, or I missing something?

> +               };
> +
> +               sysreg_fsys0: system-controller@15030000 {
> +                       compatible = "tesla,sysreg_fsys0", "syscon";
> +                       reg = <0x0 0x15030000 0x0 0x1000>;
> +               };
> +
>                 mdma0: dma-controller@10100000 {
>                         compatible = "arm,pl330", "arm,primecell";
>                         reg = <0x0 0x10100000 0x0 0x1000>;
> --
> 2.17.1
>

Re: [PATCH v2 4/6] arm64: dts: fsd: Add MCAN device node

[Krzysztof Kozlowski]

On 09/11/2022 11:09, Vivek Yadav wrote:
> Add MCAN device node and enable the same for FSD platform.
> This also adds the required pin configuration for the same.
> 
> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd-evb.dts      | 16 +++++
>  arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 28 +++++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi         | 68 ++++++++++++++++++++++
>  3 files changed, 112 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 1db6ddf03f01..af3862e9fe3b 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -34,6 +34,22 @@
>  	clock-frequency = <24000000>;
>  };
>  
> +&m_can0 {
> +	status = "okay";
> +};
> +
> +&m_can1 {
> +	status = "okay";
> +};
> +
> +&m_can2 {
> +	status = "okay";
> +};
> +
> +&m_can3 {
> +	status = "okay";
> +};
> +
>  &serial_0 {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> index d0abb9aa0e9e..bb5289ebfef3 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> @@ -339,6 +339,34 @@
>  		samsung,pin-pud = <FSD_PIN_PULL_UP>;
>  		samsung,pin-drv = <FSD_PIN_DRV_LV1>;
>  	};
> +
> +	m_can0_bus: m-can0-bus-pins {
> +		samsung,pins = "gpd0-0", "gpd0-1";
> +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> +		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
> +	};
> +
> +	m_can1_bus: m-can1-bus-pins {
> +		samsung,pins = "gpd0-2", "gpd0-3";
> +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> +		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
> +	};
> +
> +	m_can2_bus: m-can2-bus-pins {
> +		samsung,pins = "gpd0-4", "gpd0-5";
> +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> +		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
> +	};
> +
> +	m_can3_bus: m-can3-bus-pins {
> +		samsung,pins = "gpd0-6", "gpd0-7";
> +		samsung,pin-function = <FSD_PIN_FUNC_2>;
> +		samsung,pin-pud = <FSD_PIN_PULL_UP>;
> +		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
> +	};
>  };
>  
>  &pinctrl_pmu {
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> index 3d8ebbfc27f4..154fd3fc5895 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -765,6 +765,74 @@
>  			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		m_can0: can@14088000 {
> +			compatible = "bosch,m_can";
> +			reg = <0x0 0x14088000 0x0 0x0200>,
> +				<0x0 0x14080000 0x0 0x8000>;

Align with < in line before.

> +			reg-names = "m_can", "message_ram";
> +			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "int0", "int1";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&m_can0_bus>;
> +			clocks = <&clock_peric PERIC_MCAN0_IPCLKPORT_PCLK>,
> +				<&clock_peric PERIC_MCAN0_IPCLKPORT_CCLK>;

The same (unless it's the problem of diff/patch and these are actually
aligned).


Best regards,
Krzysztof

Re: [PATCH v2 5/6] can: m_can: Add ECC functionality for message RAM

[Krzysztof Kozlowski]

On 09/11/2022 11:09, Vivek Yadav wrote:
> Whenever MCAN Buffers and FIFOs are stored on message ram, there are
> inherent risks of corruption known as single-bit errors.
> 
> Enable error correction code (ECC) data integrity check for Message RAM
> to create valid ECC checksums.
> 
> ECC uses a respective number of bits, which are added to each word as a
> parity and that will raise the error signal on the corruption in the
> Interrupt Register(IR).
> 
> This indicates either bit error detected and Corrected(BEC) or No bit
> error detected when reading from Message RAM.
> 
> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>

(...)

>  
> +static int m_can_plat_init(struct m_can_classdev *cdev)
> +{
> +	struct  m_can_ecc_regmap *ecc_cfg = &cdev->ecc_cfg_sys;
> +	struct device_node *np = cdev->dev->of_node;
> +	int ret = 0;
> +
> +	if (cdev->mram_cfg_flag != ECC_ENABLE) {
> +		/* Initialize mcan message ram */
> +		ret = m_can_init_ram(cdev);
> +
> +		if (ret)
> +			return ret;
> +
> +		cdev->mram_cfg_flag = ECC_ENABLE;
> +	}
> +
> +	if (ecc_cfg->ecc_cfg_flag != ECC_ENABLE) {
> +		/* configure error code check for mram */
> +		if (!ecc_cfg->syscon) {
> +			ecc_cfg->syscon =
> +			syscon_regmap_lookup_by_phandle_args(np,
> +							     "tesla,mram-ecc-cfg"
> +							     , 1,

, goes to previous line

> +							     &ecc_cfg->reg);
> +		}
> +
> +		if (IS_ERR(ecc_cfg->syscon)) {
> +			dev_err(cdev->dev, "couldn't get the syscon reg!\n");

Didn't you just break all platforms using ECC?

Best regards,
Krzysztof

Re: [PATCH v2 6/6] arm64: dts: fsd: Add support for error correction code for message RAM

[Krzysztof Kozlowski]

On 09/11/2022 11:09, Vivek Yadav wrote:
> Add mram-ecc-cfg property which indicates the error correction code config
> and enable the same for FSD platform.
> 
> In FSD, error correction code (ECC) is configured via PERIC SYSREG
> registers.
> 
> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> ---

For net-folks: although the DTS patches are here as well, but they must
go via ARM SOC tree, so pick only network/can drivers and bindings when
they are ready.

Best regards,
Krzysztof

RE: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Vivek Yadav]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 09 November 2022 16:39
> To: Vivek Yadav <vivek.2311@samsung.com>; rcsekar@samsung.com;
> krzysztof.kozlowski+dt@linaro.org; wg@grandegger.com;
> mkl@pengutronix.de; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; pankaj.dubey@samsung.com;
> ravi.patel@samsung.com; alim.akhtar@samsung.com; linux-fsd@tesla.com;
> robh+dt@kernel.org
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; devicetree@vger.kernel.org;
> aswani.reddy@samsung.com; sriranjani.p@samsung.com
> Subject: Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific
> compatibles found on FSD SoC
> 
> On 09/11/2022 11:09, Vivek Yadav wrote:
> > From: Sriranjani P <sriranjani.p@samsung.com>
> >
> 
> Use subject prefixes matching the subsystem (git log --oneline -- ...).
> 
Okay, I will add the correct prefixes.
> > Describe the compatible properties for SYSREG controllers found on FSD
> > SoC.
> 
> This is ARM SoC patch, split it from the patchset.
>
I understand this patch is not to be subset of CAN patches, I will send these patches separately.
These patches will be used by EQos patches. As per reference, I am adding the Address link.
https://lore.kernel.org/all/20221104120517.77980-1-sriranjani.p@samsung.com/
 
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
> > Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> > Cc: devicetree@vger.kernel.org
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> 
> Drop the Cc list from commit log. It's not helpful.
> 
Okay, I will remove.
> > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> > ---
> >  .../devicetree/bindings/arm/tesla-sysreg.yaml | 50
> +++++++++++++++++++
> >  MAINTAINERS                                   |  1 +
> >  2 files changed, 51 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> > b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> > new file mode 100644
> > index 000000000000..bbcc6dd75918
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> 
> arm is only for top level stuff. This goes to soc under tesla or samsung
> directory.
> 
Okay, this is specific to Samsung fsd SoC, I will be moving this file to arm/samsung in next patch series. Hope that is fine.
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id:
> > +https://protect2.fireeye.com/v1/url?k=1ad2834a-7b59967c-1ad30805-
> 000b
> > +abff9b5d-1f65584e412e916c&q=1&e=a870a282-632a-4896-ae53-
> 3ecb50f02be5&
> > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Farm%2Ftesla-
> sysreg.yaml%23
> > +$schema:
> > +https://protect2.fireeye.com/v1/url?k=13876e33-720c7b05-1386e57c-
> 000b
> > +abff9b5d-edae3ff711999305&q=1&e=a870a282-632a-4896-ae53-
> 3ecb50f02be5&
> > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: Tesla Full Self-Driving platform's system registers
> > +
> > +maintainers:
> > +  - Alim Akhtar <alim.akhtar@samsung.com>
> > +
> > +description: |
> > +  This is a system control registers block, providing multiple low
> > +level
> > +  platform functions like board detection and identification,
> > +software
> > +  interrupt generation.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> 
> No need for oneOf.
> 
Removing this results into dt_binding_check error, so this is required.
> > +      - items:
> > +          - enum:
> > +              - tesla,sysreg_fsys0
> > +              - tesla,sysreg_peric
> 
> From where did you get underscores in compatibles?
> 
I have seen in MCAN Driver <drivers/net/can/m_can/m_can_platform.c> and also too many other yaml files.
Do you have any ref standard guideline of compatible which says underscore is not allowed.
> > +          - const: syscon
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      sysreg_fsys0: system-controller@15030000 {
> > +            compatible = "tesla,sysreg_fsys0", "syscon";
> 
> Use 4 spaces for example indentation.
> 
Okay I will make it 4 spaces indentation.
> > +            reg = <0x0 0x15030000 0x0 0x1000>;
> > +      };
> > +
> > +      sysreg_peric: system-controller@14030000 {
> > +            compatible = "tesla,sysreg_peric", "syscon";
> > +            reg = <0x0 0x14030000 0x0 0x1000>;
> > +      };
> 
> One example is enough, they are the same.
kay  I will remove 1 example.
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > a198da986146..56995e7d63ad 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2943,6 +2943,7 @@ M:	linux-fsd@tesla.com
> >  L:	linux-arm-kernel@lists.infradead.org (moderated for non-
> subscribers)
> >  L:	linux-samsung-soc@vger.kernel.org
> >  S:	Maintained
> > +F:	Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> >  F:	arch/arm64/boot/dts/tesla*
> >
> >  ARM/TETON BGA MACHINE SUPPORT
> 
> Best regards,
> Krzysztof

Thanks for reviewing the patches.

Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Krzysztof Kozlowski]

On 10/11/2022 12:18, Vivek Yadav wrote:
>>> +maintainers:
>>> +  - Alim Akhtar <alim.akhtar@samsung.com>
>>> +
>>> +description: |
>>> +  This is a system control registers block, providing multiple low
>>> +level
>>> +  platform functions like board detection and identification,
>>> +software
>>> +  interrupt generation.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>
>> No need for oneOf.
>>
> Removing this results into dt_binding_check error, so this is required.

No, this is not required. You do not have more than one condition for oneOf.

>>> +      - items:
>>> +          - enum:
>>> +              - tesla,sysreg_fsys0
>>> +              - tesla,sysreg_peric
>>
>> From where did you get underscores in compatibles?
>>
> I have seen in MCAN Driver <drivers/net/can/m_can/m_can_platform.c> and also too many other yaml files.
> Do you have any ref standard guideline of compatible which says underscore is not allowed.

git grep compatible arch/arm64/boot/dts/exynos/ | grep _
git grep compatible arch/arm/boot/dts/exynos* | grep _

Both give 0 results. For few other SoCs there such cases but that's
really, really exception. Drop underscores.


Best regards,
Krzysztof

Re: [PATCH v2 3/6] arm64: dts: fsd: add sysreg device node

[Krzysztof Kozlowski]

On 09/11/2022 12:17, Sam Protsenko wrote:
> Hi Vivek,
> 
> On Wed, 9 Nov 2022 at 11:54, Vivek Yadav <vivek.2311@samsung.com> wrote:
>>
>> From: Sriranjani P <sriranjani.p@samsung.com>
>>
>> Add SYSREG controller device node, which is available in PERIC and FSYS0
>> block of FSD SoC.
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
>> ---
>>  arch/arm64/boot/dts/tesla/fsd.dtsi | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
>> index f35bc5a288c2..3d8ebbfc27f4 100644
>> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
>> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
>> @@ -518,6 +518,16 @@
>>                                 "dout_cmu_fsys1_shared0div4";
>>                 };
>>
>> +               sysreg_peric: system-controller@14030000 {
>> +                       compatible = "tesla,sysreg_peric", "syscon";
>> +                       reg = <0x0 0x14030000 0x0 0x1000>;
> 
> Probably not related to this particular patch, but does the "reg"
> really have to have those extra 0x0s? Why it can't be just:
> 
>     reg = <0x14030000 0x1000>;
> 
> That comment applies to the whole dts/dtsi. Looks like #address-cells
> or #size-cells are bigger than they should be, or I missing something?

Yes, it looks like intention was to support some 64-bit addresses (maybe
as convention for arm64?) but none of upstreamed are above 32 bit range.
I don't have the manual/datasheet to judge whether any other
(non-upstreamed) nodes need 64bit addresses.

Best regards,
Krzysztof

RE: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Vivek Yadav]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 10 November 2022 17:42
> To: Vivek Yadav <vivek.2311@samsung.com>; rcsekar@samsung.com;
> krzysztof.kozlowski+dt@linaro.org; wg@grandegger.com;
> mkl@pengutronix.de; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; pankaj.dubey@samsung.com;
> ravi.patel@samsung.com; alim.akhtar@samsung.com; linux-fsd@tesla.com;
> robh+dt@kernel.org
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; devicetree@vger.kernel.org;
> aswani.reddy@samsung.com; sriranjani.p@samsung.com
> Subject: Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific
> compatibles found on FSD SoC
> 
> On 10/11/2022 12:18, Vivek Yadav wrote:
> >>> +maintainers:
> >>> +  - Alim Akhtar <alim.akhtar@samsung.com>
> >>> +
> >>> +description: |
> >>> +  This is a system control registers block, providing multiple low
> >>> +level
> >>> +  platform functions like board detection and identification,
> >>> +software
> >>> +  interrupt generation.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    oneOf:
> >>
> >> No need for oneOf.
> >>
> > Removing this results into dt_binding_check error, so this is required.
> 
> No, this is not required. You do not have more than one condition for oneOf.
> 
Oh, ok I got it. I was not removing "-" before items, which is resulting an error. I will update this in next patch series. Sorry for confusion.
> >>> +      - items:
> >>> +          - enum:
> >>> +              - tesla,sysreg_fsys0
> >>> +              - tesla,sysreg_peric
> >>
> >> From where did you get underscores in compatibles?
> >>
> > I have seen in MCAN Driver <drivers/net/can/m_can/m_can_platform.c>
> and also too many other yaml files.
> > Do you have any ref standard guideline of compatible which says
> underscore is not allowed.
> 
> git grep compatible arch/arm64/boot/dts/exynos/ | grep _ git grep
> compatible arch/arm/boot/dts/exynos* | grep _
> 
> Both give 0 results. For few other SoCs there such cases but that's really,
> really exception. Drop underscores.
> 
git grep compatible arch/arm64/boot/dts/ | grep _ | wc -l 
This gives me 456 location, am I missing anything here ?
Anyway I will replace with "-" in next patch series.
> 
> Best regards,
> Krzysztof
Thanks for review the patches.

Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Krzysztof Kozlowski]

On 11/11/2022 05:06, Vivek Yadav wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 10 November 2022 17:42
>> To: Vivek Yadav <vivek.2311@samsung.com>; rcsekar@samsung.com;
>> krzysztof.kozlowski+dt@linaro.org; wg@grandegger.com;
>> mkl@pengutronix.de; davem@davemloft.net; edumazet@google.com;
>> kuba@kernel.org; pabeni@redhat.com; pankaj.dubey@samsung.com;
>> ravi.patel@samsung.com; alim.akhtar@samsung.com; linux-fsd@tesla.com;
>> robh+dt@kernel.org
>> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> samsung-soc@vger.kernel.org; devicetree@vger.kernel.org;
>> aswani.reddy@samsung.com; sriranjani.p@samsung.com
>> Subject: Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific
>> compatibles found on FSD SoC
>>
>> On 10/11/2022 12:18, Vivek Yadav wrote:
>>>>> +maintainers:
>>>>> +  - Alim Akhtar <alim.akhtar@samsung.com>
>>>>> +
>>>>> +description: |
>>>>> +  This is a system control registers block, providing multiple low
>>>>> +level
>>>>> +  platform functions like board detection and identification,
>>>>> +software
>>>>> +  interrupt generation.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    oneOf:
>>>>
>>>> No need for oneOf.
>>>>
>>> Removing this results into dt_binding_check error, so this is required.
>>
>> No, this is not required. You do not have more than one condition for oneOf.
>>
> Oh, ok I got it. I was not removing "-" before items, which is resulting an error. I will update this in next patch series. Sorry for confusion.
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - tesla,sysreg_fsys0
>>>>> +              - tesla,sysreg_peric
>>>>
>>>> From where did you get underscores in compatibles?
>>>>
>>> I have seen in MCAN Driver <drivers/net/can/m_can/m_can_platform.c>
>> and also too many other yaml files.
>>> Do you have any ref standard guideline of compatible which says
>> underscore is not allowed.
>>
>> git grep compatible arch/arm64/boot/dts/exynos/ | grep _ git grep
>> compatible arch/arm/boot/dts/exynos* | grep _
>>
>> Both give 0 results. For few other SoCs there such cases but that's really,
>> really exception. Drop underscores.
>>
> git grep compatible arch/arm64/boot/dts/ | grep _ | wc -l 
> This gives me 456 location, am I missing anything here ?

You mean entries like this:

arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi:		compatible =
"qcom,pmm8155au", "qcom,spmi-pmic";

or this:

arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi:		compatible =
"gpio-leds";

or this:

arch/arm64/boot/dts/intel/socfpga_agilex.dtsi:			compatible = "fixed-clock";

And how many compatibles are with hyphen, not underscore?

Best regards,
Krzysztof

Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found on FSD SoC

[Rob Herring]

On Thu, Nov 10, 2022 at 04:48:03PM +0530, Vivek Yadav wrote:
> 
> 
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Sent: 09 November 2022 16:39
> > To: Vivek Yadav <vivek.2311@samsung.com>; rcsekar@samsung.com;
> > krzysztof.kozlowski+dt@linaro.org; wg@grandegger.com;
> > mkl@pengutronix.de; davem@davemloft.net; edumazet@google.com;
> > kuba@kernel.org; pabeni@redhat.com; pankaj.dubey@samsung.com;
> > ravi.patel@samsung.com; alim.akhtar@samsung.com; linux-fsd@tesla.com;
> > robh+dt@kernel.org
> > Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> > samsung-soc@vger.kernel.org; devicetree@vger.kernel.org;
> > aswani.reddy@samsung.com; sriranjani.p@samsung.com
> > Subject: Re: [PATCH v2 1/6] dt-bindings: Document the SYSREG specific
> > compatibles found on FSD SoC
> > 
> > On 09/11/2022 11:09, Vivek Yadav wrote:
> > > From: Sriranjani P <sriranjani.p@samsung.com>
> > >
> > 
> > Use subject prefixes matching the subsystem (git log --oneline -- ...).
> > 
> Okay, I will add the correct prefixes.
> > > Describe the compatible properties for SYSREG controllers found on FSD
> > > SoC.
> > 
> > This is ARM SoC patch, split it from the patchset.
> >
> I understand this patch is not to be subset of CAN patches, I will send these patches separately.
> These patches will be used by EQos patches. As per reference, I am adding the Address link.
> https://lore.kernel.org/all/20221104120517.77980-1-sriranjani.p@samsung.com/
>  
> > >
> > > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > > Signed-off-by: Pankaj Kumar Dubey <pankaj.dubey@samsung.com>
> > > Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
> > > Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > 
> > Drop the Cc list from commit log. It's not helpful.
> > 
> Okay, I will remove.
> > > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> > > ---
> > >  .../devicetree/bindings/arm/tesla-sysreg.yaml | 50
> > +++++++++++++++++++
> > >  MAINTAINERS                                   |  1 +
> > >  2 files changed, 51 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> > > b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> > > new file mode 100644
> > > index 000000000000..bbcc6dd75918
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> > 
> > arm is only for top level stuff. This goes to soc under tesla or samsung
> > directory.
> > 
> Okay, this is specific to Samsung fsd SoC, I will be moving this file to arm/samsung in next patch series. Hope that is fine.
> > > @@ -0,0 +1,50 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > > +---
> > > +$id:
> > > +https://protect2.fireeye.com/v1/url?k=1ad2834a-7b59967c-1ad30805-
> > 000b
> > > +abff9b5d-1f65584e412e916c&q=1&e=a870a282-632a-4896-ae53-
> > 3ecb50f02be5&
> > > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Farm%2Ftesla-
> > sysreg.yaml%23
> > > +$schema:
> > > +https://protect2.fireeye.com/v1/url?k=13876e33-720c7b05-1386e57c-
> > 000b
> > > +abff9b5d-edae3ff711999305&q=1&e=a870a282-632a-4896-ae53-
> > 3ecb50f02be5&
> > > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > > +
> > > +title: Tesla Full Self-Driving platform's system registers
> > > +
> > > +maintainers:
> > > +  - Alim Akhtar <alim.akhtar@samsung.com>
> > > +
> > > +description: |
> > > +  This is a system control registers block, providing multiple low
> > > +level
> > > +  platform functions like board detection and identification,
> > > +software
> > > +  interrupt generation.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > 
> > No need for oneOf.
> > 
> Removing this results into dt_binding_check error, so this is required.
> > > +      - items:
> > > +          - enum:
> > > +              - tesla,sysreg_fsys0
> > > +              - tesla,sysreg_peric
> > 
> > From where did you get underscores in compatibles?
> > 
> I have seen in MCAN Driver <drivers/net/can/m_can/m_can_platform.c> and also too many other yaml files.
> Do you have any ref standard guideline of compatible which says underscore is not allowed.

Section 2.3.1 defining 'compatible' in the DT spec:

The compatible string should consist only of lowercase letters, digits  
and dashes, and should start with a letter. A single comma is typically 
only used following a vendor prefix. Underscores should not be used.