* [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
@ 2023-05-31 8:32 Uwe Kleine-König
2023-05-31 9:47 ` Ilpo Järvinen
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-05-31 8:32 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Ilpo Järvinen, Johan Hovold, linux-serial, kernel
The need to handle the FSL variant of 8250 in a special way is also
present without console support. So soften the dependency for
SERIAL_8250_FSL accordingly.
This issue was identified by Dominik Andreas Schorpp.
To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
must be put in the same compilation unit as 8250_port.o because the
latter defines some functions needed in the former and so 8250_fsl.o
must not be built-in if 8250_port.o is available in a module.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,
(implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
correctly which was pointed out by the 0-day bot. (Thanks!)
That wasn't a problem before because SERIAL_8250_CONSOLE depends on
SERIAL_8250=y.
Having said that I wonder if there are a few more .o files that should
better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
obj-$(CONFIG_SERIAL_8250_XXX).
Best regards
Uwe
drivers/tty/serial/8250/Kconfig | 2 +-
drivers/tty/serial/8250/Makefile | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 5313aa31930f..10c09b19c871 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
config SERIAL_8250_FSL
bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
- depends on SERIAL_8250_CONSOLE
+ depends on SERIAL_8250
default PPC || ARM || ARM64
help
Selecting this option enables a workaround for a break-detection
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 4fc2fc1f41b6..37647ca79ec5 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_SERIAL_8250) += 8250.o 8250_base.o
8250_base-$(CONFIG_SERIAL_8250_DWLIB) += 8250_dwlib.o
8250_base-$(CONFIG_SERIAL_8250_FINTEK) += 8250_fintek.o
8250_base-$(CONFIG_SERIAL_8250_PCILIB) += 8250_pcilib.o
+8250_base-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o
obj-$(CONFIG_SERIAL_8250_PARISC) += 8250_parisc.o
obj-$(CONFIG_SERIAL_8250_PCI) += 8250_pci.o
obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
@@ -28,7 +29,6 @@ obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o
obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554) += 8250_exar_st16c554.o
obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
obj-$(CONFIG_SERIAL_8250_PCI1XXXX) += 8250_pci1xxxx.o
-obj-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o
obj-$(CONFIG_SERIAL_8250_MEN_MCB) += 8250_men_mcb.o
obj-$(CONFIG_SERIAL_8250_DFL) += 8250_dfl.o
obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o
base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.39.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-05-31 8:32 [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE Uwe Kleine-König
@ 2023-05-31 9:47 ` Ilpo Järvinen
2023-05-31 10:04 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2023-05-31 9:47 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Johan Hovold, linux-serial, kernel
[-- Attachment #1: Type: text/plain, Size: 3111 bytes --]
On Wed, 31 May 2023, Uwe Kleine-König wrote:
> The need to handle the FSL variant of 8250 in a special way is also
> present without console support. So soften the dependency for
> SERIAL_8250_FSL accordingly.
>
> This issue was identified by Dominik Andreas Schorpp.
>
> To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> must be put in the same compilation unit as 8250_port.o because the
> latter defines some functions needed in the former and so 8250_fsl.o
> must not be built-in if 8250_port.o is available in a module.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> correctly which was pointed out by the 0-day bot. (Thanks!)
That would warrant Reported-by (0-day's reports give you the tag).
> That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> SERIAL_8250=y.
>
> Having said that I wonder if there are a few more .o files that should
> better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> obj-$(CONFIG_SERIAL_8250_XXX).
>
> Best regards
> Uwe
>
> drivers/tty/serial/8250/Kconfig | 2 +-
> drivers/tty/serial/8250/Makefile | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 5313aa31930f..10c09b19c871 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
>
> config SERIAL_8250_FSL
> bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> - depends on SERIAL_8250_CONSOLE
> + depends on SERIAL_8250
Why this cannot simply be:
depends on SERIAL_8250=y
--
i.
> default PPC || ARM || ARM64
> help
> Selecting this option enables a workaround for a break-detection
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 4fc2fc1f41b6..37647ca79ec5 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SERIAL_8250) += 8250.o 8250_base.o
> 8250_base-$(CONFIG_SERIAL_8250_DWLIB) += 8250_dwlib.o
> 8250_base-$(CONFIG_SERIAL_8250_FINTEK) += 8250_fintek.o
> 8250_base-$(CONFIG_SERIAL_8250_PCILIB) += 8250_pcilib.o
> +8250_base-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o
> obj-$(CONFIG_SERIAL_8250_PARISC) += 8250_parisc.o
> obj-$(CONFIG_SERIAL_8250_PCI) += 8250_pci.o
> obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
> @@ -28,7 +29,6 @@ obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o
> obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554) += 8250_exar_st16c554.o
> obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
> obj-$(CONFIG_SERIAL_8250_PCI1XXXX) += 8250_pci1xxxx.o
> -obj-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o
> obj-$(CONFIG_SERIAL_8250_MEN_MCB) += 8250_men_mcb.o
> obj-$(CONFIG_SERIAL_8250_DFL) += 8250_dfl.o
> obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-05-31 9:47 ` Ilpo Järvinen
@ 2023-05-31 10:04 ` Uwe Kleine-König
2023-05-31 10:09 ` Ilpo Järvinen
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-05-31 10:04 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: kernel, Greg Kroah-Hartman, Johan Hovold, linux-serial
[-- Attachment #1: Type: text/plain, Size: 2609 bytes --]
On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> On Wed, 31 May 2023, Uwe Kleine-König wrote:
>
> > The need to handle the FSL variant of 8250 in a special way is also
> > present without console support. So soften the dependency for
> > SERIAL_8250_FSL accordingly.
> >
> > This issue was identified by Dominik Andreas Schorpp.
> >
> > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > must be put in the same compilation unit as 8250_port.o because the
> > latter defines some functions needed in the former and so 8250_fsl.o
> > must not be built-in if 8250_port.o is available in a module.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> >
> > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > correctly which was pointed out by the 0-day bot. (Thanks!)
>
> That would warrant Reported-by (0-day's reports give you the tag).
I'd add this tag if I created a commit that fixes the broken commit.
However I understood that if a v2 patch fixes a v1 that was broken, the
tag is not to be added?! I don't feel strong here however, so if people
agree that the tag should be there, I can add it.
> > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > SERIAL_8250=y.
> >
> > Having said that I wonder if there are a few more .o files that should
> > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > obj-$(CONFIG_SERIAL_8250_XXX).
> >
> > Best regards
> > Uwe
> >
> > drivers/tty/serial/8250/Kconfig | 2 +-
> > drivers/tty/serial/8250/Makefile | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index 5313aa31930f..10c09b19c871 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> >
> > config SERIAL_8250_FSL
> > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > - depends on SERIAL_8250_CONSOLE
> > + depends on SERIAL_8250
>
> Why this cannot simply be:
> depends on SERIAL_8250=y
This doesn't work, because then the FSL-workarounds are missing if the
8250 driver is compiled as a module.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-05-31 10:04 ` Uwe Kleine-König
@ 2023-05-31 10:09 ` Ilpo Järvinen
2023-05-31 10:40 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2023-05-31 10:09 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: kernel, Greg Kroah-Hartman, Johan Hovold, linux-serial
[-- Attachment #1: Type: text/plain, Size: 2763 bytes --]
On Wed, 31 May 2023, Uwe Kleine-König wrote:
> On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> >
> > > The need to handle the FSL variant of 8250 in a special way is also
> > > present without console support. So soften the dependency for
> > > SERIAL_8250_FSL accordingly.
> > >
> > > This issue was identified by Dominik Andreas Schorpp.
> > >
> > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > must be put in the same compilation unit as 8250_port.o because the
> > > latter defines some functions needed in the former and so 8250_fsl.o
> > > must not be built-in if 8250_port.o is available in a module.
> > >
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Hello,
> > >
> > > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > > correctly which was pointed out by the 0-day bot. (Thanks!)
> >
> > That would warrant Reported-by (0-day's reports give you the tag).
>
> I'd add this tag if I created a commit that fixes the broken commit.
> However I understood that if a v2 patch fixes a v1 that was broken, the
> tag is not to be added?! I don't feel strong here however, so if people
> agree that the tag should be there, I can add it.
>
> > > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > > SERIAL_8250=y.
> > >
> > > Having said that I wonder if there are a few more .o files that should
> > > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > > obj-$(CONFIG_SERIAL_8250_XXX).
> > >
> > > Best regards
> > > Uwe
> > >
> > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > drivers/tty/serial/8250/Makefile | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > index 5313aa31930f..10c09b19c871 100644
> > > --- a/drivers/tty/serial/8250/Kconfig
> > > +++ b/drivers/tty/serial/8250/Kconfig
> > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > >
> > > config SERIAL_8250_FSL
> > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > > - depends on SERIAL_8250_CONSOLE
> > > + depends on SERIAL_8250
> >
> > Why this cannot simply be:
> > depends on SERIAL_8250=y
>
> This doesn't work, because then the FSL-workarounds are missing if the
> 8250 driver is compiled as a module.
How can 8250 driver be a module and fsl still get enabled?
What I think (not a Kconfig expert for sure) would happen is that 8250_fsl
won't be enabled at all if CONFIG_SERIAL_8250=m because it depends on
SERIAL_8250=y.
--
i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-05-31 10:09 ` Ilpo Järvinen
@ 2023-05-31 10:40 ` Uwe Kleine-König
2023-05-31 17:51 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-05-31 10:40 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Greg Kroah-Hartman, Johan Hovold, kernel, linux-serial
[-- Attachment #1: Type: text/plain, Size: 3395 bytes --]
On Wed, May 31, 2023 at 01:09:01PM +0300, Ilpo Järvinen wrote:
> On Wed, 31 May 2023, Uwe Kleine-König wrote:
>
> > On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > >
> > > > The need to handle the FSL variant of 8250 in a special way is also
> > > > present without console support. So soften the dependency for
> > > > SERIAL_8250_FSL accordingly.
> > > >
> > > > This issue was identified by Dominik Andreas Schorpp.
> > > >
> > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > > must be put in the same compilation unit as 8250_port.o because the
> > > > latter defines some functions needed in the former and so 8250_fsl.o
> > > > must not be built-in if 8250_port.o is available in a module.
> > > >
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > ---
> > > > Hello,
> > > >
> > > > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > > > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > > > correctly which was pointed out by the 0-day bot. (Thanks!)
> > >
> > > That would warrant Reported-by (0-day's reports give you the tag).
> >
> > I'd add this tag if I created a commit that fixes the broken commit.
> > However I understood that if a v2 patch fixes a v1 that was broken, the
> > tag is not to be added?! I don't feel strong here however, so if people
> > agree that the tag should be there, I can add it.
> >
> > > > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > > > SERIAL_8250=y.
> > > >
> > > > Having said that I wonder if there are a few more .o files that should
> > > > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > > > obj-$(CONFIG_SERIAL_8250_XXX).
> > > >
> > > > Best regards
> > > > Uwe
> > > >
> > > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > > drivers/tty/serial/8250/Makefile | 2 +-
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > > index 5313aa31930f..10c09b19c871 100644
> > > > --- a/drivers/tty/serial/8250/Kconfig
> > > > +++ b/drivers/tty/serial/8250/Kconfig
> > > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > > >
> > > > config SERIAL_8250_FSL
> > > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > > > - depends on SERIAL_8250_CONSOLE
> > > > + depends on SERIAL_8250
> > >
> > > Why this cannot simply be:
> > > depends on SERIAL_8250=y
> >
> > This doesn't work, because then the FSL-workarounds are missing if the
> > 8250 driver is compiled as a module.
>
> How can 8250 driver be a module and fsl still get enabled?
It works. With my patch applied:
$ make allmodconfig
$ grep -E 'CONFIG_SERIAL_8250(_FSL)?\>' .config
CONFIG_SERIAL_8250=m
CONFIG_SERIAL_8250_FSL=y
> What I think (not a Kconfig expert for sure) would happen is that 8250_fsl
> won't be enabled at all if CONFIG_SERIAL_8250=m because it depends on
> SERIAL_8250=y.
That's not how it seems to be ...
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-05-31 10:40 ` Uwe Kleine-König
@ 2023-05-31 17:51 ` Uwe Kleine-König
2023-06-01 10:49 ` Ilpo Järvinen
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-05-31 17:51 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Greg Kroah-Hartman, Johan Hovold, kernel, linux-serial
[-- Attachment #1: Type: text/plain, Size: 3809 bytes --]
Hello Ilpo,
On Wed, May 31, 2023 at 12:40:10PM +0200, Uwe Kleine-König wrote:
> On Wed, May 31, 2023 at 01:09:01PM +0300, Ilpo Järvinen wrote:
> > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> >
> > > On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> > > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > > >
> > > > > The need to handle the FSL variant of 8250 in a special way is also
> > > > > present without console support. So soften the dependency for
> > > > > SERIAL_8250_FSL accordingly.
> > > > >
> > > > > This issue was identified by Dominik Andreas Schorpp.
> > > > >
> > > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > > > must be put in the same compilation unit as 8250_port.o because the
> > > > > latter defines some functions needed in the former and so 8250_fsl.o
> > > > > must not be built-in if 8250_port.o is available in a module.
> > > > >
> > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > > ---
> > > > > Hello,
> > > > >
> > > > > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > > > > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > > > > correctly which was pointed out by the 0-day bot. (Thanks!)
> > > >
> > > > That would warrant Reported-by (0-day's reports give you the tag).
> > >
> > > I'd add this tag if I created a commit that fixes the broken commit.
> > > However I understood that if a v2 patch fixes a v1 that was broken, the
> > > tag is not to be added?! I don't feel strong here however, so if people
> > > agree that the tag should be there, I can add it.
> > >
> > > > > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > > > > SERIAL_8250=y.
> > > > >
> > > > > Having said that I wonder if there are a few more .o files that should
> > > > > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > > > > obj-$(CONFIG_SERIAL_8250_XXX).
> > > > >
> > > > > Best regards
> > > > > Uwe
> > > > >
> > > > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > > > drivers/tty/serial/8250/Makefile | 2 +-
> > > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > > > index 5313aa31930f..10c09b19c871 100644
> > > > > --- a/drivers/tty/serial/8250/Kconfig
> > > > > +++ b/drivers/tty/serial/8250/Kconfig
> > > > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > > > >
> > > > > config SERIAL_8250_FSL
> > > > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > > > > - depends on SERIAL_8250_CONSOLE
> > > > > + depends on SERIAL_8250
> > > >
> > > > Why this cannot simply be:
> > > > depends on SERIAL_8250=y
> > >
> > > This doesn't work, because then the FSL-workarounds are missing if the
> > > 8250 driver is compiled as a module.
> >
> > How can 8250 driver be a module and fsl still get enabled?
>
> It works. With my patch applied:
>
> $ make allmodconfig
> $ grep -E 'CONFIG_SERIAL_8250(_FSL)?\>' .config
> CONFIG_SERIAL_8250=m
> CONFIG_SERIAL_8250_FSL=y
>
> > What I think (not a Kconfig expert for sure) would happen is that 8250_fsl
> > won't be enabled at all if CONFIG_SERIAL_8250=m because it depends on
> > SERIAL_8250=y.
>
> That's not how it seems to be ...
If this convinces you that the patch is fine, an ack would be nice as
gregkh signaled that there is some pending discussion he is waiting to
end before applying this patch.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-05-31 17:51 ` Uwe Kleine-König
@ 2023-06-01 10:49 ` Ilpo Järvinen
2023-06-02 9:21 ` Ilpo Järvinen
0 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2023-06-01 10:49 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Johan Hovold, kernel, linux-serial
[-- Attachment #1: Type: text/plain, Size: 3965 bytes --]
On Wed, 31 May 2023, Uwe Kleine-König wrote:
> Hello Ilpo,
>
> On Wed, May 31, 2023 at 12:40:10PM +0200, Uwe Kleine-König wrote:
> > On Wed, May 31, 2023 at 01:09:01PM +0300, Ilpo Järvinen wrote:
> > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > >
> > > > On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> > > > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > > > >
> > > > > > The need to handle the FSL variant of 8250 in a special way is also
> > > > > > present without console support. So soften the dependency for
> > > > > > SERIAL_8250_FSL accordingly.
> > > > > >
> > > > > > This issue was identified by Dominik Andreas Schorpp.
> > > > > >
> > > > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > > > > must be put in the same compilation unit as 8250_port.o because the
> > > > > > latter defines some functions needed in the former and so 8250_fsl.o
> > > > > > must not be built-in if 8250_port.o is available in a module.
> > > > > >
> > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > > > ---
> > > > > > Hello,
> > > > > >
> > > > > > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > > > > > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > > > > > correctly which was pointed out by the 0-day bot. (Thanks!)
> > > > >
> > > > > That would warrant Reported-by (0-day's reports give you the tag).
> > > >
> > > > I'd add this tag if I created a commit that fixes the broken commit.
> > > > However I understood that if a v2 patch fixes a v1 that was broken, the
> > > > tag is not to be added?! I don't feel strong here however, so if people
> > > > agree that the tag should be there, I can add it.
> > > >
> > > > > > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > > > > > SERIAL_8250=y.
> > > > > >
> > > > > > Having said that I wonder if there are a few more .o files that should
> > > > > > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > > > > > obj-$(CONFIG_SERIAL_8250_XXX).
> > > > > >
> > > > > > Best regards
> > > > > > Uwe
> > > > > >
> > > > > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > > > > drivers/tty/serial/8250/Makefile | 2 +-
> > > > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > > > > index 5313aa31930f..10c09b19c871 100644
> > > > > > --- a/drivers/tty/serial/8250/Kconfig
> > > > > > +++ b/drivers/tty/serial/8250/Kconfig
> > > > > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > > > > >
> > > > > > config SERIAL_8250_FSL
> > > > > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > > > > > - depends on SERIAL_8250_CONSOLE
> > > > > > + depends on SERIAL_8250
> > > > >
> > > > > Why this cannot simply be:
> > > > > depends on SERIAL_8250=y
> > > >
> > > > This doesn't work, because then the FSL-workarounds are missing if the
> > > > 8250 driver is compiled as a module.
> > >
> > > How can 8250 driver be a module and fsl still get enabled?
> >
> > It works. With my patch applied:
> >
> > $ make allmodconfig
> > $ grep -E 'CONFIG_SERIAL_8250(_FSL)?\>' .config
> > CONFIG_SERIAL_8250=m
> > CONFIG_SERIAL_8250_FSL=y
> >
> > > What I think (not a Kconfig expert for sure) would happen is that 8250_fsl
> > > won't be enabled at all if CONFIG_SERIAL_8250=m because it depends on
> > > SERIAL_8250=y.
> >
> > That's not how it seems to be ...
>
> If this convinces you that the patch is fine, an ack would be nice as
> gregkh signaled that there is some pending discussion he is waiting to
> end before applying this patch.
Ah, so you want to have SERIAL_8250_FSL=y always when SERIAL_8250 is
there. I can't see another way around it besides the moving you made.
Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-06-01 10:49 ` Ilpo Järvinen
@ 2023-06-02 9:21 ` Ilpo Järvinen
2023-06-02 10:09 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 9:21 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Johan Hovold, kernel, linux-serial
[-- Attachment #1: Type: text/plain, Size: 4471 bytes --]
On Thu, 1 Jun 2023, Ilpo Järvinen wrote:
> On Wed, 31 May 2023, Uwe Kleine-König wrote:
>
> > Hello Ilpo,
> >
> > On Wed, May 31, 2023 at 12:40:10PM +0200, Uwe Kleine-König wrote:
> > > On Wed, May 31, 2023 at 01:09:01PM +0300, Ilpo Järvinen wrote:
> > > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > > >
> > > > > On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> > > > > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > > > > >
> > > > > > > The need to handle the FSL variant of 8250 in a special way is also
> > > > > > > present without console support. So soften the dependency for
> > > > > > > SERIAL_8250_FSL accordingly.
> > > > > > >
> > > > > > > This issue was identified by Dominik Andreas Schorpp.
> > > > > > >
> > > > > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > > > > > must be put in the same compilation unit as 8250_port.o because the
> > > > > > > latter defines some functions needed in the former and so 8250_fsl.o
> > > > > > > must not be built-in if 8250_port.o is available in a module.
> > > > > > >
> > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > > > > ---
> > > > > > > Hello,
> > > > > > >
> > > > > > > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > > > > > > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > > > > > > correctly which was pointed out by the 0-day bot. (Thanks!)
> > > > > >
> > > > > > That would warrant Reported-by (0-day's reports give you the tag).
> > > > >
> > > > > I'd add this tag if I created a commit that fixes the broken commit.
> > > > > However I understood that if a v2 patch fixes a v1 that was broken, the
> > > > > tag is not to be added?! I don't feel strong here however, so if people
> > > > > agree that the tag should be there, I can add it.
> > > > >
> > > > > > > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > > > > > > SERIAL_8250=y.
> > > > > > >
> > > > > > > Having said that I wonder if there are a few more .o files that should
> > > > > > > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > > > > > > obj-$(CONFIG_SERIAL_8250_XXX).
> > > > > > >
> > > > > > > Best regards
> > > > > > > Uwe
> > > > > > >
> > > > > > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > > > > > drivers/tty/serial/8250/Makefile | 2 +-
> > > > > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > > > > > index 5313aa31930f..10c09b19c871 100644
> > > > > > > --- a/drivers/tty/serial/8250/Kconfig
> > > > > > > +++ b/drivers/tty/serial/8250/Kconfig
> > > > > > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > > > > > >
> > > > > > > config SERIAL_8250_FSL
> > > > > > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > > > > > > - depends on SERIAL_8250_CONSOLE
> > > > > > > + depends on SERIAL_8250
> > > > > >
> > > > > > Why this cannot simply be:
> > > > > > depends on SERIAL_8250=y
> > > > >
> > > > > This doesn't work, because then the FSL-workarounds are missing if the
> > > > > 8250 driver is compiled as a module.
> > > >
> > > > How can 8250 driver be a module and fsl still get enabled?
> > >
> > > It works. With my patch applied:
> > >
> > > $ make allmodconfig
> > > $ grep -E 'CONFIG_SERIAL_8250(_FSL)?\>' .config
> > > CONFIG_SERIAL_8250=m
> > > CONFIG_SERIAL_8250_FSL=y
> > >
> > > > What I think (not a Kconfig expert for sure) would happen is that 8250_fsl
> > > > won't be enabled at all if CONFIG_SERIAL_8250=m because it depends on
> > > > SERIAL_8250=y.
> > >
> > > That's not how it seems to be ...
> >
> > If this convinces you that the patch is fine, an ack would be nice as
> > gregkh signaled that there is some pending discussion he is waiting to
> > end before applying this patch.
>
> Ah, so you want to have SERIAL_8250_FSL=y always when SERIAL_8250 is
> there. I can't see another way around it besides the moving you made.
>
> Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
After some more thinking, it looks make allmodconfig is not going to work
because of this:
arch/powerpc/kernel/legacy_serial.c: port->handle_irq = fsl8250_handle_irq;
obj-$(CONFIG_PPC_UDBG_16550) += legacy_serial.o udbg_16550.o
config PPC_UDBG_16550
bool
--
i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
2023-06-02 9:21 ` Ilpo Järvinen
@ 2023-06-02 10:09 ` Uwe Kleine-König
0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2023-06-02 10:09 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: Greg Kroah-Hartman, Johan Hovold, kernel, linux-serial
[-- Attachment #1: Type: text/plain, Size: 5199 bytes --]
On Fri, Jun 02, 2023 at 12:21:05PM +0300, Ilpo Järvinen wrote:
> On Thu, 1 Jun 2023, Ilpo Järvinen wrote:
>
> > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> >
> > > Hello Ilpo,
> > >
> > > On Wed, May 31, 2023 at 12:40:10PM +0200, Uwe Kleine-König wrote:
> > > > On Wed, May 31, 2023 at 01:09:01PM +0300, Ilpo Järvinen wrote:
> > > > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > > > >
> > > > > > On Wed, May 31, 2023 at 12:47:54PM +0300, Ilpo Järvinen wrote:
> > > > > > > On Wed, 31 May 2023, Uwe Kleine-König wrote:
> > > > > > >
> > > > > > > > The need to handle the FSL variant of 8250 in a special way is also
> > > > > > > > present without console support. So soften the dependency for
> > > > > > > > SERIAL_8250_FSL accordingly.
> > > > > > > >
> > > > > > > > This issue was identified by Dominik Andreas Schorpp.
> > > > > > > >
> > > > > > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > > > > > > must be put in the same compilation unit as 8250_port.o because the
> > > > > > > > latter defines some functions needed in the former and so 8250_fsl.o
> > > > > > > > must not be built-in if 8250_port.o is available in a module.
> > > > > > > >
> > > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > > > > > ---
> > > > > > > > Hello,
> > > > > > > >
> > > > > > > > (implicit) v1 was already applied by Greg (a0807ca158e0 in tty-testing)
> > > > > > > > but that didn't handle CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y
> > > > > > > > correctly which was pointed out by the 0-day bot. (Thanks!)
> > > > > > >
> > > > > > > That would warrant Reported-by (0-day's reports give you the tag).
> > > > > >
> > > > > > I'd add this tag if I created a commit that fixes the broken commit.
> > > > > > However I understood that if a v2 patch fixes a v1 that was broken, the
> > > > > > tag is not to be added?! I don't feel strong here however, so if people
> > > > > > agree that the tag should be there, I can add it.
> > > > > >
> > > > > > > > That wasn't a problem before because SERIAL_8250_CONSOLE depends on
> > > > > > > > SERIAL_8250=y.
> > > > > > > >
> > > > > > > > Having said that I wonder if there are a few more .o files that should
> > > > > > > > better be used with 8250_base-$(CONFIG_SERIAL_8250_XXX) instead of
> > > > > > > > obj-$(CONFIG_SERIAL_8250_XXX).
> > > > > > > >
> > > > > > > > Best regards
> > > > > > > > Uwe
> > > > > > > >
> > > > > > > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > > > > > > drivers/tty/serial/8250/Makefile | 2 +-
> > > > > > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > > > > > > index 5313aa31930f..10c09b19c871 100644
> > > > > > > > --- a/drivers/tty/serial/8250/Kconfig
> > > > > > > > +++ b/drivers/tty/serial/8250/Kconfig
> > > > > > > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > > > > > > >
> > > > > > > > config SERIAL_8250_FSL
> > > > > > > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64)
> > > > > > > > - depends on SERIAL_8250_CONSOLE
> > > > > > > > + depends on SERIAL_8250
> > > > > > >
> > > > > > > Why this cannot simply be:
> > > > > > > depends on SERIAL_8250=y
> > > > > >
> > > > > > This doesn't work, because then the FSL-workarounds are missing if the
> > > > > > 8250 driver is compiled as a module.
> > > > >
> > > > > How can 8250 driver be a module and fsl still get enabled?
> > > >
> > > > It works. With my patch applied:
> > > >
> > > > $ make allmodconfig
> > > > $ grep -E 'CONFIG_SERIAL_8250(_FSL)?\>' .config
> > > > CONFIG_SERIAL_8250=m
> > > > CONFIG_SERIAL_8250_FSL=y
> > > >
> > > > > What I think (not a Kconfig expert for sure) would happen is that 8250_fsl
> > > > > won't be enabled at all if CONFIG_SERIAL_8250=m because it depends on
> > > > > SERIAL_8250=y.
> > > >
> > > > That's not how it seems to be ...
> > >
> > > If this convinces you that the patch is fine, an ack would be nice as
> > > gregkh signaled that there is some pending discussion he is waiting to
> > > end before applying this patch.
> >
> > Ah, so you want to have SERIAL_8250_FSL=y always when SERIAL_8250 is
> > there. I can't see another way around it besides the moving you made.
> >
> > Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> After some more thinking, it looks make allmodconfig is not going to work
> because of this:
>
> arch/powerpc/kernel/legacy_serial.c: port->handle_irq = fsl8250_handle_irq;
>
> obj-$(CONFIG_PPC_UDBG_16550) += legacy_serial.o udbg_16550.o
>
> config PPC_UDBG_16550
> bool
Yeah, the buildbot already noticed, too[1]. And I failed to add you to Cc:
in my reply.
Best regards
Uwe
[1] https://lore.kernel.org/linux-serial/20230602074219.srlgpaaypewd2q5s@pengutronix.de/
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-02 10:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 8:32 [PATCH v2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE Uwe Kleine-König
2023-05-31 9:47 ` Ilpo Järvinen
2023-05-31 10:04 ` Uwe Kleine-König
2023-05-31 10:09 ` Ilpo Järvinen
2023-05-31 10:40 ` Uwe Kleine-König
2023-05-31 17:51 ` Uwe Kleine-König
2023-06-01 10:49 ` Ilpo Järvinen
2023-06-02 9:21 ` Ilpo Järvinen
2023-06-02 10:09 ` Uwe Kleine-König
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.