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) - introduce the new defines you added in your patch under discussion here (with the new naming scheme obviously) - 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. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
