Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 10/11] video: da8xx-fb: adopt pinctrl support
From: Hebbar Gururaja @ 2013-05-31 10:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1369995191-20855-1-git-send-email-gururaja.hebbar@ti.com>

Amend the da8xx-fb controller to optionally take a pin control handle
and set the state of the pins to:

- "default" on boot, resume
- "sleep" on suspend()

By optionally putting the pins into sleep state in the suspend callback
we can accomplish two things.
- One is to minimize current leakage from pins and thus save power,
- second, we can prevent the IP from driving pins output in an
uncontrolled manner, which may happen if the power domain drops the
domain regulator.

If any of the above pin states are missing in dt, a warning message
about the missing state is displayed.
If certain pin-states are not available, to remove this warning message
pass respective state name with null phandler.

Todo:
        - if an idle state is available for pins, add support for it.

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
:100644 100644 0810939... 10c8036... M	drivers/video/da8xx-fb.c
 drivers/video/da8xx-fb.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0810939..10c8036 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/lcm.h>
+#include <linux/pinctrl/consumer.h>
 #include <video/da8xx-fb.h>
 #include <asm/div64.h>
 
@@ -182,6 +183,11 @@ struct da8xx_fb_par {
 #endif
 	void (*panel_power_ctrl)(int);
 	u32 pseudo_palette[16];
+
+	/* two pin states - default, sleep */
+	struct pinctrl			*pinctrl;
+	struct pinctrl_state		*pins_default;
+	struct pinctrl_state		*pins_sleep;
 };
 
 /* Variable Screen Information */
@@ -1306,6 +1312,36 @@ static int fb_probe(struct platform_device *device)
 	par->lcd_fck_rate = clk_get_rate(fb_clk);
 #endif
 	par->pxl_clk = lcdc_info->pixclock;
+
+	par->pinctrl = devm_pinctrl_get(&device->dev);
+	if (!IS_ERR(par->pinctrl)) {
+		par->pins_default = pinctrl_lookup_state(par->pinctrl,
+							 PINCTRL_STATE_DEFAULT);
+		if (IS_ERR(par->pins_default))
+			dev_dbg(&device->dev, "could not get default pinstate\n");
+		else
+			if (pinctrl_select_state(par->pinctrl,
+						 par->pins_default))
+				dev_err(&device->dev,
+					"could not set default pinstate\n");
+
+		par->pins_sleep = pinctrl_lookup_state(par->pinctrl,
+						       PINCTRL_STATE_SLEEP);
+		if (IS_ERR(par->pins_sleep))
+			dev_dbg(&device->dev, "could not get sleep pinstate\n");
+	} else {
+		/*
+		* Since we continue even when pinctrl node is not found,
+		* Invalidate pins as not available. This is to make sure that
+		* IS_ERR(pins_xxx) results in failure when used.
+		*/
+		par->pins_default = ERR_PTR(-ENODATA);
+		par->pins_sleep = ERR_PTR(-ENODATA);
+
+		dev_dbg(&device->dev, "did not get pins for lcd error: %li\n",
+			PTR_ERR(par->pinctrl));
+	}
+
 	if (fb_pdata->panel_power_ctrl) {
 		par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
 		par->panel_power_ctrl(1);
@@ -1551,6 +1587,12 @@ static int fb_suspend(struct platform_device *dev, pm_message_t state)
 	pm_runtime_put_sync(&dev->dev);
 	console_unlock();
 
+	/* Optionally let pins go into sleep states */
+	if (!IS_ERR(par->pins_sleep))
+		if (pinctrl_select_state(par->pinctrl, par->pins_sleep))
+			dev_err(&dev->dev,
+				"could not set pins to sleep state\n");
+
 	return 0;
 }
 static int fb_resume(struct platform_device *dev)
@@ -1560,6 +1602,12 @@ static int fb_resume(struct platform_device *dev)
 
 	console_lock();
 	pm_runtime_get_sync(&dev->dev);
+
+	/* Optionaly enable pins to be muxed in and configured */
+	if (!IS_ERR(par->pins_default))
+		if (pinctrl_select_state(par->pinctrl, par->pins_default))
+			dev_err(&dev->dev, "could not set default pins\n");
+
 	lcd_context_restore();
 	if (par->blank = FB_BLANK_UNBLANK) {
 		lcd_enable_raster();
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v3 0/8] xilinxfb changes
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

