| Message ID | E1Ud4rN-0004Jc-CL@rmk-PC.arm.linux.org.uk |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, 16 May 2013 21:33:09 +0100 Russell King <rmk+kernel@arm.linux.org.uk> wrote: > Eliminate reg_base_p and reg_size, mv64xxx_i2c_unmap_regs() and an > unchecked ioremap() return from this driver by using the devm_* > API for requesting and ioremapping resources. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > drivers/i2c/busses/i2c-mv64xxx.c | 46 +++++--------------------------------- > 1 files changed, 6 insertions(+), 40 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c > index 0339cd8..19cc9bf 100644 > --- a/drivers/i2c/busses/i2c-mv64xxx.c > +++ b/drivers/i2c/busses/i2c-mv64xxx.c [snip] > @@ -704,7 +671,6 @@ mv64xxx_i2c_remove(struct platform_device *dev) > > rc = i2c_del_adapter(&drv_data->adapter); > free_irq(drv_data->irq, drv_data); > - mv64xxx_i2c_unmap_regs(drv_data); > #if defined(CONFIG_HAVE_CLK) > /* Not all platforms have a clk */ > if (!IS_ERR(drv_data->clk)) { The patch does not apply: it seems it lacks: + int rc; - i2c_del_adapter(&drv_data->adapter); + rc = i2c_del_adapter(&drv_data->adapter); - return 0; + return rc; BTW, is this return code useful?
On Fri, May 17, 2013 at 11:23:16AM +0200, Jean-Francois Moine wrote: > On Thu, 16 May 2013 21:33:09 +0100 > Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > > Eliminate reg_base_p and reg_size, mv64xxx_i2c_unmap_regs() and an > > unchecked ioremap() return from this driver by using the devm_* > > API for requesting and ioremapping resources. > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > drivers/i2c/busses/i2c-mv64xxx.c | 46 +++++--------------------------------- > > 1 files changed, 6 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c > > index 0339cd8..19cc9bf 100644 > > --- a/drivers/i2c/busses/i2c-mv64xxx.c > > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > [snip] > > @@ -704,7 +671,6 @@ mv64xxx_i2c_remove(struct platform_device *dev) > > > > rc = i2c_del_adapter(&drv_data->adapter); > > free_irq(drv_data->irq, drv_data); > > - mv64xxx_i2c_unmap_regs(drv_data); > > #if defined(CONFIG_HAVE_CLK) > > /* Not all platforms have a clk */ > > if (!IS_ERR(drv_data->clk)) { > > The patch does not apply: it seems it lacks: I should've mentioned that the patches are against v3.9, not v3.10-rc1. > + int rc; > > - i2c_del_adapter(&drv_data->adapter); > + rc = i2c_del_adapter(&drv_data->adapter); > > - return 0; > + return rc; > > BTW, is this return code useful? No it isn't. Removals *can't* fail. Once the remove function is called, the device _will_ be unbound no matter what the return value of that function is. The module will also be unloaded (if that's why it's being unbound) whether or not you return an error. So, removals must always succeed. Why their return type hasn't been void from the start I've no idea - but that's a question for Patrick Mochel who implemented the original driver model design. -- 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
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index 0339cd8..19cc9bf 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -92,8 +92,6 @@ struct mv64xxx_i2c_data { u32 aborting; u32 cntl_bits; void __iomem *reg_base; - u32 reg_base_p; - u32 reg_size; u32 addr1; u32 addr2; u32 bytes_left; @@ -495,40 +493,6 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = { * ***************************************************************************** */ -static int -mv64xxx_i2c_map_regs(struct platform_device *pd, - struct mv64xxx_i2c_data *drv_data) -{ - int size; - struct resource *r = platform_get_resource(pd, IORESOURCE_MEM, 0); - - if (!r) - return -ENODEV; - - size = resource_size(r); - - if (!request_mem_region(r->start, size, drv_data->adapter.name)) - return -EBUSY; - - drv_data->reg_base = ioremap(r->start, size); - drv_data->reg_base_p = r->start; - drv_data->reg_size = size; - - return 0; -} - -static void -mv64xxx_i2c_unmap_regs(struct mv64xxx_i2c_data *drv_data) -{ - if (drv_data->reg_base) { - iounmap(drv_data->reg_base); - release_mem_region(drv_data->reg_base_p, drv_data->reg_size); - } - - drv_data->reg_base = NULL; - drv_data->reg_base_p = 0; -} - #ifdef CONFIG_OF static int mv64xxx_calc_freq(const int tclk, const int n, const int m) @@ -610,6 +574,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) { struct mv64xxx_i2c_data *drv_data; struct mv64xxx_i2c_pdata *pdata = pd->dev.platform_data; + struct resource *r; int rc; if ((!pdata && !pd->dev.of_node)) @@ -619,9 +584,12 @@ mv64xxx_i2c_probe(struct platform_device *pd) if (!drv_data) return -ENOMEM; - rc = mv64xxx_i2c_map_regs(pd, drv_data); - if (rc) + r = platform_get_resource(pd, IORESOURCE_MEM, 0); + drv_data->reg_base = devm_ioremap_resource(&pd->dev, r); + if (IS_ERR(drv_data->reg_base)) { + rc = PTR_ERR(drv_data->reg_base); goto exit_kfree; + } strlcpy(drv_data->adapter.name, MV64XXX_I2C_CTLR_NAME " adapter", sizeof(drv_data->adapter.name)); @@ -690,7 +658,6 @@ mv64xxx_i2c_probe(struct platform_device *pd) clk_unprepare(drv_data->clk); } #endif - mv64xxx_i2c_unmap_regs(drv_data); exit_kfree: kfree(drv_data); return rc; @@ -704,7 +671,6 @@ mv64xxx_i2c_remove(struct platform_device *dev) rc = i2c_del_adapter(&drv_data->adapter); free_irq(drv_data->irq, drv_data); - mv64xxx_i2c_unmap_regs(drv_data); #if defined(CONFIG_HAVE_CLK) /* Not all platforms have a clk */ if (!IS_ERR(drv_data->clk)) {
Eliminate reg_base_p and reg_size, mv64xxx_i2c_unmap_regs() and an unchecked ioremap() return from this driver by using the devm_* API for requesting and ioremapping resources. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/i2c/busses/i2c-mv64xxx.c | 46 +++++--------------------------------- 1 files changed, 6 insertions(+), 40 deletions(-)