[PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

[Andy Shevchenko]

On Thu, Apr 2, 2020 at 7:55 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> On Thu, Apr 2, 2020 at 1:55 AM Simon Glass <sjg at chromium.org> wrote:
> > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko <andy.shevchenko at gmail.com> wrote:
> > > On Wed, Apr 01, 2020 at 10:56:26AM -0600, Simon Glass wrote:
> > > > On Wed, 1 Apr 2020 at 08:45, Andy Shevchenko <andy.shevchenko at gmail.com> wrote:
> > > > > On Wed, Apr 1, 2020 at 5:32 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > On Wed, Apr 1, 2020 at 9:58 PM Andy Shevchenko
> > > > > > <andriy.shevchenko at linux.intel.com> wrote:
> > > > > > >
> > > > > > > The commit breaks serial console on the Intel Edison.
> > > > > > >
> > > > > > > This reverts commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> > > > > > >
> > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> > > > > > > ---
> > > > > > >  drivers/serial/ns16550.c | 40 ++++++++++++----------------------------
> > > > > > >  1 file changed, 12 insertions(+), 28 deletions(-)
> > > > > > >
> > > > > >
> > > > > > Could you please spend some time to investigate why this breaks Intel Edison?
> > > > > >
> > > > > > Reverting this patch would mean we break other boards too as
> > > > > > Wolfgang's patch wanted to fix the breakage in the first place. Much
> > > > > > appreciated!
> > > > >
> > > > > I guess I'm wrong person here. The DM code is a complete black box to me.
> > > > > Nevertheless, I may test any provided fix / debug / etc patch by request.
> > > > >
> > > > > And I think it's fair to investigate by the one who made a regression
> > > > > in the first place.
> > > >
> > > > Given that we have conflicting breakages, we need to debug Edison.
> > >
> > > I would glad to test any suggested change or debug patch!
> > >
> > > > Does it enable the debug UART?
> > >
> > > If I am not mistaken, it does not.
> >
> > Looks like you are right. If you know the address you could do that -
> > see minnowmax for an example.
>
> Please suggest what's the best approach to proceed.

I think I understand what happened, and Wolfgang's patch *is* a culprit.

In serial_intel_mid.c we setup a divisor before probing the actual
device. Now, since the address retrieving has been moved further in
the initialization, we are writing to unknown registers and thus don't
properly initialize hardware.

So, the proper fix would be in my opinion to introduce a call back or
some other way to make ordering possible, like registering hw_init
callback in probe which will be called in the ns16550, or even do this
as a callback for all serial class drivers.

I don't dare to dive into this anticipating that crap will hit the fan.
So, please, who is knowing serial code, fix this asap!

-- 
With Best Regards,
Andy Shevchenko