linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] xilinxfb changes
@ 2013-05-31 12:55 Michal Simek
       [not found] ` <cover.1370004925.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Timur Tabi, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Michal Simek,
	Tomi Valkeinen, Grant Likely

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

Hi,

I have done more changes in the driver to support probing
on little and big endian system where detection is done
directly on the hardware.
I have also done some cleanups to get it to the better shape.

Thanks for your review,
Michal

Changes in v3:
- Remove out_be IO name from function name
- Change patch subject from "Do not use out_be32 IO function"
  to "Do not name out_be32 in function name"
- New patch in this patchset based on discussions
- New patch in this patchset based on discussions
- New patch in this patchset
- New patch in this patchset based on discussions
- New patch in this patchset

Changes in v2:
- use of_property_read_u32 helper function
Series-changes: 3
- fix commit message

Michal Simek (8):
  video: xilinxfb: Fix OF probing on little-endian systems
  video: xilinxfb: Do not name out_be32 in function name
  video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG
  video: xilinxfb: Use drvdata->regs_phys instead of physaddr
  video: xilinxfb: Group bus initialization
  video: xilinxfb: Add support for little endian accesses
  video: xilinxfb: Fix sparse warnings
  video: xilinxfb: Use driver for Xilinx ARM Zynq

 drivers/video/Kconfig    |   2 +-
 drivers/video/xilinxfb.c | 157 ++++++++++++++++++++++++-----------------------
 2 files changed, 81 insertions(+), 78 deletions(-)

--
1.8.2.3


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems
       [not found] ` <cover.1370004925.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
@ 2013-05-31 12:55   ` Michal Simek
  2013-05-31 13:05     ` Timur Tabi
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Michal Simek,
	Rob Herring, Timur Tabi, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Grant Likely

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

From: Michal Simek <monstr@monstr.eu>

DTB is always big-endian that's why it is necessary
to properly convert value (*p).
It is automatically done in of_property_read_u32().

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3: None
Changes in v2:
- use of_property_read_u32 helper function
Series-changes: 3
- fix commit message

 drivers/video/xilinxfb.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index af0b4fd..aecd15d 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -406,8 +406,7 @@ static int xilinxfb_release(struct device *dev)
 static int xilinxfb_of_probe(struct platform_device *op)
 {
 	const u32 *prop;
-	u32 *p;
-	u32 tft_access;
+	u32 tft_access = 0;
 	struct xilinxfb_platform_data pdata;
 	struct resource res;
 	int size, rc;
@@ -427,8 +426,8 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	 * To check whether the core is connected directly to DCR or PLB
 	 * interface and initialize the tft_access accordingly.
 	 */
-	p = (u32 *)of_get_property(op->dev.of_node, "xlnx,dcr-splb-slave-if", NULL);
-	tft_access = p ? *p : 0;
+	of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if",
+			     &tft_access);

 	/*
 	 * Fill the resource structure if its direct PLB interface
--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 2/8] video: xilinxfb: Do not name out_be32 in function name
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
       [not found] ` <cover.1370004925.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
@ 2013-05-31 12:55 ` Michal Simek
  2013-05-31 12:55 ` [PATCH v3 3/8] video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG Michal Simek
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

out_be32 IO function is not supported by ARM.
It is only available for PPC and Microblaze.
Because this driver can be used on ARM let's
remove out_be32 from function name.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- Remove out_be IO name from function name
- Change patch subject from "Do not use out_be32 IO function"
  to "Do not name out_be32 in function name"

Changes in v2: None

 drivers/video/xilinxfb.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index aecd15d..c9b442b 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -57,7 +57,7 @@
  * In case of direct PLB access the second control register will be at
  * an offset of 4 as compared to the DCR access where the offset is 1
  * i.e. REG_CTRL. So this is taken care in the function
- * xilinx_fb_out_be32 where it left shifts the offset 2 times in case of
+ * xilinx_fb_out32 where it left shifts the offset 2 times in case of
  * direct PLB access.
  */
 #define NUM_REGS	2
