[PATCH 1/3] video: xilinxfb: Use standard variable name convention

[PATCH 1/3] video: xilinxfb: Use standard variable name convention

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2571 bytes --]

s/op/pdev/ in xilinxfb_of_probe().
No functional chagnes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/video/xilinxfb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 84c664e..123cd70 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -413,7 +413,7 @@ static int xilinxfb_release(struct device *dev)
  * OF bus binding
  */

-static int xilinxfb_of_probe(struct platform_device *op)
+static int xilinxfb_of_probe(struct platform_device *pdev)
 {
 	const u32 *prop;
 	u32 tft_access = 0;
@@ -427,7 +427,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	/* Allocate the driver data region */
 	drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata) {
-		dev_err(&op->dev, "Couldn't allocate device private record\n");
+		dev_err(&pdev->dev, "Couldn't allocate device private record\n");
 		return -ENOMEM;
 	}

@@ -435,7 +435,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	 * To check whether the core is connected directly to DCR or BUS
 	 * interface and initialize the tft_access accordingly.
 	 */
-	of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if",
+	of_property_read_u32(pdev->dev.of_node, "xlnx,dcr-splb-slave-if",
 			     &tft_access);

 	/*
@@ -459,29 +459,29 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	}
 #endif

-	prop = of_get_property(op->dev.of_node, "phys-size", &size);
+	prop = of_get_property(pdev->dev.of_node, "phys-size", &size);
 	if ((prop) && (size >= sizeof(u32)*2)) {
 		pdata.screen_width_mm = prop[0];
 		pdata.screen_height_mm = prop[1];
 	}

-	prop = of_get_property(op->dev.of_node, "resolution", &size);
+	prop = of_get_property(pdev->dev.of_node, "resolution", &size);
 	if ((prop) && (size >= sizeof(u32)*2)) {
 		pdata.xres = prop[0];
 		pdata.yres = prop[1];
 	}

-	prop = of_get_property(op->dev.of_node, "virtual-resolution", &size);
+	prop = of_get_property(pdev->dev.of_node, "virtual-resolution", &size);
 	if ((prop) && (size >= sizeof(u32)*2)) {
 		pdata.xvirt = prop[0];
 		pdata.yvirt = prop[1];
 	}

-	if (of_find_property(op->dev.of_node, "rotate-display", NULL))
+	if (of_find_property(pdev->dev.of_node, "rotate-display", NULL))
 		pdata.rotate_screen = 1;

-	dev_set_drvdata(&op->dev, drvdata);
-	return xilinxfb_assign(op, drvdata, &pdata);
+	dev_set_drvdata(&pdev->dev, drvdata);
+	return xilinxfb_assign(pdev, drvdata, &pdata);
 }

 static int xilinxfb_of_remove(struct platform_device *op)
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 1/3] video: xilinxfb: Use standard variable name convention

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2571 bytes --]

s/op/pdev/ in xilinxfb_of_probe().
No functional chagnes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/video/xilinxfb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 84c664e..123cd70 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -413,7 +413,7 @@ static int xilinxfb_release(struct device *dev)
  * OF bus binding
  */

-static int xilinxfb_of_probe(struct platform_device *op)
+static int xilinxfb_of_probe(struct platform_device *pdev)
 {
 	const u32 *prop;
 	u32 tft_access = 0;
@@ -427,7 +427,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	/* Allocate the driver data region */
 	drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata) {
-		dev_err(&op->dev, "Couldn't allocate device private record\n");
+		dev_err(&pdev->dev, "Couldn't allocate device private record\n");
 		return -ENOMEM;
 	}

@@ -435,7 +435,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	 * To check whether the core is connected directly to DCR or BUS
 	 * interface and initialize the tft_access accordingly.
 	 */
