Re: [PATCH] pinctrl: dra: dt-bindings: Fix output pull up/down

[Roger Quadros <rogerq@ti.com>]

On 10/31/2014 03:50 PM, Nishanth Menon wrote:
> On 12:52-20141031, Roger Quadros wrote:
>> For PIN_OUTPUT_PULLUP and PIN_OUTPUT_PULLDOWN we must not set the
>> PULL_DIS bit which disables the PULLs.
>>
>> While at that get rid for the PULL_ENA defination. It is misleading
>> as there is no PULL enable bit in the register. And a zero bit defination
>> makes no sense.
> It was done to make it readable. Pull is enabled when that bit is 0 and
> disabled when that bit is 1. it is counter intutive enough for a macro

But you can't enable the PULL by ORing it. you have to AND it's inverted version with the rest.
It is confusing because it doesn't follow the logic of the other bit masks.

> to be defined. I suggest retaining that and not mixing with the current
> patch.

Agreed. I'll revise this patch to not touch that macro.

cheers,
-roger

> 
> 
>>
>> Fixes: 23d9cec07c58 (:pinctrl: dra: dt-bindings: Fix pull enable/disable")
> s/:pinctrl/"pinctrl/ ?
> 
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  include/dt-bindings/pinctrl/dra.h | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/dt-bindings/pinctrl/dra.h b/include/dt-bindings/pinctrl/dra.h
>> index 3d33794..71098e4 100644
>> --- a/include/dt-bindings/pinctrl/dra.h
>> +++ b/include/dt-bindings/pinctrl/dra.h
>> @@ -30,7 +30,6 @@
>>  #define MUX_MODE14	0xe
>>  #define MUX_MODE15	0xf
>>  
>> -#define PULL_ENA		(0 << 16)
>>  #define PULL_DIS		(1 << 16)
>>  #define PULL_UP			(1 << 17)
>>  #define INPUT_EN		(1 << 18)
>> @@ -40,12 +39,12 @@
>>  
>>  /* Active pin states */
>>  #define PIN_OUTPUT		(0 | PULL_DIS)
>> -#define PIN_OUTPUT_PULLUP	(PIN_OUTPUT | PULL_ENA | PULL_UP)
>> -#define PIN_OUTPUT_PULLDOWN	(PIN_OUTPUT | PULL_ENA)
>> +#define PIN_OUTPUT_PULLUP	(PULL_UP)
>> +#define PIN_OUTPUT_PULLDOWN	(0)
>>  #define PIN_INPUT		(INPUT_EN | PULL_DIS)
>>  #define PIN_INPUT_SLEW		(INPUT_EN | SLEWCONTROL)
>> -#define PIN_INPUT_PULLUP	(PULL_ENA | INPUT_EN | PULL_UP)
>> -#define PIN_INPUT_PULLDOWN	(PULL_ENA | INPUT_EN)
>> +#define PIN_INPUT_PULLUP	(INPUT_EN | PULL_UP)
>> +#define PIN_INPUT_PULLDOWN	(INPUT_EN)
>>  
>>  #endif
>>  
>> -- 
>> 1.8.3.2
>>
>