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:[email protected]]
> Sent: 2015年1月7日 22:36
> To: Wu, Aaron; [email protected];
> [email protected]; [email protected]; [email protected]
> 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 <[email protected]>
> 
> [...]
> 
> > @@ -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   |

Reply via email to