RE: [RFC 1/1] video: da8xx-fb: adding dt support

From: Prabhakar Lad [mailto:prabhakar.csengg@gmail.com]

Sent: Thursday, August 08, 2013 11:07 PM
Subject: Re: [RFC 1/1] video: da8xx-fb: adding dt support

Hi Darren,

Thanks for the patch, below are few nits!

Prabhakar,

Thanks for reviewing, somehow I missed your reply so sorry about late response.

On Fri, Aug 9, 2013 at 1:45 AM, Darren Etheridge [off-list ref]
wrote:

quoted

Enhancing driver to enable probe triggered by a corresponding dt entry.

<snip>

quoted

+
+       cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode),

GFP_KERNEL);

quoted

+       if (!cfg) {
+               dev_err(&dev->dev, "memory allocation failed\n");

devm_* api will print the message if allocation has failed, please remove the
above err statement and also similarly below.

Will fix.

<snip>

quoted

+               lcdc_info = devm_kzalloc(&dev->dev,
+                                        sizeof(struct fb_videomode),
+                                        GFP_KERNEL);
+               if (!lcdc_info) {
+                       dev_err(&dev->dev, "memory allocation
+ failed\n");

ditto

Will fix

<snip>

quoted

@@ -1390,7 +1439,7 @@ static int fb_probe(struct platform_device

*device)

quoted

        par->dev = &device->dev;
        par->lcdc_clk = tmp_lcdc_clk;
        par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
-       if (fb_pdata->panel_power_ctrl) {
+       if (fb_pdata && fb_pdata->panel_power_ctrl) {
                par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
                par->panel_power_ctrl(1);

What happens to this data in DT case ?

Good find,  right now we don't do any power control in the DT case - this is something that needs to be addressed.

 > >         }

quoted

@@ -1638,6 +1687,17 @@ static int fb_resume(struct platform_device

*dev)  #define fb_resume NULL  #endif

+static const struct of_device_id da8xx_fb_of_match[] = {
+       /*
+        * this driver supports version 1 and version 2 of the
+        * Texas Instruments lcd controller (lcdc) hardware block
+        */
+       {.compatible = "ti,da830-lcdc", },
+       {.compatible = "ti,am3352-lcdc", },
+       {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+

you can bound this structure and the macro in IS_ENABLED(CONFIG_OF)

Ok will look at that.

Thanks,
Darren