From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel RAYNAL Subject: Re: [PATCH 16/16] arm64: dts: marvell: armada-3720-espressobin: fill UART nodes Date: Fri, 13 Oct 2017 09:01:19 +0200 Message-ID: <20171013090119.44e9a3c5@xps13> References: <20171006101344.15590-1-miquel.raynal@free-electrons.com> <20171006101344.15590-17-miquel.raynal@free-electrons.com> <87fuawe8gx.fsf@free-electrons.com> <20171006151521.440418a2@windsurf.lan> <20171009093025.78d45a9a@xps13> <87zi8wbocl.fsf@free-electrons.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <87zi8wbocl.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Gregory CLEMENT Cc: Thomas Petazzoni , Greg Kroah-Hartman , Linus Walleij , Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Jiri Slaby , Catalin Marinas , Will Deacon , linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Antoine Tenart , Nadav Haklai , Wilson Ding List-Id: linux-gpio@vger.kernel.org Hello Gregory, On Thu, 12 Oct 2017 13:24:42 +0200 Gregory CLEMENT wrote: > Hi Miquel, > > On lun., oct. 09 2017, Miquel RAYNAL > wrote: > > > Hi Thomas, Gregory, > > > > On Fri, 6 Oct 2017 15:15:21 +0200 > > Thomas Petazzoni 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 > -- Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: miquel.raynal@free-electrons.com (Miquel RAYNAL) Date: Fri, 13 Oct 2017 09:01:19 +0200 Subject: [PATCH 16/16] arm64: dts: marvell: armada-3720-espressobin: fill UART nodes In-Reply-To: <87zi8wbocl.fsf@free-electrons.com> References: <20171006101344.15590-1-miquel.raynal@free-electrons.com> <20171006101344.15590-17-miquel.raynal@free-electrons.com> <87fuawe8gx.fsf@free-electrons.com> <20171006151521.440418a2@windsurf.lan> <20171009093025.78d45a9a@xps13> <87zi8wbocl.fsf@free-electrons.com> Message-ID: <20171013090119.44e9a3c5@xps13> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hello Gregory, On Thu, 12 Oct 2017 13:24:42 +0200 Gregory CLEMENT wrote: > Hi Miquel, > > On lun., oct. 09 2017, Miquel RAYNAL > wrote: > > > Hi Thomas, Gregory, > > > > On Fri, 6 Oct 2017 15:15:21 +0200 > > Thomas Petazzoni 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 > -- Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com