Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210414134539.42332-1-alcooperx@gmail.com> <CAHp75VfQetCJJZ_U70xEduVBQHEx_GkHeKBs2uzNtvcq9H0BqQ@mail.gmail.com>
In-Reply-To: <CAHp75VfQetCJJZ_U70xEduVBQHEx_GkHeKBs2uzNtvcq9H0BqQ@mail.gmail.com>
From:   Alan Cooper <alcooperx@gmail.com>
Date:   Wed, 21 Apr 2021 15:04:45 -0400
Message-ID: <CAOGqxeUiFE0348Y=yEoD7bnMaGnFkbvNy3WZ2oSZZzR4D-xz_w@mail.gmail.com>
Subject: Re: [-next] serial: 8250: Match legacy NS16550A UARTs
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Florian Fainelli <f.fainelli@gmail.com>,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Apr 15, 2021 at 6:44 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Apr 14, 2021 at 7:13 PM Al Cooper <alcooperx@gmail.com> wrote:
> >
> > From: Florian Fainelli <f.fainelli@gmail.com>
> >
> > Older 32-bit only Broadcom STB chips used a NS16550A compatible UART,
> > the 8250_bcm7271.c driver can drive those UARTs just fine provided that
> > we let it match the appropriate compatible string.
>
> This sounds not correct to me, ns16550a is a national semiconductor product.
>
> Why is it here and not in generic 8250_of?

Andy,

While double checking in preparation for this response I was surprised
to find that the problem that this was meant to solve no longer
exists. The original issue was a result of the two UART drivers
returning probe DEFERs when getting their clocks. This was happening
because we use SCMI based clocks and they were coming up late in the
initialization process. The problem no longer exists because the SCMI
clock driver is now being called by subsys_init instead of module_init
and the drivers don't DEFER on clocks.

Here's a more detailed description of the problem we were trying to solve.

Our SoC's have always had a NS16650A UART core and older SoC's would
have a compatible string of: 'compatible = ""ns16550a"' and use the
8250_of driver. Our newer SoC's have added enhancements to the base
core to add support for DMA and accurate high speed baud rates and use
this newer 8250_bcm7271 driver. The Device Tree node for our enhanced
UARTs has a compatible string of: 'compatible = "brcm,bcm7271-uart",
"ns16550a"''. With both drivers running and the link order setup so
that the 8250_bcm7217 driver is initialized before the 8250_of driver,
we should bind the 8250_bcm7271 driver to the enhanced UART, or for
upstream kernels that don't have the 8250_bcm7271 driver, we bind to
the 8250_of driver.

The problem is that when both the 8250_of and 8250_bcm7271 drivers
were running, occasionally the 8250_of driver would be bound to the
enhanced UART instead of the 8250_bcm7271 driver. This was happening
because we use SCMI based clocks which come up late in initialization
and cause probe DEFER's when the two drivers get their clocks.
Occasionally the SCMI clock would become ready between the
8250_bcm7271 probe and the 8250_of probe and the 8250_of driver would
be bound. To fix this we decided to config only our 8250_bcm7271
driver and added "ns16665a0" to the compatible string so the driver
would work on our older system.

I was surprised by a couple of things while working on this issue.
1. The compatible string left to right order has no effect on which
driver is bound for builtin drivers.
2. I thought that the new "device links" added by the OF framework for
clocks would fix this order issue but it did not because the link was
made to the parent of the SCMI clock protocol which comes up long
before the clock protocol.

Thanks
Al


>
> > +       {
> > +               .compatible = "ns16550a",
> > +               .data = brcmstb_rate_table_16550a,
> > +       },
>
>
> --
> With Best Regards,
> Andy Shevchenko