[PATCH v9 4/6] ARM: davinci: Add a remoteproc driver implementation for OMAP-L13x DSP

[Tivy, Robert]

Hi Sergei,

> -----Original Message-----
> From: Sergei Shtylyov [mailto:sergei.shtylyov at cogentembedded.com]
> Sent: Sunday, April 07, 2013 10:55 AM
> 
> Hello.
> 
> On 04/07/2013 05:02 PM, Ohad Ben-Cohen wrote:
> 
> >
> >> +static int da8xx_rproc_probe(struct platform_device *pdev)
> >> +{
> >> +       struct device *dev = &pdev->dev;
> >> +       struct da8xx_rproc *drproc;
> >> +       struct rproc *rproc;
> >> +       struct irq_data *irq_data;
> >> +       struct resource *bootreg_res;
> >> +       struct resource *chipsig_res;
> >> +       struct clk *dsp_clk;
> >> +       void __iomem *chipsig;
> >> +       void __iomem *bootreg;
> >> +       int irq;
> >> +       int ret;
> >> +
> [...]
> >> +       bootreg = devm_request_and_ioremap(dev, bootreg_res);
> >> +       if (!bootreg) {
> >> +               dev_err(dev, "unable to map boot register\n");
> >> +               return -EADDRNOTAVAIL;
> >> +       }
> >> +
> >> +       chipsig = devm_request_and_ioremap(dev, chipsig_res);
> 
>     I suggest that you use more modern (yes, already a newer interface
> :-)
> devm_ioremap_resource() instead -- it returns the error code (as a
> pointer)
> in case of error, and it certainly doesn't require you to print error
> messages.

Thanks, will do.

I appreciate the notice of a more modern function, it's really tough to keep up with the flurry of activity to the kernel.

Regarding this change, should the code use
	return PTR_ERR(bootreg);
or
	return PTR_RET(bootreg);
I ask because PTR_ERR() returns 'long' whereas PTR_RET() returns 'int' (and probe returns 'int'), but I see that the majority of existing code uses "return PTR_ERR()" in probe functions.

> 
> >> +       if (!chipsig) {
> >> +               dev_err(dev, "unable to map CHIPSIG register\n");
> >> +               return -EADDRNOTAVAIL;
> >> +       }
> >>
> 
> WBR, Sergei

Regards,

- Rob