| Message ID | 4DCB8937.2060101@compulab.co.il (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Tomi Valkeinen |
| Headers | show |
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/ sorry...
> -----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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/12/11 11:47, Janorkar, Mayuresh wrote: >> On 05/12/11 10:16, Igor Grinberg wrote: >> 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)); yeah, sorry for that that is what happening when making patches in a hurry and no checks done > 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, > Right. Do both fixes make it work? If it is, then Tomi, will you add both fixes to the patch set with our SOBs? or do you want this to be sent properly as a follow up?
On Thu, 2011-05-12 at 14:20 +0300, Igor Grinberg wrote: > Right. Do both fixes make it work? > > If it is, then Tomi, > will you add both fixes to the patch set with our SOBs? > or do you want this to be sent properly as a follow up? Yes, I can add these all to the same patch and handle it through DSS tree. Although due to the reluctance to make changes to arch/arm/*omap*, I guess we'll pass these patches in this merge window. Mayuresh, if you're able to test this and it works, can you send me the full diff so that I don't make any mistakes combining the code? Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
arch/arm/mach-omap2/board-ldp.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-ldp.c b/arch/arm/mach-omap2/board-ldp.c index f7d6038..ca92e43 100644 --- a/arch/arm/mach-omap2/board-ldp.c +++ b/arch/arm/mach-omap2/board-ldp.c @@ -203,10 +203,27 @@ static struct twl4030_usb_data ldp_usb_data = { .usb_mode = T2_USB_MODE_ULPI, }; +static int ldp_twl_gpio_setup(struct device *dev, unsigned gpio, unsigned ngpio) +{ + int r; + + struct gpio twl_gpios[] = { + {gpio + 7 , GPIOF_OUT_INIT_LOW, "LCD ENABLE"}, + {gpio + 15, GPIOF_OUT_INIT_LOW, "LCD BACKLIGHT"}, + }; + + r = gpio_request_array(twl_gpios, ARRAY_SIZE(gpios)); + if (r) + pr_err("Cannot request LCD GPIOs, error %d\n", r); + + return r; +} + static struct twl4030_gpio_platform_data ldp_gpio_data = { .gpio_base = OMAP_MAX_GPIO_LINES, .irq_base = TWL4030_GPIO_IRQ_BASE, .irq_end = TWL4030_GPIO_IRQ_END, + .setup = ldp_twl_gpios_setup, }; static struct twl4030_madc_platform_data ldp_madc_data = {