On Fri, Apr 03, 2020 at 05:46:19AM -0700, Bin Meng wrote:
Currently the driver gets ns16550 base address in the driver probe() routine, which may potentially break any ns16550 wrapper driver that does additional initialization before calling ns16550_serial_probe().
Things are complicated that we need consider ns16550 devices on both simple-bus and PCI bus. To fix the issue we move the base address assignment for simple-bus ns16550 device back to the ofdata_to_platdata(), and assign base address for PCI ns16550 device in ns16550_serial_probe().
This is still not perfect. Ideally if any PCI bus based ns16550 wrapper driver tries to access plat->base before calling probe(), it is subject to break.
Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from ofdata_to_platdata() to probe()") Reported-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Signed-off-by: Bin Meng bmeng.cn@gmail.com Tested-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
...
- if (addr == FDT_ADDR_T_NONE)
return -EINVAL;-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
- plat->base = addr;
-#else
- plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
-#endif
- return 0;
(1) for below.
...
addr = devfdt_get_addr_pci(dev);
if (addr == FDT_ADDR_T_NONE)return -EINVAL;+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
plat->base = addr;+#else
plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);#endif
And now is the question why we can't use above as a helper in both cases (either with check for address correctness or without)?
- }
...
- addr = dev_read_addr(dev);
- if (addr != FDT_ADDR_T_NONE) {
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
plat->base = addr;+#else
plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);+#endif
- } else if (!device_is_on_pci_bus(dev)) {
return -EINVAL;- }
Something like
addr = dev_read_addr(dev); ret = ns16550_assign_base(addr, plat); if (ret && !device_is_on_pci_bus(...)) return ret;