MIME-Version: 1.0
References: <20211127223253.19098-1-semen.protsenko@linaro.org>
 <20211127223253.19098-7-semen.protsenko@linaro.org> <YaORtBO4b9AyFYyd@kroah.com>
In-Reply-To: <YaORtBO4b9AyFYyd@kroah.com>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Mon, 29 Nov 2021 01:54:18 +0200
Message-ID: <CAPLW+4=mL6ZV73zY+AoF6mo8Tka0cO8pst2mBYOoEHpX4nrRbA@mail.gmail.com>
Subject: Re: [PATCH 6/8] tty: serial: Make SERIAL_SAMSUNG=y impossible when EXYNOS_USI_V2=m
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Brown <broonie@kernel.org>,
        Jaewon Kim <jaewon02.kim@samsung.com>,
        Chanho Park <chanho61.park@samsung.com>,
        David Virag <virag.david003@gmail.com>,
        Youngmin Nam <youngmin.nam@samsung.com>,
        devicetree@vger.kernel.org, linux-spi@vger.kernel.org,
        linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
        linux-i2c@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, 28 Nov 2021 at 16:27, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Nov 28, 2021 at 12:32:51AM +0200, Sam Protsenko wrote:
> > When UART is encapsulated in USIv2 block (e.g. in Exynos850), USIv2
> > driver must be loaded first, as it's preparing USI hardware for
> > particular protocol use. Make it impossible for Samsung serial driver to
> > be built-in when USIv2 driver is built as a module, to prevent incorrect
> > booting order for those drivers.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> >  drivers/tty/serial/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 0e5ccb25bdb1..47bc24e74041 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE
> >  config SERIAL_SAMSUNG
> >       tristate "Samsung SoC serial support"
> >       depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || ARCH_APPLE || COMPILE_TEST
> > +     depends on EXYNOS_USI_V2 || !EXYNOS_USI_V2
>
> That's odd, and is not going to help if everything is built as a module
> and loaded that way.
>
> This needs to be done properly in code to handle the issues if the
> "wrong" code is loaded first.  Please trigger off of the hardware type
> correctly so you don't have to worry about this at all.
>

You are right. The only thing that should be done is "__init" should
be removed from s3c24xx_serial_console_setup() and
s3c24xx_serial_get_options() functions. Because in case when USIv2
driver instantiates the serial driver via of_platform_populate(), when
USI_V2=m and SERIAL_SAMSUNG=y, those symbols will be thrown away
already. And of course "[PATCH 5/8] tty: serial: samsung: Enable
console as module" is needed as well. Correct init order (USI vs
serial) is already ensured by embedding serial node in USI node (as a
child node).

We'll still have some weird init order in that case (USI_V2=m and
SERIAL_SAMSUNG=y), like doing serial console init first (and
earlycon), then registering USI driver as a module (reconfiguring USI
IP-core), and then doing serial probe. But at least that doesn't crash
and works fine (only causing some delay once, in the middle of dmesg
output). But I guess that would be a problem of people who decided to
go with such weird config.

Bottom line is, this patch is not needed. I'll re-send v2 soon,
excluding it from there, and will also add that mentioned "__init"
removal.

Thanks for review!

> thanks,
>
> greg k-h