Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

From: Katsuhiro Suzuki <katsuhiro@katsuster.net>
To: Tony McKahan <tonymckahan@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org,
	Klaus Goger <klaus.goger@theobroma-systems.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Brian Norris <briannorris@chromium.org>,
	linux-rockchip@lists.infradead.org,
	Shawn Lin <shawn.lin@rock-chips.com>,
	Akash Gajjar <Akash_Gajjar@mentor.com>,
	Vicente Bergas <vicencb@gmail.com>, Levin Du <djw@t-chip.com.cn>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	linux-arm-kernel@lists.infradead.org,
	Shohei Maruyama <cheat.sc.linux@outlook.com>,
	Matthias Brugger <mbrugger@suse.com>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Douglas Anderson <dianders@chromium.org>,
	Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>,
	Oskari Lemmela <oskari@lemmela.net>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Christoph Muellner <christoph.muellner@theobroma-systems.com>
Subject: Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2
Date: Mon, 4 Mar 2019 22:56:46 +0900	[thread overview]
Message-ID: <19be5c97-cdfa-d3e5-efcb-f2c4e7351d9f@katsuster.net> (raw)
In-Reply-To: <CAERgok=cxtLqRmYXdrCaUF8mK2pPfGAPc6w3hdLaPgPmqKmkcw@mail.gmail.com>

No problem! Thank you for discussion :)

On 2019/03/04 4:12, Tony McKahan wrote:
> On Sun, Mar 3, 2019 at 1:45 PM Tony McKahan <tonymckahan@gmail.com> wrote:
>>
>> Hello Katsushiro,
> 
> And apologies for the extra "s", typing too quickly I'm afraid.
>>
>> On Sun, Mar 3, 2019 at 12:31 PM Katsuhiro Suzuki
>> <katsuhiro@katsuster.net> wrote:
>>>
>>> Hello Tony,
>>>
>>> On 2019/03/04 0:13, Tony McKahan wrote:
>>>> On Sun, Mar 3, 2019 at 9:04 AM Katsuhiro Suzuki <katsuhiro@katsuster.net> wrote:
>>>>>
>>>>> Hello Heiko,
>>>>>
>>>>> Thank you for comments.
>>>>>
>>>>> On 2019/03/03 22:19, Heiko Stuebner wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:
>>>>>>> This patch increases drive strength of UART2 from 3mA to 12mA for
>>>>>>> getting more faster rising edge.
>>>>>>>
>>>>>>> RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
>>>>>>> this setting, a bit width of UART is about 667ns.
>>>>>>>
>>>>>>> In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
>>>>>>> converter), falling time of RockPro64 UART2 is 40ns, but riging time
>>>>>>> is over 650ns. So UART receiver will get wrong data, because receiver
>>>>>>> read intermediate data of rising edge.
>>>>>>>
>>>>>>> Rising time becomes 300ns from 650ns if apply this patch. This is not
>>>>>>> perfect solution but better than now.
>>>>>>>
>>>>>>> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
>>>>>>> ---
>>>>>>>     arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++++++--
>>>>>>>     1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> your changing a core rk3399 property here, so I'd really like to get
>>>>>> input from other board stakeholders on this before applying a core
>>>>>> change.
>>>>>>
>>>>>> Could you either include the submitters of other rk3399-boards in the
>>>>>> recipient list so that they're aware or limit the change to rockpro64 for
>>>>>> the time being (aka overriding the property in the board-dts) please?
>>>>>>
>>>>>
>>>>> OK, I'm adding other boards members.
>>>>> by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts
>>>>>
>>>>>
>>>>> RockPro64 directly connect UART2 pins of RK3399 to external connector.
>>>>> I think maybe other RK3399 boards are facing same problem, but I cannot
>>>>> check it because I have RockPro64 only...
>>>>>
>>>>> I'm happy if someone tell me other boards situation.
>>>>
>>>> I'm pulling out other rockchip boards momentarily to see what kind of
>>>> population we have.
>>>>
>>>> Note these are not all running 5.x kernels, however none of them have
>>>> the UART2 drive levels modified to my knowledge, and regardless, none
>>>> show over 100 ns.
>>>>
>>>> board:    rise/fall
>>>>
>>>> rk3399-roc-pc: 90ns/90ns
>>>> rk3399-rockpro64 V2.0:  90ns/45ns
>>>> rk3399-rockpro64 V2.1:  40ns/41ns
>>>>
>>>> Please make sure there's not a large amount of flux or something
>>>> around the terminals on your board, that seems excessively high.
>>>>
>>>
>>> Thank you for valuable information. For more deeply discussion,
>>> I tried other conditions and watch the rise/fall times.
>>>
>>> 1) Not connect
>>> The rise/fall times are 40ns/5ns when nothing connect (impedance is
>>> very high) to external pin of RockPro64.
>>>
>>> What UART device are you using with RockPro64? If you use some device
>>> with RockPro64 and board shows rise/fall times = 90ns/45ns, my device
>>> is not suitable for RockPro64 by some reason. So it's better to drop
>>> my patch.
>>
>> The adapter is an FTDI FT232RL breakout board, attached with some
>> generic Dupont connector jumpers.
>> Interesting your RockPro is showing this symptom, perhaps there is a
>> cold solder joint somewhere introducing resistance?
>>
>>>
>>> 2) Other SoC
>>> I have other SoC board rk3328-rock64, Rock64 shows rise/fall times =
>>> 90ns/80ns when same UART-USB device is connected to UART pin.
>>
>> I measured similar on my Rock64 as well.
>>
>>>
>>> I think it shows rk3399's (or RockPro64's?) drive strength is a little
>>> weak. So it's better to increase the drive strength of UART of rk3399.
>>
>> I do not think this is a bad idea generally, it simply allows for more
>> available current from the interface.  I'll let others be the judge of
>> that, however.
>>
>>>
>>> Best Regards,
>>> Katsuhiro Suzuki
>>>
>>>>>
>>>>> Best Regards,
>>>>> Katsuhiro Suzuki
>>>>>
>>>>>
>>>>>> Thanks
>>>>>> Heiko
>>>>>>
>>>>>>
>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>>>>>> index beaa92744a64..e3c8f91ead50 100644
>>>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>>>>>> @@ -2000,6 +2000,11 @@
>>>>>>>                        drive-strength = <8>;
>>>>>>>                };
>>>>>>>
>>>>>>> +            pcfg_pull_up_12ma: pcfg-pull-up-12ma {
>>>>>>> +                    bias-pull-up;
>>>>>>> +                    drive-strength = <12>;
>>>>>>> +            };
>>>>>>> +
>>>>>>>                pcfg_pull_up_18ma: pcfg-pull-up-18ma {
>>>>>>>                        bias-pull-up;
>>>>>>>                        drive-strength = <18>;
>>>>>>> @@ -2521,8 +2526,8 @@
>>>>>>>                uart2c {
>>>>>>>                        uart2c_xfer: uart2c-xfer {
>>>>>>>                                rockchip,pins =
>>>>>>> -                                    <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up>,
>>>>>>> -                                    <4 RK_PC4 RK_FUNC_1 &pcfg_pull_none>;
>>>>>>> +                                    <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up_12ma>,
>>>>>>> +                                    <4 RK_PC4 RK_FUNC_1 &pcfg_pull_none_12ma>;
>>>>>>>                        };
>>>>>>>                };
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-rockchip mailing list
>>>>> Linux-rockchip@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>>
>>>>
>>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> 