-	of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if",
+	of_property_read_u32(pdev->dev.of_node, "xlnx,dcr-splb-slave-if",
 			     &tft_access);

 	/*
@@ -459,29 +459,29 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	}
 #endif

-	prop = of_get_property(op->dev.of_node, "phys-size", &size);
+	prop = of_get_property(pdev->dev.of_node, "phys-size", &size);
 	if ((prop) && (size >= sizeof(u32)*2)) {
 		pdata.screen_width_mm = prop[0];
 		pdata.screen_height_mm = prop[1];
 	}

-	prop = of_get_property(op->dev.of_node, "resolution", &size);
+	prop = of_get_property(pdev->dev.of_node, "resolution", &size);
 	if ((prop) && (size >= sizeof(u32)*2)) {
 		pdata.xres = prop[0];
 		pdata.yres = prop[1];
 	}

-	prop = of_get_property(op->dev.of_node, "virtual-resolution", &size);
+	prop = of_get_property(pdev->dev.of_node, "virtual-resolution", &size);
 	if ((prop) && (size >= sizeof(u32)*2)) {
 		pdata.xvirt = prop[0];
 		pdata.yvirt = prop[1];
 	}

-	if (of_find_property(op->dev.of_node, "rotate-display", NULL))
+	if (of_find_property(pdev->dev.of_node, "rotate-display", NULL))
 		pdata.rotate_screen = 1;

-	dev_set_drvdata(&op->dev, drvdata);
-	return xilinxfb_assign(op, drvdata, &pdata);
+	dev_set_drvdata(&pdev->dev, drvdata);
+	return xilinxfb_assign(pdev, drvdata, &pdata);
 }

 static int xilinxfb_of_remove(struct platform_device *op)
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 2/3] video: xilinxfb: Use devm_kzalloc instead of kzalloc

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Simplify driver probe and release function.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/video/xilinxfb.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 123cd70..fd9c430 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -368,7 +368,6 @@ err_fbmem:
 		devm_iounmap(dev, drvdata->regs);

 err_region:
-	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);

 	return rc;
@@ -403,7 +402,6 @@ static int xilinxfb_release(struct device *dev)
 		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 #endif

-	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);

 	return 0;
@@ -425,7 +423,7 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
 	pdata = xilinx_fb_default_pdata;

 	/* Allocate the driver data region */
-	drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL);
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata) {
 		dev_err(&pdev->dev, "Couldn't allocate device private record\n");
 		return -ENOMEM;
@@ -453,7 +451,6 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
 		drvdata->dcr_host = dcr_map(op->dev.of_node, start, drvdata->dcr_len);
 		if (!DCR_MAP_OK(drvdata->dcr_host)) {
 			dev_err(&op->dev, "invalid DCR address\n");
-			kfree(drvdata);
 			return -ENODEV;
 		}
 	}
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 2/3] video: xilinxfb: Use devm_kzalloc instead of kzalloc

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Simplify driver probe and release function.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/video/xilinxfb.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 123cd70..fd9c430 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -368,7 +368,6 @@ err_fbmem:
 		devm_iounmap(dev, drvdata->regs);

 err_region:
-	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);

 	return rc;
@@ -403,7 +402,6 @@ static int xilinxfb_release(struct device *dev)
 		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 #endif

-	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);

 	return 0;
@@ -425,7 +423,7 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
 	pdata = xilinx_fb_default_pdata;

 	/* Allocate the driver data region */
-	drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL);
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata) {
 		dev_err(&pdev->dev, "Couldn't allocate device private record\n");
 		return -ENOMEM;
@@ -453,7 +451,6 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
 		drvdata->dcr_host = dcr_map(op->dev.of_node, start, drvdata->dcr_len);
 		if (!DCR_MAP_OK(drvdata->dcr_host)) {
 			dev_err(&op->dev, "invalid DCR address\n");
-			kfree(drvdata);
 			return -ENODEV;
 		}
 	}
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

