Re: [PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

2012-07-05 Thread Sylwester Nawrocki
On 07/04/2012 06:11 PM, Mark Brown wrote:
 Saves some error handling and a small amount of code.
 
 Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
 Acked-by: Linus Walleij linus.wall...@linaro.org
 ---
  drivers/spi/spi-s3c64xx.c |   25 +
  1 file changed, 1 insertion(+), 24 deletions(-)
 
 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index f4e2341..b7aeb5d 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -1028,19 +1028,7 @@ static int __init s3c64xx_spi_probe(struct 
 platform_device *pdev)
   /* the spi-mode bits understood by this driver: */
   master-mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
  
 - if (request_mem_region(mem_res-start,
 - resource_size(mem_res), pdev-name) == NULL) {
 - dev_err(pdev-dev, Req mem region failed\n);
 - ret = -ENXIO;
 - goto err0;
 - }
 -
 - sdd-regs = ioremap(mem_res-start, resource_size(mem_res));
 - if (sdd-regs == NULL) {
 - dev_err(pdev-dev, Unable to remap IO\n);
 - ret = -ENXIO;
 - goto err1;
 - }
 + sdd-regs = devm_request_and_ioremap(pdev-dev, mem_res);

It doesn't seem right. Why is is the check for valid sdd-regs removed ?
This should have rather been:

+   sdd-regs = devm_request_and_ioremap(pdev-dev, mem_res);
+   if (sdd-regs == NULL) {
+   dev_err(pdev-dev, Failed to request memory region\n);
+   return -ENXIO;
+   }

Currently whne devm_request_and_ioremap() fails and returns NULL kernel oops
would happen right on first writel() in s3c64xx_spi_hwninit().


Thanks,
Sylwester

   if (sci-cfg_gpio == NULL || sci-cfg_gpio(pdev)) {
   dev_err(pdev-dev, Unable to config gpio\n);
 @@ -1124,10 +1112,6 @@ err4:
   clk_put(sdd-clk);
  err3:
  err2:
 - iounmap((void *) sdd-regs);
 -err1:
 - release_mem_region(mem_res-start, resource_size(mem_res));
 -err0:
   platform_set_drvdata(pdev, NULL);
   spi_master_put(master);
  
 @@ -1138,7 +1122,6 @@ static int s3c64xx_spi_remove(struct platform_device 
 *pdev)
  {
   struct spi_master *master = spi_master_get(platform_get_drvdata(pdev));
   struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
 - struct resource *mem_res;
  
   pm_runtime_disable(pdev-dev);
  
 @@ -1154,12 +1137,6 @@ static int s3c64xx_spi_remove(struct platform_device 
 *pdev)
   clk_disable(sdd-clk);
   clk_put(sdd-clk);
  
 - iounmap((void *) sdd-regs);
 -
 - mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 - if (mem_res != NULL)
 - release_mem_region(mem_res-start, resource_size(mem_res));
 -
   platform_set_drvdata(pdev, NULL);
   spi_master_put(master);
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

2012-07-05 Thread Mark Brown
On Thu, Jul 05, 2012 at 10:41:04AM +0200, Sylwester Nawrocki wrote:

 It doesn't seem right. Why is is the check for valid sdd-regs removed ?
 This should have rather been:

Mostly just because the structure of the code is a bit error prone when
making quick updates with the if statement afterwards that looks like
error handling.


signature.asc
Description: Digital signature


Re: [PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

2012-06-28 Thread Linus Walleij
On Thu, Jun 28, 2012 at 3:23 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:

 Saves some error handling and a small amount of code.

 Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com

Elegant, monsieur.
Acked-by: Linus Walleij linus.wall...@linaro.org

I'm starting to wonder if it would not be possible to mass-convert these
using coccinelle.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

2012-06-28 Thread Mark Brown
On Thu, Jun 28, 2012 at 08:51:44PM +0200, Linus Walleij wrote:

 I'm starting to wonder if it would not be possible to mass-convert these
 using coccinelle.

Probably, yes.  I'm not actually going through these particularly,
really what I'm doing is looking to find drivers I run on my systems
that might be able to use regmap-mmio and fixing things I find as I
look.


signature.asc
Description: Digital signature