netdev - Re: [PATCH 3/4] powerpc/p1010: FlexCAN Controller for of

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 09 Aug 2011 12:56:15 +0200
From:	Marc Kleine-Budde <mkl@...gutronix.de>
To:	U Bhaskar-B22300 <B22300@...escale.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"holt@....com" <holt@....com>,
	"wg@...ndegger.com" <wg@...ndegger.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"linuxppc-release@...ux.freescale.net" 
	<linuxppc-release@...ux.freescale.net>,
	"socketcan-core@...ts.berlios.de" <socketcan-core@...ts.berlios.de>
Subject: Re: [PATCH 3/4] powerpc/p1010: FlexCAN Controller for of_ type

On 08/09/2011 12:29 PM, U Bhaskar-B22300 wrote:
>> NACK - your patch does more than the description states (debug code).
> [Bhaskar] Ok I will change description as below
> 	This patch introduces the following features
> 	1. provides the support for FlexCAN on P1010 SoC.
> 	2. Provides the common read/write interface for ARM and PowerPC based FlexCAN.
> 	3. Provides the clock interface for PowerPC based FlexCAN.
> 	Will it be OK ?
> 
>> Further you still add bugs to the driver. I've send you patches to fix
>> them.
> [Bhaskar] Please mention what sort of BUG you are seeing ..

I have, it's in that mail:

> @@ -957,24 +952,24 @@ static int __devinit register_flexcandev(struct net_device *dev)
> >  
> >  	/* select "bus clock", chip must be disabled */
> >  	flexcan_chip_disable(priv);
> > -	reg = readl(&regs->ctrl);
> > +	reg = flexcan_read(&regs->ctrl);
> >  	reg |= FLEXCAN_CTRL_CLK_SRC;
> > -	writel(reg, &regs->ctrl);
> > +	flexcan_write(reg, &regs->ctrl);
> >  
> >  	flexcan_chip_enable(priv);
> >  
> >  	/* set freeze, halt and activate FIFO, restrict register access */
> > -	reg = readl(&regs->mcr);
> > +	reg = flexcan_read(&regs->mcr);
> >  	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
> >  		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
> > -	writel(reg, &regs->mcr);
> > +	flexcan_write(reg, &regs->mcr);
> >  
> >  	/*
> >  	 * Currently we only support newer versions of this core
> >  	 * featuring a RX FIFO. Older cores found on some Coldfire
> >  	 * derivates are not yet supported.
> >  	 */
> > -	reg = readl(&regs->mcr);
> > +	reg = flexcan_read(&regs->mcr);
> >  	if (!(reg & FLEXCAN_MCR_FEN)) {
> >  		dev_err(dev->dev.parent,
> >  			"Could not enable RX FIFO, unsupported core\n");
> > @@ -984,6 +979,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
> >  
> >  	err = register_candev(dev);
> >  
> > +	return err;
If you return here, the clock stays enabled....not good

> >   out:
> >  	/* disable core and turn off clocks */
> >  	flexcan_chip_disable(priv);
> > @@ -992,7 +988,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
> >  	return err;
> >  }

For reference bugreport + fix:
https://lists.berlios.de/pipermail/socketcan-core/2011-July/005838.html

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


Download attachment "signature.asc" of type "application/pgp-signature" (263 bytes)