diff mbox series

[v4,1/4] can: m_can: move Message RAM initialization to function

Message ID 20170505135033.8349-1-quentin.schulz@free-electrons.com
State Accepted
Commit 6a792e8158a2597418a0833c7e350328e209f318
Headers show
Series [v4,1/4] can: m_can: move Message RAM initialization to function | expand

Commit Message

Quentin Schulz May 5, 2017, 1:50 p.m. UTC 
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>

---

v4:
  - remove unused variables from m_can_of_parse_mram,

 drivers/net/can/m_can/m_can.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

-- 
2.11.0

Comments

Quentin Schulz May 12, 2017, 6:37 a.m. UTC | #1 
Hi all,

On 05/05/2017 15:50, Quentin Schulz wrote:
> 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>


It's been a week since I sent this patch series. Any comments?

Thanks,
Quentin

> ---

> 

> v4:

>   - remove unused variables from m_can_of_parse_mram,

> 

>  drivers/net/can/m_can/m_can.c | 26 +++++++++++++++-----------

>  1 file changed, 15 insertions(+), 11 deletions(-)

> 

> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c

> index bf8fdaeb955e..5da1bdb202a3 100644

> --- a/drivers/net/can/m_can/m_can.c

> +++ b/drivers/net/can/m_can/m_can.c

> @@ -1489,11 +1489,23 @@ 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)

>  {

> -	int i, start, end;

> -

>  	priv->mcfg[MRAM_SIDF].off = mram_config_vals[0];

>  	priv->mcfg[MRAM_SIDF].num = mram_config_vals[1];

>  	priv->mcfg[MRAM_XIDF].off = priv->mcfg[MRAM_SIDF].off +

> @@ -1529,15 +1541,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)

> 


-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Oliver Hartkopp May 16, 2017, 3:51 a.m. UTC | #2 
On 05/15/2017 06:50 AM, Marc Kleine-Budde wrote:
> On 05/12/2017 08:37 AM, Quentin Schulz wrote:

>> Hi all,

>>

>> On 05/05/2017 15:50, Quentin Schulz wrote:

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

>>

>> It's been a week since I sent this patch series. Any comments?

>

> Looks good, added to linux-can-next.


Isn't this a fix for linux-can instead?

At least it would make no sense to me to have the upgraded M_CAN driver 
in Linux 4.12 without this fix.

Regards,
Oliver
Alexandre Belloni May 16, 2017, 8 a.m. UTC | #3 
Hi,

On 15/05/2017 at 20:51:30 -0700, Oliver Hartkopp wrote:
> On 05/15/2017 06:50 AM, Marc Kleine-Budde wrote:

> > On 05/12/2017 08:37 AM, Quentin Schulz wrote:

> > > On 05/05/2017 15:50, Quentin Schulz wrote:

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

> > > 

> > > It's been a week since I sent this patch series. Any comments?

> > 

> > Looks good, added to linux-can-next.

> 

> Isn't this a fix for linux-can instead?

> 

> At least it would make no sense to me to have the upgraded M_CAN driver in

> Linux 4.12 without this fix.

> 


The related suspend mode on the sama5d2 is not present in v4.12 so I
think this can wait v4.13.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
diff mbox series

Patch

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index bf8fdaeb955e..5da1bdb202a3 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1489,11 +1489,23 @@  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)
 {
-	int i, start, end;
-
 	priv->mcfg[MRAM_SIDF].off = mram_config_vals[0];
 	priv->mcfg[MRAM_SIDF].num = mram_config_vals[1];
 	priv->mcfg[MRAM_XIDF].off = priv->mcfg[MRAM_SIDF].off +
@@ -1529,15 +1541,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)