diff mbox

[PATCHv6,3/5] video: mx3fb: Introduce regulator support.

Message ID 1393408800-8946-3-git-send-email-denis@eukrea.com (mailing list archive)
State New, archived
Headers show

Commit Message

Denis Carikli Feb. 26, 2014, 9:59 a.m. UTC
This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Eric BĂ©nard <eric@eukrea.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v5->v6:
- Shrinked the Cc list.
- still permit non-dt boards to use that driver without a regulator.

ChangeLog v4->v5:
- Added Shawn Guo in the Cc list.
- Rebased to make it apply.

ChangeLog v3->v4:
- Some code style fixes.
- Improved error handling in eremap.

ChangeLog v2->v3:
- The prints are now replaced with non line wrapped prints.
- The regulator retrival has been adapted to the new DT bindings which looks
  more like the IPUv3 ones.
- The regulator_is_enabled checks were kept, because regulator_disable do not
  do such check.
---
 drivers/video/mx3fb.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

Comments

Denis Carikli Feb. 26, 2014, 11:40 a.m. UTC | #1
On 02/26/2014 11:20 AM, Alexander Shiyan wrote:

 > Why you want to use an excess "regulator-name" property?
I'll fix that.

>> +	/* In dt mode,
>> +	 * using devm_regulator_get would require that the proprety referencing
>> +	 * the regulator phandle has to be inside the mx3fb node.
>
> What???
[...]
 > Just use devm_regulator_get(dev, "lcd") for both dt/non-dt case.

About the use of devm_regulator_get instead of regulator_get:

There is a "dma ipu driver" for the mx3* at drivers/dma/ipu/ipu_idmac.c
This framebuffer driver (mx3fb) uses that "dma ipu driver".

In non-dt mode("board files"), this framebuffer driver requires some 
platform data which has resource informations about the ipu.

So to get device tree bindings support for the mx3fb driver, at first I 
exported the "dma ipu driver" as DMA bindings, to be used in the dts, 
and then made the mx3fb driver use that.

The comment[1] to that patchset was to instead have similar bindings 
that looks like the IPUv3 ones(IPUv3 is a staging drm driver), and not 
to export the "dma ipu driver" bindings to the dts.

So I made the bindings look like that:

display0: display@di0 {
   [...]
   display-timings {
     [...]
   };
};

ipu: ipu@53fc0000 {
   compatible = "fsl,imx35-ipu";
   reg = <0x53fc0000 0x4000>;
   clocks = <&clks 55>;
   display = <&display0>;
};

So that is very similar to the imx-drm binding[2].

To achieve that, I've set the .compatible property of the mx3fb driver 
to "fsl,<chip>-ipu", so it would look like the IPUv3 bindings, and then 
I handled the "dma ipu driver" registration in a way that doesn't export 
it to the dts.

The difference is that the imx-drm driver has separate drivers for 
handling each display type(parallel display, tve, lvds and HDMI) while 
the mx3fb doesn't.

using devm_regulator_get(NULL, "lcd") would result in a crash, because 
of devres_add.

using devm_regulator_get(dev, "lcd") would be better but it would 
instead mean that the regulator handle will have to be a child of the 
mx3fb's ipu node (ipu@53fc0000).

That's because devm_regulator_get will end calling of_get_regulator 
(trough _regulator_get, and regulator_dev_lookup), which will in turn 
lookup that regulator in the childs of dev->of_node.

That's why I want to pass it a NULL instead of a device struct, and I 
can't do it in devm_regulator_get because of devres_add, so I ended up 
using regulator_get directly.

References.
-----------
[1]http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205846.html
[2]Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt

Denis.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 952d2b5..40b47dd 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -27,6 +27,7 @@ 
 #include <linux/clk.h>
 #include <linux/mutex.h>
 #include <linux/dma/ipu-dma.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
@@ -273,6 +274,7 @@  struct mx3fb_info {
 	struct dma_async_tx_descriptor	*txd;
 	dma_cookie_t			cookie;
 	struct scatterlist		sg[2];
+	struct regulator		*reg_lcd;
 
 	struct fb_var_screeninfo	cur_var; /* current var info */
 	uint32_t			flags;
@@ -1042,6 +1044,12 @@  static void __blank(int blank, struct fb_info *fbi)
 	case FB_BLANK_HSYNC_SUSPEND:
 	case FB_BLANK_NORMAL:
 		sdc_set_brightness(mx3fb, 0);
