Message ID | 20241120153554.860710-1-sjg@chromium.org |
---|---|
Headers | show |
Series | serial: Support a disabled serial port | expand |
Hi Simon, On 20/11/2024 16:35, Simon Glass wrote: > This series provides a way to tell a serial UART that it can't actually > work, at runtime. The main motivation is to deal with a coreboot feature > where it does not provide UART details in the sysinfo structure unless > the UART is also enabled in coreboot. Why is the UART driver probed if coreboot doesn't provide the necessary info for it? Couldn't you disable CONFIG_REQUIRE_SERIAL_CONSOLE and skip probing it? Kind regards, > > Attempts to introduce a way to enable a silent UART in coreboot have > lead to a large amount of discussion but no result, sadly. > > This series reworks a patch sent last year, which was not quite ready to > be applied. A coreboot user (on irc) hit this problem, so it needs to be > resolved. > > Link: https://patchwork.ozlabs.org/project/uboot/patch/20231101180447.99361-1-sjg@chromium.org/ > > Changes in v3: > - Put the feature behind a Kconfig > - Move the feature to the serial uclass, so any serial driver can use it > > Changes in v2: > - Drop RFC tag since there were no comments > > Simon Glass (3): > serial: Allow a serial port to be silent disabled > serial: ns16550: Avoid probing hardware when disabled > x86: coreboot: Make use of disabled console > > arch/x86/cpu/coreboot/Kconfig | 1 + > drivers/serial/Kconfig | 9 +++++++++ > drivers/serial/ns16550.c | 3 ++- > drivers/serial/serial-uclass.c | 19 ++++++++++++++++++- > drivers/serial/serial_coreboot.c | 2 ++ > include/serial.h | 25 +++++++++++++++++++++++++ > 6 files changed, 57 insertions(+), 2 deletions(-) >
Hi Caleb, On Wed, 20 Nov 2024 at 08:56, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > Hi Simon, > > On 20/11/2024 16:35, Simon Glass wrote: > > This series provides a way to tell a serial UART that it can't actually > > work, at runtime. The main motivation is to deal with a coreboot feature > > where it does not provide UART details in the sysinfo structure unless > > the UART is also enabled in coreboot. > > Why is the UART driver probed if coreboot doesn't provide the necessary > info for it? U-Boot doesn't do well without some sort of serial device. That could perhaps be improved. But the point here is that we need a single U-Boot build which can start from coreboot, regardless of what coreboot decides to do. > > Couldn't you disable CONFIG_REQUIRE_SERIAL_CONSOLE and skip probing it? No, because I want it to work if the serial info is available. [..] Regards, SImon
On 20/11/2024 17:03, Simon Glass wrote: > Hi Caleb, > > On Wed, 20 Nov 2024 at 08:56, Caleb Connolly <caleb.connolly@linaro.org> wrote: >> >> Hi Simon, >> >> On 20/11/2024 16:35, Simon Glass wrote: >>> This series provides a way to tell a serial UART that it can't actually >>> work, at runtime. The main motivation is to deal with a coreboot feature >>> where it does not provide UART details in the sysinfo structure unless >>> the UART is also enabled in coreboot. >> >> Why is the UART driver probed if coreboot doesn't provide the necessary >> info for it? > > U-Boot doesn't do well without some sort of serial device. That could > perhaps be improved. But the point here is that we need a single > U-Boot build which can start from coreboot, regardless of what > coreboot decides to do. Right, I'm suggesting that at runtime you notice that coreboot didn't provide a serial port definition (or gave you a bogus address or something) and skip probing the serial port in that case. > >> >> Couldn't you disable CONFIG_REQUIRE_SERIAL_CONSOLE and skip probing it? > > No, because I want it to work if the serial info is available. This option doesn't disable the serial port, it just makes U-Boot not panic if it can't find one. For example I enable this when building U-Boot for the upstream supported Qualcomm phones since many don't enable the serial port in their DTS. > > [..] > > Regards, > SImon
On Wed, Nov 20, 2024 at 05:37:35PM +0100, Caleb Connolly wrote: > > > On 20/11/2024 17:03, Simon Glass wrote: > > Hi Caleb, > > > > On Wed, 20 Nov 2024 at 08:56, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >> > >> Hi Simon, > >> > >> On 20/11/2024 16:35, Simon Glass wrote: > >>> This series provides a way to tell a serial UART that it can't actually > >>> work, at runtime. The main motivation is to deal with a coreboot feature > >>> where it does not provide UART details in the sysinfo structure unless > >>> the UART is also enabled in coreboot. > >> > >> Why is the UART driver probed if coreboot doesn't provide the necessary > >> info for it? > > > > U-Boot doesn't do well without some sort of serial device. That could > > perhaps be improved. But the point here is that we need a single > > U-Boot build which can start from coreboot, regardless of what > > coreboot decides to do. > > Right, I'm suggesting that at runtime you notice that coreboot didn't > provide a serial port definition (or gave you a bogus address or > something) and skip probing the serial port in that case. > > > >> > >> Couldn't you disable CONFIG_REQUIRE_SERIAL_CONSOLE and skip probing it? > > > > No, because I want it to work if the serial info is available. > > This option doesn't disable the serial port, it just makes U-Boot not > panic if it can't find one. For example I enable this when building > U-Boot for the upstream supported Qualcomm phones since many don't > enable the serial port in their DTS. Yes, this feels like an oddly coreboot specific solution to a generic problem that we should be able to handle already I thought? Like a Pi can have or not have a serial port enabled and still have video and we use that. We might well need something specific to know the port is disabled since I gather we aren't getting a devicetree that says status = disabled in it. But this ought to be a common enough general case?