RE: [PATCH 4/6] OMAP: LDP: Port the display driver to new DSS2

Janorkar, Mayuresh
2011-05-12T08:47:32Z

> -----Original Message-----
> From: Igor Grinberg [mailto:grinberg@compulab.co.il]
> Sent: Thursday, May 12, 2011 2:08 PM
> To: Valkeinen, Tomi
> Cc: Janorkar, Mayuresh; tony@atomide.com; linux-omap@vger.kernel.org;
> Stanley Miao
> Subject: Re: [PATCH 4/6] OMAP: LDP: Port the display driver to new DSS2
>
> On 05/12/11 10:16, Igor Grinberg wrote:
>
> > On 05/12/11 09:40, Igor Grinberg wrote:
> >> On 05/11/11 09:32, Tomi Valkeinen wrote:
> >>> On Wed, 2011-05-11 at 10:28 +0530, Janorkar, Mayuresh wrote:
> >>>>> +static void __init ldp_display_init(void)
> >>>>> +{
> >>>>> + int r;
> >>>>> +
> >>>>> + struct gpio gpios[] = {
> >>>>> + {LCD_PANEL_RESET_GPIO, GPIOF_OUT_INIT_HIGH, "LCD
> RESET"},
> >>>>> + {LCD_PANEL_QVGA_GPIO, GPIOF_OUT_INIT_HIGH, "LCD QVGA"},
> >>>>> + {LCD_PANEL_ENABLE_GPIO, GPIOF_OUT_INIT_LOW, "LCD
> ENABLE"},
> >>>>> + {LCD_PANEL_BACKLIGHT_GPIO, GPIOF_OUT_INIT_LOW, "LCD
> >>>>> BACKLIGHT"},
> >>>>> + };
> >>>>> +
> >>>>> + r = gpio_request_array(gpios, ARRAY_SIZE(gpios));
> >>>>> + if (r) {
> >>>>> + pr_err("Cannot request LCD GPIOs, error %d\n", r);
> >>>>> + return;
> >>>>> + }
> >>>> If I test with this patch, this request is returning error.
> >>> Hmm. Well, the GPIOs are the same as in the old driver. However, the
> old
> >>> driver doesn't even seem to check if it manages to request the GPIOs,
> so
> >>> it may well be that it was failing also.
> >>>
> >>> Can you check from /sys/class/gpio/ if some of the GPIOs are already
> >>> allocated: 207, 199, 55, 56?
> >>>
> >>> The definition of two of those GPIOs look kinda funny to me: (15 +
> >>> OMAP_MAX_GPIO_LINES). I don't know what OMAP_MAX_GPIO_LINES means, but
> >>> it sure doesn't sound ok to have a GPIO number that is 15 over the
> >>> maximum =).
> >>>
> >> This means that some kind of gpio expander is used.
> >> I bet those are twl gpios (or may be some other discrete chip).
> >> If those are twl gpios then you need to provide the .setup callback
> >> in struct twl4030_gpio_platform_data which will request and setup those
> gpios.
> >>
> > No need to wonder ;)
> >
> > Mayuresh,
> > try the attached patch (though some changes to Tomi's patch will be
> needed)
> > if it works, then we're good.
>
> there is a typo in the function name:
>
> s/ldp_twl_gpio_setup/ldp_twl_gpios_setup/

Also,
> + };
> +
> + r = gpio_request_array(twl_gpios, ARRAY_SIZE(gpios));

This should be ARRAY_SIZE(twl_gpios));

And one more thing:
We need to use regulator for OMAP3430.

Something like this:
+/* VPLL2 for digital video outputs */
+static struct regulator_consumer_supply ldp_vpll2_supplies[] = {
+ REGULATOR_SUPPLY("vdds_dsi", "omapdss"),
+ REGULATOR_SUPPLY("vdds_dsi", "omapdss_dsi1"),
+};
+
+static struct regulator_init_data ldp_vpll2 = {
+ .constraints = {
+ .name = "VDVI",
+ .min_uV = 1800000,
+ .max_uV = 1800000,
+ .apply_uV = true,
+ .valid_modes_mask = REGULATOR_MODE_NORMAL
+ | REGULATOR_MODE_STANDBY,
+ .valid_ops_mask = REGULATOR_CHANGE_MODE
+ | REGULATOR_CHANGE_STATUS,
+ },
+ .num_consumer_supplies = ARRAY_SIZE(ldp_vpll2_supplies),
+ .consumer_supplies = ldp_vpll2_supplies,
+};

@@ -340,6 +514,7 @@ static struct twl4030_platform_data ldp_twldata = {
.vmmc1 = &ldp_vmmc1,
.gpio = &ldp_gpio_data,
.keypad = &ldp_kp_twl4030_data,
+ .vpll2 = &ldp_vpll2,

>
> sorry...
>
>
> --
> Regards,
> Igor.