i2c: pxa: convert to devm_* API
RFC PATCH v3 2/3 i2c: pxa: convert to devm_* API
when i2c malloc faild, we should not try to call release_mem_region. aovid this, convert them to devm_* API.
Signed-off-by Libo Chen <[email protected]>
--- drivers/i2c/busses/i2c-pxa.c | 63 ++++++++++-------------------------------- 1 files changed, 15 insertions(+), 48 deletions(-)
drivers/i2c/busses/i2c-pxa.c +15 -48
@@ -1091,10 +1091,9 @@ static int i2c_pxa_probe(struct platform_device *dev)struct resource *res =NULL ;int ret, irq; - i2c =kzalloc (sizeof (struct pxa_i2c ),GFP_KERNEL ); + i2c =devm_kzalloc (&dev->dev,sizeof (struct pxa_i2c ),GFP_KERNEL );if (!i2c) { - ret = -ENOMEM ; - goto emalloc; +return -ENOMEM ; }/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
@@ -1104,19 +1103,11 @@ static int i2c_pxa_probe(struct platform_device *dev)if (ret >0 ) ret =i2c_pxa_probe_pdata (dev, i2c, &i2c_type);if (ret <0 ) - goto eclk; +return ret; - res =platform_get_resource (dev,IORESOURCE_MEM ,0 ); irq =platform_get_irq (dev,0 ); -if (res ==NULL || irq <0 ) { - ret = -ENODEV ; - goto eclk; - } - -if (!request_mem_region (res->start,resource_size (res), res->name)) { - ret = -ENOMEM ; - goto eclk; - } +if (irq <0 ) +return -ENODEV ; i2c->adap.owner =THIS_MODULE ; i2c->adap.retries =5 ;
@@ -1126,17 +1117,15 @@ static int i2c_pxa_probe(struct platform_device *dev)strlcpy (i2c->adap.name,"pxa_i2c-i2c" ,sizeof (i2c->adap.name)); - i2c->clk =clk_get (&dev->dev,NULL ); + i2c->clk =devm_clk_get (&dev->dev,NULL );if (IS_ERR (i2c->clk)) { - ret =PTR_ERR (i2c->clk); - goto eclk; +return PTR_ERR (i2c->clk); } - i2c->reg_base =ioremap (res->start,resource_size (res)); -if (!i2c->reg_base) { - ret = -EIO ; - goto eremap; - } + res =platform_get_resource (dev,IORESOURCE_MEM ,0 ); + i2c->reg_base =devm_ioremap_resource (&dev->dev, res); +if (IS_ERR (i2c->reg_base)) +return PTR_ERR (i2c->reg_bas); i2c->reg_ibmr = i2c->reg_base + pxa_reg_layout[i2c_type].ibmr; i2c->reg_idbr = i2c->reg_base + pxa_reg_layout[i2c_type].idbr;
@@ -1166,10 +1155,10 @@ static int i2c_pxa_probe(struct platform_device *dev) i2c->adap.algo = &i2c_pxa_pio_algorithm; }else { i2c->adap.algo = &i2c_pxa_algorithm; - ret =request_irq (irq, i2c_pxa_handler,IRQF_SHARED , -dev_name (&dev->dev), i2c); + ret =devm_request_irq (&dev->dev, irq, i2c_pxa_handler, +IRQF_SHARED ,dev_name (&dev->dev), i2c);if (ret) - goto ereqirq; +return ret; }i2c_pxa_reset (i2c);
@@ -1183,7 +1172,7 @@ static int i2c_pxa_probe(struct platform_device *dev) ret =i2c_add_numbered_adapter (&i2c->adap);if (ret <0 ) {printk (KERN_INFO "I2C: Failed to add bus\n" ); - goto eadapt; +return ret; }of_i2c_register_devices (&i2c->adap);
@@ -1198,19 +1187,6 @@ static int i2c_pxa_probe(struct platform_device *dev)#endif return 0 ; -eadapt: -if (!i2c->use_pio) -free_irq (irq, i2c); -ereqirq: -clk_disable (i2c->clk); -iounmap (i2c->reg_base); -eremap: -clk_put (i2c->clk); -eclk: -kfree (i2c); -emalloc: -release_mem_region (res->start,resource_size (res)); -return ret; }static int i2c_pxa_remove (struct platform_device *dev)
@@ -1218,15 +1194,6 @@ static int i2c_pxa_remove(struct platform_device *dev)struct pxa_i2c *i2c =platform_get_drvdata (dev);i2c_del_adapter (&i2c->adap); -if (!i2c->use_pio) -free_irq (i2c->irq, i2c); - -clk_disable (i2c->clk); -clk_put (i2c->clk); - -iounmap (i2c->reg_base); -release_mem_region (i2c->iobase, i2c->iosize); -kfree (i2c);return 0 ; }
Signature · 1 line
1.7.1
Hi Libo,
I think you can merge patch 1/3 and 2/3, they do the same thing that using devm_* API to simplify
and make the code clean, and the additional goal is it also can fix a bug. Besides, maybe you need to change the title and make it clear and self-described.
Thanks,
Gu
Quoted 13 lines · Libo Chen
when i2c malloc faild, we should not try to call
release_mem_region. aovid this, convert them to devm_* API.Signed-off-by Libo Chen <[email protected]>--- drivers/i2c/busses/i2c-pxa.c | 63 ++++++++++-------------------------------- 1 files changed, 15 insertions(+), 48 deletions(-)drivers/i2c/busses/i2c-pxa.c +0
-0
drivers/i2c/busses/i2c-pxa.c static int i2c_pxa_probe(struct platform_device *dev) · line 1097
Quoted 10 lines
drivers/i2c/busses/i2c-pxa.c +2
-3@@ -1091,10 +1091,9 @@ static int i2c_pxa_probe(struct platform_device *dev)struct resource *res =NULL ;int ret, irq; - i2c =kzalloc (sizeof (struct pxa_i2c ),GFP_KERNEL ); + i2c =devm_kzalloc (&dev->dev,sizeof (struct pxa_i2c ),GFP_KERNEL );if (!i2c) { - ret = -ENOMEM ; - goto emalloc; +return -ENOMEM ; }
The {} pair is no longer needed.
Quoted 26 lines
drivers/i2c/busses/i2c-pxa.c +3
-11/* Default adapter num to device id; i2c_pxa_probe_dt can override. */ @@ -1104,19 +1103,11 @@ static int i2c_pxa_probe(struct platform_device *dev)if (ret >0 ) ret =i2c_pxa_probe_pdata (dev, i2c, &i2c_type);if (ret <0 ) - goto eclk; +return ret; - res =platform_get_resource (dev,IORESOURCE_MEM ,0 ); irq =platform_get_irq (dev,0 ); -if (res ==NULL || irq <0 ) { - ret = -ENODEV ; - goto eclk; - } - -if (!request_mem_region (res->start,resource_size (res), res->name)) { - ret = -ENOMEM ; - goto eclk; - } +if (irq <0 ) +return -ENODEV ; i2c->adap.owner =THIS_MODULE ; i2c->adap.retries =5 ;
drivers/i2c/busses/i2c-pxa.c static int i2c_pxa_probe(struct platform_device *dev) · line 1123
Quoted 10 lines
drivers/i2c/busses/i2c-pxa.c +2
-3@@ -1126,17 +1117,15 @@ static int i2c_pxa_probe(struct platform_device *dev)strlcpy (i2c->adap.name,"pxa_i2c-i2c" ,sizeof (i2c->adap.name)); - i2c->clk =clk_get (&dev->dev,NULL ); + i2c->clk =devm_clk_get (&dev->dev,NULL );if (IS_ERR (i2c->clk)) { - ret =PTR_ERR (i2c->clk); - goto eclk; +return PTR_ERR (i2c->clk); }
the same.
Quoted 72 lines
drivers/i2c/busses/i2c-pxa.c +8
-31- i2c->reg_base =ioremap (res->start,resource_size (res)); -if (!i2c->reg_base) { - ret = -EIO ; - goto eremap; - } + res =platform_get_resource (dev,IORESOURCE_MEM ,0 ); + i2c->reg_base =devm_ioremap_resource (&dev->dev, res); +if (IS_ERR (i2c->reg_base)) +return PTR_ERR (i2c->reg_bas); i2c->reg_ibmr = i2c->reg_base + pxa_reg_layout[i2c_type].ibmr; i2c->reg_idbr = i2c->reg_base + pxa_reg_layout[i2c_type].idbr;@@ -1166,10 +1155,10 @@ static int i2c_pxa_probe(struct platform_device *dev) i2c->adap.algo = &i2c_pxa_pio_algorithm; }else { i2c->adap.algo = &i2c_pxa_algorithm; - ret =request_irq (irq, i2c_pxa_handler,IRQF_SHARED , -dev_name (&dev->dev), i2c); + ret =devm_request_irq (&dev->dev, irq, i2c_pxa_handler, +IRQF_SHARED ,dev_name (&dev->dev), i2c);if (ret) - goto ereqirq; +return ret; }i2c_pxa_reset (i2c);@@ -1183,7 +1172,7 @@ static int i2c_pxa_probe(struct platform_device *dev) ret =i2c_add_numbered_adapter (&i2c->adap);if (ret <0 ) {printk (KERN_INFO "I2C: Failed to add bus\n" ); - goto eadapt; +return ret; }of_i2c_register_devices (&i2c->adap);@@ -1198,19 +1187,6 @@ static int i2c_pxa_probe(struct platform_device *dev)#endif return 0 ; -eadapt: -if (!i2c->use_pio) -free_irq (irq, i2c); -ereqirq: -clk_disable (i2c->clk); -iounmap (i2c->reg_base); -eremap: -clk_put (i2c->clk); -eclk: -kfree (i2c); -emalloc: -release_mem_region (res->start,resource_size (res)); -return ret; }static int i2c_pxa_remove (struct platform_device *dev)@@ -1218,15 +1194,6 @@ static int i2c_pxa_remove(struct platform_device *dev)struct pxa_i2c *i2c =platform_get_drvdata (dev);i2c_del_adapter (&i2c->adap); -if (!i2c->use_pio) -free_irq (i2c->irq, i2c); - -clk_disable (i2c->clk); -clk_put (i2c->clk); - -iounmap (i2c->reg_base); -release_mem_region (i2c->iobase, i2c->iosize); -kfree (i2c);return 0 ; }
Quoted 3 lines · Gu Zheng
Hi Libo,
I think you can merge patch 1/3 and 2/3, they do the same thing that using devm_* API to simplify
and make the code clean, and the additional goal is it also can fix a bug.
nope. they should be separated.
Quoted 3 lines
Besides, maybe you need to
change the title and make it clear and self-described.
Quoted 7 lines · Gu Zheng
Hi Libo,
I think you can merge patch 1/3 and 2/3, they e thdo the saming that using devm_* API to simplify
and make the code clean, and the additional goal is it also can fix a bug. Besides, maybe you need to change the title and make it clear and self-described.Thanks,
Gu
thanks for your suggestion. But, I had referenced other guy`s patch title, they usually do like this, so I think it is enough.
Regards,
Libo
Quoted 4 lines · Libo Chen
when i2c malloc faild, we should not try to call
release_mem_region. aovid this, convert them to devm_* API.Signed-off-by Libo Chen <[email protected]>
First, you did it right. This patch should not be combined with 1/3. It is not a trivial cleanup, so I prefer to keep them seperated.
BTW did you test these patches on real hardware?
Quoted 23 lines
drivers/i2c/busses/i2c-pxa.c +0
-13@@ -1198,19 +1187,6 @@ static int i2c_pxa_probe(struct platform_device *dev)#endif return 0 ; -eadapt: -if (!i2c->use_pio) -free_irq (irq, i2c); -ereqirq: -clk_disable (i2c->clk); -iounmap (i2c->reg_base); -eremap: -clk_put (i2c->clk); -eclk: -kfree (i2c); -emalloc: -release_mem_region (res->start,resource_size (res)); -return ret; }static int i2c_pxa_remove (struct platform_device *dev)@@ -1218,15 +1194,6 @@ static int i2c_pxa_remove(struct platform_device *dev)struct pxa_i2c *i2c =platform_get_drvdata (dev);
drivers/i2c/busses/i2c-pxa.c static int i2c_pxa_remove(struct platform_device *dev) · line 1229
Quoted 10 lines
drivers/i2c/busses/i2c-pxa.c +0
-9i2c_del_adapter (&i2c->adap); -if (!i2c->use_pio) -free_irq (i2c->irq, i2c); - -clk_disable (i2c->clk); -clk_put (i2c->clk); - -iounmap (i2c->reg_base); -release_mem_region (i2c->iobase, i2c->iosize); -kfree (i2c);
You remove too much. Who disables the clock now?