All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Protsenko <semen.protsenko@linaro.org>
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
Subject: Re: [PATCH 6/8] tty: serial: Make SERIAL_SAMSUNG=y impossible when EXYNOS_USI_V2=m
Date: Mon, 29 Nov 2021 01:54:18 +0200	[thread overview]
Message-ID: <CAPLW+4=mL6ZV73zY+AoF6mo8Tka0cO8pst2mBYOoEHpX4nrRbA@mail.gmail.com> (raw)
In-Reply-To: <YaORtBO4b9AyFYyd@kroah.com>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Sam Protsenko <semen.protsenko@linaro.org>
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
Subject: Re: [PATCH 6/8] tty: serial: Make SERIAL_SAMSUNG=y impossible when EXYNOS_USI_V2=m
Date: Mon, 29 Nov 2021 01:54:18 +0200	[thread overview]
Message-ID: <CAPLW+4=mL6ZV73zY+AoF6mo8Tka0cO8pst2mBYOoEHpX4nrRbA@mail.gmail.com> (raw)
In-Reply-To: <YaORtBO4b9AyFYyd@kroah.com>

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

  reply	other threads:[~2021-11-28 23:55 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-27 22:32 [PATCH 0/8] soc: samsung: Add USIv2 driver Sam Protsenko
2021-11-27 22:32 ` Sam Protsenko
2021-11-27 22:32 ` [PATCH 1/8] dt-bindings: soc: samsung: Add Exynos USIv2 bindings Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-27 22:32 ` [PATCH 2/8] dt-bindings: soc: samsung: Add Exynos USIv2 bindings doc Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-29  8:33   ` Krzysztof Kozlowski
2021-11-29  8:33     ` Krzysztof Kozlowski
2021-11-27 22:32 ` [PATCH 3/8] soc: samsung: Add USIv2 driver Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-29  8:49   ` Krzysztof Kozlowski
2021-11-29  8:49     ` Krzysztof Kozlowski
2021-11-27 22:32 ` [PATCH 4/8] tty: serial: samsung: Remove USI initialization Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-28 14:28   ` Greg Kroah-Hartman
2021-11-28 14:28     ` Greg Kroah-Hartman
2021-11-28 16:26     ` Sam Protsenko
2021-11-28 16:26       ` Sam Protsenko
2021-11-28 17:03       ` Sam Protsenko
2021-11-28 17:03         ` Sam Protsenko
2021-11-27 22:32 ` [PATCH 5/8] tty: serial: samsung: Enable console as module Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-29  8:52   ` Krzysztof Kozlowski
2021-11-29  8:52     ` Krzysztof Kozlowski
2021-11-29 20:18     ` Sam Protsenko
2021-11-29 20:18       ` Sam Protsenko
2021-11-29 20:38       ` Sam Protsenko
2021-11-29 20:38         ` Sam Protsenko
2021-11-27 22:32 ` [PATCH 6/8] tty: serial: Make SERIAL_SAMSUNG=y impossible when EXYNOS_USI_V2=m Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-28 14:27   ` Greg Kroah-Hartman
2021-11-28 14:27     ` Greg Kroah-Hartman
2021-11-28 23:54     ` Sam Protsenko [this message]
2021-11-28 23:54       ` Sam Protsenko
2021-11-27 22:32 ` [PATCH 7/8] i2c: Make I2C_EXYNOS5=y " Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-29  0:02   ` Sam Protsenko
2021-11-29  0:02     ` Sam Protsenko
2021-11-27 22:32 ` [PATCH 8/8] spi: Make SPI_S3C64XX=y " Sam Protsenko
2021-11-27 22:32   ` Sam Protsenko
2021-11-29  0:02   ` Sam Protsenko
2021-11-29  0:02     ` Sam Protsenko
2021-11-28  3:15 ` [PATCH 0/8] soc: samsung: Add USIv2 driver David Virag
2021-11-28  3:15   ` David Virag
2021-11-29  9:02   ` Krzysztof Kozlowski
2021-11-29  9:02     ` Krzysztof Kozlowski
2021-11-29 13:56   ` Sam Protsenko
2021-11-29 13:56     ` Sam Protsenko
2021-11-29 17:35     ` Krzysztof Kozlowski
2021-11-29 17:35       ` Krzysztof Kozlowski
2021-11-29 19:19     ` David Virag
2021-11-29 19:19       ` David Virag
2021-11-30  0:01       ` Sam Protsenko
2021-11-30  0:01         ` Sam Protsenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPLW+4=mL6ZV73zY+AoF6mo8Tka0cO8pst2mBYOoEHpX4nrRbA@mail.gmail.com' \
    --to=semen.protsenko@linaro.org \
    --cc=broonie@kernel.org \
    --cc=chanho61.park@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jaewon02.kim@samsung.com \
    --cc=krzysztof.kozlowski@canonical.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=virag.david003@gmail.com \
    --cc=youngmin.nam@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.