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

[Marek Vasut]

On 12/21/2018 10:08 PM, Paul Burton wrote:
> Hi Marek,
> 
> On Fri, Dec 21, 2018 at 09:08:28PM +0100, Marek Vasut wrote:
>> On 12/16/2018 11:28 PM, Paul Burton wrote:
>>> On Sun, Dec 16, 2018 at 11:01:18PM +0100, Marek Vasut wrote:
>>>>>>> I did suggest an alternative approach which would rename
>>>>>>> serial8250_clear_fifos() and split it into 2 variants - one that
>>>>>>> disables FIFOs & one that does not, then use the latter in
>>>>>>> __do_stop_tx_rs485():
>>>>>>>
>>>>>>> https://lore.kernel.org/lkml/20181213014805.77u5dzydo23cm6fq@pburton-laptop/
>>>>>>>
>>>>>>> However I have no access to the OMAP3 hardware that Marek's patch was
>>>>>>> attempting to fix & have heard nothing back with regards to him testing
>>>>>>> that approach, so here's a simple revert that fixes the Ingenic JZ4780.
>>>>>>>
>>>>>>> I've marked for stable back to v4.10 presuming that this is how far the
>>>>>>> broken patch may be backported, given that this is where commit
>>>>>>> 2bed8a8e7072 ("Clearing FIFOs in RS485 emulation mode causes subsequent
>>>>>>> transmits to break") that it tried to fix was introduced.
>>>>>>
>>>>>> OK, I tested this on AM335x / OMAP3 and the system is again broken, so
>>>>>> that's a NAK.
>>>>>
>>>>> To be clear - what did you test? This revert or the patch linked to
>>>>> above?
>>>>>
>>>>> This revert would of course reintroduce your RS485 issue because it just
>>>>> undoes your change.
>>>>
>>>> The revert. Which of the two patches do you need me to test.
>>>
>>> The one in the email I sent on Thursday 13th at 01:48:06 UTC, linked to
>>> at lore.kernel.org in the quote right above:
>>>
>>> https://lore.kernel.org/lkml/20181213014805.77u5dzydo23cm6fq@pburton-laptop/
>>>
>>> You replied with comments on the patch, you just never tested it or
>>> never told me if you did. The lack of response means I don't know
>>> whether that potential patch even still works for your system, hence the
>>> revert.
>>
>> I shared the entire testcase, which now fails on AM335x due to this
>> revert. Is there any progress on a proper fix from your side which does
>> not break the AM335x ?
> 
> No.
> 
> Let's be clear - I didn't break your AM335x system, your broken patch
> says that Daniel did with a commit applied back in v4.10. As such I
> don't consider it my responsibility to fix your problem at all, I don't
> have any access to the hardware anyway & I won't be buying hardware to
> fix a bug for you.
> 
> Despite all that I did write a patch which I expect would have solved
> the problem for both of us, which is linked *twice* in the quoted emails
> above & which as far as I can tell you *still* have not tested. I can
> only surmise that you're trying deliberately to be annoying at this
> point.
> 
> If you want to try the patch I already wrote, and confirm whether it
> actually works for you, then let's talk. Otherwise we're done here.

Understood. However I did test your patch, but it was generating
spurious IRQs and overruns (ttyS ttyS2: 91 input overrun(s)) on the
AM335x, so that's not a proper solution.

I even brought the CI20 V2 I have back to life (yes, I bought one). The
patch Ezequiel posted did fix the problem on the CI20, and did not break
the AM335x, so I'd prefer if it was revisited instead of a heavy-handed
revert.

And I'd prefer to keep this thread alive, since I am still convinced
that the FIFO handling code is wrong. Moreover, considering the UME bit
on JZ4780 in FCR, the original code should confuse the UART on the
JZ4780 too, although this might be hidden by some other surrounding code
specific to the 8250 core on the JZ4780.

I am also on mipslinux IRC channel, in case you want to discuss this.

-- 
Best regards,
Marek Vasut