* [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.