| Message ID | 1393408800-8946-3-git-send-email-denis@eukrea.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
?????, 26 ??????? 2014, 10:59 +01:00 ?? Denis Carikli <denis@eukrea.com>: > This commit is based on the following commit by Fabio Estevam: > 4344429 video: mxsfb: Introduce regulator support ... > +++ b/drivers/video/mx3fb.c ... > @@ -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", > + ®ulator_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. What??? > + */ > + 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"); > + } Why you want to use an excess "regulator-name" property? Just use devm_regulator_get(dev, "lcd") for both dt/non-dt case.
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.
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", + ®ulator_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);
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(-)