video: mx3fb: Use devm_kzalloc

Replace kzalloc by devm_kzalloc and remove the kfree() calls.

---
ChangeLog v6->v7:
- Removed the Cc from the patch, they went into
git send-email instead.
ChangeLog v5->v6:
- New patch need

---
 drivers/video/fbdev/mx3fb.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index 142e860..ee95de8 100644
@@ -1496,7 +1496,7 @@ static int mx3fb_probe(struct platform_device *pdev)
 	if (!sdc_reg)
 		return -EINVAL;
 
-	mx3fb = kzalloc(sizeof(*mx3fb), GFP_KERNEL);
+	mx3fb = devm_kzalloc(&pdev->dev, sizeof(*mx3fb), GFP_KERNEL);
 	if (!mx3fb)
 		return -ENOMEM;
 

@@ -1542,7 +1542,6 @@ ersdc0:
 	dmaengine_put();
 	iounmap(mx3fb->reg_base);
 eremap:
-	kfree(mx3fb);
 	dev_err(dev, "mx3fb: failed to register fb\n");
 	return ret;
 }

@@ -1561,7 +1560,6 @@ static int mx3fb_remove(struct platform_device *dev)
 	dmaengine_put();
 
 	iounmap(mx3fb->reg_base);
-	kfree(mx3fb);
 	return 0;
 }
 

1.7.9.5

This patch is based on:
838bdf7 video: mxsfb: fix broken videomode selection

---
ChangeLog v6->v7:
- Removed the Cc from the patch, they went into
git send-email instead.

ChangeLog v5->v6:
- Shrinked the Cc list.
- de-active and pixelclk-active dt properties are
now handled in this patch to get rid of the
"fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*" patch

ChangeLog v4->v5:
- Added some people in the Cc list.
- The full ipu register range is now passed to the driver,
the code and the documentation were adapted to it.
- Updated the documentation not to mention the lcd controller, the ipu was mentioned instead.
- The ipu patch was removed from this patchset, as a consequence the mx3fb code has been adapted not to expect the dma ipu driver to be probed trough the device tree.

ChangeLog v3->v4:
- Updated bindings.
- Updated documentation accordinly.
- Updated code accordinly.
- Fixed the lack of "ret =" in of_property_read_string(display_np, "model", &name);
- Supressed some compilation warnings.

ChangeLog v2->v3:
- The device tree bindings were reworked in order to make it look more like the IPUv3 bindings.
- The interface_pix_fmt property now looks like the IPUv3 one.

---
 .../devicetree/bindings/video/fsl,mx3-fb.txt       |   44 +++++
 drivers/video/fbdev/Kconfig                        |    2 +
 drivers/video/fbdev/mx3fb.c                        |  185 +++++++++++++++++---
 3 files changed, 208 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt

new file mode 100644
index 0000000..c0409a4
@@ -0,0 +1,44 @@
+Freescale MX3 IPU.
+==================
+
+Required properties:
+- compatible: Should be "fsl,<chip>-ipu". compatible chips include the imx31 and the
+  imx35.
+- reg: should be register base and length as documented in the datasheet.
+- clocks: Handle to the ipu_gate clock.
+- display: Phandle to a "fsl,mx3-parallel-display" compatible display node
+  which is described below.
+
+Example:
+
+ipu: ipu at 53fc0000 {
+	compatible = "fsl,imx35-ipu";
+	reg = <0x53fc0000 0x4000>;
+	clocks = <&clks 55>;
+};
+
+Parallel display support
+========================
+
+Required properties:
+- compatible: Should be "fsl,mx3-parallel-display".
+- model : The user-visible name of the display.
+
+Optional properties:
+- interface_pix_fmt: How this display is connected to the
+  crtc. Currently supported types: "rgb24", "rgb565", "rgb666".
+
+It can also have an optional timing subnode as described in
+  Documentation/devicetree/bindings/video/display-timing.txt.
+
+Example:
+
+display0: display at di0 {
+	compatible = "fsl,mx3-parallel-display";
+	interface-pix-fmt = "rgb666";
+	model = "CMO-QVGA";
+};
+
+&ipu {
+	display = <&display0>;
+}

index 67409e0..c996b96 100644
@@ -2330,6 +2330,8 @@ config FB_MX3
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	select FB_MODE_HELPERS
 	default y
 	help
 	  This is a framebuffer device for the i.MX31 LCD Controller. So

1.7.9.5

This commit is based on the following commit by Fabio Estevam:
4344429 video: mxsfb: Introduce regulator support

---
ChangeLog v6->v7:
- Removed the Cc from the patch, they went into
git send-email instead.

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(-)

index 952d2b5..40b47dd 100644
@@ -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);
 

1.7.9.5

???????, 14 ????? 2014, 10:12 +01:00 ?? Denis Carikli <[email protected]>:

...

