diff mbox

serial: imx: regression triggered by newly introduced DSR irq handling

Message ID 1471120532.1923.4.camel@googlemail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christoph Fritz Aug. 13, 2016, 8:35 p.m. UTC 
On Wed, 2016-08-10 at 17:54 -0300, Fabio Estevam wrote:
> On Fri, Aug 5, 2016 at 9:03 AM, Christoph Fritz
> <chf.fritz@googlemail.com> wrote:
> 
> > SION is necessary for pinconfig MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1
> > if bit 17 in IOMUXC_GPR_GPR1 is set and bit 13 not.
> >
> > So that ref_enetpll1 provides a clock not only for the external PHY but
> > also for the internal controller.
> >
> > Using SION is a quirk here, because the silicon doesn't feed the clock
> > back to the internal controller automatically.
> >
> > On the other hand, NXP could argue: You need to add a wire on your PCB
> > between ENET1_REF_CLK1 and ENET1_TX_CLK -- but referring to the
> > datasheet ENET1_TX_CLK isn't used in RMII config...
> >
> > So if Fabio or Shawn agrees with my assumption above, I'll add something
> > like 27e16501052e5341934d3 "serial: imx: implement DSR irq
> > handling for DTE mode".

To correct myself, I'm referring to commit da4fa6fa8016dba54ae "ARM:
imx25-pinfunc: document SION being important for
MX25_PAD_SD1_CMD__SD1_CMD".

> >
> > Okay?
> 
> Could you please post your suggestion as a patch or RFC so that we can
> understand what your proposal is?

Sure, here is my proposal:

---
 arch/arm/boot/dts/imx6sx-pinfunc.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

--

Comments

Uwe Kleine-König Aug. 15, 2016, 5:22 a.m. UTC | #1 
Hello Christoph,

On Sat, Aug 13, 2016 at 10:35:32PM +0200, Christoph Fritz wrote:
> On Wed, 2016-08-10 at 17:54 -0300, Fabio Estevam wrote:
> > Could you please post your suggestion as a patch or RFC so that we can
> > understand what your proposal is?
> 
> Sure, here is my proposal:

I like it, just a few minor nits below.

> +/*
> + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> + * PHY in RMII mode. This configuration is true if:

Sounds a bit strage to me. Maybe s/true/necessary/? Or "valid"?

> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
> + * It seems to be a silicon bug that in this configuration ENET1_TX reference
> + * clock isn't provided automatically.  According to i.mx6sx reference manual

s/i.mx6sx/i.MX6SX/

> + * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
> + * should be the case.
> + * This might have side effects for other hardware units that are connected to
> + * that pin and use the respective function as input (e.g. DSR irq handling).

s/DSR irq/UART1's DTR/. Then it's more obvious to relate to
MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B.

> + */
>  #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0

Thanks
Uwe
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6sx-pinfunc.h b/arch/arm/boot/dts/imx6sx-pinfunc.h
index bb9c6b7..67a3dd7 100644
--- a/arch/arm/boot/dts/imx6sx-pinfunc.h
+++ b/arch/arm/boot/dts/imx6sx-pinfunc.h
@@ -308,6 +308,19 @@ 
 #define MX6SX_PAD_ENET1_RX_CLK__VDEC_DEBUG_35                     0x008C 0x03D4 0x0000 0x8 0x0
 #define MX6SX_PAD_ENET1_RX_CLK__PCIE_CTRL_DEBUG_29                0x008C 0x03D4 0x0000 0x9 0x0
 #define MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK                      0x0090 0x03D8 0x0000 0x0 0x0
+/*
+ * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
+ * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
+ * PHY in RMII mode. This configuration is true if:
+ *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
+ *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
+ * It seems to be a silicon bug that in this configuration ENET1_TX reference
+ * clock isn't provided automatically.  According to i.mx6sx reference manual
+ * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
+ * should be the case.
+ * This might have side effects for other hardware units that are connected to
+ * that pin and use the respective function as input (e.g. DSR irq handling).
+ */
 #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
 #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
 #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0