linux-kernel - RE: [PATCH] bfin_can: rewrite the blackfin CAN driver into arch independent style

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <649EF91064D35D40B9C93A225BF41674612E3EDD@NWD2MBX7.ad.analog.com>
Date:	Thu, 8 Jan 2015 07:13:53 +0000
From:	"Wu, Aaron" <Aaron.Wu@...log.com>
To:	Marc Kleine-Budde <mkl@...gutronix.de>,
	"adi-buildroot-devel@...ts.sourceforge.net" 
	<adi-buildroot-devel@...ts.sourceforge.net>,
	"wg@...ndegger.com" <wg@...ndegger.com>,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] bfin_can: rewrite the blackfin CAN driver into arch
 independent style

Hi Marc

Thanks for your comments. I've split it into two parts and submit patches of V2.
Do you mind if I fix potential bugs with future patches? 

Regards
Aaron

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@...gutronix.de]
> Sent: 2015年1月7日 22:36
> To: Wu, Aaron; adi-buildroot-devel@...ts.sourceforge.net;
> wg@...ndegger.com; linux-can@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH] bfin_can: rewrite the blackfin CAN driver into arch
> independent style
> 
> On 12/24/2014 04:30 AM, Aaron Wu wrote:
> > Rewrite the blackfin CAN driver into arch independent style
> >
> > Signed-off-by: Aaron Wu <Aaron.wu@...log.com>
> 
> [...]
> 
> > @@ -547,6 +543,11 @@ static int bfin_can_probe(struct platform_device
> *pdev)
> >  	}
> >
> >  	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (res_mem == NULL) {
> > +		dev_err(&pdev->dev, "Cannot get IORESOURCE_MEM\n");
> > +		return -ENOENT;
> > +	}
> > +
> >  	rx_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> >  	tx_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
> >  	err_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 2); @@ -
> 555,12
> > +556,6 @@ static int bfin_can_probe(struct platform_device *pdev)
> >  		goto exit;
> >  	}
> >
> > -	if (!request_mem_region(res_mem->start, resource_size(res_mem),
> > -				dev_name(&pdev->dev))) {
> > -		err = -EBUSY;
> > -		goto exit;
> > -	}
> 
> You should remove the corresponding release_mem_region() from the
> remove() function, too.
> 
> > -
> >  	/* request peripheral pins */
> >  	err = peripheral_request_list(pdata, dev_name(&pdev->dev));
> >  	if (err)
> > @@ -573,7 +568,13 @@ static int bfin_can_probe(struct platform_device
> *pdev)
> >  	}
> >
> >  	priv = netdev_priv(dev);
> > -	priv->membase = (void __iomem *)res_mem->start;
> > +
> > +	priv->membase = devm_ioremap_resource(&pdev->dev, res_mem);
> > +	if (IS_ERR((void *)priv->membase)) {
> > +		dev_err(&pdev->dev, "Cannot map IO\n");
> > +		return PTR_ERR((void *)priv->membase);
> 
> Is this cast still needed?
> 
> Can you move the devm_ conversion into a separate patch?
> 
> 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   |