[PATCH v3 0/2] can: mcan: Add MCAN support for FSD SoC

[PATCH v3 0/2] can: mcan: Add MCAN support for FSD SoC

[Vivek Yadav]

Add support for MCAN instances present on the FSD platform.

Vivek Yadav (2):
  can: m_can: Move mram init to mcan device setup
  arm64: dts: fsd: Add MCAN device node

changes since v2:

[PATCH v2 1/6] dt-bindings: Document the SYSREG specific compatibles found
on FSD SoC
link:
https://lore.kernel.org/lkml/20221109100928.109478-2-vivek.2311@samsung.com/
    1: Addressed review comment given by Krzysztof Kozlowski
        a) As per suggestion separated this patch and posted separately.
          ref: https://www.spinics.net/lists/kernel/msg4597970.html 

[PATCH v2 2/6] dt-bindings: can: mcan: Add ECC functionality to message ram
link: 
https://lore.kernel.org/lkml/20221109100928.109478-3-vivek.2311@samsung.com/
    1: Addressed review comment given by Krzysztof Kozlowski
       a) For now I am dropping this. I will reconsider the implementation and
          will resend as separate patch.

[PATCH v2 3/6] arm64: dts: fsd: add sysreg device node
link:
https://lore.kernel.org/lkml/20221109100928.109478-4-vivek.2311@samsung.com/
    1: Addressed review comment given by Krzysztof Kozlowski
       a) Dropped Cc from commit message.
       b) As per suggestion separated this and corresponding DT-binding
	  patch and posted separately.
          ref: https://www.spinics.net/lists/kernel/msg4597921.html

[PATCH v2 4/6] arm64: dts: fsd: Add MCAN device node
link: 
https://lore.kernel.org/lkml/20221109100928.109478-5-vivek.2311@samsung.com/
    1: Addressed review comment given by Krzysztof Kozlowski
       a) Aligned the lines.

[PATCH v2 5/6] can: m_can: Add ECC functionality for message RAM
link: 
https://lore.kernel.org/lkml/20221109100928.109478-6-vivek.2311@samsung.com/
    1: Addressed review comment given by Krzysztof Kozlowski
       a) We are dropping this for now and will reconsider it's
          implementation and resend as separate patch.

[PATCH v2 6/6] arm64: dts: fsd: Add support for error correction code for
message RAM
link: 
https://lore.kernel.org/lkml/20221109100928.109478-7-vivek.2311@samsung.com/
    1: Addressed review comment given by Krzysztof Kozlowski
       a) Subject is updated and patch go via ARM SOC tree, we will
          resend this as separate patch along with ECC patch.

changes since v1:

[PATCH 0/7] can: mcan: Add MCAN support for FSD SoC 
    1: Addressed review comment given by  Marc Kleine-Budde
       a) Added my signed off.

[PATCH 2/7] dt-bindings: can: mcan: Add ECC functionality to message ram
link: 
https://lore.kernel.org/netdev/87k04oxsvb.fsf@hardanger.blackshift.org/ 
    1: Addressed review comment given by  Marc Kleine-Budde
       a) Added an example to the yaml that makes use of the 
          mram-ecc-cfg property.
       b) Added prefix to "mram-ecc-cfg" property and
          "$ref: /schemas/types.yaml#/definitions/phandle".

[PATCH 4/7] can: mcan: enable peripheral clk to access mram
link:
https://lore.kernel.org/netdev/20221021095833.62406-5-vivek.2311@samsung.com/
    1: Addressed review comment given by  Marc Kleine-Budde
       a) Moved mram init into m_can_dev_setup function by then
          clocks are enabled and prevent probe failure.
       b) Added the platform init ops in m_can_plat_ops and
          moved mram init into it.

[PATCH 5/7] arm64: dts: fsd: Add MCAN device node
link:
https://lore.kernel.org/netdev/20221021095833.62406-6-vivek.2311@samsung.com/
    1: Addressed review comment given by  Marc Kleine-Budde
       a) Added the DT people on Cc.

[PATCH 6/7] can: m_can: Add ECC functionality for message RAM
link:
https://lore.kernel.org/netdev/20221021095833.62406-7-vivek.2311@samsung.com/
    1: Addressed review comment given by kernel test robot.
       a) Addressed missing prototypes warnings.

    2: Addressed review comment given by  Marc Kleine-Budde
       a) Sorted the declaration of local variable by reverse Christmas 
          tree.
       b) Used syscon_regmap_lookup_by_phandle_args to get the syscon
          Base Address and offset.
       c) Used FIELD_PREP instead of logical operation.
       d) Used regmap_read_poll_timeout API to give timeout condition
          for ECC cfg done status instead of using while loop counter.
       e) Moved all the ECC mcaros in m_can.c file and changed the name
          with a common prefix M_CAN.
       f) Moved ECC init into platform init function called during m_can
          device setup.
 

 .../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 v3 1/2] can: m_can: Move mram init to mcan device setup

