Re: [PATCH] Revert "serial: 8250: Fix clearing FIFOs in RS485 mode again"

From: Marek Vasut <marex@denx.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Paul Burton <paul.burton@mips.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Paul Burton <pburton@wavecomp.com>,
	Daniel Jedrychowski <avistel@gmail.com>,
	"linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
	stable <stable@vger.kernel.org>,
	Ezequiel Garcia <ezequiel@collabora.com>
Subject: Re: [PATCH] Revert "serial: 8250: Fix clearing FIFOs in RS485 mode again"
Date: Mon, 17 Dec 2018 16:43:06 +0100	[thread overview]
Message-ID: <52a4b322-5710-1ac0-fa3e-11ed6ef2236d@denx.de> (raw)
In-Reply-To: <20181217151851.GA21564@kroah.com>

On 12/17/2018 04:18 PM, Greg Kroah-Hartman wrote:
> On Sun, Dec 16, 2018 at 10:35:12PM +0000, Paul Burton wrote:
>> Hi Ezequiel,
>>
>> On Sun, Dec 16, 2018 at 07:28:22PM -0300, Ezequiel Garcia wrote:
>>> On Sun, 16 Dec 2018 at 19:24, Paul Burton <paul.burton@mips.com> wrote:
>>>> This helps, but it only addresses one part of one of the 4 reasons I
>>>> listed as motivation for my revert. For example serial8250_do_shutdown()
>>>> also clearly intends to disable the FIFOs.
>>>>
>>>
>>> OK. So, let's fix that :-)
>>
>> I already did, or at least tried to, on Thursday [1].
>>
>>> By all means, it would be really nice to push forward and fix the garbage
>>> issue on JZ4780, as well as the transmission issue on AM335x.
>>>
>>> AM335x is a wildly popular platform, and it's not funny to break it.
>>
>> Well, clearly not if it was broken in v4.10 & only just fixed..? And
>> from Marek's commit message the patch in v4.10 doesn't break the whole
>> system just RS485.
>>
>>> So, let's please stop discussing which board we'll break and just fix both.
>>
>> I completely agree that would be ideal and I wrote a patch hoping to do
>> that on Thursday, but didn't get any response on testing. It's late in
>> the cycle hence a revert made sense. Simple as that.
> 
> A revert makes sense now, I'll go queue this up, thanks.

I don't like this for multiple reasons.
1) There is a better patch posted which doesn't break the AM335x and
   clearly identifies and fixes the problem on the JZ4780 / CI20
2) The JZ4780 8250 core is not a standard 8250 core, since it has extra
   bits in the FCR register (like the UME bit, which disables the whole
   UART block), so the revert IMO would break that core too, it just
   hides the breakage. I'm still trying to understand the implications
   of that in detail, but the discussion wasn't quite constructive.

I'd much rather see Ezequiel's patch applied, since that's far less
destructive approach to fixing the problem than the revert.

-- 
Best regards,
Marek Vasut