| Message ID | 002801cefaf4$69243d70$3b6cb850$%han@samsung.com |
|---|---|
| State | Deferred |
| Headers | show |
Hi Jingoo Han, On Tue, 17 Dec 2013 15:51:28 +0900, Jingoo Han wrote: > Use devm_request_region() to make cleanup paths simpler. You need to include <linux/ioport.h> for this function. > Signed-off-by: Jingoo Han <jg1.han@samsung.com> > --- > drivers/i2c/busses/i2c-isch.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c > index 8c38aaa..fb02df2 100644 > --- a/drivers/i2c/busses/i2c-isch.c > +++ b/drivers/i2c/busses/i2c-isch.c > @@ -275,7 +275,8 @@ static int smbus_sch_probe(struct platform_device *dev) > if (!res) > return -EBUSY; > > - if (!request_region(res->start, resource_size(res), dev->name)) { > + if (!devm_request_region(&dev->dev, res->start, resource_size(res), > + dev->name)) { > dev_err(&dev->dev, "SMBus region 0x%x already in use!\n", > sch_smba); > return -EBUSY; > @@ -294,7 +295,6 @@ static int smbus_sch_probe(struct platform_device *dev) > retval = i2c_add_adapter(&sch_adapter); > if (retval) { > dev_err(&dev->dev, "Couldn't register adapter!\n"); > - release_region(res->start, resource_size(res)); > sch_smba = 0; > } > > @@ -307,7 +307,6 @@ static int smbus_sch_remove(struct platform_device *pdev) > if (sch_smba) { > i2c_del_adapter(&sch_adapter); > res = platform_get_resource(pdev, IORESOURCE_IO, 0); > - release_region(res->start, resource_size(res)); > sch_smba = 0; > } "res" becomes useless in the remove function as you removed the only use of it. So you can simplify this function further.
On Tuesday, December 17, 2013 4:53 PM, Jean Delvare wrote: > On Tue, 17 Dec 2013 15:51:28 +0900, Jingoo Han wrote: > > Use devm_request_region() to make cleanup paths simpler. > > You need to include <linux/ioport.h> for this function. According to the i2c tree, <linux/ioport.h> is already included. :-) ./ drivers/i2c/busses/i2c-isch.c #include <linux/module.h> #include <linux/platform_device.h> #include <linux/kernel.h> #include <linux/delay.h> #include <linux/stddef.h> #include <linux/ioport.h> <-- Here! > > > Signed-off-by: Jingoo Han <jg1.han@samsung.com> > > --- > > drivers/i2c/busses/i2c-isch.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) [.....] > > @@ -307,7 +307,6 @@ static int smbus_sch_remove(struct platform_device *pdev) > > if (sch_smba) { > > i2c_del_adapter(&sch_adapter); > > res = platform_get_resource(pdev, IORESOURCE_IO, 0); > > - release_region(res->start, resource_size(res)); > > sch_smba = 0; > > } > > "res" becomes useless in the remove function as you removed the only > use of it. So you can simplify this function further. OK, you're right. I will remove tge unnecessary "res" variable. I will send V2 patch, soon. Thank you for your comment. :-) Best regards, Jingoo Han -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 18 Dec 2013 10:42:39 +0900, Jingoo Han wrote: > On Tuesday, December 17, 2013 4:53 PM, Jean Delvare wrote: > > On Tue, 17 Dec 2013 15:51:28 +0900, Jingoo Han wrote: > > > Use devm_request_region() to make cleanup paths simpler. > > > > You need to include <linux/ioport.h> for this function. > > According to the i2c tree, <linux/ioport.h> is already included. :-) > > ./ drivers/i2c/busses/i2c-isch.c > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/kernel.h> > #include <linux/delay.h> > #include <linux/stddef.h> > #include <linux/ioport.h> <-- Here! Oops, you're right, don't know how I managed to miss it, sorry.
diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c index 8c38aaa..fb02df2 100644 --- a/drivers/i2c/busses/i2c-isch.c +++ b/drivers/i2c/busses/i2c-isch.c @@ -275,7 +275,8 @@ static int smbus_sch_probe(struct platform_device *dev) if (!res) return -EBUSY; - if (!request_region(res->start, resource_size(res), dev->name)) { + if (!devm_request_region(&dev->dev, res->start, resource_size(res), + dev->name)) { dev_err(&dev->dev, "SMBus region 0x%x already in use!\n", sch_smba); return -EBUSY; @@ -294,7 +295,6 @@ static int smbus_sch_probe(struct platform_device *dev) retval = i2c_add_adapter(&sch_adapter); if (retval) { dev_err(&dev->dev, "Couldn't register adapter!\n"); - release_region(res->start, resource_size(res)); sch_smba = 0; } @@ -307,7 +307,6 @@ static int smbus_sch_remove(struct platform_device *pdev) if (sch_smba) { i2c_del_adapter(&sch_adapter); res = platform_get_resource(pdev, IORESOURCE_IO, 0); - release_region(res->start, resource_size(res)); sch_smba = 0; }
Use devm_request_region() to make cleanup paths simpler. Signed-off-by: Jingoo Han <jg1.han@samsung.com> --- drivers/i2c/busses/i2c-isch.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)