| Message ID | 1414752745-13696-1-git-send-email-rogerq@ti.com |
|---|---|
| State | New |
| Headers | show |
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 to be defined. I suggest retaining that and not mixing with the current patch. > > 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 >
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 >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. Fixes: 23d9cec07c58 (:pinctrl: dra: dt-bindings: Fix pull enable/disable") Signed-off-by: Roger Quadros <rogerq@ti.com> --- include/dt-bindings/pinctrl/dra.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)