devm_iounmap is called automatically that's why remove it from the code
dev_set_drvdata(dev, NULL) is called by generic code
after device_release or on probe failure.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/video/xilinxfb.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index fd9c430..7e3036c 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -260,10 +260,9 @@ static int xilinxfb_assign(struct platform_device *pdev,

 		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 		drvdata->regs = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(drvdata->regs)) {
-			rc = PTR_ERR(drvdata->regs);
-			goto err_region;
-		}
+		if (IS_ERR(drvdata->regs))
+			return PTR_ERR(drvdata->regs);
+
 		drvdata->regs_phys = res->start;
 	}

@@ -279,11 +278,7 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	if (!drvdata->fb_virt) {
 		dev_err(dev, "Could not allocate frame buffer memory\n");
-		rc = -ENOMEM;
-		if (drvdata->flags & BUS_ACCESS_FLAG)
-			goto err_fbmem;
-		else
-			goto err_region;
+		return -ENOMEM;
 	}

 	/* Clear (turn to black) the framebuffer */
@@ -363,13 +358,6 @@ err_cmap:
 	/* Turn off the display */
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

-err_fbmem:
-	if (drvdata->flags & BUS_ACCESS_FLAG)
-		devm_iounmap(dev, drvdata->regs);
-
-err_region:
-	dev_set_drvdata(dev, NULL);
-
 	return rc;
 }

@@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
 	/* Turn off the display */
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

-	/* Release the resources, as allocated based on interface */
-	if (drvdata->flags & BUS_ACCESS_FLAG)
-		devm_iounmap(dev, drvdata->regs);
 #ifdef CONFIG_PPC_DCR
-	else
+	/* Release the resources, as allocated based on interface */
+	if (!(drvdata->flags & BUS_ACCESS_FLAG))
 		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 #endif

-	dev_set_drvdata(dev, NULL);
-
 	return 0;
 }

--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

devm_iounmap is called automatically that's why remove it from the code
dev_set_drvdata(dev, NULL) is called by generic code
after device_release or on probe failure.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/video/xilinxfb.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index fd9c430..7e3036c 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -260,10 +260,9 @@ static int xilinxfb_assign(struct platform_device *pdev,

 		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 		drvdata->regs = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(drvdata->regs)) {
-			rc = PTR_ERR(drvdata->regs);
-			goto err_region;
-		}
+		if (IS_ERR(drvdata->regs))
+			return PTR_ERR(drvdata->regs);
+
 		drvdata->regs_phys = res->start;
 	}

@@ -279,11 +278,7 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	if (!drvdata->fb_virt) {
 		dev_err(dev, "Could not allocate frame buffer memory\n");
-		rc = -ENOMEM;
-		if (drvdata->flags & BUS_ACCESS_FLAG)
-			goto err_fbmem;
-		else
-			goto err_region;
+		return -ENOMEM;
 	}

 	/* Clear (turn to black) the framebuffer */
@@ -363,13 +358,6 @@ err_cmap:
 	/* Turn off the display */
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

-err_fbmem:
-	if (drvdata->flags & BUS_ACCESS_FLAG)
-		devm_iounmap(dev, drvdata->regs);
-
-err_region:
-	dev_set_drvdata(dev, NULL);
-
 	return rc;
 }

@@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
 	/* Turn off the display */
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

-	/* Release the resources, as allocated based on interface */
-	if (drvdata->flags & BUS_ACCESS_FLAG)
-		devm_iounmap(dev, drvdata->regs);
 #ifdef CONFIG_PPC_DCR
-	else
+	/* Release the resources, as allocated based on interface */
+	if (!(drvdata->flags & BUS_ACCESS_FLAG))
 		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 #endif

-	dev_set_drvdata(dev, NULL);
-
 	return 0;
 }

--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] video: xilinxfb: Use devm_kzalloc instead of kzalloc

[Jingoo Han]

On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:
> 
> Simplify driver probe and release function.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

> ---
>  drivers/video/xilinxfb.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

Re: [PATCH 2/3] video: xilinxfb: Use devm_kzalloc instead of kzalloc

[Jingoo Han]

On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:
> 
> Simplify driver probe and release function.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

