[patch v2] usb: phy: msm: change devm_ioremap() to devm_ioremap_resource()

[patch v2] usb: phy: msm: change devm_ioremap() to devm_ioremap_resource()

[Dan Carpenter]

There are several issues here:
1) platform_get_resource() can return NULL and that wasn't handled.
2) We should request the memory before we remap it, and
   devm_ioremap_resource() does that.
3) devm_ioremap() returns a NULL but we were checking for IS_ERR().

Fixes: 6b99c68ec1f9 ('usb: phy: msm: Migrate to Managed Device Resource allocation')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: the first version just swapped the IS_ERR() check for a NULL check

Thanks everyone for the review.  In the end, it was still a one line fix
so I forgive you Ivan.

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index c522c4f..4f88174 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1586,7 +1586,7 @@ static int msm_otg_probe(struct platform_device *pdev)
 				      np ? "alt_core" : "usb_hs_core_clk");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	motg->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	motg->regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(motg->regs))
 		return PTR_ERR(motg->regs);
 

Re: [patch v2] usb: phy: msm: change devm_ioremap() to devm_ioremap_resource()

[Ivan T. Ivanov]

On Wed, 2014-05-14 at 16:54 +0300, Dan Carpenter wrote:
> There are several issues here:
> 1) platform_get_resource() can return NULL and that wasn't handled.
> 2) We should request the memory before we remap it, and
>    devm_ioremap_resource() does that.
> 3) devm_ioremap() returns a NULL but we were checking for IS_ERR().
> 
> Fixes: 6b99c68ec1f9 ('usb: phy: msm: Migrate to Managed Device Resource allocation')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: the first version just swapped the IS_ERR() check for a NULL check
> 
> Thanks everyone for the review.  In the end, it was still a one line fix
> so I forgive you Ivan.
> 
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> index c522c4f..4f88174 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -1586,7 +1586,7 @@ static int msm_otg_probe(struct platform_device *pdev)
>  				      np ? "alt_core" : "usb_hs_core_clk");
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	motg->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	motg->regs = devm_ioremap_resource(&pdev->dev, res);


Thank you Dan, but this will not going to work. Address space is 
shared between this PHY driver and CI device controller driver.
First version plus adding required check for 'res' as suggested
by Sergei will fix this.

Regards,
Ivan

Re: [patch v2] usb: phy: msm: change devm_ioremap() to devm_ioremap_resource()

[Dan Carpenter]

On Mon, May 19, 2014 at 12:19:10PM +0300, Ivan T. Ivanov wrote:
> On Wed, 2014-05-14 at 16:54 +0300, Dan Carpenter wrote:
> > There are several issues here:
> > 1) platform_get_resource() can return NULL and that wasn't handled.
> > 2) We should request the memory before we remap it, and
> >    devm_ioremap_resource() does that.
> > 3) devm_ioremap() returns a NULL but we were checking for IS_ERR().
> > 
> > Fixes: 6b99c68ec1f9 ('usb: phy: msm: Migrate to Managed Device Resource allocation')
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: the first version just swapped the IS_ERR() check for a NULL check
> > 
> > Thanks everyone for the review.  In the end, it was still a one line fix
> > so I forgive you Ivan.
> > 
> > diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> > index c522c4f..4f88174 100644
> > --- a/drivers/usb/phy/phy-msm-usb.c
> > +++ b/drivers/usb/phy/phy-msm-usb.c
> > @@ -1586,7 +1586,7 @@ static int msm_otg_probe(struct platform_device *pdev)
> >  				      np ? "alt_core" : "usb_hs_core_clk");
> >  
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	motg->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> > +	motg->regs = devm_ioremap_resource(&pdev->dev, res);
> 
> 
> Thank you Dan, but this will not going to work. Address space is 
> shared between this PHY driver and CI device controller driver.
> First version plus adding required check for 'res' as suggested
> by Sergei will fix this.
> 

People, I'm really sorry about this.  I knew there was a risk when I
sent the patch.  I don't have a way to test this and my changelog was
too confident, more than it should have been.  This was my fault.

I will send a patch to fix it.

regards,
dan carpenter