@@ -150,7 +150,7 @@ struct xilinxfb_drvdata {
  * To perform the read/write on the registers we need to check on
  * which bus its connected and call the appropriate write API.
  */
-static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
+static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
 				u32 val)
 {
 	if (drvdata->flags & PLB_ACCESS_FLAG)
@@ -197,7 +197,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
 	switch (blank_mode) {
 	case FB_BLANK_UNBLANK:
 		/* turn on panel */
-		xilinx_fb_out_be32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
+		xilinx_fb_out32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
 		break;

 	case FB_BLANK_NORMAL:
@@ -205,7 +205,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
 	case FB_BLANK_HSYNC_SUSPEND:
 	case FB_BLANK_POWERDOWN:
 		/* turn off panel */
-		xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+		xilinx_fb_out32(drvdata, REG_CTRL, 0);
 	default:
 		break;

@@ -280,13 +280,13 @@ static int xilinxfb_assign(struct device *dev,
 	memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);

 	/* Tell the hardware where the frame buffer is */
-	xilinx_fb_out_be32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
+	xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);

 	/* Turn on the display */
 	drvdata->reg_ctrl_default = REG_CTRL_ENABLE;
 	if (pdata->rotate_screen)
 		drvdata->reg_ctrl_default |= REG_CTRL_ROTATE;
-	xilinx_fb_out_be32(drvdata, REG_CTRL,
+	xilinx_fb_out32(drvdata, REG_CTRL,
 					drvdata->reg_ctrl_default);

 	/* Fill struct fb_info */
@@ -345,7 +345,7 @@ err_cmap:
 		iounmap(drvdata->fb_virt);

 	/* Turn off the display */
-	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+	xilinx_fb_out32(drvdata, REG_CTRL, 0);

 err_fbmem:
 	if (drvdata->flags & PLB_ACCESS_FLAG)
@@ -381,7 +381,7 @@ static int xilinxfb_release(struct device *dev)
 		iounmap(drvdata->fb_virt);

 	/* Turn off the display */
-	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+	xilinx_fb_out32(drvdata, REG_CTRL, 0);

 	/* Release the resources, as allocated based on interface */
 	if (drvdata->flags & PLB_ACCESS_FLAG) {
--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 3/8] video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
       [not found] ` <cover.1370004925.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
  2013-05-31 12:55 ` [PATCH v3 2/8] video: xilinxfb: Do not name out_be32 in function name Michal Simek
@ 2013-05-31 12:55 ` Michal Simek
  2013-05-31 12:55 ` [PATCH v3 4/8] video: xilinxfb: Use drvdata->regs_phys instead of physaddr Michal Simek
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

Using only PLB name is wrong for a long time because
the same access functions are also used for AXI.
s/PLB/BUS/g

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- New patch in this patchset based on discussions

Changes in v2: None

 drivers/video/xilinxfb.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index c9b442b..d94c992 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -44,7 +44,7 @@


 /*
- * Xilinx calls it "PLB TFT LCD Controller" though it can also be used for
+ * Xilinx calls it "TFT LCD Controller" though it can also be used for
  * the VGA port on the Xilinx ML40x board. This is a hardware display
  * controller for a 640x480 resolution TFT or VGA screen.
  *
@@ -54,11 +54,11 @@
  * don't start thinking about scrolling).  The second allows the LCD to
  * be turned on or off as well as rotated 180 degrees.
  *
- * In case of direct PLB access the second control register will be at
+ * In case of direct BUS access the second control register will be at
  * an offset of 4 as compared to the DCR access where the offset is 1
  * i.e. REG_CTRL. So this is taken care in the function
  * xilinx_fb_out32 where it left shifts the offset 2 times in case of
- * direct PLB access.
+ * direct BUS access.
  */
 #define NUM_REGS	2
 #define REG_FB_ADDR	0
@@ -116,7 +116,7 @@ static struct fb_var_screeninfo xilinx_fb_var = {
 };


-#define PLB_ACCESS_FLAG	0x1		/* 1 = PLB, 0 = DCR */
+#define BUS_ACCESS_FLAG		0x1 /* 1 = BUS, 0 = DCR */

 struct xilinxfb_drvdata {

@@ -146,14 +146,14 @@ struct xilinxfb_drvdata {
 	container_of(_info, struct xilinxfb_drvdata, info)

 /*
- * The XPS TFT Controller can be accessed through PLB or DCR interface.
+ * The XPS TFT Controller can be accessed through BUS or DCR interface.
  * To perform the read/write on the registers we need to check on
  * which bus its connected and call the appropriate write API.
  */
 static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
 				u32 val)
 {
-	if (drvdata->flags & PLB_ACCESS_FLAG)
+	if (drvdata->flags & BUS_ACCESS_FLAG)
 		out_be32(drvdata->regs + (offset << 2), val);
 #ifdef CONFIG_PPC_DCR
 	else
@@ -235,10 +235,10 @@ static int xilinxfb_assign(struct device *dev,
 	int rc;
 	int fbsize = pdata->xvirt * pdata->yvirt * BYTES_PER_PIXEL;

-	if (drvdata->flags & PLB_ACCESS_FLAG) {
+	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		/*
 		 * Map the control registers in if the controller
-		 * is on direct PLB interface.
+		 * is on direct BUS interface.
 		 */
 		if (!request_mem_region(physaddr, 8, DRIVER_NAME)) {
 			dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
@@ -270,7 +270,7 @@ static int xilinxfb_assign(struct device *dev,
 	if (!drvdata->fb_virt) {
 		dev_err(dev, "Could not allocate frame buffer memory\n");
 		rc = -ENOMEM;
-		if (drvdata->flags & PLB_ACCESS_FLAG)
+		if (drvdata->flags & BUS_ACCESS_FLAG)
 			goto err_fbmem;
 		else
 			goto err_region;
@@ -323,7 +323,7 @@ static int xilinxfb_assign(struct device *dev,
 		goto err_regfb;
 	}

-	if (drvdata->flags & PLB_ACCESS_FLAG) {
+	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		/* Put a banner in the log (for DEBUG) */
 		dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr,
 					drvdata->regs);
@@ -348,11 +348,11 @@ err_cmap:
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

 err_fbmem:
-	if (drvdata->flags & PLB_ACCESS_FLAG)
+	if (drvdata->flags & BUS_ACCESS_FLAG)
 		iounmap(drvdata->regs);

 err_map:
-	if (drvdata->flags & PLB_ACCESS_FLAG)
+	if (drvdata->flags & BUS_ACCESS_FLAG)
 		release_mem_region(physaddr, 8);

 err_region:
@@ -384,7 +384,7 @@ static int xilinxfb_release(struct device *dev)
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

 	/* Release the resources, as allocated based on interface */
-	if (drvdata->flags & PLB_ACCESS_FLAG) {
+	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		iounmap(drvdata->regs);
 		release_mem_region(drvdata->regs_phys, 8);
 	}
@@ -423,18 +423,18 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	}

 	/*
-	 * To check whether the core is connected directly to DCR or PLB
+	 * 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",
 			     &tft_access);

 	/*
-	 * Fill the resource structure if its direct PLB interface
+	 * Fill the resource structure if its direct BUS interface
 	 * otherwise fill the dcr_host structure.
 	 */
 	if (tft_access) {
-		drvdata->flags |= PLB_ACCESS_FLAG;
+		drvdata->flags |= BUS_ACCESS_FLAG;
 		rc = of_address_to_resource(op->dev.of_node, 0, &res);
 		if (rc) {
 			dev_err(&op->dev, "invalid address\n");
--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 4/8] video: xilinxfb: Use drvdata->regs_phys instead of physaddr
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
                   ` (2 preceding siblings ...)
  2013-05-31 12:55 ` [PATCH v3 3/8] video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG Michal Simek
@ 2013-05-31 12:55 ` Michal Simek
  2013-05-31 12:55 ` [PATCH v3 5/8] video: xilinxfb: Group bus initialization Michal Simek
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

physaddr will be remove in the next patch.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- New patch in this patchset based on discussions

Changes in v2: None

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

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index d94c992..1b55f18 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -325,7 +325,7 @@ static int xilinxfb_assign(struct device *dev,

 	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		/* Put a banner in the log (for DEBUG) */
-		dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr,
+		dev_dbg(dev, "regs: phys=%x, virt=%p\n", drvdata->regs_phys,
 					drvdata->regs);
 	}
 	/* Put a banner in the log (for DEBUG) */
@@ -353,7 +353,7 @@ err_fbmem:

 err_map:
 	if (drvdata->flags & BUS_ACCESS_FLAG)
-		release_mem_region(physaddr, 8);
+		release_mem_region(drvdata->regs_phys, 8);

 err_region:
 	kfree(drvdata);
--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 5/8] video: xilinxfb: Group bus initialization
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
                   ` (3 preceding siblings ...)
  2013-05-31 12:55 ` [PATCH v3 4/8] video: xilinxfb: Use drvdata->regs_phys instead of physaddr Michal Simek
@ 2013-05-31 12:55 ` Michal Simek
  2013-05-31 12:55 ` [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses Michal Simek
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

Move of_address_to_resource() to xilinxfb_assign()
which simplify driver probing.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- New patch in this patchset

Changes in v2: None

 drivers/video/xilinxfb.c | 56 +++++++++++++-----------------------------------
 1 file changed, 15 insertions(+), 41 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 1b55f18..bd3b85d 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -227,33 +227,23 @@ static struct fb_ops xilinxfb_ops =
  * Bus independent setup/teardown
  */

-static int xilinxfb_assign(struct device *dev,
+static int xilinxfb_assign(struct platform_device *pdev,
 			   struct xilinxfb_drvdata *drvdata,
-			   unsigned long physaddr,
 			   struct xilinxfb_platform_data *pdata)
 {
 	int rc;
+	struct device *dev = &pdev->dev;
 	int fbsize = pdata->xvirt * pdata->yvirt * BYTES_PER_PIXEL;

 	if (drvdata->flags & BUS_ACCESS_FLAG) {
-		/*
-		 * Map the control registers in if the controller
-		 * is on direct BUS interface.
-		 */
-		if (!request_mem_region(physaddr, 8, DRIVER_NAME)) {
-			dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-				physaddr);
-			rc = -ENODEV;
-			goto err_region;
-		}
+		struct resource *res;

-		drvdata->regs_phys = physaddr;
-		drvdata->regs = ioremap(physaddr, 8);
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		drvdata->regs_phys = res->start;
+		drvdata->regs = devm_request_and_ioremap(&pdev->dev, res);
 		if (!drvdata->regs) {
-			dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-				physaddr);
-			rc = -ENODEV;
-			goto err_map;
+			rc = -EADDRNOTAVAIL;
+			goto err_region;
 		}
 	}

@@ -349,11 +339,7 @@ err_cmap:

 err_fbmem:
 	if (drvdata->flags & BUS_ACCESS_FLAG)
-		iounmap(drvdata->regs);
-
-err_map:
-	if (drvdata->flags & BUS_ACCESS_FLAG)
-		release_mem_region(drvdata->regs_phys, 8);
+		devm_iounmap(dev, drvdata->regs);

 err_region:
 	kfree(drvdata);
@@ -384,10 +370,8 @@ static int xilinxfb_release(struct device *dev)
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);

 	/* Release the resources, as allocated based on interface */
-	if (drvdata->flags & BUS_ACCESS_FLAG) {
-		iounmap(drvdata->regs);
-		release_mem_region(drvdata->regs_phys, 8);
-	}
+	if (drvdata->flags & BUS_ACCESS_FLAG)
+		devm_iounmap(dev, drvdata->regs);
 #ifdef CONFIG_PPC_DCR
 	else
 		dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
@@ -408,8 +392,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	const u32 *prop;
 	u32 tft_access = 0;
 	struct xilinxfb_platform_data pdata;
-	struct resource res;
-	int size, rc;
+	int size;
 	struct xilinxfb_drvdata *drvdata;

 	/* Copy with the default pdata (not a ptr reference!) */
@@ -435,22 +418,17 @@ static int xilinxfb_of_probe(struct platform_device *op)
 	 */
 	if (tft_access) {
 		drvdata->flags |= BUS_ACCESS_FLAG;
-		rc = of_address_to_resource(op->dev.of_node, 0, &res);
-		if (rc) {
-			dev_err(&op->dev, "invalid address\n");
-			goto err;
-		}
 	}
 #ifdef CONFIG_PPC_DCR
 	else {
 		int start;
-		res.start = 0;
 		start = dcr_resource_start(op->dev.of_node, 0);
 		drvdata->dcr_len = dcr_resource_len(op->dev.of_node, 0);
 		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");
-			goto err;
+			kfree(drvdata);
+			return -ENODEV;
 		}
 	}
 #endif
@@ -477,11 +455,7 @@ static int xilinxfb_of_probe(struct platform_device *op)
 		pdata.rotate_screen = 1;

 	dev_set_drvdata(&op->dev, drvdata);
-	return xilinxfb_assign(&op->dev, drvdata, res.start, &pdata);
-
- err:
-	kfree(drvdata);
-	return -ENODEV;
+	return xilinxfb_assign(op, drvdata, &pdata);
 }

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


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
                   ` (4 preceding siblings ...)
  2013-05-31 12:55 ` [PATCH v3 5/8] video: xilinxfb: Group bus initialization Michal Simek
@ 2013-05-31 12:55 ` Michal Simek
  2013-05-31 13:38   ` Arnd Bergmann
  2013-05-31 12:55 ` [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings Michal Simek
  2013-05-31 12:55 ` [PATCH v3 8/8] video: xilinxfb: Use driver for Xilinx ARM Zynq Michal Simek
  7 siblings, 1 reply; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

Dynamically detect endianess on IP and use
ioread/iowrite functions instead of powerpc and microblaze
specific out_be32.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- New patch in this patchset based on discussions

Changes in v2: None

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

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index bd3b85d..f3d4a69 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -117,6 +117,7 @@ static struct fb_var_screeninfo xilinx_fb_var = {


 #define BUS_ACCESS_FLAG		0x1 /* 1 = BUS, 0 = DCR */
+#define LITTLE_ENDIAN_ACCESS	0x2 /* LITTLE ENDIAN IO functions */

 struct xilinxfb_drvdata {

@@ -153,14 +154,33 @@ struct xilinxfb_drvdata {
 static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
 				u32 val)
 {
-	if (drvdata->flags & BUS_ACCESS_FLAG)
-		out_be32(drvdata->regs + (offset << 2), val);
+	if (drvdata->flags & BUS_ACCESS_FLAG) {
+		if (drvdata->flags & LITTLE_ENDIAN_ACCESS)
+			iowrite32(val, drvdata->regs + (offset << 2));
+		else
+			iowrite32be(val, drvdata->regs + (offset << 2));
+	}
 #ifdef CONFIG_PPC_DCR
 	else
 		dcr_write(drvdata->dcr_host, offset, val);
 #endif
 }

+static u32 xilinx_fb_in32(struct xilinxfb_drvdata *drvdata, u32 offset)
+{
+	if (drvdata->flags & BUS_ACCESS_FLAG) {
+		if (drvdata->flags & LITTLE_ENDIAN_ACCESS)
+			return ioread32(drvdata->regs + (offset << 2));
+		else
+			return ioread32be(drvdata->regs + (offset << 2));
+	}
+#ifdef CONFIG_PPC_DCR
+	else
+		return dcr_read(drvdata->dcr_host, offset);
+#endif
+	return 0;
+}
+
 static int
 xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
 	unsigned transp, struct fb_info *fbi)
@@ -271,6 +291,12 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	/* Tell the hardware where the frame buffer is */
 	xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
+	rc = xilinx_fb_in32(drvdata, REG_FB_ADDR);
+	/* Endianess detection */
+	if (rc != drvdata->fb_phys) {
+		drvdata->flags |= LITTLE_ENDIAN_ACCESS;
+		xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
+	}

 	/* Turn on the display */
 	drvdata->reg_ctrl_default = REG_CTRL_ENABLE;
--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
                   ` (5 preceding siblings ...)
  2013-05-31 12:55 ` [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses Michal Simek
@ 2013-05-31 12:55 ` Michal Simek
  2013-05-31 13:26   ` Timur Tabi
  2013-05-31 12:55 ` [PATCH v3 8/8] video: xilinxfb: Use driver for Xilinx ARM Zynq Michal Simek
  7 siblings, 1 reply; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

Use proper casting for fb_virt variable.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- New patch in this patchset

Changes in v2: None

 drivers/video/xilinxfb.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index f3d4a69..e27a4f6 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -131,7 +131,7 @@ struct xilinxfb_drvdata {
 	dcr_host_t      dcr_host;
 	unsigned int    dcr_len;
 #endif
-	void		*fb_virt;	/* virt. address of the frame buffer */
+	void __iomem	*fb_virt;	/* virt. address of the frame buffer */
 	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
 	int		fb_alloced;	/* Flag, was the fb memory alloced? */

@@ -273,8 +273,10 @@ static int xilinxfb_assign(struct platform_device *pdev,
 		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
 	} else {
 		drvdata->fb_alloced = 1;
-		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
-					&drvdata->fb_phys, GFP_KERNEL);
+		drvdata->fb_virt = (__force void __iomem *)
+				   dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
+						      &drvdata->fb_phys,
+						      GFP_KERNEL);
 	}

 	if (!drvdata->fb_virt) {
@@ -287,7 +289,7 @@ static int xilinxfb_assign(struct platform_device *pdev,
 	}

 	/* Clear (turn to black) the framebuffer */
-	memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);
+	memset_io(drvdata->fb_virt, 0, fbsize);

 	/* Tell the hardware where the frame buffer is */
 	xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
@@ -307,7 +309,7 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	/* Fill struct fb_info */
 	drvdata->info.device = dev;
-	drvdata->info.screen_base = (void __iomem *)drvdata->fb_virt;
+	drvdata->info.screen_base = drvdata->fb_virt;
 	drvdata->info.fbops = &xilinxfb_ops;
 	drvdata->info.fix = xilinx_fb_fix;
 	drvdata->info.fix.smem_start = drvdata->fb_phys;
@@ -341,8 +343,8 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		/* Put a banner in the log (for DEBUG) */
-		dev_dbg(dev, "regs: phys=%x, virt=%p\n", drvdata->regs_phys,
-					drvdata->regs);
+		dev_dbg(dev, "regs: phys=%x, virt=%p\n",
+			(u32)drvdata->regs_phys, drvdata->regs);
 	}
 	/* Put a banner in the log (for DEBUG) */
 	dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
@@ -355,8 +357,9 @@ err_regfb:

 err_cmap:
 	if (drvdata->fb_alloced)
-		dma_free_coherent(dev, PAGE_ALIGN(fbsize), drvdata->fb_virt,
-			drvdata->fb_phys);
+		dma_free_coherent(dev, PAGE_ALIGN(fbsize),
+				  (__force void *)drvdata->fb_virt,
+				  drvdata->fb_phys);
 	else
 		iounmap(drvdata->fb_virt);

@@ -388,7 +391,8 @@ static int xilinxfb_release(struct device *dev)

 	if (drvdata->fb_alloced)
 		dma_free_coherent(dev, PAGE_ALIGN(drvdata->info.fix.smem_len),
-				  drvdata->fb_virt, drvdata->fb_phys);
+				  (__force void *)drvdata->fb_virt,
+				  drvdata->fb_phys);
 	else
 		iounmap(drvdata->fb_virt);

--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 8/8] video: xilinxfb: Use driver for Xilinx ARM Zynq
  2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
                   ` (6 preceding siblings ...)
  2013-05-31 12:55 ` [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings Michal Simek
@ 2013-05-31 12:55 ` Michal Simek
  7 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 12:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michal Simek, Michal Simek, Arnd Bergmann, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

From: Michal Simek <monstr@monstr.eu>

Enable this driver for all Xilinx platforms.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3: None
Changes in v2: None

 drivers/video/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..2c301f8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2188,7 +2188,7 @@ config FB_PS3_DEFAULT_SIZE_M

 config FB_XILINX
 	tristate "Xilinx frame buffer support"
-	depends on FB && (XILINX_VIRTEX || MICROBLAZE)
+	depends on FB && (XILINX_VIRTEX || MICROBLAZE || ARCH_ZYNQ)
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
--
1.8.2.3


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems
  2013-05-31 12:55   ` [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems Michal Simek
@ 2013-05-31 13:05     ` Timur Tabi
  2013-05-31 13:09       ` Michal Simek
  0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2013-05-31 13:05 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, Michal Simek, Arnd Bergmann,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Grant Likely,
	Rob Herring, linux-fbdev, devicetree-discuss

On 05/31/2013 07:55 AM, Michal Simek wrote:
> -	p = (u32 *)of_get_property(op->dev.of_node, "xlnx,dcr-splb-slave-if", NULL);
> -	tft_access = p ? *p : 0;
> +	of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if",
> +			     &tft_access);

This is okay, but just FYI, you could instead have just used be32_to_cpup().

-- 
Timur Tabi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems
  2013-05-31 13:05     ` Timur Tabi
@ 2013-05-31 13:09       ` Michal Simek
  0 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 13:09 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Michal Simek, linux-kernel, Arnd Bergmann,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Grant Likely,
	Rob Herring, linux-fbdev, devicetree-discuss

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

On 05/31/2013 03:05 PM, Timur Tabi wrote:
> On 05/31/2013 07:55 AM, Michal Simek wrote:
>> -	p = (u32 *)of_get_property(op->dev.of_node, "xlnx,dcr-splb-slave-if", NULL);
>> -	tft_access = p ? *p : 0;
>> +	of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if",
>> +			     &tft_access);
> 
> This is okay, but just FYI, you could instead have just used be32_to_cpup().

yep. I was there in v1 but then Soren suggested to use of_property_read_u32
http://lkml.org/lkml/2013/5/29/365

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 12:55 ` [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings Michal Simek
@ 2013-05-31 13:26   ` Timur Tabi
  2013-05-31 13:37     ` Michal Simek
  0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2013-05-31 13:26 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, Michal Simek, Arnd Bergmann,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On 05/31/2013 07:55 AM, Michal Simek wrote:

> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index f3d4a69..e27a4f6 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -131,7 +131,7 @@ struct xilinxfb_drvdata {
>  	dcr_host_t      dcr_host;
>  	unsigned int    dcr_len;
>  #endif
> -	void		*fb_virt;	/* virt. address of the frame buffer */
> +	void __iomem	*fb_virt;	/* virt. address of the frame buffer */
>  	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
>  	int		fb_alloced;	/* Flag, was the fb memory alloced? */
> 
> @@ -273,8 +273,10 @@ static int xilinxfb_assign(struct platform_device *pdev,
>  		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
>  	} else {
>  		drvdata->fb_alloced = 1;
> -		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
> -					&drvdata->fb_phys, GFP_KERNEL);
> +		drvdata->fb_virt = (__force void __iomem *)
> +				   dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
> +						      &drvdata->fb_phys,
> +						      GFP_KERNEL);

I think this is wrong.  At least, it would be on PowerPC.
dma_alloc_coherent() allocates regular RAM, not I/O memory.  So it
should not be __iomem.



-- 
Timur Tabi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 13:26   ` Timur Tabi
@ 2013-05-31 13:37     ` Michal Simek
  2013-05-31 13:41       ` Timur Tabi
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Simek @ 2013-05-31 13:37 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Michal Simek, linux-kernel, Arnd Bergmann,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

On 05/31/2013 03:26 PM, Timur Tabi wrote:
> On 05/31/2013 07:55 AM, Michal Simek wrote:
> 
>> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
>> index f3d4a69..e27a4f6 100644
>> --- a/drivers/video/xilinxfb.c
>> +++ b/drivers/video/xilinxfb.c
>> @@ -131,7 +131,7 @@ struct xilinxfb_drvdata {
>>  	dcr_host_t      dcr_host;
>>  	unsigned int    dcr_len;
>>  #endif
>> -	void		*fb_virt;	/* virt. address of the frame buffer */
>> +	void __iomem	*fb_virt;	/* virt. address of the frame buffer */
>>  	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
>>  	int		fb_alloced;	/* Flag, was the fb memory alloced? */
>>
>> @@ -273,8 +273,10 @@ static int xilinxfb_assign(struct platform_device *pdev,
>>  		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
>>  	} else {
>>  		drvdata->fb_alloced = 1;
>> -		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
>> -					&drvdata->fb_phys, GFP_KERNEL);
>> +		drvdata->fb_virt = (__force void __iomem *)
>> +				   dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
>> +						      &drvdata->fb_phys,
>> +						      GFP_KERNEL);
> 
> I think this is wrong.  At least, it would be on PowerPC.
> dma_alloc_coherent() allocates regular RAM, not I/O memory.  So it
> should not be __iomem.

The same is for Microblaze. Driver shares fb_virt for IO memory
and for allocated memory. The purpose of this driver wasn't
to change the driver logic just resolved sparse warnings.
The other way is also wrong.
I have compiled this driver with ppc toolchain and it should
remove sparse warnings for PPC.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses
  2013-05-31 12:55 ` [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses Michal Simek
@ 2013-05-31 13:38   ` Arnd Bergmann
  0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:38 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, Michal Simek, Timur Tabi,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On Friday 31 May 2013 14:55:36 Michal Simek wrote:
> Dynamically detect endianess on IP and use
> ioread/iowrite functions instead of powerpc and microblaze
> specific out_be32.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 13:37     ` Michal Simek
@ 2013-05-31 13:41       ` Timur Tabi
  2013-05-31 14:22         ` Michal Simek
  0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2013-05-31 13:41 UTC (permalink / raw)
  To: monstr
  Cc: Michal Simek, linux-kernel, Arnd Bergmann,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On 05/31/2013 08:37 AM, Michal Simek wrote:
> The same is for Microblaze. Driver shares fb_virt for IO memory
> and for allocated memory. The purpose of this driver wasn't
> to change the driver logic just resolved sparse warnings.
> The other way is also wrong.
> I have compiled this driver with ppc toolchain and it should
> remove sparse warnings for PPC.

But it's not I/O memory.  It's regular memory.  __iomem is for
memory-mapped I/O, which is limited to a specific range of memory locations.

If sometimes you use regular memory for the framebuffer, and other times
you use real I/O memory for the framebuffer, then you should have two
different pointers.

-- 
Timur Tabi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 13:41       ` Timur Tabi
@ 2013-05-31 14:22         ` Michal Simek
  2013-05-31 14:56           ` Arnd Bergmann
  2013-05-31 15:28           ` Arnd Bergmann
  0 siblings, 2 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 14:22 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Michal Simek, linux-kernel, Arnd Bergmann,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

On 05/31/2013 03:41 PM, Timur Tabi wrote:
> On 05/31/2013 08:37 AM, Michal Simek wrote:
>> The same is for Microblaze. Driver shares fb_virt for IO memory
>> and for allocated memory. The purpose of this driver wasn't
>> to change the driver logic just resolved sparse warnings.
>> The other way is also wrong.
>> I have compiled this driver with ppc toolchain and it should
>> remove sparse warnings for PPC.
> 
> But it's not I/O memory.  It's regular memory.  __iomem is for
> memory-mapped I/O, which is limited to a specific range of memory locations.
> 
> If sometimes you use regular memory for the framebuffer, and other times
> you use real I/O memory for the framebuffer, then you should have two
> different pointers.

I agree with you and if you like I can change it.
But there will be at least one retype because dma_alloc_coherent returns void *
but struct fb_info expects that pointer is __iomem (char __iomem *screen_base).
Patch is below.

Thanks,
Michal


diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index f3d4a69..885f294 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -132,8 +132,8 @@ struct xilinxfb_drvdata {
 	unsigned int    dcr_len;
 #endif
 	void		*fb_virt;	/* virt. address of the frame buffer */
+	void __iomem	*fb_virt_io;	/* virt. address of the frame buffer */
 	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
-	int		fb_alloced;	/* Flag, was the fb memory alloced? */

 	u8 		flags;		/* features of the driver */

@@ -270,24 +270,36 @@ static int xilinxfb_assign(struct platform_device *pdev,
 	/* Allocate the framebuffer memory */
 	if (pdata->fb_phys) {
 		drvdata->fb_phys = pdata->fb_phys;
-		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
+		drvdata->fb_virt_io = ioremap(pdata->fb_phys, fbsize);
+
+		if (!drvdata->fb_virt_io) {
+			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;
+		}
+
+		/* Clear (turn to black) the framebuffer */
+		memset_io(drvdata->fb_virt_io, 0, fbsize);
 	} else {
-		drvdata->fb_alloced = 1;
 		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
 					&drvdata->fb_phys, GFP_KERNEL);
-	}

-	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;
+		if (!drvdata->fb_virt_io) {
+			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;
+		memset(drvdata->fb_virt, 0, fbsize);
 	}

-	/* Clear (turn to black) the framebuffer */
-	memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);
+

 	/* Tell the hardware where the frame buffer is */
 	xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
@@ -307,7 +319,11 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	/* Fill struct fb_info */
 	drvdata->info.device = dev;
-	drvdata->info.screen_base = (void __iomem *)drvdata->fb_virt;
+	if (drvdata->fb_virt)
+		drvdata->info.screen_base = (__force void __iomem *)
+							drvdata->fb_virt;
+	else
+		drvdata->info.screen_base = drvdata->fb_virt_io;
 	drvdata->info.fbops = &xilinxfb_ops;
 	drvdata->info.fix = xilinx_fb_fix;
 	drvdata->info.fix.smem_start = drvdata->fb_phys;
@@ -341,8 +357,8 @@ static int xilinxfb_assign(struct platform_device *pdev,

 	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		/* Put a banner in the log (for DEBUG) */
-		dev_dbg(dev, "regs: phys=%x, virt=%p\n", drvdata->regs_phys,
-					drvdata->regs);
+		dev_dbg(dev, "regs: phys=%x, virt=%p\n",
+			(u32)drvdata->regs_phys, drvdata->regs);
 	}
 	/* Put a banner in the log (for DEBUG) */
 	dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
@@ -354,11 +370,11 @@ err_regfb:
 	fb_dealloc_cmap(&drvdata->info.cmap);

 err_cmap:
-	if (drvdata->fb_alloced)
+	if (drvdata->fb_virt)
 		dma_free_coherent(dev, PAGE_ALIGN(fbsize), drvdata->fb_virt,
 			drvdata->fb_phys);
 	else
-		iounmap(drvdata->fb_virt);
+		iounmap(drvdata->fb_virt_io);

 	/* Turn off the display */
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);
@@ -386,11 +402,11 @@ static int xilinxfb_release(struct device *dev)

 	fb_dealloc_cmap(&drvdata->info.cmap);

-	if (drvdata->fb_alloced)
+	if (drvdata->fb_virt)
 		dma_free_coherent(dev, PAGE_ALIGN(drvdata->info.fix.smem_len),
 				  drvdata->fb_virt, drvdata->fb_phys);
 	else
-		iounmap(drvdata->fb_virt);
+		iounmap(drvdata->fb_virt_io);

 	/* Turn off the display */
 	xilinx_fb_out32(drvdata, REG_CTRL, 0);


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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 14:22         ` Michal Simek
@ 2013-05-31 14:56           ` Arnd Bergmann
  2013-05-31 15:06             ` Timur Tabi
  2013-05-31 15:28           ` Arnd Bergmann
  1 sibling, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2013-05-31 14:56 UTC (permalink / raw)
  To: monstr
  Cc: Timur Tabi, Michal Simek, linux-kernel,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On Friday 31 May 2013 16:22:24 Michal Simek wrote:
> @@ -307,7 +319,11 @@ static int xilinxfb_assign(struct platform_device *pdev,
> 
>         /* Fill struct fb_info */
>         drvdata->info.device = dev;
> -       drvdata->info.screen_base = (void __iomem *)drvdata->fb_virt;
> +       if (drvdata->fb_virt)
> +               drvdata->info.screen_base = (__force void __iomem *)
> +                                                       drvdata->fb_virt;
> +       else
> +               drvdata->info.screen_base = drvdata->fb_virt_io;

Yes, unfortunately, this is what all other frame buffer drivers do
at the moment. It is technically not correct, but most architectures
are able to call readl/writel on regular memory, or dereference
__iomem tokens, so we often get away with it. It's probably not
worth fixing it in the fbdev code base as that would be a huge
change, and people are migrating to DRM/KMS.

	Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 14:56           ` Arnd Bergmann
@ 2013-05-31 15:06             ` Timur Tabi
  2013-05-31 15:29               ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2013-05-31 15:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: monstr, Michal Simek, linux-kernel,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On 05/31/2013 09:56 AM, Arnd Bergmann wrote:
> Yes, unfortunately, this is what all other frame buffer drivers do
> at the moment. It is technically not correct, but most architectures
> are able to call readl/writel on regular memory, or dereference
> __iomem tokens, so we often get away with it. It's probably not
> worth fixing it in the fbdev code base as that would be a huge
> change, and people are migrating to DRM/KMS.

But why bother fixing this bug if it just makes things worse?  Sparse is
supposed to warn us about bad code.  This patch doesn't fix the bug, it
just masks the warnings!

-- 
Timur Tabi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 14:22         ` Michal Simek
  2013-05-31 14:56           ` Arnd Bergmann
@ 2013-05-31 15:28           ` Arnd Bergmann
  1 sibling, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2013-05-31 15:28 UTC (permalink / raw)
  To: monstr
  Cc: Timur Tabi, Michal Simek, linux-kernel,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On Friday 31 May 2013 16:22:24 Michal Simek wrote:
>         if (pdata->fb_phys) {
>                 drvdata->fb_phys = pdata->fb_phys;
> -               drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
> +               drvdata->fb_virt_io = ioremap(pdata->fb_phys, fbsize);
> +
> +               if (!drvdata->fb_virt_io) {
> +                       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;
> +               }
> +
> +               /* Clear (turn to black) the framebuffer */
> +               memset_io(drvdata->fb_virt_io, 0, fbsize);
>         } else {
> -               drvdata->fb_alloced = 1;
>                 drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
>                                         &drvdata->fb_phys, GFP_KERNEL);
> -       }
> 

I think you also want to use ioremap_wc or dma_alloc_writecombine
here, to get a write-combining mapping, rather than a device mapping
that you would use for MMIO register access.

There is also a builtin assumption in the code above that the DMA
address space pointer (which you pass into REG_FB_ADDR) is the
same as what you pass into drvdata->info.fix.smem_start. That is
not the case in general, but I don't see a good way around it
when pdata->fb_phys is set by the platform to something outside
of system memory. It should probably have a comment next to it.

	Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 15:06             ` Timur Tabi
@ 2013-05-31 15:29               ` Arnd Bergmann
  2013-05-31 16:33                 ` Michal Simek
  0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2013-05-31 15:29 UTC (permalink / raw)
  To: Timur Tabi
  Cc: monstr, Michal Simek, linux-kernel,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

On Friday 31 May 2013 10:06:43 Timur Tabi wrote:
> On 05/31/2013 09:56 AM, Arnd Bergmann wrote:
> > Yes, unfortunately, this is what all other frame buffer drivers do
> > at the moment. It is technically not correct, but most architectures
> > are able to call readl/writel on regular memory, or dereference
> > __iomem tokens, so we often get away with it. It's probably not
> > worth fixing it in the fbdev code base as that would be a huge
> > change, and people are migrating to DRM/KMS.
> 
> But why bother fixing this bug if it just makes things worse?  Sparse is
> supposed to warn us about bad code.  This patch doesn't fix the bug, it
> just masks the warnings!

Yes, good point. It's probably best cast the ioremap() output to
a regular pointer here, as that is actually just uncached RAM,
not an MMIO register.

	Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
  2013-05-31 15:29               ` Arnd Bergmann
@ 2013-05-31 16:33                 ` Michal Simek
  0 siblings, 0 replies; 21+ messages in thread
From: Michal Simek @ 2013-05-31 16:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Timur Tabi, Michal Simek, linux-kernel,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev

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

On 05/31/2013 05:29 PM, Arnd Bergmann wrote:
> On Friday 31 May 2013 10:06:43 Timur Tabi wrote:
>> On 05/31/2013 09:56 AM, Arnd Bergmann wrote:
>>> Yes, unfortunately, this is what all other frame buffer drivers do
>>> at the moment. It is technically not correct, but most architectures
>>> are able to call readl/writel on regular memory, or dereference
>>> __iomem tokens, so we often get away with it. It's probably not
>>> worth fixing it in the fbdev code base as that would be a huge
>>> change, and people are migrating to DRM/KMS.
>>
>> But why bother fixing this bug if it just makes things worse?  Sparse is
>> supposed to warn us about bad code.  This patch doesn't fix the bug, it
>> just masks the warnings!
> 
> Yes, good point. It's probably best cast the ioremap() output to
> a regular pointer here, as that is actually just uncached RAM,
> not an MMIO register.

ok. It means I will just remove this patch from this patchset.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2013-05-31 16:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-31 12:55 [PATCH v3 0/8] xilinxfb changes Michal Simek
     [not found] ` <cover.1370004925.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2013-05-31 12:55   ` [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems Michal Simek
2013-05-31 13:05     ` Timur Tabi
2013-05-31 13:09       ` Michal Simek
2013-05-31 12:55 ` [PATCH v3 2/8] video: xilinxfb: Do not name out_be32 in function name Michal Simek
2013-05-31 12:55 ` [PATCH v3 3/8] video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG Michal Simek
2013-05-31 12:55 ` [PATCH v3 4/8] video: xilinxfb: Use drvdata->regs_phys instead of physaddr Michal Simek
2013-05-31 12:55 ` [PATCH v3 5/8] video: xilinxfb: Group bus initialization Michal Simek
2013-05-31 12:55 ` [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses Michal Simek
2013-05-31 13:38   ` Arnd Bergmann
2013-05-31 12:55 ` [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings Michal Simek
2013-05-31 13:26   ` Timur Tabi
2013-05-31 13:37     ` Michal Simek
2013-05-31 13:41       ` Timur Tabi
2013-05-31 14:22         ` Michal Simek
2013-05-31 14:56           ` Arnd Bergmann
2013-05-31 15:06             ` Timur Tabi
2013-05-31 15:29               ` Arnd Bergmann
2013-05-31 16:33                 ` Michal Simek
2013-05-31 15:28           ` Arnd Bergmann
2013-05-31 12:55 ` [PATCH v3 8/8] video: xilinxfb: Use driver for Xilinx ARM Zynq Michal Simek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).