[PATCH 1/2] can: m_can: move Message RAM initialization to function

[PATCH 1/2] can: m_can: move Message RAM initialization to function

[Quentin Schulz]

To avoid possible ECC/parity checksum errors when reading an
uninitialized buffer, the entire Message RAM is initialized when probing
the driver. This initialization is done in the same function reading the
Device Tree properties.

This patch moves the RAM initialization to a separate function so it can
be called separately from device initialization from Device Tree.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/net/can/m_can/m_can.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index bf8fdaeb955e..3f0445440146 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1489,6 +1489,20 @@ static int register_m_can_dev(struct net_device *dev)
 	return register_candev(dev);
 }
 
+static void m_can_init_ram(struct m_can_priv *priv)
+{
+	int end, i, start;
+
+	/* initialize the entire Message RAM in use to avoid possible
+	 * ECC/parity checksum errors when reading an uninitialized buffer
+	 */
+	start = priv->mcfg[MRAM_SIDF].off;
+	end = priv->mcfg[MRAM_TXB].off +
+		priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
+	for (i = start; i < end; i += 4)
+		writel(0x0, priv->mram_base + i);
+}
+
 static void m_can_of_parse_mram(struct m_can_priv *priv,
 				const u32 *mram_config_vals)
 {
@@ -1529,15 +1543,7 @@ static void m_can_of_parse_mram(struct m_can_priv *priv,
 		priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
 		priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
 
-	/* initialize the entire Message RAM in use to avoid possible
-	 * ECC/parity checksum errors when reading an uninitialized buffer
-	 */
-	start = priv->mcfg[MRAM_SIDF].off;
-	end = priv->mcfg[MRAM_TXB].off +
-		priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
-	for (i = start; i < end; i += 4)
-		writel(0x0, priv->mram_base + i);
-
+	m_can_init_ram(priv);
 }
 
 static int m_can_plat_probe(struct platform_device *pdev)
-- 
2.11.0

[PATCH 2/2] can: m_can: add deep Suspend/Resume support

[Quentin Schulz]

This adds Power Management deep Suspend/Resume support for Bosch M_CAN
chip.

When the chip resumes from deep sleep, the RAM needs to be initialized
as it is done when the driver probes. The net interface also needs to be
closed and reopened to be fully functional.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/net/can/m_can/m_can.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 3f0445440146..9e0143b528f1 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1670,12 +1670,10 @@ static __maybe_unused int m_can_suspend(struct device *dev)
 	struct m_can_priv *priv = netdev_priv(ndev);
 
 	if (netif_running(ndev)) {
-		netif_stop_queue(ndev);
 		netif_device_detach(ndev);
+		m_can_close(ndev);
 	}
 
-	/* TODO: enter low power */
-
 	priv->can.state = CAN_STATE_SLEEPING;
 
 	return 0;
@@ -1686,13 +1684,13 @@ static __maybe_unused int m_can_resume(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct m_can_priv *priv = netdev_priv(ndev);
 
-	/* TODO: exit low power */
+	m_can_init_ram(priv);
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	if (netif_running(ndev)) {
+		m_can_open(ndev);
 		netif_device_attach(ndev);
-		netif_start_queue(ndev);
 	}
 
 	return 0;
-- 
2.11.0

Re: [PATCH 2/2] can: m_can: add deep Suspend/Resume support

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 756 bytes --]

On 05/03/2017 02:11 PM, Quentin Schulz wrote:
> This adds Power Management deep Suspend/Resume support for Bosch M_CAN
> chip.
> 
> When the chip resumes from deep sleep, the RAM needs to be initialized
> as it is done when the driver probes. The net interface also needs to be
> closed and reopened to be fully functional.

Are you sure it's the closing and opening of the net interface. Maybe
it's the m_can_start() and/or the subsequent m_can_chip_config()?

Marc

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


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

Re: [PATCH 2/2] can: m_can: add deep Suspend/Resume support

[Quentin Schulz]

[-- Attachment #1.1: Type: text/plain, Size: 906 bytes --]

Hi Marc,

On 03/05/2017 14:16, Marc Kleine-Budde wrote:
> On 05/03/2017 02:11 PM, Quentin Schulz wrote:
>> This adds Power Management deep Suspend/Resume support for Bosch M_CAN
>> chip.
>>
>> When the chip resumes from deep sleep, the RAM needs to be initialized
>> as it is done when the driver probes. The net interface also needs to be
>> closed and reopened to be fully functional.
> 
> Are you sure it's the closing and opening of the net interface. Maybe
> it's the m_can_start() and/or the subsequent m_can_chip_config()?
> 

You're right. The chip needs to be completely reinitialized (clocks,
chip itself) and it also closes and reopens the interface if I'm correct
with close_candev and open_candev.
I'm sending a v2 right away to fix the commit log.

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/2] can: m_can: add deep Suspend/Resume support

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1225 bytes --]

On 05/03/2017 02:28 PM, Quentin Schulz wrote:
> Hi Marc,
> 
> On 03/05/2017 14:16, Marc Kleine-Budde wrote:
>> On 05/03/2017 02:11 PM, Quentin Schulz wrote:
>>> This adds Power Management deep Suspend/Resume support for Bosch M_CAN
>>> chip.
>>>
>>> When the chip resumes from deep sleep, the RAM needs to be initialized
>>> as it is done when the driver probes. The net interface also needs to be
>>> closed and reopened to be fully functional.
>>
>> Are you sure it's the closing and opening of the net interface. Maybe
>> it's the m_can_start() and/or the subsequent m_can_chip_config()?
>>
> 
> You're right. The chip needs to be completely reinitialized (clocks,
> chip itself) and it also closes and reopens the interface if I'm correct
> with close_candev and open_candev.
> I'm sending a v2 right away to fix the commit log.

:) I mean to fix the call to the proper function and _not_ calling
close() open().

Marc

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


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