Patchwork ARM: iomux-mx51: Always define PUE for pins used as GPIO

login
register
mail settings
Submitter Alexander Shiyan
Date May 21, 2013, 8:27 a.m.
Message ID <1369124845-19244-1-git-send-email-shc_work@mail.ru>
Download mbox | patch
Permalink /patch/245308/
State New
Headers show

Comments

Alexander Shiyan - May 21, 2013, 8:27 a.m.
This patch adds a PUE bit for pins which used as GPIO. This allow
to use the GPIOs correctly as inputs when source is open-drain with
external pullup resistor (IRQ open-drain outputs for example).

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/mach-imx/iomux-mx51.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Sascha Hauer - May 21, 2013, 7:24 p.m.
On Tue, May 21, 2013 at 12:27:25PM +0400, Alexander Shiyan wrote:
> This patch adds a PUE bit for pins which used as GPIO. This allow
> to use the GPIOs correctly as inputs when source is open-drain with
> external pullup resistor (IRQ open-drain outputs for example).

I don't understand. If there's an external pullup why do you want to
turn on the internal one?

Sascha

> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  arch/arm/mach-imx/iomux-mx51.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/iomux-mx51.h b/arch/arm/mach-imx/iomux-mx51.h
> index 75bbcc4..7dc187c 100644
> --- a/arch/arm/mach-imx/iomux-mx51.h
> +++ b/arch/arm/mach-imx/iomux-mx51.h
> @@ -34,7 +34,8 @@
>  #define MX51_SDHCI_PAD_CTRL	(PAD_CTL_PKE | PAD_CTL_DSE_HIGH | \
>  				PAD_CTL_PUS_47K_UP | PAD_CTL_PUE | \
>  				PAD_CTL_SRE_FAST | PAD_CTL_DVS)
> -#define MX51_GPIO_PAD_CTRL	(PAD_CTL_DSE_HIGH | PAD_CTL_PKE | PAD_CTL_SRE_FAST)
> +#define MX51_GPIO_PAD_CTRL	(PAD_CTL_DSE_HIGH | PAD_CTL_PKE | \
> +				PAD_CTL_PUE | PAD_CTL_SRE_FAST)
>  
>  #define MX51_PAD_CTRL_2		(PAD_CTL_PKE | PAD_CTL_HYS)
>  #define MX51_PAD_CTRL_3		(PAD_CTL_PKE | PAD_CTL_PUS_100K_UP)
> -- 
> 1.8.1.5
> 
>
Alexander Shiyan - May 22, 2013, 6:19 a.m.
> On Tue, May 21, 2013 at 12:27:25PM +0400, Alexander Shiyan wrote:
> > This patch adds a PUE bit for pins which used as GPIO. This allow
> > to use the GPIOs correctly as inputs when source is open-drain with
> > external pullup resistor (IRQ open-drain outputs for example).
> 
> I don't understand. If there's an external pullup why do you want to
> turn on the internal one?

It is just switch off keeper circuit.

Some pads not have internal pullup, probably this is affected only for such pins.
This is example of this issue:
Used pad MX51_PAD_DISPB2_SER_CLK__GPIO3_7, (GPIO3.7 = 71).
External pullup resistor is used.

gpio_direction output 71 1
gpio_direction_input 71
gpio_get_value 71; echo $?
1
gpio_direction output 71 0
gpio_direction_input 71
gpio_get_value 71; echo $?
0

So, we cannot get "1" here anymore if only pullup is used for this pad. The pad is keep last logic state...
The second solution for this is disable PKE, i am not sure which solution is the best...

> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  arch/arm/mach-imx/iomux-mx51.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-imx/iomux-mx51.h b/arch/arm/mach-imx/iomux-mx51.h
> > index 75bbcc4..7dc187c 100644
> > --- a/arch/arm/mach-imx/iomux-mx51.h
> > +++ b/arch/arm/mach-imx/iomux-mx51.h
> > @@ -34,7 +34,8 @@
> >  #define MX51_SDHCI_PAD_CTRL	(PAD_CTL_PKE | PAD_CTL_DSE_HIGH | \
> >  				PAD_CTL_PUS_47K_UP | PAD_CTL_PUE | \
> >  				PAD_CTL_SRE_FAST | PAD_CTL_DVS)
> > -#define MX51_GPIO_PAD_CTRL	(PAD_CTL_DSE_HIGH | PAD_CTL_PKE | PAD_CTL_SRE_FAST)
> > +#define MX51_GPIO_PAD_CTRL	(PAD_CTL_DSE_HIGH | PAD_CTL_PKE | \
> > +				PAD_CTL_PUE | PAD_CTL_SRE_FAST)
> >  
> >  #define MX51_PAD_CTRL_2		(PAD_CTL_PKE | PAD_CTL_HYS)
> >  #define MX51_PAD_CTRL_3		(PAD_CTL_PKE | PAD_CTL_PUS_100K_UP)
> > -- 

