| Message ID | 1363818997-23137-7-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 20, 2013 at 07:36:27PM -0300, Ezequiel Garcia wrote: > Convert devm_request_and_ioremap() to the newly introduced > devm_ioremap_resource() which provides more consistent error handling. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > drivers/thermal/mvebu_thermal.c | 6 ++---- > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > index ef04e4e..089b43d 100644 > --- a/drivers/thermal/mvebu_thermal.c > +++ b/drivers/thermal/mvebu_thermal.c > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > - if (!priv->sensor) { > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > + if (!priv->sensor) > return -EADDRNOTAVAIL; > - } Hi Ezequiel It would be better to return priv->sensor, not a fixed error code. Andrew
On Thu, Mar 21, 2013 at 07:10:24AM +0100, Andrew Lunn wrote: > On Wed, Mar 20, 2013 at 07:36:27PM -0300, Ezequiel Garcia wrote: > > Convert devm_request_and_ioremap() to the newly introduced > > devm_ioremap_resource() which provides more consistent error handling. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > --- > > drivers/thermal/mvebu_thermal.c | 6 ++---- > > 1 files changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > > index ef04e4e..089b43d 100644 > > --- a/drivers/thermal/mvebu_thermal.c > > +++ b/drivers/thermal/mvebu_thermal.c > > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > > if (!priv) > > return -ENOMEM; > > > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > > - if (!priv->sensor) { > > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > > + if (!priv->sensor) > > return -EADDRNOTAVAIL; > > - } > > > It would be better to return priv->sensor, not a fixed error code. > Good catch! I missed that. Thanks,
On Thu, Mar 21, 2013 at 06:43:26AM -0300, Ezequiel Garcia wrote: > On Thu, Mar 21, 2013 at 07:10:24AM +0100, Andrew Lunn wrote: > > On Wed, Mar 20, 2013 at 07:36:27PM -0300, Ezequiel Garcia wrote: > > > Convert devm_request_and_ioremap() to the newly introduced > > > devm_ioremap_resource() which provides more consistent error handling. > > > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > > --- > > > drivers/thermal/mvebu_thermal.c | 6 ++---- > > > 1 files changed, 2 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > > > index ef04e4e..089b43d 100644 > > > --- a/drivers/thermal/mvebu_thermal.c > > > +++ b/drivers/thermal/mvebu_thermal.c > > > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > > > if (!priv) > > > return -ENOMEM; > > > > > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > > > - if (!priv->sensor) { > > > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > > > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > > > + if (!priv->sensor) > > > return -EADDRNOTAVAIL; > > > - } > > > > > > It would be better to return priv->sensor, not a fixed error code. > > > > Good catch! I missed that. And the same where the priv->control is introduced. Andrew
Hello. On 21-03-2013 2:36, Ezequiel Garcia wrote: > Convert devm_request_and_ioremap() to the newly introduced > devm_ioremap_resource() which provides more consistent error handling. > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > drivers/thermal/mvebu_thermal.c | 6 ++---- > 1 files changed, 2 insertions(+), 4 deletions(-) > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > index ef04e4e..089b43d 100644 > --- a/drivers/thermal/mvebu_thermal.c > +++ b/drivers/thermal/mvebu_thermal.c > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > - if (!priv->sensor) { > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > + if (!priv->sensor) devm_ioremap_resource() returns error code, not NULL. WBR, Sergei
Hi Sergei, On Thu, Mar 21, 2013 at 06:04:58PM +0400, Sergei Shtylyov wrote: > On 21-03-2013 2:36, Ezequiel Garcia wrote: > > > Convert devm_request_and_ioremap() to the newly introduced > > devm_ioremap_resource() which provides more consistent error handling. > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > --- > > drivers/thermal/mvebu_thermal.c | 6 ++---- > > 1 files changed, 2 insertions(+), 4 deletions(-) > > > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > > index ef04e4e..089b43d 100644 > > --- a/drivers/thermal/mvebu_thermal.c > > +++ b/drivers/thermal/mvebu_thermal.c > > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > > if (!priv) > > return -ENOMEM; > > > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > > - if (!priv->sensor) { > > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > > + if (!priv->sensor) > > devm_ioremap_resource() returns error code, not NULL. > Yes, you're right. Andrew has already pointed this out and I'll fix it in the next round. Thanks for the review!
On Wed, Mar 20, 2013 at 07:36:27PM -0300, Ezequiel Garcia wrote: > Convert devm_request_and_ioremap() to the newly introduced > devm_ioremap_resource() which provides more consistent error handling. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > drivers/thermal/mvebu_thermal.c | 6 ++---- > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > index ef04e4e..089b43d 100644 > --- a/drivers/thermal/mvebu_thermal.c > +++ b/drivers/thermal/mvebu_thermal.c > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > - if (!priv->sensor) { > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > + if (!priv->sensor) > return -EADDRNOTAVAIL; NAK. It pays to understand the interface that you're using. In this case, it *doesn't* return NULL on error, so you'll never reach -EADDRNOTAVAIL. Please look this function up in lib/devres.c and understand its API.
On Thu, Mar 21, 2013 at 07:17:51PM +0000, Russell King - ARM Linux wrote: > On Wed, Mar 20, 2013 at 07:36:27PM -0300, Ezequiel Garcia wrote: > > Convert devm_request_and_ioremap() to the newly introduced > > devm_ioremap_resource() which provides more consistent error handling. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > --- > > drivers/thermal/mvebu_thermal.c | 6 ++---- > > 1 files changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c > > index ef04e4e..089b43d 100644 > > --- a/drivers/thermal/mvebu_thermal.c > > +++ b/drivers/thermal/mvebu_thermal.c > > @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) > > if (!priv) > > return -ENOMEM; > > > > - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); > > - if (!priv->sensor) { > > - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); > > + priv->sensor = devm_ioremap_resource(&pdev->dev, res); > > + if (!priv->sensor) > > return -EADDRNOTAVAIL; > > NAK. It pays to understand the interface that you're using. In this > case, it *doesn't* return NULL on error, so you'll never reach > -EADDRNOTAVAIL. Please look this function up in lib/devres.c and > understand its API. > Yes, you're right, I'll fix this on v2. Thanks,
diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c index ef04e4e..089b43d 100644 --- a/drivers/thermal/mvebu_thermal.c +++ b/drivers/thermal/mvebu_thermal.c @@ -86,11 +86,9 @@ static int mvebu_thermal_probe(struct platform_device *pdev) if (!priv) return -ENOMEM; - priv->sensor = devm_request_and_ioremap(&pdev->dev, res); - if (!priv->sensor) { - dev_err(&pdev->dev, "Failed to request_ioremap memory\n"); + priv->sensor = devm_ioremap_resource(&pdev->dev, res); + if (!priv->sensor) return -EADDRNOTAVAIL; - } thermal = thermal_zone_device_register("mvebu_thermal", 0, 0, priv, &ops, NULL, 0, 0);
Convert devm_request_and_ioremap() to the newly introduced devm_ioremap_resource() which provides more consistent error handling. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- drivers/thermal/mvebu_thermal.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)