| Message ID | 20171006101344.15590-17-miquel.raynal@free-electrons.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support armada-37xx second UART port | expand |
Hi Miquel, On ven., oct. 06 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Fill ESPRESSObin uart0 node with pinctrl information like in the > Armada-3720-DB device tree (which uses the same node). > > Also explain how to enable the second UART port available on the > headers. This second port is not enabled by default because both > headers are dedicated to expose general purpose pins and remapping > some of them to use the second UART would break existing users. > > Suggested-by: László ÁSHIN <laszlo@ashin.hu> > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > --- > arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > index 2ce52ba74f73..c05b274ab1a9 100644 > --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > @@ -98,9 +98,17 @@ > > /* Exported on the micro USB connector J5 through an FTDI */ > &uart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart1_pins>; > status = "okay"; > }; > > +/* > + * Enabling the second UART on J17 (pins 24,26) is just a matter of copying the > + * uart1 node from armada-3720-db.dts with one difference: it works with 1.8V > + * TTL levels. This difference is not related to the device tree. So I would write: /* * To enable the second UART on J17 (pins 24,26) refer to the uart1 * node from armada-3720-db.dts. * Note that TX and RX signal are the ones coming directly from the SoC: * 1.8V TTL. */ Thanks, Gregory > + */ > + > /* J7 */ > &usb3 { > status = "okay"; > -- > 2.11.0 >
Hello, On Fri, 06 Oct 2017 15:01:18 +0200, Gregory CLEMENT wrote: > /* > * To enable the second UART on J17 (pins 24,26) refer to the uart1 > * node from armada-3720-db.dts. > * Note that TX and RX signal are the ones coming directly from the SoC: > * 1.8V TTL. > */ One issue with this comment (and Miquèl's version as well) is that it does not explain why you don't enable this UART by default. The real reason is in the commit log from Miquèl, and should probably be part of the comment. Perhaps something like: /* * Connector J17 (pins X, Y, Z) exposes a number of different * features: * - UART1 (pins 24 = RX, pins 26 = TX), see armada-3720-db.dts for an * example on how to enable UART1. Beware that the signals are 1.8V * TTL. * - SPIxyz * - I2Cxyz */ Otherwise, it's not clear at all why you don't just enable UART1. Or perhaps I misunderstood Miquèl's commit log ? Best regards, Thomas
Hi Thomas, Gregory, On Fri, 6 Oct 2017 15:15:21 +0200 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Fri, 06 Oct 2017 15:01:18 +0200, Gregory CLEMENT wrote: > > > /* > > * To enable the second UART on J17 (pins 24,26) refer to the uart1 > > * node from armada-3720-db.dts. > > * Note that TX and RX signal are the ones coming directly from > > the SoC: > > * 1.8V TTL. > > */ > > One issue with this comment (and Miquèl's version as well) is that it > does not explain why you don't enable this UART by default. > > The real reason is in the commit log from Miquèl, and should probably > be part of the comment. Perhaps something like: > > /* > > * Connector J17 (pins X, Y, Z) exposes a number of different > * features: > * - UART1 (pins 24 = RX, pins 26 = TX), see armada-3720-db.dts for > an > * example on how to enable UART1. Beware that the signals are 1.8V > * TTL. > * - SPIxyz > * - I2Cxyz > */ Thanks for both your comments, there is my version, inspired from both comments: /* * Connector J17 exposes a number of different features. Some pins are * multiplexed. This is the case for the UART1 feature (pins 24 = RX, * pins 26 = TX). See armada-3720-db.dts for an example of how to enable it. * Beware that the signals are 1.8V TTL. */ Thanks, Miquèl > > Otherwise, it's not clear at all why you don't just enable UART1. Or > perhaps I misunderstood Miquèl's commit log ? > > Best regards, > > Thomas
Hi Miquel, On lun., oct. 09 2017, Miquel RAYNAL <miquel.raynal@free-electrons.com> wrote: > Hi Thomas, Gregory, > > On Fri, 6 Oct 2017 15:15:21 +0200 > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > >> Hello, >> >> On Fri, 06 Oct 2017 15:01:18 +0200, Gregory CLEMENT wrote: >> >> > /* >> > * To enable the second UART on J17 (pins 24,26) refer to the uart1 >> > * node from armada-3720-db.dts. >> > * Note that TX and RX signal are the ones coming directly from >> > the SoC: >> > * 1.8V TTL. >> > */ >> >> One issue with this comment (and Miquèl's version as well) is that it >> does not explain why you don't enable this UART by default. >> >> The real reason is in the commit log from Miquèl, and should probably >> be part of the comment. Perhaps something like: >> >> /* >> >> * Connector J17 (pins X, Y, Z) exposes a number of different >> * features: >> * - UART1 (pins 24 = RX, pins 26 = TX), see armada-3720-db.dts for >> an >> * example on how to enable UART1. Beware that the signals are 1.8V >> * TTL. >> * - SPIxyz >> * - I2Cxyz >> */ > > Thanks for both your comments, there is my version, inspired from both > comments: > > /* > * Connector J17 exposes a number of different features. Some pins are > * multiplexed. This is the case for the UART1 feature (pins 24 = RX, > * pins 26 = TX). See armada-3720-db.dts for an example of how to enable it. > * Beware that the signals are 1.8V TTL. > */ Seems good for me however I prefer Thomas version, easier to read and to extend latter with the description of other pins if needed. Gregory > > Thanks, > Miquèl > >> >> Otherwise, it's not clear at all why you don't just enable UART1. Or >> perhaps I misunderstood Miquèl's commit log ? >> >> Best regards, >> >> Thomas > > > > -- > Miquel Raynal, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
Hello Gregory, On Thu, 12 Oct 2017 13:24:42 +0200 Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Miquel, > > On lun., oct. 09 2017, Miquel RAYNAL > <miquel.raynal@free-electrons.com> wrote: > > > Hi Thomas, Gregory, > > > > On Fri, 6 Oct 2017 15:15:21 +0200 > > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > > > >> Hello, > >> > >> On Fri, 06 Oct 2017 15:01:18 +0200, Gregory CLEMENT wrote: > >> > >> > /* > >> > * To enable the second UART on J17 (pins 24,26) refer to the > >> > uart1 > >> > * node from armada-3720-db.dts. > >> > * Note that TX and RX signal are the ones coming directly from > >> > the SoC: > >> > * 1.8V TTL. > >> > */ > >> > >> One issue with this comment (and Miquèl's version as well) is that > >> it does not explain why you don't enable this UART by default. > >> > >> The real reason is in the commit log from Miquèl, and should > >> probably be part of the comment. Perhaps something like: > >> > >> /* > >> > >> * Connector J17 (pins X, Y, Z) exposes a number of different > >> * features: > >> * - UART1 (pins 24 = RX, pins 26 = TX), see armada-3720-db.dts > >> for an > >> * example on how to enable UART1. Beware that the signals are > >> 1.8V > >> * TTL. > >> * - SPIxyz > >> * - I2Cxyz > >> */ > > > > Thanks for both your comments, there is my version, inspired from > > both comments: > > > > /* > > * Connector J17 exposes a number of different features. Some pins > > are > > * multiplexed. This is the case for the UART1 feature (pins 24 = > > RX, > > * pins 26 = TX). See armada-3720-db.dts for an example of how to > > enable it. > > * Beware that the signals are 1.8V TTL. > > */ > > Seems good for me however I prefer Thomas version, easier to read and > to extend latter with the description of other pins if needed. Ok, I reused the 'dash' presentation to be later extended. See v2 coming soon. Thanks, Miquèl > > Gregory > > > > > Thanks, > > Miquèl > > > >> > >> Otherwise, it's not clear at all why you don't just enable UART1. > >> Or perhaps I misunderstood Miquèl's commit log ? > >> > >> Best regards, > >> > >> Thomas > > > > > > > > -- > > Miquel Raynal, Free Electrons > > Embedded Linux and Kernel engineering > > http://free-electrons.com >
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts index 2ce52ba74f73..c05b274ab1a9 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts @@ -98,9 +98,17 @@ /* Exported on the micro USB connector J5 through an FTDI */ &uart0 { + pinctrl-names = "default"; + pinctrl-0 = <&uart1_pins>; status = "okay"; }; +/* + * Enabling the second UART on J17 (pins 24,26) is just a matter of copying the + * uart1 node from armada-3720-db.dts with one difference: it works with 1.8V + * TTL levels. + */ + /* J7 */ &usb3 { status = "okay";
Fill ESPRESSObin uart0 node with pinctrl information like in the Armada-3720-DB device tree (which uses the same node). Also explain how to enable the second UART port available on the headers. This second port is not enabled by default because both headers are dedicated to expose general purpose pins and remapping some of them to use the second UART would break existing users. Suggested-by: László ÁSHIN <laszlo@ashin.hu> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> --- arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 8 ++++++++ 1 file changed, 8 insertions(+)