[Vivek Yadav]

When we try to access the mcan message ram addresses, hclk is
gated by any other drivers or disabled, because of that probe gets
failed.

Move the mram init functionality to mcan device setup called by
mcan class register from mcan probe function, by that time clocks
are enabled.

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          |  4 ++--
 drivers/net/can/m_can/m_can.h          |  1 +
 drivers/net/can/m_can/m_can_platform.c | 21 +++++++++++++++++----
 3 files changed, 20 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..5956f0b4d5b1 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1516,9 +1516,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)
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 401410022823..c6e77b93f4a2 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -93,6 +93,7 @@ struct m_can_classdev {
 	int is_peripheral;
 
 	struct mram_cfg mcfg[MRAM_CFG_NUM];
+	bool mram_cfg_flag;
 };
 
 struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, int sizeof_priv);
diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index b5a5bedb3116..1a65b0d256fb 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -67,11 +67,28 @@ static int iomap_write_fifo(struct m_can_classdev *cdev, int offset,
 	return 0;
 }
 
+static int plat_init(struct m_can_classdev *cdev)
+{
+	int ret = 0;
+
+	if (!cdev->mram_cfg_flag) {
+		/* Initialize mcan message ram */
+		ret = m_can_init_ram(cdev);
+		if (ret)
+			return ret;
+
+		cdev->mram_cfg_flag = true;
+	}
+
+	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 = plat_init,
 };
 
 static int m_can_plat_probe(struct platform_device *pdev)
@@ -140,10 +157,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 v3 2/2] 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>
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 f35bc5a288c2..dfdb32514887 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -755,6 +755,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

Re: [PATCH v3 2/2] arm64: dts: fsd: Add MCAN device node

[Krzysztof Kozlowski]

On 22/11/2022 11:54, 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>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>

Thank you for the patch.

Looks OK. It is too late in the cycle for me to pick it up. I will take
it after the merge window via Samsung SoC.

Note for networking maintainers: please do not pick up DTS via netdev tree.

Best regards,
Krzysztof

Re: [PATCH v3 1/2] can: m_can: Move mram init to mcan device setup

[Marc Kleine-Budde]

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

On 22.11.2022 16:24:54, Vivek Yadav wrote:
> When we try to access the mcan message ram addresses, hclk is
> gated by any other drivers or disabled, because of that probe gets
> failed.
> 
> Move the mram init functionality to mcan device setup called by
> mcan class register from mcan probe function, by that time clocks
> are enabled.

Why not call the RAM init directly from m_can_chip_config()?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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

RE: [PATCH v3 1/2] can: m_can: Move mram init to mcan device setup

[Vivek Yadav]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 24 November 2022 04:12
> To: Vivek Yadav <vivek.2311@samsung.com>
> Cc: rcsekar@samsung.com; krzysztof.kozlowski+dt@linaro.org;
> wg@grandegger.com; 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; 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 v3 1/2] can: m_can: Move mram init to mcan device
> setup
> 
> On 22.11.2022 16:24:54, Vivek Yadav wrote:
> > When we try to access the mcan message ram addresses, hclk is gated by
> > any other drivers or disabled, because of that probe gets failed.
> >
> > Move the mram init functionality to mcan device setup called by mcan
> > class register from mcan probe function, by that time clocks are
> > enabled.
> 
> Why not call the RAM init directly from m_can_chip_config()?
> 
m_can_chip_config function is called from m_can open.

Configuring RAM init every time we open the CAN instance is not needed, I think only once during the probe is enough. 

If message RAM init failed then fifo Transmit and receive will fail and there will be no communication. So there is no point to "open and Configure CAN chip".

From my understanding it's better to keep RAM init inside the probe and if there is a failure happened goes to CAN probe failure.
> Marc
Thanks for reviewing the patch.
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   |
> https://protect2.fireeye.com/v1/url?k=818c3690-e0f1dc13-818dbddf-
> 74fe48600158-a08b6a4bfa0b043e&q=1&e=315ed8d1-1645-4c16-b5e7-
> 2a250ae36941&u=https%3A%2F%2Fwww.pengutronix.de%2F  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Re: RE: [PATCH v3 1/2] can: m_can: Move mram init to mcan device setup

[Marc Kleine-Budde]

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

On 24.11.2022 14:36:48, Vivek Yadav wrote:
> > Why not call the RAM init directly from m_can_chip_config()?
> > 
> m_can_chip_config function is called from m_can open.
> 
> Configuring RAM init every time we open the CAN instance is not
> needed, I think only once during the probe is enough.

That probably depends on you power management. If I add a regulator to
power the external tcan4x5x chip and power it up during open(), I need
to initialize the RAM.