> ---
>  drivers/video/xilinxfb.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Jingoo Han]

On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:
> 
> devm_iounmap is called automatically that's why remove it from the code
> dev_set_drvdata(dev, NULL) is called by generic code
> after device_release or on probe failure.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

> ---
>  drivers/video/xilinxfb.c | 28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Jingoo Han]

On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:
> 
> devm_iounmap is called automatically that's why remove it from the code
> dev_set_drvdata(dev, NULL) is called by generic code
> after device_release or on probe failure.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

> ---
>  drivers/video/xilinxfb.c | 28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On 12/09/13 08:54, Michal Simek wrote:

> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>  	/* Turn off the display */
>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
> 
> -	/* Release the resources, as allocated based on interface */
> -	if (drvdata->flags & BUS_ACCESS_FLAG)
> -		devm_iounmap(dev, drvdata->regs);
>  #ifdef CONFIG_PPC_DCR
> -	else
> +	/* Release the resources, as allocated based on interface */
> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>  #endif

I might be mistaken, and it's not strictly part of this series, but
shouldn't dcr_unmap be called somewhere else also, if the probe fails?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On 12/09/13 08:54, Michal Simek wrote:

> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>  	/* Turn off the display */
>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
> 
> -	/* Release the resources, as allocated based on interface */
> -	if (drvdata->flags & BUS_ACCESS_FLAG)
> -		devm_iounmap(dev, drvdata->regs);
>  #ifdef CONFIG_PPC_DCR
> -	else
> +	/* Release the resources, as allocated based on interface */
> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>  #endif

I might be mistaken, and it's not strictly part of this series, but
shouldn't dcr_unmap be called somewhere else also, if the probe fails?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
> On 12/09/13 08:54, Michal Simek wrote:
> 
>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>  	/* Turn off the display */
>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>
>> -	/* Release the resources, as allocated based on interface */
>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>> -		devm_iounmap(dev, drvdata->regs);
>>  #ifdef CONFIG_PPC_DCR
>> -	else
>> +	/* Release the resources, as allocated based on interface */
>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>  #endif
> 
> I might be mistaken, and it's not strictly part of this series, but
> shouldn't dcr_unmap be called somewhere else also, if the probe fails?

yes. It should.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
> On 12/09/13 08:54, Michal Simek wrote:
> 
>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>  	/* Turn off the display */
>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>
>> -	/* Release the resources, as allocated based on interface */
>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>> -		devm_iounmap(dev, drvdata->regs);
>>  #ifdef CONFIG_PPC_DCR
>> -	else
>> +	/* Release the resources, as allocated based on interface */
>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>  #endif
> 
> I might be mistaken, and it's not strictly part of this series, but
> shouldn't dcr_unmap be called somewhere else also, if the probe fails?

yes. It should.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On 16/09/13 13:33, Michal Simek wrote:
> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>> On 12/09/13 08:54, Michal Simek wrote:
>>
>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>  	/* Turn off the display */
>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>
>>> -	/* Release the resources, as allocated based on interface */
>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>> -		devm_iounmap(dev, drvdata->regs);
>>>  #ifdef CONFIG_PPC_DCR
>>> -	else
>>> +	/* Release the resources, as allocated based on interface */
>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>  #endif
>>
>> I might be mistaken, and it's not strictly part of this series, but
>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
> 
> yes. It should.

Do you want me to apply these patches as they are, or do you want to
improve the series to include the dcr_unmap fix?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On 16/09/13 13:33, Michal Simek wrote:
> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>> On 12/09/13 08:54, Michal Simek wrote:
>>
>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>  	/* Turn off the display */
>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>
>>> -	/* Release the resources, as allocated based on interface */
>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>> -		devm_iounmap(dev, drvdata->regs);
>>>  #ifdef CONFIG_PPC_DCR
>>> -	else
>>> +	/* Release the resources, as allocated based on interface */
>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>  #endif
>>
>> I might be mistaken, and it's not strictly part of this series, but
>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
> 
> yes. It should.

