* iomux-mx3.h: possible macro precedence issue
@ 2016-09-20 20:02 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2016-09-20 20:02 UTC (permalink / raw)
To: linux-arm-kernel
Julia Lawall wrote a script
Link:?http://lkml.kernel.org/r/alpine.DEB.2.10.1609201503260.2914 at hadrien
that found a possible issue with macro argument precedence.
diff -u -p a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
--- a/arch/arm/mach-imx/iomux-mx3.h
+++ b/arch/arm/mach-imx/iomux-mx3.h
@@ -529,7 +529,7 @@ enum iomux_pins {
#define MX31_PIN_DCD_DTE1__DCD_DTE2 IOMUX_MODE(MX31_PIN_DCD_DTE1, IOMUX_CONFIG_ALT1)
#define MX31_PIN_RI_DTE1__RI_DTE2 IOMUX_MODE(MX31_PIN_RI_DTE1, IOMUX_CONFIG_ALT1)
#define MX31_PIN_DSR_DTE1__DSR_DTE2 IOMUX_MODE(MX31_PIN_DSR_DTE1, IOMUX_CONFIG_ALT1)
-#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
+#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
#define MX31_PIN_PC_RST__CTS5 IOMUX_MODE(MX31_PIN_PC_RST, IOMUX_CONFIG_ALT2)
#define MX31_PIN_PC_VS2__RTS5 IOMUX_MODE(MX31_PIN_PC_VS2, IOMUX_CONFIG_ALT2)
#define MX31_PIN_PC_BVD2__TXD5 IOMUX_MODE(MX31_PIN_PC_BVD2, IOMUX_CONFIG_ALT2)
this may be intentional, but perhaps a solution
would be to use parentheses in the #define IOMUX_MODE
---
diff --git a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
index 368667b..3f9ede2 100644
--- a/arch/arm/mach-imx/iomux-mx3.h
+++ b/arch/arm/mach-imx/iomux-mx3.h
@@ -156,7 +156,7 @@ void mxc_iomux_mode(unsigned int pin_mode);
(((gpionum << IOMUX_GPIONUM_SHIFT) & IOMUX_GPIONUM_MASK) | \
(padnum & IOMUX_PADNUM_MASK))
-#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
+#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
#define IOMUX_TO_GPIO(iomux_pin) \
((iomux_pin & IOMUX_GPIONUM_MASK) >> IOMUX_GPIONUM_SHIFT)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* iomux-mx3.h: possible macro precedence issue
@ 2016-09-20 20:02 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2016-09-20 20:02 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Julia Lawall, linux-arm-kernel, LKML
Julia Lawall wrote a script
Link: http://lkml.kernel.org/r/alpine.DEB.2.10.1609201503260.2914@hadrien
that found a possible issue with macro argument precedence.
diff -u -p a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
--- a/arch/arm/mach-imx/iomux-mx3.h
+++ b/arch/arm/mach-imx/iomux-mx3.h
@@ -529,7 +529,7 @@ enum iomux_pins {
#define MX31_PIN_DCD_DTE1__DCD_DTE2 IOMUX_MODE(MX31_PIN_DCD_DTE1, IOMUX_CONFIG_ALT1)
#define MX31_PIN_RI_DTE1__RI_DTE2 IOMUX_MODE(MX31_PIN_RI_DTE1, IOMUX_CONFIG_ALT1)
#define MX31_PIN_DSR_DTE1__DSR_DTE2 IOMUX_MODE(MX31_PIN_DSR_DTE1, IOMUX_CONFIG_ALT1)
-#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
+#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
#define MX31_PIN_PC_RST__CTS5 IOMUX_MODE(MX31_PIN_PC_RST, IOMUX_CONFIG_ALT2)
#define MX31_PIN_PC_VS2__RTS5 IOMUX_MODE(MX31_PIN_PC_VS2, IOMUX_CONFIG_ALT2)
#define MX31_PIN_PC_BVD2__TXD5 IOMUX_MODE(MX31_PIN_PC_BVD2, IOMUX_CONFIG_ALT2)
this may be intentional, but perhaps a solution
would be to use parentheses in the #define IOMUX_MODE
---
diff --git a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
index 368667b..3f9ede2 100644
--- a/arch/arm/mach-imx/iomux-mx3.h
+++ b/arch/arm/mach-imx/iomux-mx3.h
@@ -156,7 +156,7 @@ void mxc_iomux_mode(unsigned int pin_mode);
(((gpionum << IOMUX_GPIONUM_SHIFT) & IOMUX_GPIONUM_MASK) | \
(padnum & IOMUX_PADNUM_MASK))
-#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
+#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
#define IOMUX_TO_GPIO(iomux_pin) \
((iomux_pin & IOMUX_GPIONUM_MASK) >> IOMUX_GPIONUM_SHIFT)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* iomux-mx3.h: possible macro precedence issue
2016-09-20 20:02 ` Joe Perches
@ 2016-09-21 6:06 ` Uwe Kleine-König
-1 siblings, 0 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2016-09-21 6:06 UTC (permalink / raw)
To: linux-arm-kernel
Cc += Shawn
Hello,
On Tue, Sep 20, 2016 at 01:02:24PM -0700, Joe Perches wrote:
> Julia Lawall wrote a script
>
> Link:?http://lkml.kernel.org/r/alpine.DEB.2.10.1609201503260.2914 at hadrien
>
> that found a possible issue with macro argument precedence.
>
>
> diff -u -p a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> --- a/arch/arm/mach-imx/iomux-mx3.h
> +++ b/arch/arm/mach-imx/iomux-mx3.h
> @@ -529,7 +529,7 @@ enum iomux_pins {
> #define MX31_PIN_DCD_DTE1__DCD_DTE2 IOMUX_MODE(MX31_PIN_DCD_DTE1, IOMUX_CONFIG_ALT1)
> #define MX31_PIN_RI_DTE1__RI_DTE2 IOMUX_MODE(MX31_PIN_RI_DTE1, IOMUX_CONFIG_ALT1)
> #define MX31_PIN_DSR_DTE1__DSR_DTE2 IOMUX_MODE(MX31_PIN_DSR_DTE1, IOMUX_CONFIG_ALT1)
> -#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
> +#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
> #define MX31_PIN_PC_RST__CTS5 IOMUX_MODE(MX31_PIN_PC_RST, IOMUX_CONFIG_ALT2)
> #define MX31_PIN_PC_VS2__RTS5 IOMUX_MODE(MX31_PIN_PC_VS2, IOMUX_CONFIG_ALT2)
> #define MX31_PIN_PC_BVD2__TXD5 IOMUX_MODE(MX31_PIN_PC_BVD2, IOMUX_CONFIG_ALT2)
I assume this is the only problematic definition? If so, there is a
single affected platform: arch/arm/mach-imx/mach-kzm_arm11_01.c
> this may be intentional, but perhaps a solution
> would be to use parentheses in the #define IOMUX_MODE
>
> ---
>
> diff --git a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> index 368667b..3f9ede2 100644
> --- a/arch/arm/mach-imx/iomux-mx3.h
> +++ b/arch/arm/mach-imx/iomux-mx3.h
> @@ -156,7 +156,7 @@ void mxc_iomux_mode(unsigned int pin_mode);
> (((gpionum << IOMUX_GPIONUM_SHIFT) & IOMUX_GPIONUM_MASK) | \
> (padnum & IOMUX_PADNUM_MASK))
>
> -#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
> +#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
>
> #define IOMUX_TO_GPIO(iomux_pin) \
> ((iomux_pin & IOMUX_GPIONUM_MASK) >> IOMUX_GPIONUM_SHIFT)
My preference is to fix IOMUX_MODE instead of adding the parents to
MX31_PIN_DTR_DTE1__DTR_DTE2.
Joe, do you create a proper patch?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: iomux-mx3.h: possible macro precedence issue
@ 2016-09-21 6:06 ` Uwe Kleine-König
0 siblings, 0 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2016-09-21 6:06 UTC (permalink / raw)
To: Joe Perches; +Cc: Sascha Hauer, Julia Lawall, LKML, linux-arm-kernel, Shawn Guo
Cc += Shawn
Hello,
On Tue, Sep 20, 2016 at 01:02:24PM -0700, Joe Perches wrote:
> Julia Lawall wrote a script
>
> Link: http://lkml.kernel.org/r/alpine.DEB.2.10.1609201503260.2914@hadrien
>
> that found a possible issue with macro argument precedence.
>
>
> diff -u -p a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> --- a/arch/arm/mach-imx/iomux-mx3.h
> +++ b/arch/arm/mach-imx/iomux-mx3.h
> @@ -529,7 +529,7 @@ enum iomux_pins {
> #define MX31_PIN_DCD_DTE1__DCD_DTE2 IOMUX_MODE(MX31_PIN_DCD_DTE1, IOMUX_CONFIG_ALT1)
> #define MX31_PIN_RI_DTE1__RI_DTE2 IOMUX_MODE(MX31_PIN_RI_DTE1, IOMUX_CONFIG_ALT1)
> #define MX31_PIN_DSR_DTE1__DSR_DTE2 IOMUX_MODE(MX31_PIN_DSR_DTE1, IOMUX_CONFIG_ALT1)
> -#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
> +#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
> #define MX31_PIN_PC_RST__CTS5 IOMUX_MODE(MX31_PIN_PC_RST, IOMUX_CONFIG_ALT2)
> #define MX31_PIN_PC_VS2__RTS5 IOMUX_MODE(MX31_PIN_PC_VS2, IOMUX_CONFIG_ALT2)
> #define MX31_PIN_PC_BVD2__TXD5 IOMUX_MODE(MX31_PIN_PC_BVD2, IOMUX_CONFIG_ALT2)
I assume this is the only problematic definition? If so, there is a
single affected platform: arch/arm/mach-imx/mach-kzm_arm11_01.c
> this may be intentional, but perhaps a solution
> would be to use parentheses in the #define IOMUX_MODE
>
> ---
>
> diff --git a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> index 368667b..3f9ede2 100644
> --- a/arch/arm/mach-imx/iomux-mx3.h
> +++ b/arch/arm/mach-imx/iomux-mx3.h
> @@ -156,7 +156,7 @@ void mxc_iomux_mode(unsigned int pin_mode);
> (((gpionum << IOMUX_GPIONUM_SHIFT) & IOMUX_GPIONUM_MASK) | \
> (padnum & IOMUX_PADNUM_MASK))
>
> -#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
> +#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
>
> #define IOMUX_TO_GPIO(iomux_pin) \
> ((iomux_pin & IOMUX_GPIONUM_MASK) >> IOMUX_GPIONUM_SHIFT)
My preference is to fix IOMUX_MODE instead of adding the parents to
MX31_PIN_DTR_DTE1__DTR_DTE2.
Joe, do you create a proper patch?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 8+ messages in thread
* iomux-mx3.h: possible macro precedence issue
2016-09-21 6:06 ` Uwe Kleine-König
@ 2016-09-21 6:12 ` Joe Perches
-1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2016-09-21 6:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2016-09-21 at 08:06 +0200, Uwe Kleine-K?nig wrote:\
> On Tue, Sep 20, 2016 at 01:02:24PM -0700, Joe Perches wrote:
> > Julia Lawall wrote a script
> >
> > Link:?http://lkml.kernel.org/r/alpine.DEB.2.10.1609201503260.2914 at hadrien
> >
> > that found a possible issue with macro argument precedence.
> >
> >
> > diff -u -p a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> > --- a/arch/arm/mach-imx/iomux-mx3.h
> > +++ b/arch/arm/mach-imx/iomux-mx3.h
> > @@ -529,7 +529,7 @@ enum iomux_pins {
> > ?#define MX31_PIN_DCD_DTE1__DCD_DTE2 IOMUX_MODE(MX31_PIN_DCD_DTE1, IOMUX_CONFIG_ALT1)
> > ?#define MX31_PIN_RI_DTE1__RI_DTE2 IOMUX_MODE(MX31_PIN_RI_DTE1, IOMUX_CONFIG_ALT1)
> > ?#define MX31_PIN_DSR_DTE1__DSR_DTE2 IOMUX_MODE(MX31_PIN_DSR_DTE1, IOMUX_CONFIG_ALT1)
> > -#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
> > +#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
> > ?#define MX31_PIN_PC_RST__CTS5 IOMUX_MODE(MX31_PIN_PC_RST, IOMUX_CONFIG_ALT2)
> > ?#define MX31_PIN_PC_VS2__RTS5 IOMUX_MODE(MX31_PIN_PC_VS2, IOMUX_CONFIG_ALT2)
> > ?#define MX31_PIN_PC_BVD2__TXD5 IOMUX_MODE(MX31_PIN_PC_BVD2, IOMUX_CONFIG_ALT2)
>
>
> I assume this is the only problematic definition?
yes..
> If so, there is a
> single affected platform: arch/arm/mach-imx/mach-kzm_arm11_01.c
> ?
> > this may be intentional, but perhaps a solution
> > would be to use parentheses in the #define IOMUX_MODE?
> >
> > ---
> >
> > diff --git a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> > index 368667b..3f9ede2 100644
> > --- a/arch/arm/mach-imx/iomux-mx3.h
> > +++ b/arch/arm/mach-imx/iomux-mx3.h
> > @@ -156,7 +156,7 @@ void mxc_iomux_mode(unsigned int pin_mode);
> > > > ? (((gpionum << IOMUX_GPIONUM_SHIFT) & IOMUX_GPIONUM_MASK) | \
> > > > ? (padnum & IOMUX_PADNUM_MASK))
> > ?
> > -#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
> > +#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
> > ?
> > ?#define IOMUX_TO_GPIO(iomux_pin) \
> > ? ((iomux_pin & IOMUX_GPIONUM_MASK) >> IOMUX_GPIONUM_SHIFT)
>
>
> My preference is to fix IOMUX_MODE instead of adding the parents to
> MX31_PIN_DTR_DTE1__DTR_DTE2.
Mine too.
> Joe, do you create a proper patch?
I just don't have the hardware and don't know what was intended
by the existing code. It does appear to be a defect though.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: iomux-mx3.h: possible macro precedence issue
@ 2016-09-21 6:12 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2016-09-21 6:12 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sascha Hauer, Julia Lawall, LKML, linux-arm-kernel, Shawn Guo
On Wed, 2016-09-21 at 08:06 +0200, Uwe Kleine-König wrote:\
> On Tue, Sep 20, 2016 at 01:02:24PM -0700, Joe Perches wrote:
> > Julia Lawall wrote a script
> >
> > Link: http://lkml.kernel.org/r/alpine.DEB.2.10.1609201503260.2914@hadrien
> >
> > that found a possible issue with macro argument precedence.
> >
> >
> > diff -u -p a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> > --- a/arch/arm/mach-imx/iomux-mx3.h
> > +++ b/arch/arm/mach-imx/iomux-mx3.h
> > @@ -529,7 +529,7 @@ enum iomux_pins {
> > #define MX31_PIN_DCD_DTE1__DCD_DTE2 IOMUX_MODE(MX31_PIN_DCD_DTE1, IOMUX_CONFIG_ALT1)
> > #define MX31_PIN_RI_DTE1__RI_DTE2 IOMUX_MODE(MX31_PIN_RI_DTE1, IOMUX_CONFIG_ALT1)
> > #define MX31_PIN_DSR_DTE1__DSR_DTE2 IOMUX_MODE(MX31_PIN_DSR_DTE1, IOMUX_CONFIG_ALT1)
> > -#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
> > +#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
> > #define MX31_PIN_PC_RST__CTS5 IOMUX_MODE(MX31_PIN_PC_RST, IOMUX_CONFIG_ALT2)
> > #define MX31_PIN_PC_VS2__RTS5 IOMUX_MODE(MX31_PIN_PC_VS2, IOMUX_CONFIG_ALT2)
> > #define MX31_PIN_PC_BVD2__TXD5 IOMUX_MODE(MX31_PIN_PC_BVD2, IOMUX_CONFIG_ALT2)
>
>
> I assume this is the only problematic definition?
yes..
> If so, there is a
> single affected platform: arch/arm/mach-imx/mach-kzm_arm11_01.c
>
> > this may be intentional, but perhaps a solution
> > would be to use parentheses in the #define IOMUX_MODE
> >
> > ---
> >
> > diff --git a/arch/arm/mach-imx/iomux-mx3.h b/arch/arm/mach-imx/iomux-mx3.h
> > index 368667b..3f9ede2 100644
> > --- a/arch/arm/mach-imx/iomux-mx3.h
> > +++ b/arch/arm/mach-imx/iomux-mx3.h
> > @@ -156,7 +156,7 @@ void mxc_iomux_mode(unsigned int pin_mode);
> > > > (((gpionum << IOMUX_GPIONUM_SHIFT) & IOMUX_GPIONUM_MASK) | \
> > > > (padnum & IOMUX_PADNUM_MASK))
> >
> > -#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
> > +#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
> >
> > #define IOMUX_TO_GPIO(iomux_pin) \
> > ((iomux_pin & IOMUX_GPIONUM_MASK) >> IOMUX_GPIONUM_SHIFT)
>
>
> My preference is to fix IOMUX_MODE instead of adding the parents to
> MX31_PIN_DTR_DTE1__DTR_DTE2.
Mine too.
> Joe, do you create a proper patch?
I just don't have the hardware and don't know what was intended
by the existing code. It does appear to be a defect though.
^ permalink raw reply [flat|nested] 8+ messages in thread
* iomux-mx3.h: possible macro precedence issue
2016-09-21 6:12 ` Joe Perches
@ 2016-09-21 6:39 ` Uwe Kleine-König
-1 siblings, 0 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2016-09-21 6:39 UTC (permalink / raw)
To: linux-arm-kernel
Hello Joe,
On Tue, Sep 20, 2016 at 11:12:59PM -0700, Joe Perches wrote:
> > > -#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
> > > +#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
> > > [...]
> > > -#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
> > > +#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
> >
> > Joe, do you create a proper patch?
>
> I just don't have the hardware and don't know what was intended
> by the existing code. It does appear to be a defect though.
I don't have that hardware either, but fixing this with just checking
the hardware manual is good enough.
The current definition makes MX31_PIN_DTR_DTE1__DTR_DTE2:
MX31_PIN_DTR_DTE1__DTR_DTE2
= IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
= (MX31_PIN_DTR_DTE1 | IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE << IOMUX_MODE_SHIFT)
= (IOMUX_PIN(44, 109) | (4 << 4) | 0 << 17)
= (0x586d | 0x40)
= 0x586d
while the expected value is
(MX31_PIN_DTR_DTE1 | (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE) << IOMUX_MODE_SHIFT)
= (IOMUX_PIN(44, 109) | ((4 << 4) | 0) << 17)
= (0x586d | 0x800000)
= 0x80586d
The effect is that the input is routed nowhere (which is the case also
with the issue fixed) and output is controlled by the GPIO module
instead of the UART and so the pin doesn't operate as handshaking
signal. (Assuming I interpreted everything correctly which I'm not sure
given that I didn't touch i.MX31 for long and it's different that the
other i.MX SoCs.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: iomux-mx3.h: possible macro precedence issue
@ 2016-09-21 6:39 ` Uwe Kleine-König
0 siblings, 0 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2016-09-21 6:39 UTC (permalink / raw)
To: Joe Perches; +Cc: Sascha Hauer, Julia Lawall, LKML, linux-arm-kernel, Shawn Guo
Hello Joe,
On Tue, Sep 20, 2016 at 11:12:59PM -0700, Joe Perches wrote:
> > > -#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
> > > +#define MX31_PIN_DTR_DTE1__DTR_DTE2 IOMUX_MODE(MX31_PIN_DTR_DTE1, (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE))
> > > [...]
> > > -#define IOMUX_MODE(pin, mode) (pin | mode << IOMUX_MODE_SHIFT)
> > > +#define IOMUX_MODE(pin, mode) ((pin) | ((mode) << IOMUX_MODE_SHIFT))
> >
> > Joe, do you create a proper patch?
>
> I just don't have the hardware and don't know what was intended
> by the existing code. It does appear to be a defect though.
I don't have that hardware either, but fixing this with just checking
the hardware manual is good enough.
The current definition makes MX31_PIN_DTR_DTE1__DTR_DTE2:
MX31_PIN_DTR_DTE1__DTR_DTE2
= IOMUX_MODE(MX31_PIN_DTR_DTE1, IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE)
= (MX31_PIN_DTR_DTE1 | IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE << IOMUX_MODE_SHIFT)
= (IOMUX_PIN(44, 109) | (4 << 4) | 0 << 17)
= (0x586d | 0x40)
= 0x586d
while the expected value is
(MX31_PIN_DTR_DTE1 | (IOMUX_OCONFIG_ALT3 | IOMUX_ICONFIG_NONE) << IOMUX_MODE_SHIFT)
= (IOMUX_PIN(44, 109) | ((4 << 4) | 0) << 17)
= (0x586d | 0x800000)
= 0x80586d
The effect is that the input is routed nowhere (which is the case also
with the issue fixed) and output is controlled by the GPIO module
instead of the UART and so the pin doesn't operate as handshaking
signal. (Assuming I interpreted everything correctly which I'm not sure
given that I didn't touch i.MX31 for long and it's different that the
other i.MX SoCs.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-21 6:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-20 20:02 iomux-mx3.h: possible macro precedence issue Joe Perches
2016-09-20 20:02 ` Joe Perches
2016-09-21 6:06 ` Uwe Kleine-König
2016-09-21 6:06 ` Uwe Kleine-König
2016-09-21 6:12 ` Joe Perches
2016-09-21 6:12 ` Joe Perches
2016-09-21 6:39 ` Uwe Kleine-König
2016-09-21 6:39 ` Uwe Kleine-König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.