> If message RAM init failed then fifo Transmit and receive will fail
> and there will be no communication. So there is no point to "open and
> Configure CAN chip".

For mmio devices the RAM init will probably not fail. There are return
values and error checking for the SPI attached devices. Where the SPI
communication will fail. However if this is problem, I assume the chip
will not be detected in the first place.

> From my understanding it's better to keep RAM init inside the probe
> and if there is a failure happened goes to CAN probe failure.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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

RE: RE: [PATCH v3 1/2] can: m_can: Move mram init to mcan device setup

[Vivek Yadav]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 24 November 2022 20:24
> To: Vivek Yadav <vivek.2311@samsung.com>
> Cc: rcsekar@samsung.com; krzysztof.kozlowski+dt@linaro.org;
> wg@grandegger.com; 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; 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: RE: [PATCH v3 1/2] can: m_can: Move mram init to mcan device
> setup
> 
> On 24.11.2022 14:36:48, Vivek Yadav wrote:
> > > Why not call the RAM init directly from m_can_chip_config()?
> > >
> > m_can_chip_config function is called from m_can open.
> >
> > Configuring RAM init every time we open the CAN instance is not
> > needed, I think only once during the probe is enough.
> 
> That probably depends on you power management. If I add a regulator to
> power the external tcan4x5x chip and power it up during open(), I need to
> initialize the RAM.
> 
Thanks for the clarification,
There is one doubt for which I need clarity if I add ram init in m_can_chip_config.

In the current implementation, m_can_ram_init is added in the probe and m_can_class_resume function.
If I add the ram init function in chip_config which is getting called from m_can_start, then m_can_init_ram will be called two times, once in resume and next from m_can_start also.

Can we add ram init inside the m_can_open function itself?
Because it is independent of m_can resume functionality.

> > If message RAM init failed then fifo Transmit and receive will fail
> > and there will be no communication. So there is no point to "open and
> > Configure CAN chip".
> 
> For mmio devices the RAM init will probably not fail. There are return values
> and error checking for the SPI attached devices. Where the SPI
> communication will fail. However if this is problem, I assume the chip will not
> be detected in the first place.
> 
> > From my understanding it's better to keep RAM init inside the probe
> > and if there is a failure happened goes to CAN probe failure.
> 
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   |
> https://protect2.fireeye.com/v1/url?k=2053d9ab-7fc8e0b4-205252e4-
> 000babdfecba-a8c309c53e3358f5&q=1&e=c0cfd0e2-a422-4821-a49d-
> 113cfa4da9cb&u=https%3A%2F%2Fwww.pengutronix.de%2F  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Re: RE: RE: [PATCH v3 1/2] can: m_can: Move mram init to mcan device setup

[Marc Kleine-Budde]

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

On 01.12.2022 09:40:50, Vivek Yadav wrote:
> > That probably depends on you power management. If I add a regulator
> > to power the external tcan4x5x chip and power it up during open(), I
> > need to initialize the RAM.
> > 
> Thanks for the clarification,
>
> There is one doubt for which I need clarity if I add ram init in
> m_can_chip_config.
> 
> In the current implementation, m_can_ram_init is added in the probe
> and m_can_class_resume function.
>
> If I add the ram init function in chip_config which is getting called
> from m_can_start, then m_can_init_ram will be called two times, once
> in resume and next from m_can_start also.

As m_can_start() is called from resume, remove the direct call to
m_can_init_ram() from m_can_class_resume().

> Can we add ram init inside the m_can_open function itself? Because it
> is independent of m_can resume functionality.

See above.

mainline implementation:

m_can_class_resume()
        -> m_can_init_ram()

m_can_open()
        -> m_can_start()
        -> m_can_chip_config()
        -> ops->init
                m_can_init_ram() (for tcan only)


proposed:

m_can_class_resume()
        -> m_can_start()
        -> m_can_chip_config()
        -> m_can_init_ram()

m_can_open()
        -> m_can_start()
        -> m_can_chip_config()
        -> m_can_init_ram()

In mainline m_can_init_ram() is called for the tcan during open(). So if
you call m_can_init_ram() from m_can_chip_config(), remove it from the
tcan's tcan4x5x_init() functions, and from m_can_class_resume() it
should only be called once for open and once for resume.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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

RE: [PATCH v3 2/2] arm64: dts: fsd: Add MCAN device node

[Pankaj Dubey]

> -----Original Message-----
> From: Vivek Yadav <vivek.2311@samsung.com>
> Sent: Tuesday, November 22, 2022 4:25 PM
> To: 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; Vivek Yadav
> <vivek.2311@samsung.com>
> Subject: [PATCH v3 2/2] arm64: dts: fsd: Add MCAN device node
> 
> 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>
> Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
> ---

Looks good to me. 
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>