RE: [PATCH] ARM: dts: imx6sx: Add missing uart mux function

[Anson Huang]

Hi, Uwe

> Subject: Re: [PATCH] ARM: dts: imx6sx: Add missing uart mux function
> 
> Hello Anson,
> 
> On Thu, Feb 13, 2020 at 09:18:10AM +0000, Anson Huang wrote:
> > > On Thu, Feb 13, 2020 at 02:43:09PM +0800, Anson Huang wrote:
> > > > From: Anson Huang <b20788@xxxxxxxxxxxxx>
> > > >
> > > > Update i.MX6SX pinfunc header to add uart mux function.
> > >
> > > I'm aware you add the macros in a consistent way to the already
> > > existing stuff. Still I think there is something to improve here. We
> > > now have definitions like:
> > >
> > > 	MX6SX_PAD_GPIO1_IO06__UART1_RTS_B
> > > 	MX6SX_PAD_GPIO1_IO06__UART1_CTS_B
> > >
> > > 	MX6SX_PAD_GPIO1_IO07__UART1_CTS_B
> > > 	MX6SX_PAD_GPIO1_IO07__UART1_RTS_B
> > >
> > > where (ignoring other pins that could be used) only the following
> > > combinations are valid:
> > >
> > > 	MX6SX_PAD_GPIO1_IO04__UART1_TX
> > > 	MX6SX_PAD_GPIO1_IO05__UART1_RX
> > > 	MX6SX_PAD_GPIO1_IO06__UART1_RTS_B
> > > 	MX6SX_PAD_GPIO1_IO07__UART1_CTS_B
> > >
> > > (in DCE mode) and
> > >
> > > 	MX6SX_PAD_GPIO1_IO04__UART1_RX
> > > 	MX6SX_PAD_GPIO1_IO05__UART1_TX
> > > 	MX6SX_PAD_GPIO1_IO06__UART1_CTS_B
> > > 	MX6SX_PAD_GPIO1_IO07__UART1_RTS_B
> > >
> > > (in DTE mode).
> >
> > Is it possible the using below combination, if possible, then I think
> > the prefix "DTE/DCE" are NOT impacting real functions, they are just
> different names for better identification:
> >
> > MX6SX_PAD_GPIO1_IO04__UART1_TX
> > MX6SX_PAD_GPIO1_IO05__UART1_RX
> > MX6SX_PAD_GPIO1_IO06__UART1_CTS_B
> > MX6SX_PAD_GPIO1_IO07__UART1_RTS_B
> 
> This is wrong according to my experience. If you look at the diagram in the
> i.MX6SX RM in the External Signals chapter (page 4111 in the IMX6SXRM Rev.
> 2, 9/2017) you can only either use RX/TX and RTS/CTS for their original
> purpose, or swap both pairs together.
> 
> > > For i.MX6SLL, i.MX6UL, imx6ULL and i.MX7 the naming convention is
> > > saner, a typical definition there is:
> > >
> > > 	MX7D_PAD_LPSR_GPIO1_IO04__UART5_DTE_RTS
> > >
> > > where the name includes DTE and where is it (more) obvious that this
> > > cannot be combined with
> > >
> > > 	MX7D_PAD_LPSR_GPIO1_IO07__UART5_DCE_TX
> > >
> > > .
> > >
> > > I suggest to adapt the latter naming convention also for the other
> > > i.MX pinfunc headers, probably with introducing defines for not
> > > breaking existing dts files.
> >
> > If to improve the name, just change the existing dts files which use
> > them should be OK, as this header file ONLY used by DT and should be
> > no compatible issues. So should I change the dts files together?
> 
> My approach would be one patch for each of:
> 
>  - rename existing imx6sx symbols to contain DTE or DCE
>    (introducing defines that map the old name to the new)

Is the introducing defines that map to old name to the new mainly for
NOT breaking bisect? As pinfunc.h is changed in a separate patch other than dts files. 

> 
>  - introduce the new defines you added in your patch under discussion
>    here (with the new naming scheme obviously)

Make sense. Current head file ONLY has DCE/DTE for TX/RX but miss the RTS/CTS.

> 
>  - switch all in-tree consumers to the new names
>    (maybe offering to split per machine)
> 
> I would also drop the _B suffix in the first patch which serves no useful
> purpose.

Make sense.

Thanks,
Anson