* Tomi Valkeinen <tomi.valkeinen@xxxxxx> [140428 03:44]: > On 26/04/14 02:53, Tony Lindgren wrote: > > * Tomi Valkeinen <tomi.valkeinen@xxxxxx> [140424 02:53]: > >> On 18/04/14 18:51, Tony Lindgren wrote: > >> > >>>> + gpio = of_get_gpio(node, 0); > >>>> + if (gpio_is_valid(gpio) || gpio == -ENOENT) { > >>>> + ddata->enable_gpio = gpio; > >>>> + } else { > >>>> + dev_err(&pdev->dev, "failed to parse enable gpio\n"); > >>>> + return gpio; > >>>> + } > >>> > >>> We should set the GPIO polarity based on the OF_GPIO_ACTIVE_LOW like > >>> gpio_backlight_probe_dt is doing. > >> > >> Instead of doing it with the old gpio API, and checking the 'active' > >> flag everywhere, I think we can use the new gpiod API which handles the > >> polarity automatically. > >> > >> I attached prototype patches (based on -rc2) for panel dpi using that > >> approach. It's a bit messier than I'd like, because for non-DT boot we > >> need to request the gpio using the old API, and then convert it to > >> gpio_desc. We can remove that code when all the boards use DT. > >> > >> I've compiled tested this only, as I don't have DPI panels I could use. > >> I did try similar approach for TFP410, and it seemed to work fine. > > > > Got these working by updating my test patch to use enable-gpios instead > > of gpios, and had to change from GPIO_ACTIVE_LOW to GPIO_ACTIVE_HIGH. > > Are we now also breaking legacy booting by reversing the polarity? > > I don't think so. The GPIOs should be active-high by default, if I'm not > mistaken, so the polarities should be the same for legacy boot with or > without those patches. Of course, I don't have the boards so I have no > idea if the polarities have been correct even before. OK > debugfs/gpio shows the actual value of the gpio, so you could check from > there what it is. Yeah that should be checked. > > In any case, looks like we have some duplicate panel code.. Turns > > out most panel dpi users for omap3 board-*.c files are just > > sharp-ls037v7dw01 panels but configured in QVGA mode. At least for > > EVM and and LDP based on looking at the pictures and the configuration > > Hmm, true, board-ldp.c's panel looks very much like sharp-ls037v7dw01. Yes it seems so also based on the photos of the LCD panel. And 3430sdp also seems to have something similar but it's upside down. > Which EVM are you talking about? The LogicPD omap3 EVMs TMDSEVM3530 and TMDSEVM3730. The am335x EVM has a different larger panel. > > pins (using the names kernel): > > > > QVGA = lcd MO > > reset = lcd RESB > > ... > > > > Then the enable_gpio should be just a GPIO controlled 3.3V regulator > > in most cases. I suggest we move them over to ls037v7dw01 and allow > > configuring them both for VGA and QVGA depending on the orientation. > > Looking at the panel spec, it has the following pins: > > RESB - reset > MO - VGA/QVGA > UD - vertical scanning direction > LR - horizontal scanning direction > INI - power on control > > And it needs 3.3V power. > > Are you saying that on some boards the gpio used for enable_gpio is > actually used to switch on a 3.3V regulator? The 3.3V GPIO regulator is needed in addition to the configurable pins, I guess some ls037v7dw01 panels only have a subset of the pins controllable by GPIOs, and the GPIO regulator may be used instead of the INI. But that's hard to know without schematics. > > I guess you do have some device with ls037v7dw01 since you've been > > patching it? > > No, I don't have any boards with that panel. OK. I'll move my boards over to ls037v7dw01 and do a DT conversion patch for it that just sets a standard gpios property for panel ls037v7dw01. The gpios entry can have 0 entries in the middle depending on wiring configuration and there should not be any need to name each GPIO. As far as I'm concerned, your panel-dpi patch is fine with me, and we should not add more configuration to it for the ls037v7dw01 panels. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html