2022-11-09 11:07:53

by Vivek Yadav

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

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



2022-11-09 11:11:15

by Vivek Yadav

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

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 <[email protected]>
Signed-off-by: Vivek Yadav <[email protected]>
---
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


2022-11-09 11:12:24

by Vivek Yadav

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

From: Sriranjani P <[email protected]>

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

Signed-off-by: Alim Akhtar <[email protected]>
Signed-off-by: Pankaj Kumar Dubey <[email protected]>
Signed-off-by: Ravi Patel <[email protected]>
Signed-off-by: Vivek Yadav <[email protected]>
Cc: [email protected]
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Sriranjani P <[email protected]>
---
.../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 <[email protected]>
+
+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: [email protected]
L: [email protected] (moderated for non-subscribers)
L: [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
F: arch/arm64/boot/dts/tesla*

ARM/TETON BGA MACHINE SUPPORT
--
2.17.1


2022-11-09 11:28:01

by Vivek Yadav

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

From: Sriranjani P <[email protected]>

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

Signed-off-by: Alim Akhtar <[email protected]>
Signed-off-by: Pankaj Kumar Dubey <[email protected]>
Cc: [email protected]
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Sriranjani P <[email protected]>
---
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


2022-11-09 11:30:48

by Krzysztof Kozlowski

[permalink] [raw]
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 <[email protected]>
>

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 <[email protected]>
> Signed-off-by: Pankaj Kumar Dubey <[email protected]>
> Signed-off-by: Ravi Patel <[email protected]>
> Signed-off-by: Vivek Yadav <[email protected]>
> Cc: [email protected]
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Rob Herring <[email protected]>

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

> Signed-off-by: Sriranjani P <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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: [email protected]
> L: [email protected] (moderated for non-subscribers)
> L: [email protected]
> S: Maintained
> +F: Documentation/devicetree/bindings/arm/tesla-sysreg.yaml
> F: arch/arm64/boot/dts/tesla*
>
> ARM/TETON BGA MACHINE SUPPORT

Best regards,
Krzysztof


2022-11-09 11:35:39

by Vivek Yadav

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

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 <[email protected]>
Cc: [email protected]
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Vivek Yadav <[email protected]>
---
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


2022-11-09 11:49:55

by Vivek Yadav

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

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 <[email protected]>
Cc: [email protected]
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Vivek Yadav <[email protected]>
---
.../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


2022-11-09 11:52:00

by Krzysztof Kozlowski

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

On 09/11/2022 11:09, Vivek Yadav wrote:
> From: Sriranjani P <[email protected]>
>
> Add SYSREG controller device node, which is available in PERIC and FSYS0
> block of FSD SoC.
> > Signed-off-by: Alim Akhtar <[email protected]>
> Signed-off-by: Pankaj Kumar Dubey <[email protected]>
> Cc: [email protected]
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Rob Herring <[email protected]>

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

> Signed-off-by: Sriranjani P <[email protected]>
> ---
> 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


2022-11-09 11:52:44

by Krzysztof Kozlowski

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

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 <[email protected]>
> Signed-off-by: Vivek Yadav <[email protected]>

(...)

>
> +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


2022-11-09 11:54:14

by Sam Protsenko

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

Hi Vivek,

On Wed, 9 Nov 2022 at 11:54, Vivek Yadav <[email protected]> wrote:
>
> From: Sriranjani P <[email protected]>
>
> Add SYSREG controller device node, which is available in PERIC and FSYS0
> block of FSD SoC.
>
> Signed-off-by: Alim Akhtar <[email protected]>
> Signed-off-by: Pankaj Kumar Dubey <[email protected]>
> Cc: [email protected]
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Sriranjani P <[email protected]>
> ---
> 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
>

2022-11-09 11:55:00

by Krzysztof Kozlowski

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

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 <[email protected]>
> Cc: [email protected]
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Vivek Yadav <[email protected]>
> ---
> .../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


2022-11-10 12:04:55

by Vivek Yadav

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



> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: 09 November 2022 16:39
> To: Vivek Yadav <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> >
>
> 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/[email protected]/

> >
> > Signed-off-by: Alim Akhtar <[email protected]>
> > Signed-off-by: Pankaj Kumar Dubey <[email protected]>
> > Signed-off-by: Ravi Patel <[email protected]>
> > Signed-off-by: Vivek Yadav <[email protected]>
> > Cc: [email protected]
> > Cc: Krzysztof Kozlowski <[email protected]>
> > Cc: Rob Herring <[email protected]>
>
> Drop the Cc list from commit log. It's not helpful.
>
Okay, I will remove.
> > Signed-off-by: Sriranjani P <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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: [email protected]
> > L: [email protected] (moderated for non-
> subscribers)
> > L: [email protected]
> > 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.



2022-11-10 12:24:05

by Krzysztof Kozlowski

[permalink] [raw]
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 <[email protected]>
>>> +
>>> +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


2022-11-10 13:02:12

by Krzysztof Kozlowski

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

On 09/11/2022 12:17, Sam Protsenko wrote:
> Hi Vivek,
>
> On Wed, 9 Nov 2022 at 11:54, Vivek Yadav <[email protected]> wrote:
>>
>> From: Sriranjani P <[email protected]>
>>
>> Add SYSREG controller device node, which is available in PERIC and FSYS0
>> block of FSD SoC.
>>
>> Signed-off-by: Alim Akhtar <[email protected]>
>> Signed-off-by: Pankaj Kumar Dubey <[email protected]>
>> Cc: [email protected]
>> Cc: Krzysztof Kozlowski <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Signed-off-by: Sriranjani P <[email protected]>
>> ---
>> 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


2022-11-11 05:31:39

by Vivek Yadav

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



> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: 10 November 2022 17:42
> To: Vivek Yadav <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> >>> +
> >>> +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.



2022-11-11 08:26:38

by Krzysztof Kozlowski

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

On 11/11/2022 05:06, Vivek Yadav wrote:
>
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: 10 November 2022 17:42
>> To: Vivek Yadav <[email protected]>; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> 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 <[email protected]>
>>>>> +
>>>>> +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


2022-11-16 16:53:16

by Rob Herring (Arm)

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

On Thu, Nov 10, 2022 at 04:48:03PM +0530, Vivek Yadav wrote:
>
>
> > -----Original Message-----
> > From: Krzysztof Kozlowski <[email protected]>
> > Sent: 09 November 2022 16:39
> > To: Vivek Yadav <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > 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 <[email protected]>
> > >
> >
> > 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/[email protected]/
>
> > >
> > > Signed-off-by: Alim Akhtar <[email protected]>
> > > Signed-off-by: Pankaj Kumar Dubey <[email protected]>
> > > Signed-off-by: Ravi Patel <[email protected]>
> > > Signed-off-by: Vivek Yadav <[email protected]>
> > > Cc: [email protected]
> > > Cc: Krzysztof Kozlowski <[email protected]>
> > > Cc: Rob Herring <[email protected]>
> >
> > Drop the Cc list from commit log. It's not helpful.
> >
> Okay, I will remove.
> > > Signed-off-by: Sriranjani P <[email protected]>
> > > ---
> > > .../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 <[email protected]>
> > > +
> > > +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.