Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
added compile-test support to the Freescale 16550 driver. However, as
SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
enables this driver.
Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
prevent asking the user about this driver when configuring a kernel
without appropriate Freescale SoC or ACPI support.
Fixes: b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Yes, it's ugly, but I see no better solution. Do you?
drivers/tty/serial/8250/Kconfig | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Johan Hovold <johan@kernel.org> Date: 2021-09-16 08:46:05
On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
added compile-test support to the Freescale 16550 driver. However, as
SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
enables this driver.
Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
prevent asking the user about this driver when configuring a kernel
without appropriate Freescale SoC or ACPI support.
This tightening is arguable a separate change which risk introducing
regressions if you get it wrong and should go in a separate patch at
least.
quoted hunk
Fixes: b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Yes, it's ugly, but I see no better solution. Do you?
drivers/tty/serial/8250/Kconfig | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -361,9 +361,13 @@ config SERIAL_8250_BCM2835AUXIfunsure,sayN.configSERIAL_8250_FSL-bool+bool"Freescale 16550-style UART support (8250 based driver)"depends onSERIAL_8250_CONSOLE-defaultPPC||ARM||ARM64||COMPILE_TEST+depends onFSL_SOC||ARCH_LAYERSCAPE||SOC_LS1021A||(ARM64&&ACPI)||COMPILE_TEST+defaultFSL_SOC||ARCH_LAYERSCAPE||SOC_LS1021A||(ARM64&&ACPI)
I'd suggest just doing
bool "Freescale 16550-style UART support (8250 based driver)"
depends on SERIAL_8250_CONSOLE
default PPC || ARM || ARM64
Since neither of the symbols you add to that "depends on" line is an
actual build or runtime dependency.
Then you can refine the "default" line in a follow up (or argue why you
think there should be a "depends on FSL_SOC || ...").
+ help
+ Selecting this option will add support for the 16550-style serial
+ port hardware found on Freescale SoCs.
config SERIAL_8250_DW
tristate "Support for Synopsys DesignWare 8250 quirks"
Hi Johan,
On Thu, Sep 16, 2021 at 10:46 AM Johan Hovold [off-list ref] wrote:
On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
quoted
Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
added compile-test support to the Freescale 16550 driver. However, as
SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
enables this driver.
Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
prevent asking the user about this driver when configuring a kernel
without appropriate Freescale SoC or ACPI support.
This tightening is arguable a separate change which risk introducing
regressions if you get it wrong and should go in a separate patch at
least.
Getting it wrong would indeed be a regression, but not tightening
that at the same time would mean I have to send a separate patch with
a Fixes tag referring to this fix, following this template:
foo should depend on bar
The foo hardware is only present on bar SoCs. Hence add a
dependency on bar, to prevent asking the user about this driver
when configuring a kernel without bar support.
quoted
Fixes: b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Yes, it's ugly, but I see no better solution. Do you?
drivers/tty/serial/8250/Kconfig | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -361,9 +361,13 @@ config SERIAL_8250_BCM2835AUXIfunsure,sayN.configSERIAL_8250_FSL-bool+bool"Freescale 16550-style UART support (8250 based driver)"depends onSERIAL_8250_CONSOLE-defaultPPC||ARM||ARM64||COMPILE_TEST+depends onFSL_SOC||ARCH_LAYERSCAPE||SOC_LS1021A||(ARM64&&ACPI)||COMPILE_TEST+defaultFSL_SOC||ARCH_LAYERSCAPE||SOC_LS1021A||(ARM64&&ACPI)
I'd suggest just doing
bool "Freescale 16550-style UART support (8250 based driver)"
depends on SERIAL_8250_CONSOLE
default PPC || ARM || ARM64
Since neither of the symbols you add to that "depends on" line is an
actual build or runtime dependency.
They are.
Then you can refine the "default" line in a follow up (or argue why you
think there should be a "depends on FSL_SOC || ...").
quoted
+ help
+ Selecting this option will add support for the 16550-style serial
+ port hardware found on Freescale SoCs.
config SERIAL_8250_DW
tristate "Support for Synopsys DesignWare 8250 quirks"
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Johan Hovold <johan@kernel.org> Date: 2021-09-16 09:08:43
On Thu, Sep 16, 2021 at 10:55:49AM +0200, Geert Uytterhoeven wrote:
Hi Johan,
On Thu, Sep 16, 2021 at 10:46 AM Johan Hovold [off-list ref] wrote:
quoted
On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
quoted
Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
added compile-test support to the Freescale 16550 driver. However, as
SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
enables this driver.
Fix this by making SERIAL_8250_FSL visible. Tighten the dependencies to
prevent asking the user about this driver when configuring a kernel
without appropriate Freescale SoC or ACPI support.
This tightening is arguable a separate change which risk introducing
regressions if you get it wrong and should go in a separate patch at
least.
Getting it wrong would indeed be a regression, but not tightening
that at the same time would mean I have to send a separate patch with
a Fixes tag referring to this fix, following this template:
foo should depend on bar
The foo hardware is only present on bar SoCs. Hence add a
dependency on bar, to prevent asking the user about this driver
when configuring a kernel without bar support.
I know this is a pet peeve of yours, but asking users about one more
symbol when configuring their kernels is hardly something that requires
a Fixes tag.
Either way it's a pretty weak argument for not separating the change.
Johan