On Tue, 28 Jul 2015 08:23:13 -0400
Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
> Hi David,
>
> On 07/28/2015 02:07 AM, David Jander wrote:
> > On Mon, 27 Jul 2015 15:15:59 -0300
> > Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx> wrote:
> >
> >> This reverts commit e95044ba4fee93f5ea8a1a24b2d921e148503833.
> >>
> >> Commit e95044ba4fee93 ("tty: serial: imx.c: Reset UART before activating
> >> interrupts") terribly messes up with the console on mx6 boards, so
> >> let's revert it.
> >
> > IMHO Spewing out a few corrupt characters during startup (which curiously
> > also appear sometimes due to other reasons), is a lot less of a problem
> > that a hanging system!
>
> Seems you already know the symptoms. Was the console corruption a known issue
> before submitting? I didn't see anything in the changelog or discussion at
> submission time?
Honestly I must admit that I have not cared enough about these symptoms until
now, although I remember regularly seeing a few garbled characters coming from
the UART during kernel boot, and I thought those might be related to early
console handover or something similar. I remember seeing this since long
before DAM support was added for i.MX6.
> > Please don't revert this patch without providing a fix for the crash!
>
> This clearly isn't the correct fix, and it makes no sense to build a proper
> fix on top of a broken one.
If I understand you correctly, you refer to resetting the UART at driver init,
while it is already transmitting due to something like early-console?
Is the UART driver really initialized twice for that?
Because otherwise I don't see how this could do any harm...
The driver clearly assumes the UART is coming straight out of reset, so if it
does not, it needs to be reset before the driver can safely do anything.
Please explain how this can in any way _not_ be the correct solution (or at
least part of it).
> The right way is to revert this and try again.
I am not convinced of that, please explain.
> This time I would look at moving the request_irq() into the startup() method
> like every other serial driver does.
This sounds indeed like a good solution to avoid the interrupt race, but IMHO
my patch is a better solution, because the driver still assumes the UART comes
straight out of reset. This is evident by the fact that the driver only ever
changes register settings that are not reset defaults. This driver is clearly
not meant to be used with an UART that has been used before the driver
initializes it for the first time. The only robust fix for that is to either
forcibly reset the UART at the beginning or to fix the driver in that it makes
no assumption about historic register settings, which will lead to basically
resetting it at startup anyway.
Best regards,
--
David Jander
Protonic Holland.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