Do you want me to apply these patches as they are, or do you want to
improve the series to include the dcr_unmap fix?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

Hi Tomi,

On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:
> On 16/09/13 13:33, Michal Simek wrote:
>> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>>> On 12/09/13 08:54, Michal Simek wrote:
>>>
>>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>>  	/* Turn off the display */
>>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>>
>>>> -	/* Release the resources, as allocated based on interface */
>>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>>> -		devm_iounmap(dev, drvdata->regs);
>>>>  #ifdef CONFIG_PPC_DCR
>>>> -	else
>>>> +	/* Release the resources, as allocated based on interface */
>>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>>  #endif
>>>
>>> I might be mistaken, and it's not strictly part of this series, but
>>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
>>
>> yes. It should.
> 
> Do you want me to apply these patches as they are, or do you want to
> improve the series to include the dcr_unmap fix?

Sorry I have missed this email.

Yes please apply it as is. I don't have ppc hw here to be able to test this
change.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

Hi Tomi,

On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:
> On 16/09/13 13:33, Michal Simek wrote:
>> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>>> On 12/09/13 08:54, Michal Simek wrote:
>>>
>>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>>  	/* Turn off the display */
>>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>>
>>>> -	/* Release the resources, as allocated based on interface */
>>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>>> -		devm_iounmap(dev, drvdata->regs);
>>>>  #ifdef CONFIG_PPC_DCR
>>>> -	else
>>>> +	/* Release the resources, as allocated based on interface */
>>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>>  #endif
>>>
>>> I might be mistaken, and it's not strictly part of this series, but
>>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
>>
>> yes. It should.
> 
> Do you want me to apply these patches as they are, or do you want to
> improve the series to include the dcr_unmap fix?

Sorry I have missed this email.

Yes please apply it as is. I don't have ppc hw here to be able to test this
change.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

On 30/09/13 15:05, Michal Simek wrote:
> Hi Tomi,
> 
> On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:
>> On 16/09/13 13:33, Michal Simek wrote:
>>> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>>>> On 12/09/13 08:54, Michal Simek wrote:
>>>>
>>>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>>>  	/* Turn off the display */
>>>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>>>
>>>>> -	/* Release the resources, as allocated based on interface */
>>>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>>>> -		devm_iounmap(dev, drvdata->regs);
>>>>>  #ifdef CONFIG_PPC_DCR
>>>>> -	else
>>>>> +	/* Release the resources, as allocated based on interface */
>>>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>>>  #endif
>>>>
>>>> I might be mistaken, and it's not strictly part of this series, but
>>>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
>>>
>>> yes. It should.
>>
>> Do you want me to apply these patches as they are, or do you want to
>> improve the series to include the dcr_unmap fix?
> 
> Sorry I have missed this email.
> 
> Yes please apply it as is. I don't have ppc hw here to be able to test this
> change.

This series does not apply. Can you rebase on top of linux-next, and resend?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

On 30/09/13 15:05, Michal Simek wrote:
> Hi Tomi,
> 
> On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:
>> On 16/09/13 13:33, Michal Simek wrote:
>>> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>>>> On 12/09/13 08:54, Michal Simek wrote:
>>>>
>>>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>>>  	/* Turn off the display */
>>>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>>>
>>>>> -	/* Release the resources, as allocated based on interface */
>>>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>>>> -		devm_iounmap(dev, drvdata->regs);
>>>>>  #ifdef CONFIG_PPC_DCR
>>>>> -	else
>>>>> +	/* Release the resources, as allocated based on interface */
>>>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>>>  #endif
>>>>
>>>> I might be mistaken, and it's not strictly part of this series, but
>>>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
>>>
>>> yes. It should.
>>
>> Do you want me to apply these patches as they are, or do you want to
>> improve the series to include the dcr_unmap fix?
> 
> Sorry I have missed this email.
> 
> Yes please apply it as is. I don't have ppc hw here to be able to test this
> change.

