On 2013-12-10 12:56, Enric Balletbo Serra wrote:
> Hi all,
>
> 2013/12/9 Javier Martinez Canillas <javier@xxxxxxxxxxxx>:
>> Hi Tomi,
>>
>> On Mon, Dec 9, 2013 at 4:30 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
>>> On 2013-12-09 17:09, Javier Martinez Canillas wrote:
>>>> Hi Tomi,
>>>>
>>>> On Mon, Dec 9, 2013 at 1:56 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
>>>>> On 2013-12-06 10:57, Javier Martinez Canillas wrote:
>>>>>
>>>>>>> + tfp410: encoder@0 {
>>>>>>> + compatible = "ti,tfp410";
>>>>>>> + gpios = <&gpio1 0 0>; /* 0, power-down */
>>>>>>> +
>>>>>>
>>>>>> Please use the constants from include/dt-bindings/ instead of magic
>>>>>> numbers, i.e:
>>>>>>
>>>>>> gpios = <&gpio1 0 GPIO_ACTIVE_HIGH>; /* 0, power-down */
>>>>>
>>>>> Thanks, fixed now (for all .dts files)
>>>>>
>>>>> However... The TFP410 gpio is "power-down". I think we should actually
>>>>> mark it as GPIO_ACTIVE_LOW, as setting it to 0 powers down the device.
>>>>>
>>>>
>>>> yes, I looked at the TFP410 datasheet [0] and the Power Down pin is
>>>> indeed an active-low, I just replaced to GPIO_ACTIVE_HIGH since you
>>>> were using a constant 0 and include/dt-bindings/gpio/gpio.h defines
>>>> GPIO_ACTIVE_HIGH as 0.
>>>>
>>>> I just asked to Enric why we use GPIO_ACTIVE_HIGH for the PD pin on
>>>> the IGEPv2 DTS instad and is because the IGEP board uses a hardware
>>>> signal inverter but that is a special case. I don't know about the
>>>> Panda board since I haven't looked at its datasheet.
>>>
>>> Oh. Does it work on igep? The TFP410 driver always handles the PD GPIO
>>> as it were active-low. The flag is ignored.
>>>
>>
>> How weird, it does work on the IGEPv2 but you are right I just looked
>> at at drivers/video/omap2/displays-new/encoder-tfp410.c and I see
>> that it indeed just does:
>>
>> r = devm_gpio_request_one(&pdev->dev, ddata->pd_gpio,
>> GPIOF_OUT_INIT_LOW, "tfp410 PD");
>>
>> So I don't know how it is working... I'm on the road and won't have
>> access to my IGEPv2 to dig further on this. Maybe Enric can shed more
>> light on this.
>>
>
> On IGEPv2 the GPIO that controls the power-down pin is connected
> through a dual/buffer driver [1]. This driver is only a buffer, not
> inverts the signal (I had told you wrong, sorry Javier ), so the pin
> continues being active low.
>
> As both of you pointed the driver ignores the flag to handle the PD
> GPIO, so doesn't matter if in the device tree we put GPIO_ACTIVE_HIGH
> or GPIO_ACTIVE_LOW, so simply it works. About the patch to support
> display for IGEP, to be coherent, the gpio should be defined as
> GPIO_ACTIVE_LOW not GPIO_ACTIVE_HIGH. I have tested, and of course,
> works.
>
>
> diff --git a/arch/arm/boot/dts/omap3-igep0020.dts
> b/arch/arm/boot/dts/omap3-igep0020.dts
> index 2569d60..d185e06 100644
> --- a/arch/arm/boot/dts/omap3-igep0020.dts
> +++ b/arch/arm/boot/dts/omap3-igep0020.dts
> @@ -233,7 +233,7 @@
>
> tfp410: encoder@0 {
> compatible = "ti,tfp410";
> - gpios = <&gpio6 10 GPIO_ACTIVE_HIGH>; /* 170, power-down */
> + gpios = <&gpio6 10 GPIO_ACTIVE_LOW>; /* 170, power-down */
>
> ports {
> #address-cells = <1>;
>
>
> [1] http://www.ti.com/product/sn74lvc2g07
Ok, looks good. I have changed the TFP gpios to active-low in my series
for all .dts files, which includes the igep0020.dts.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
