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

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

Signed-off-by: Michal Simek <redacted>
---
 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

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 <redacted>
---
 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

Simplify driver probe and release function.

Signed-off-by: Michal Simek <redacted>
---
 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

On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:

Simplify driver probe and release function.

Signed-off-by: Michal Simek <redacted>

Reviewed-by: Jingoo Han <redacted>

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

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 <redacted>

Reviewed-by: Jingoo Han <redacted>

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

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

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

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:

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

quoted

@@ -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

On 16/09/13 13:33, Michal Simek wrote:

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:

quoted

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

quoted

@@ -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

Hi Tomi,

On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:

On 16/09/13 13:33, Michal Simek wrote:

quoted

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:

quoted

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

quoted

@@ -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

On 30/09/13 15:05, Michal Simek wrote:

Hi Tomi,

On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:

quoted

On 16/09/13 13:33, Michal Simek wrote:

quoted

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:

quoted

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

quoted

@@ -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

On 10/09/2013 11:02 AM, Tomi Valkeinen wrote:

On 30/09/13 15:05, Michal Simek wrote:

quoted

Hi Tomi,

On 09/16/2013 12:34 PM, Tomi Valkeinen wrote:

quoted

On 16/09/13 13:33, Michal Simek wrote:

quoted

On 09/16/2013 11:51 AM, Tomi Valkeinen wrote:

quoted

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

quoted

@@ -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

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

quoted

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