This series does not apply. Can you rebase on top of linux-next, and resend?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

On 10/09/2013 11:02 AM, Tomi Valkeinen wrote:
> On 30/09/13 15:05, Michal Simek wrote:
>> Hi Tomi,
>>
>> On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:
>>> On 16/09/13 13:33, Michal Simek wrote:
>>>> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>>>>> On 12/09/13 08:54, Michal Simek wrote:
>>>>>
>>>>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>>>>  	/* Turn off the display */
>>>>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>>>>
>>>>>> -	/* Release the resources, as allocated based on interface */
>>>>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>>>>> -		devm_iounmap(dev, drvdata->regs);
>>>>>>  #ifdef CONFIG_PPC_DCR
>>>>>> -	else
>>>>>> +	/* Release the resources, as allocated based on interface */
>>>>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>>>>  #endif
>>>>>
>>>>> I might be mistaken, and it's not strictly part of this series, but
>>>>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
>>>>
>>>> yes. It should.
>>>
>>> Do you want me to apply these patches as they are, or do you want to
>>> improve the series to include the dcr_unmap fix?
>>
>> Sorry I have missed this email.
>>
>> Yes please apply it as is. I don't have ppc hw here to be able to test this
>> change.
> 
> This series does not apply. Can you rebase on top of linux-next, and resend?

Do you mean Stephen Rothwell linux-next or any your linux-next branch?
No problem to do so if you send me link to the repo.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

On 10/09/2013 11:02 AM, Tomi Valkeinen wrote:
> On 30/09/13 15:05, Michal Simek wrote:
>> Hi Tomi,
>>
>> On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:
>>> On 16/09/13 13:33, Michal Simek wrote:
>>>> On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:
>>>>> On 12/09/13 08:54, Michal Simek wrote:
>>>>>
>>>>>> @@ -394,16 +382,12 @@ static int xilinxfb_release(struct device *dev)
>>>>>>  	/* Turn off the display */
>>>>>>  	xilinx_fb_out32(drvdata, REG_CTRL, 0);
>>>>>>
>>>>>> -	/* Release the resources, as allocated based on interface */
>>>>>> -	if (drvdata->flags & BUS_ACCESS_FLAG)
>>>>>> -		devm_iounmap(dev, drvdata->regs);
>>>>>>  #ifdef CONFIG_PPC_DCR
>>>>>> -	else
>>>>>> +	/* Release the resources, as allocated based on interface */
>>>>>> +	if (!(drvdata->flags & BUS_ACCESS_FLAG))
>>>>>>  		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>>>>>>  #endif
>>>>>
>>>>> I might be mistaken, and it's not strictly part of this series, but
>>>>> shouldn't dcr_unmap be called somewhere else also, if the probe fails?
>>>>
>>>> yes. It should.
>>>
>>> Do you want me to apply these patches as they are, or do you want to
>>> improve the series to include the dcr_unmap fix?
>>
>> Sorry I have missed this email.
>>
>> Yes please apply it as is. I don't have ppc hw here to be able to test this
>> change.
> 
> This series does not apply. Can you rebase on top of linux-next, and resend?

Do you mean Stephen Rothwell linux-next or any your linux-next branch?
No problem to do so if you send me link to the repo.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

On 09/10/13 13:25, Michal Simek wrote:

>> This series does not apply. Can you rebase on top of linux-next, and resend?
> 
> Do you mean Stephen Rothwell linux-next or any your linux-next branch?
> No problem to do so if you send me link to the repo.

Either one, my for-next is in Stephen's tree:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git for-next

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 3/3] video: xilinxfb: Simplify error path

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

On 09/10/13 13:25, Michal Simek wrote:

>> This series does not apply. Can you rebase on top of linux-next, and resend?
> 
> Do you mean Stephen Rothwell linux-next or any your linux-next branch?
> No problem to do so if you send me link to the repo.

Either one, my for-next is in Stephen's tree:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git for-next

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]