Re: [PATCH 16/16] arm64: dts: marvell: armada-3720-espressobin: fill UART nodes

[Gregory CLEMENT <gregory.clement@xxxxxxxxxxxxxxxxxx>]

Hi Miquel,
 
 On lun., oct. 09 2017, Miquel RAYNAL <miquel.raynal@xxxxxxxxxxxxxxxxxx> wrote:

> Hi Thomas, Gregory,
>
> On Fri, 6 Oct 2017 15:15:21 +0200
> Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> 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

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html