---
Lothar Waßmann - May 22, 2013, 7:02 a.m.
Hi,

Alexander Shiyan writes:
> > On Tue, May 21, 2013 at 12:27:25PM +0400, Alexander Shiyan wrote:
> > > This patch adds a PUE bit for pins which used as GPIO. This allow
> > > to use the GPIOs correctly as inputs when source is open-drain with
> > > external pullup resistor (IRQ open-drain outputs for example).
> > 
> > I don't understand. If there's an external pullup why do you want to
> > turn on the internal one?
> 
> It is just switch off keeper circuit.
> 
If you want to switch the keeper circuit off why don't you do that by
removing PAD_CTL_PKE rather than adding PAD_CTL_PUE?


Lothar Waßmann
Alexander Shiyan - May 22, 2013, 7:17 a.m.
> Alexander Shiyan writes:
> > > On Tue, May 21, 2013 at 12:27:25PM +0400, Alexander Shiyan wrote:
> > > > This patch adds a PUE bit for pins which used as GPIO. This allow
> > > > to use the GPIOs correctly as inputs when source is open-drain with
> > > > external pullup resistor (IRQ open-drain outputs for example).
> > > 
> > > I don't understand. If there's an external pullup why do you want to
> > > turn on the internal one?
> > 
> > It is just switch off keeper circuit.
> > 
> If you want to switch the keeper circuit off why don't you do that by
> removing PAD_CTL_PKE rather than adding PAD_CTL_PUE?

Again, I do not know which the solution here is better. If we remove
the PKE bit, for some pins it will be removed the default pullup/pulldown (PUS = 0),
so it may affect the some running configuration.

---
Lothar Waßmann - May 22, 2013, 8:36 a.m.
Hi,

Alexander Shiyan writes:
> > Alexander Shiyan writes:
> > > > On Tue, May 21, 2013 at 12:27:25PM +0400, Alexander Shiyan wrote:
> > > > > This patch adds a PUE bit for pins which used as GPIO. This allow
> > > > > to use the GPIOs correctly as inputs when source is open-drain with
> > > > > external pullup resistor (IRQ open-drain outputs for example).
> > > > 
> > > > I don't understand. If there's an external pullup why do you want to
> > > > turn on the internal one?
> > > 
> > > It is just switch off keeper circuit.
> > > 
> > If you want to switch the keeper circuit off why don't you do that by
> > removing PAD_CTL_PKE rather than adding PAD_CTL_PUE?
> 
> Again, I do not know which the solution here is better. If we remove
> the PKE bit, for some pins it will be removed the default pullup/pulldown (PUS = 0),
> so it may affect the some running configuration.
> 
IMO the current approach of defining the pad config with the pad
definition is broken anyway. It would be much better to specify the
pad configuration at the point where a specific pad is used, rather
than where it is defined.
The pin mux setting for a specific function of a pin is always the
same for any platform but the pad config may be different for
different platforms.


Lothar Waßmann

Patch

diff --git a/arch/arm/mach-imx/iomux-mx51.h b/arch/arm/mach-imx/iomux-mx51.h
index 75bbcc4..7dc187c 100644
--- a/arch/arm/mach-imx/iomux-mx51.h
+++ b/arch/arm/mach-imx/iomux-mx51.h
@@ -34,7 +34,8 @@ 
 #define MX51_SDHCI_PAD_CTRL	(PAD_CTL_PKE | PAD_CTL_DSE_HIGH | \
 				PAD_CTL_PUS_47K_UP | PAD_CTL_PUE | \
 				PAD_CTL_SRE_FAST | PAD_CTL_DVS)
-#define MX51_GPIO_PAD_CTRL	(PAD_CTL_DSE_HIGH | PAD_CTL_PKE | PAD_CTL_SRE_FAST)
+#define MX51_GPIO_PAD_CTRL	(PAD_CTL_DSE_HIGH | PAD_CTL_PKE | \
+				PAD_CTL_PUE | PAD_CTL_SRE_FAST)
 
 #define MX51_PAD_CTRL_2		(PAD_CTL_PKE | PAD_CTL_HYS)
 #define MX51_PAD_CTRL_3		(PAD_CTL_PKE | PAD_CTL_PUS_100K_UP)