| Message ID | 1367503462-24742-3-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Fabio, > On mx23 and mx28 the pullup bits are defined as: > > - 0: Enable the internal pad keeper > - 1: Disable the internal pad keeper > > Fix the definitions as they are currently the opposite. Sorry, but I think this is not correct. Please have a look at the reference manual for i.MX28 page 786: It seems that some pins have an internal pullup, but other pins "only" have the internal keeper you mentioned. Quote from : bank 0 pin 24: "Set this bit to one to _enable_ the internal pullup..." bank 0 pin 25: "Set this bit to one to _disable_ the internal keeper..." So I think the current implementation is correct. However, if you are trying to enable a pullup for a pin which does not have this function (e.g. because the hardware guys trimmed the BOM and did not spend an external pullup) you are actually disabling the keeper which makes the situation even worse in some situations. I'm still wondering what an ideal solution could be... Best regards, Michael
Hi Michael, On Thu, May 2, 2013 at 3:44 PM, Michael Heimpold <mhei@heimpold.de> wrote: > Sorry, but I think this is not correct. Please have a look > at the reference manual for i.MX28 page 786: > It seems that some pins have an internal pullup, but other > pins "only" have the internal keeper you mentioned. > > Quote from : > bank 0 pin 24: "Set this bit to one to _enable_ the internal pullup..." > bank 0 pin 25: "Set this bit to one to _disable_ the internal keeper..." > > So I think the current implementation is correct. Ok, I see. mx23 does not have the "Set this bit to one to _enable_ the internal pullup" option. > > However, if you are trying to enable a pullup for a pin which does not > have this function (e.g. because the hardware guys trimmed the BOM > and did not spend an external pullup) you are actually disabling the > keeper which makes the situation even worse in some situations. > > I'm still wondering what an ideal solution could be... So I plan to keep mx28 bits untouched and do the reverse definition only for mx23: #if defined CONFIG_MX28 #define PAD_PULLUP 1 #define PAD_NOPULL 0 #else #define PAD_PULLUP 0 #define PAD_NOPULL 1 #endif
Dear Fabio Estevam, > Hi Michael, > > On Thu, May 2, 2013 at 3:44 PM, Michael Heimpold <mhei@heimpold.de> wrote: > > Sorry, but I think this is not correct. Please have a look > > at the reference manual for i.MX28 page 786: > > It seems that some pins have an internal pullup, but other > > pins "only" have the internal keeper you mentioned. > > > > Quote from : > > bank 0 pin 24: "Set this bit to one to _enable_ the internal pullup..." > > bank 0 pin 25: "Set this bit to one to _disable_ the internal keeper..." > > > > So I think the current implementation is correct. > > Ok, I see. > > mx23 does not have the "Set this bit to one to _enable_ the internal > pullup" option. > > > However, if you are trying to enable a pullup for a pin which does not > > have this function (e.g. because the hardware guys trimmed the BOM > > and did not spend an external pullup) you are actually disabling the > > keeper which makes the situation even worse in some situations. > > > > I'm still wondering what an ideal solution could be... > > So I plan to keep mx28 bits untouched and do the reverse definition > only for mx23: > > #if defined CONFIG_MX28 > #define PAD_PULLUP 1 > #define PAD_NOPULL 0 > #else > #define PAD_PULLUP 0 > #define PAD_NOPULL 1 > #endif Wait, this code is pulled from Linux kernel. How does Linux solve that? Is it also broken in Linux? Best regards, Marek Vasut
Hi Fabio, > mx23 does not have the "Set this bit to one to _enable_ the internal > pullup" option. Hm, in the RM for i.MX233, page 1480 and above (http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf) I can only find definitions like "Set this bit to one to _disable_ the internal pad keeper and _enable_ the internal pull up resistor..." or for EMI pads at bank 3 "Set this bit to one to _disable_ the internal keeper" (without a note about pull ups). Could you please link me to your reference where the reverse definition comes from? Best regards, Michael
Hi Michael, On Thu, May 2, 2013 at 6:45 PM, Michael Heimpold <mhei@heimpold.de> wrote: > Could you please link me to your reference where the reverse > definition comes from? Sorry for the confusion, current code is correct for both mx23 and mx28. I discarded this patch and will send v3 shortly. Thanks, Fabio Estevam
diff --git a/arch/arm/include/asm/arch-mxs/iomux.h b/arch/arm/include/asm/arch-mxs/iomux.h index 66dcd36..70300d5 100644 --- a/arch/arm/include/asm/arch-mxs/iomux.h +++ b/arch/arm/include/asm/arch-mxs/iomux.h @@ -78,8 +78,8 @@ typedef u32 iomux_cfg_t; #define PAD_3V3 0 #endif -#define PAD_NOPULL 0 -#define PAD_PULLUP 1 +#define PAD_PULLUP 0 +#define PAD_NOPULL 1 #define MXS_PAD_4MA ((PAD_4MA << MXS_PAD_MA_SHIFT) | \ MXS_PAD_MA_VALID_MASK)
On mx23 and mx28 the pullup bits are defined as: - 0: Enable the internal pad keeper - 1: Disable the internal pad keeper Fix the definitions as they are currently the opposite. Cc: Lauri Hintsala <lauri.hintsala@bluegiga.com> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- This affects mx28 as well, so also adding Lauri. I also tested on mx28evk and behavior is fine after this change. Changes since v1: - Newly introduced arch/arm/include/asm/arch-mxs/iomux.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)