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

[Wu, Aaron]

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@xxxxxxxxxxxxxx]
> Sent: 2015å1æ7æ 22:36
> To: Wu, Aaron; adi-buildroot-devel@xxxxxxxxxxxxxxxxxxxxx;
> wg@xxxxxxxxxxxxxx; linux-can@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxx>
> 
> [...]
> 
> > @@ -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   |