I need to add device tree support to the mx3fb driver. My first approach gave a binding that looked like that:

cmo_qvga: display {
model = "CMO-QVGA";
[...]

   display-timings {
     qvga_timings: 320x240 {
       hactive = <320>;
       vactive = <240>;
       [...]
    };
};

ipu: ipu at 53fc0000 {
compatible = "fsl,imx31-ipu";
reg = < 0x53fc0000 0x5f
0x53fc0088 0x2b >;
interrupts = <42 41>;
dma-channels = <32>;
#dma-cells = <1>;
clocks = <&clks 55>;
clock-names = "";
};

lcdc: mx3fb at 53fc00b4 {
compatible = "fsl,mx3-fb";
reg = <0x53fc00b4 0x0b>;
clocks = <&clks 55>;
dmas = <&ipu 14>;
dma-names = "tx";
display = <&cmo_qvga>;
};

The issue was that exporting the "dma ipu driver" was not a good idea. I was told to instead make bindings that looks very similar to the ipuv3 driver[1]
So at the end that gave something like that:

cmo_qvga: display at di0 {
compatible = "fsl,mx3-parallel-display";
regulator-name = "lcd";
lcd-supply = <&reg_lcd_3v3>;
model = "CMO-QVGA";

   display-timings {
     qvga_timings: 320x240 {
       hactive = <320>;
       vactive = <240>;
       [...]
    };
};

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

So here fsl,imx35-ipu is binded to the mx3fb driver.
But the mx3fb driver still need to use the dma-ipu driver somehow. That's why the dma-ipu driver is handled behind the scenes, that way it's not exported to the device tree bindings.

Now, since the mx3fb driver is binded to the "fsl,imx35-ipu" compatible, if I would do a "mx3fbi->reg_lcd = devm_regulator_get(dev, "lcd");", that would then lookup for the regulator in the mx3fb node (The last "ipu at 53fc0000" here).

Instead the regulator can be found in the display node, which has no driver associated with it.

In the case of the ipuv3, the parallel display driver is associated with the display at di0 node, so the device matches with the device tree node directly.

References:
-----------
[1] The ipuv3 driver is in drivers/staging/imx-drm/
[2] the dma ipu driver is in drivers/dma/ipu/

Denis.

???????, 14 ????? 2014, 12:23 +01:00 ?? Denis Carikli <[email protected]>:

Sascha, Shawn, can you comment on that?

---

[Added Mark to Cc]

[...]

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Here, the display devicenode has no struct device associated with it like mentioned above.
Because of that, retriving the regulator from the devicetree, for instance like regulator_dev_lookup() does, would require a different approach.

As I understand it, what happen in regulator_dev_lookup when regulator_get(NULL, regulator_name) is used is the following:

Since the struct device is NULL, it skips the struct device based lookup, but also the devicetree lookup (it uses dev->of_node for that) At the end it falls back on a match on the regulator name to find it.

would keeping regulator_get(NULL, regulator_name); in the driver instead be better for now?

Denis.

It would be much better to have an actual device to get things for,
using a hard coded regulator name is very much deprecated. Using a hard
coded name is fragile and we don't have support for mapping this
properly in the device tree.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140822/16a51093/attachment.sig>

There is already a "dma ipu driver" for the mx3* that is currently at drivers/dma/ipu/ipu_idmac.c

The mx3fb framebuffer driver uses this "dma ipu driver".

The goal is not to expose the "dma ipu driver" in the device tree bindings, to make the mx3fb bindings look similar to the ipuv3 ones.

In the device tree bindings, the "dma ipu driver" is absent, and the mx3fb compatible is "fsl,imx<soc>-ipu".

An node for the mx3fb was also added in the imx35 dtsi.

---
ChangeLog v6->v7:
- Removed the Cc from the patch, they went into
git send-email instead.
- Removed the comment, and instead a commit messaage was added to explain the non-straingtforward part of the change.

ChangeLog v5->v6:
- Shrinked the Cc list.

ChangeLog v4->v5:
- Added Grant Likely and Shawn Guo in the Cc list.
- Adapted to the new non-dma ipu bindings.
- Adapted to the use of imx35-pingrp.h
- The pinctrl ipu node addition was moved in this commit.

ChangeLog v3->v4:
- Splitted the imx35.dtsi display support (new patch).

---
 arch/arm/boot/dts/imx35.dtsi |    7 +++++++
 arch/arm/mach-imx/imx35-dt.c |    6 ++++++
 2 files changed, 13 insertions(+)

index e59ccb4..b9bf6a0 100644
@@ -230,6 +230,13 @@
 				status = "disabled";
 			};
 
+			ipu: ipu at 53fc0000 {
+				compatible = "fsl,imx35-ipu";
+				reg = <0x53fc0000 0x4000>;
+				clocks = <&clks 55>;
+				status = "disabled";
+			};
+
 			audmux: audmux at 53fc4000 {
 				compatible = "fsl,imx35-audmux", "fsl,imx31-audmux";
 				reg = <0x53fc4000 0x4000>;

index 9d48e00..657cc5d 100644
@@ -18,14 +18,20 @@
 #include <asm/mach/time.h>
 #include <asm/hardware/cache-l2x0.h>
 #include "common.h"
+#include "devices-imx35.h"
 #include "mx35.h"
 
 static void __init imx35_dt_init(void)
 {
+	struct device_node *np;
 	mxc_arch_reset_init_dt();
 
 	of_platform_populate(NULL, of_default_bus_match_table,
 			     NULL, NULL);
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx35-ipu");
+	if (of_device_is_available(np))
+		imx35_add_ipu_core();
 }
 
 static void __init imx35_irq_init(void)

1.7.9.5

---
ChangeLog v6->v7:
- Removed the Cc from the patch, they went into
git send-email instead.

ChangeLog v5->v6:
- Shrinked the Cc list.
- Added the targets in the Makefile
- rebased not to use the now gone pingrp headers.

ChangeLog v4->v5:
- Added Grant Likely and Shawn Guo in the Cc list.
- Adapted to the new non-dma ipu bindings.

ChangeLog v3->v4:
- Shortened the licenses.
- adapted the dts(i) to the new bindings.
ChangeLog v2->v3:
- The dts were adapted to the new DT bindings which looks more like the IPUv3 ones.

---
 arch/arm/boot/dts/Makefile                         |    3 +
 arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi       |   28 ++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts  |   58 ++++++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts  |   47 ++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts   |   47 ++++++++++++++++
 5 files changed, 183 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts

index cf6c523..1df8945 100644
@@ -173,6 +173,9 @@ dtb-$(CONFIG_ARCH_MXC) += \
 	imx27-phytec-phycard-s-rdk.dtb \
 	imx31-bug.dtb \
 	imx35-eukrea-mbimxsd35-baseboard.dtb \
+	imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dtb \
+	imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dtb \
+	imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dtb \
 	imx50-evk.dtb \
 	imx51-apf51.dtb \
 	imx51-apf51dev.dtb \

index 906ae93..6596009 100644
@@ -70,6 +70,34 @@
 				MX35_PAD_I2C1_DAT__I2C1_SDA		0x80000000
 			>;
 		};
+
+		pinctrl_ipu_disp0: ipudisp0grp {
+			fsl,pins = <
+				MX35_PAD_LD0__IPU_DISPB_DAT_0         0x80000000
+				MX35_PAD_LD1__IPU_DISPB_DAT_1         0x80000000
+				MX35_PAD_LD2__IPU_DISPB_DAT_2         0x80000000
+				MX35_PAD_LD3__IPU_DISPB_DAT_3         0x80000000
+				MX35_PAD_LD4__IPU_DISPB_DAT_4         0x80000000
+				MX35_PAD_LD5__IPU_DISPB_DAT_5         0x80000000
+				MX35_PAD_LD6__IPU_DISPB_DAT_6         0x80000000
+				MX35_PAD_LD7__IPU_DISPB_DAT_7         0x80000000
+				MX35_PAD_LD8__IPU_DISPB_DAT_8         0x80000000
+				MX35_PAD_LD9__IPU_DISPB_DAT_9         0x80000000
+				MX35_PAD_LD10__IPU_DISPB_DAT_10       0x80000000
+				MX35_PAD_LD11__IPU_DISPB_DAT_11       0x80000000
+				MX35_PAD_LD12__IPU_DISPB_DAT_12       0x80000000
+				MX35_PAD_LD13__IPU_DISPB_DAT_13       0x80000000
+				MX35_PAD_LD14__IPU_DISPB_DAT_14       0x80000000
+				MX35_PAD_LD15__IPU_DISPB_DAT_15       0x80000000
+				MX35_PAD_LD16__IPU_DISPB_DAT_16       0x80000000
+				MX35_PAD_LD17__IPU_DISPB_DAT_17       0x80000000
+				MX35_PAD_D3_HSYNC__IPU_DISPB_D3_HSYNC 0x80000000
+				MX35_PAD_D3_FPSHIFT__IPU_DISPB_D3_CLK 0x80000000
+				MX35_PAD_D3_DRDY__IPU_DISPB_D3_DRDY   0x80000000
+				MX35_PAD_D3_VSYNC__IPU_DISPB_D3_VSYNC 0x80000000
+				MX35_PAD_CONTRAST__IPU_DISPB_CONTR    0x80000000
+			>;
+		};
 	};
 };
 

1.7.9.5

I've picked this patch for 3.16. I didn't pick the DT related patches in this series as there seemed to be unanswered questions about them.

Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140508/25a73595/attachment.sig>