[4/7] can: mcan: enable peripheral clk to access mram

Message ID 20221021095833.62406-5-vivek.2311@samsung.com
State New
Headers
Series can: mcan: Add MCAN support for FSD SoC |

Commit Message

Vivek Yadav Oct. 21, 2022, 9:58 a.m. UTC
  When we try to access the mcan message ram addresses, make sure hclk is
not gated by any other drivers or disabled. Enable the clock (hclk) before
accessing the mram and disable it after that.

This is required in case if by-default hclk is gated.

Signed-off-by: Ravi Patel <ravi.patel@samsung.com>
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
---
 drivers/net/can/m_can/m_can_platform.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
  

Comments

Marc Kleine-Budde Oct. 25, 2022, 7:44 a.m. UTC | #1
On 21.10.2022 15:28:30, Vivek Yadav wrote:
> When we try to access the mcan message ram addresses, make sure hclk is
> not gated by any other drivers or disabled. Enable the clock (hclk) before
> accessing the mram and disable it after that.
> 
> This is required in case if by-default hclk is gated.

From my point of view it makes no sense to init the RAM during probe.
Can you move the init_ram into the m_can_chip_config() function? The
clocks should be enabled then.

Marc
  
Vivek Yadav Nov. 9, 2022, 9:55 a.m. UTC | #2
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 25 October 2022 13:14
> To: Vivek Yadav <vivek.2311@samsung.com>
> Cc: rcsekar@samsung.com; 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-can@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 4/7] can: mcan: enable peripheral clk to access mram
> 
> On 21.10.2022 15:28:30, Vivek Yadav wrote:
> > When we try to access the mcan message ram addresses, make sure hclk
> > is not gated by any other drivers or disabled. Enable the clock (hclk)
> > before accessing the mram and disable it after that.
> >
> > This is required in case if by-default hclk is gated.
> 
> From my point of view it makes no sense to init the RAM during probe.
> Can you move the init_ram into the m_can_chip_config() function? The
> clocks should be enabled then.
> 
As per my understanding, we should not remove message ram init from probe because if message ram init failed then there will be no 
Storing of Tx/Rx messages onto message ram, so it's better to confirm write operations onto message ram before CAN communication.

So we can kept init_ram in the probe function only, but we will shift init_ram into m_can_dev_setup function by the time clks are already enabled.
> Marc
> 
Thanks for the review.
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://protect2.fireeye.com/v1/url?k=fc7bf79b-
> 9c996ac6-fc7a7cd4-000babd9f1ba-3024ea0d5d83d168&q=1&e=87d053cd-
> e4ab-41e4-a21b-
> c348747c0ce5&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 |
  

Patch

diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index eee47bad0592..5aab025775f9 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -140,10 +140,17 @@  static int m_can_plat_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mcan_class);
 
-	ret = m_can_init_ram(mcan_class);
+	/* clock needs to be enabled to access mram block */
+	ret = clk_prepare_enable(mcan_class->hclk);
 	if (ret)
 		goto probe_fail;
 
+	ret = m_can_init_ram(mcan_class);
+	if (ret)
+		goto mram_fail;
+
+	clk_disable_unprepare(mcan_class->hclk);
+
 	pm_runtime_enable(mcan_class->dev);
 	ret = m_can_class_register(mcan_class);
 	if (ret)
@@ -153,6 +160,8 @@  static int m_can_plat_probe(struct platform_device *pdev)
 
 out_runtime_disable:
 	pm_runtime_disable(mcan_class->dev);
+mram_fail:
+	clk_disable_unprepare(mcan_class->hclk);
 probe_fail:
 	m_can_class_free_dev(mcan_class->net);
 	return ret;