+		if (!IS_ERR(mx3_fbi->reg_lcd)) {
+			if (regulator_disable(mx3_fbi->reg_lcd)) {
+				dev_err(fbi->device,
+					"Failed to disable regulator.\n");
+			}
+		}
 		memset((char *)fbi->screen_base, 0, fbi->fix.smem_len);
 		/* Give LCD time to update - enough for 50 and 60 Hz */
 		msleep(25);
@@ -1049,6 +1057,12 @@  static void __blank(int blank, struct fb_info *fbi)
 		break;
 	case FB_BLANK_UNBLANK:
 		sdc_enable_channel(mx3_fbi);
+		if (!IS_ERR(mx3_fbi->reg_lcd)) {
+			if (regulator_enable(mx3_fbi->reg_lcd)) {
+				dev_err(fbi->device,
+					"Failed to enable regulator.\n");
+			}
+		}
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
 		break;
 	}
@@ -1233,7 +1247,12 @@  static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 	if (mx3_fbi->blank == FB_BLANK_UNBLANK) {
 		sdc_disable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, 0);
-
+		if (!IS_ERR(mx3_fbi->reg_lcd)) {
+			if (regulator_disable(mx3_fbi->reg_lcd)) {
+				dev_err(&pdev->dev,
+					"Failed to disable regulator.\n");
+			}
+		}
 	}
 	return 0;
 }
@@ -1249,6 +1268,12 @@  static int mx3fb_resume(struct platform_device *pdev)
 	if (mx3_fbi->blank == FB_BLANK_UNBLANK) {
 		sdc_enable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
+		if (!IS_ERR(mx3_fbi->reg_lcd)) {
+			if (regulator_enable(mx3_fbi->reg_lcd)) {
+				dev_err(&pdev->dev,
+					"Failed to enable regulator.\n");
+			}
+		}
 	}
 
 	console_lock();
@@ -1394,6 +1419,7 @@  static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
 	struct device_node *np = dev->of_node;
 	const char *name;
+	const char *regulator_name;
 	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
@@ -1409,6 +1435,9 @@  static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 			return -EINVAL;
 		}
 
+		of_property_read_string(display_np, "regulator-name",
+					&regulator_name);
+
 		of_property_read_string(display_np, "interface-pix-fmt",
 					&ipu_disp_format);
 		if (!ipu_disp_format) {
@@ -1526,6 +1555,21 @@  static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	if (ret < 0)
 		goto esetpar;
 
+	/* In dt mode,
+	 * using devm_regulator_get would require that the proprety referencing
+	 * the regulator phandle has to be inside the mx3fb node.
+	 */
+	if (np) {
+		if (regulator_name)
+			mx3fbi->reg_lcd = regulator_get(NULL, regulator_name);
+
+		if (IS_ERR(mx3fbi->reg_lcd))
+			return PTR_ERR(mx3fbi->reg_lcd);
+	} else {
+		/* Permit that driver without a regulator in non-dt mode */
+		mx3fbi->reg_lcd = regulator_get(dev, "lcd");
+	}
+
 	__blank(FB_BLANK_UNBLANK, fbi);
 
 	dev_info(dev, "registered, using mode %s\n", fb_mode);
@@ -1589,6 +1633,7 @@  static int mx3fb_probe(struct platform_device *pdev)
 	dma_cap_mask_t mask;
 	struct dma_chan *chan;
 	struct dma_chan_request rq;
+	struct mx3fb_info *mx3_fbi;
 	struct device_node *np = dev->of_node;
 	struct videomode *vm;
 	struct fb_videomode *fb_vm;
@@ -1673,6 +1718,12 @@  ersdc0:
 	dmaengine_put();
 	iounmap(mx3fb->reg_base);
 eremap:
+	if (mx3fb->fbi) {
+		mx3_fbi = mx3fb->fbi->par;
+
+		if ((!IS_ERR(mx3_fbi->reg_lcd)) && mx3_fbi->reg_lcd)
+			regulator_put(mx3_fbi->reg_lcd);
+	}
 	dev_err(dev, "mx3fb: failed to register fb\n");
 	return ret;
 }
@@ -1684,6 +1735,9 @@  static int mx3fb_remove(struct platform_device *dev)
 	struct mx3fb_info *mx3_fbi = fbi->par;
 	struct dma_chan *chan;
 
+	if ((!IS_ERR(mx3_fbi->reg_lcd)) && mx3_fbi->reg_lcd)
+		regulator_put(mx3_fbi->reg_lcd);
+
 	chan = &mx3_fbi->idmac_channel->dma_chan;
 	release_fbi(fbi);