* [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems
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
In-Reply-To: <cover.1370004925.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>

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

* [PATCH v3 2/8] video: xilinxfb: Do not name out_be32 in function name
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* [PATCH v3 3/8] video: xilinxfb: Rename PLB_ACCESS_FLAG to BUS_ACCESS_FLAG
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* [PATCH v3 4/8] video: xilinxfb: Use drvdata->regs_phys instead of physaddr
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* [PATCH v3 5/8] video: xilinxfb: Group bus initialization
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* [PATCH v3 8/8] video: xilinxfb: Use driver for Xilinx ARM Zynq
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
In-Reply-To: <cover.1370004925.git.michal.simek@xilinx.com>

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

* Re: [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems
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
In-Reply-To: <0a546c3de84c4650d019274debef0ded9007b36f.1370004925.git.michal.simek@xilinx.com>

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

* Re: [PATCH v3 1/8] video: xilinxfb: Fix OF probing on little-endian systems
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
In-Reply-To: <51A8A016.3000306@tabi.org>

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

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <ea3d6fcbb9f7a00a9cdbd3d0a7a4d3401265ebf0.1370004925.git.michal.simek@xilinx.com>

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

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <51A8A4ED.6070104@tabi.org>

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

* Re: [PATCH v3 6/8] video: xilinxfb: Add support for little endian accesses
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
In-Reply-To: <c1791574b211528b9f59af634018b7c83678f963.1370004925.git.michal.simek@xilinx.com>

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

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <51A8A7B1.3040709@monstr.eu>

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

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <51A8A89C.8030404@tabi.org>

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

* [PATCH 1/2] trivial: atmel_lcdfb: add missing error message
From: Richard Genoud @ 2013-05-31 14:28 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
	linux-kernel, Richard Genoud

When a too small framebuffer is given, the atmel_lcdfb_check_var
silently fails.
Adding an error message will save some head scratching.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/video/atmel_lcdfb.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 540909d..6e6491f 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -461,8 +461,11 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
 	if (info->fix.smem_len) {
 		unsigned int smem_len = (var->xres_virtual * var->yres_virtual
 					 * ((var->bits_per_pixel + 7) / 8));
-		if (smem_len > info->fix.smem_len)
+		if (smem_len > info->fix.smem_len) {
+			dev_err(dev, "Frame buffer is too small (%u) for screen size (need at least %u)\n",
+				info->fix.smem_len, smem_len);
 			return -EINVAL;
+		}
 	}
 
 	/* Saturate vertical and horizontal timings at maximum values */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 2/2] atmel_lcdfb: blank the backlight on remove
From: Richard Genoud @ 2013-05-31 14:28 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
	linux-kernel, Richard Genoud
In-Reply-To: <1370010519-18691-1-git-send-email-richard.genoud@gmail.com>

When removing atmel_lcdfb module, the backlight is unregistered but not
blanked. (only for CONFIG_BACKLIGHT_ATMEL_LCDC case).
This can result in the screen going full white depending on how the PWM
is wired.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/video/atmel_lcdfb.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 6e6491f..e00318f 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -223,8 +223,13 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)
 
 static void exit_backlight(struct atmel_lcdfb_info *sinfo)
 {
-	if (sinfo->backlight)
+	if (sinfo->backlight) {
+		if (sinfo->backlight->ops) {
+			sinfo->backlight->props.power = FB_BLANK_POWERDOWN;
+			sinfo->backlight->ops->update_status(sinfo->backlight);
+		}
 		backlight_device_unregister(sinfo->backlight);
+	}
 }
 
 #else
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <51A8B220.6070308@monstr.eu>

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

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <1466524.sgAzczhnBg@wuerfel>

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

* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Grygorii Strashko @ 2013-05-31 15:17 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Tomi Valkeinen, linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <874ndk5sse.fsf@linaro.org>

On 05/30/2013 07:36 PM, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
>
>> When using DT, dss device does not have platform data. However,
>> dss_get_ctx_loss_count() uses dss device's platform data to find the
>> get_ctx_loss_count function pointer.
>>
>> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
>> platform data from the omapdss device, which is a "virtual" device and
>> always has platform data.
> Dumb question (since the DSS device model has always been beyond my
> grasp): how/why does the virtual device still have platform_data on a DT
> boot?
>
> Also, this context_loss_count and DT boot is starting to get cumbersome,
> and there's currently no (good) way of handling the context loss in a
> generic way without pdata function pointers.
>
> I'm starting to think we should move towards dropping this OMAP-specific
> context loss counting, and just assume context is always lost.  If there
> are performance problems, runtime PM autosuspend timeouts can be used to
> avoid the transitions.
>
> Or, on some devices, I suspect comparing a few registers against their
> power-on reset values might be a quicker way of detecting lost context
> and would avoid having to call into platform code all together.
Hi Kevin,
I've a question:
Why don't add API in "Kernel/Base PM" framework for context lost detection?
Like pm_was_ctx_lost().

If this topic has been discussed already (many times)) - sorry for the 
noise.
And I'll be very appreciate if you can point me on corresponding 
discussions.
>
> Kevin
>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dss.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
>> index 94f66f9..bd01608 100644
>> --- a/drivers/video/omap2/dss/dss.c
>> +++ b/drivers/video/omap2/dss/dss.c
>> @@ -157,7 +157,8 @@ static void dss_restore_context(void)
>>   
>>   int dss_get_ctx_loss_count(void)
>>   {
>> -	struct omap_dss_board_info *board_data = dss.pdev->dev.platform_data;
>> +	struct platform_device *core_pdev = dss_get_core_pdev();
>> +	struct omap_dss_board_info *board_data = core_pdev->dev.platform_data;
>>   	int cnt;
>>   
>>   	if (!board_data->get_context_loss_count)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH 2/2] atmel_lcdfb: blank the backlight on remove
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-31 15:19 UTC (permalink / raw)
  To: Richard Genoud; +Cc: Nicolas Ferre, Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1370010519-18691-2-git-send-email-richard.genoud@gmail.com>

On 16:28 Fri 31 May     , Richard Genoud wrote:
> When removing atmel_lcdfb module, the backlight is unregistered but not
> blanked. (only for CONFIG_BACKLIGHT_ATMEL_LCDC case).
> This can result in the screen going full white depending on how the PWM
> is wired.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/video/atmel_lcdfb.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 6e6491f..e00318f 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -223,8 +223,13 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)
>  
>  static void exit_backlight(struct atmel_lcdfb_info *sinfo)
>  {
> -	if (sinfo->backlight)
> +	if (sinfo->backlight) {
if (!sinfo->backlight)
	return;

	other look ok 

if you want ot hte next RC you have 3h to send a v2 before I work off for few
days

Best Regards,
J.

^ permalink raw reply

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <51A8B220.6070308@monstr.eu>

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

* Re: [PATCH v3 7/8] video: xilinxfb: Fix sparse warnings
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
In-Reply-To: <51A8BC83.6050809@tabi.org>

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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox