On Tue, 1 Nov 2022, Martin Hundebøll wrote: > On 28/10/2022 11.40, Ilpo Järvinen wrote: > > On Fri, 28 Oct 2022, Ilpo Järvinen wrote: > > > > > On Wed, 26 Oct 2022, Ilpo Järvinen wrote: > > > > > > > On Tue, 25 Oct 2022, Martin Hundebøll wrote: > > > > > > > > > Skip registration of the platform device used for built-in ports, if > > > > > no > > > > > such ports are configured/created. > > > > > > > > > > Signed-off-by: Martin Hundebøll <martin@xxxxxxxxxx> > > > > > > > > For patches 1-3: > > > > > > > > Tested-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> > > > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> > > > > > > > > Please include these tags into the next version of your submission. > > > > Thank you. > > > > > > Actually, I just found out that some set of cmdline parameters do no > > > longer work the same. So I'm retracting both of my tags for now. > > > > > > =0 did work as expected due to this change which I tested and some other > > > values >4 but there now seems to be problem of the console not showing up > > > like previously when I don't give nr_uarts at all. > > > > NAK from me until the problem is resolved adequately. > > > > Already the patch 1/4 causes an unacceptable reassignment of ttySx > > targets. This is going to break people's cmdline console setups so you > > need to find a better way. > > > > Before any of these patches: > > > > [ 0.000000] Command line: console=ttyS0,115200n8 8250.nr_uarts=4 > > [ 0.021031] Kernel command line: console=ttyS0,115200n8 8250.nr_uarts=4 > > [ 0.441924] printk: console [ttyS0] enabled > > [ 2.243165] printk: console [ttyS0] disabled > > [ 2.245682] dw-apb-uart.6: ttyS0 at MMIO 0x4010006000 (irq = 33, > > base_baud = 115200) is a 16550A > > [ 4.010237] printk: console [ttyS0] enabled > > [ 5.933887] dw-apb-uart.7: ttyS1 at MMIO 0x4010007000 (irq = 16, > > base_baud = 6250000) is a 16550A > > [ 5.952829] dw-apb-uart.8: ttyS2 at MMIO 0x4010008000 (irq = 17, > > base_baud = 6250000) is a 16550A > > > > After 1/4 ttyS0 is no longer the same: > > > > [ 0.000000] Command line: console=ttyS0,115200n8 8250.nr_uarts=4 > > [ 0.021023] Kernel command line: console=ttyS0,115200n8 8250.nr_uarts=4 > > [ 0.441872] printk: console [ttyS0] enabled > > [ 2.233584] dw-apb-uart.6: ttyS4 at MMIO 0x4010006000 (irq = 33, > > base_baud = 115200) is a 16550A > > [ 2.241955] dw-apb-uart.7: ttyS5 at MMIO 0x4010007000 (irq = 16, > > base_baud = 6250000) is a 16550A > > [ 2.249804] dw-apb-uart.8: ttyS6 at MMIO 0x4010008000 (irq = 17, > > base_baud = 6250000) is a 16550A > > Thanks for testing this. > > The old behavior is wrong: your designware ports replace the built-in ones (0 > to 3) instead of using the unused ones (4 to 31). With these patches, it acts > as I'd expect: the built-in ports are kept, and any later discovered ports > follow. > > Yes, breaking existing systems in this way is unacceptable. I'm not sure how > to approach this, but I'm inclined to introduce a new config variable to keep > the broken behavior? To me this looks now more an attempt to repurpose the nr_uarts to mean something it really hasn't. Clearly it hasn't meant non-discoverable ports earlier but something related to the maximum number ports. This also explains why we had that misunderstanding about the meaning of nr_uarts between us earlier. The code which makes this "wrong behavior" to happen is the last loop in serial8250_find_match_or_unused(). Given its comment, I think it has been very much intentional behavior. Pretending that non-discoverable port is more real/precious than a port that was later _discovered_ and is very much a real one is what brings you to this renumbering issue. I'm not anymore sure about your goals really. Now it sounds one of the goals is preserving the non-discoverable ports, whereas previously it was just about allowing 0 of them. How about you add entirely new CONFIG and/or param for this minimum number of non-discoverable ports and make the last resort loop (or perhaps all but the first loop) in serial8250_find_match_or_unused() to honor that (start looking only from port above that index). If it defaults to 0, I think this renumbering issue is avoided. Would it work for all the goals you have? With that change, I don't know what condition should trigger what you do in 3/4 though. -- i.
