[U-Boot] [PATCH 0/5] x86: Support PCI based UART as the U-Boot serial console

Newer x86 Platform Controller Hub chipset (like Topcliff, BayTrail) starts to integrate NS16550 compatible PCI UART devices. In order to use them, we have to scan the PCI bus and allocate memory/io address in the early phase.
On Intel Crown Bay board, there are 4 UART DB9 connectors, one of which is from the superio legacy serial port while the other 3 are connected to the Topcliff PCH UART devices.
The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Bin Meng (5): x86: Add missing DECLARE_GLOBAL_DATA_PTR for pci.c x86: Support pci bus scan in the early phase x86: Add an API for finding pci devices in the early phase x86: Support PCI UART in the x86_serial driver x86: Add PCI UART related defines in crownbay.h
arch/x86/cpu/pci.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ include/configs/crownbay.h | 5 +++++ 4 files changed, 81 insertions(+)

Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/pci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index f3492c3..404fbb6 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -15,6 +15,8 @@ #include <pci.h> #include <asm/pci.h>
+DECLARE_GLOBAL_DATA_PTR; + static struct pci_controller x86_hose;
int pci_early_init_hose(struct pci_controller **hosep)

On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 2 ++ 1 file changed, 2 insertions(+)
Please can you always add a commit message even if only one line?
Regards, Simon

Hi Simon,
On Sat, Dec 20, 2014 at 1:40 AM, Simon Glass sjg@chromium.org wrote:
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 2 ++ 1 file changed, 2 insertions(+)
Please can you always add a commit message even if only one line?
OK.
Regards, Bin

On x86, some peripherals on pci buses need to be accessed in the early phase (eg: pci uart) with a valid pci memory/io address, thus scan the pci bus and do the corresponding resource allocation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/pci.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 404fbb6..1eee08b 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -29,6 +29,7 @@ int pci_early_init_hose(struct pci_controller **hosep)
board_pci_setup_hose(hose); pci_setup_type1(hose); + hose->last_busno = pci_hose_scan(hose); gd->arch.hose = hose; *hosep = hose;

This new API pci_early_find_devices() is derived from the generic version of pci_find_devices() with modifications required in the early phase (like hose, config space access routines).
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 1eee08b..cdfb981 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -111,3 +111,44 @@ void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) { pci_hose_write_config_dword(get_hose(), dev, where, value); } + +pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index) +{ + struct pci_controller *hose = gd->arch.hose; + u16 vendor, device; + u8 header_type; + pci_dev_t bdf; + int i, bus, found_multi = 0; + + for (bus = hose->first_busno; bus <= hose->last_busno; bus++) + for (bdf = PCI_BDF(bus, 0, 0); + bdf < PCI_BDF(bus + 1, 0, 0); + bdf += PCI_BDF(0, 0, 1)) { + if (pci_skip_dev(hose, bdf)) + continue; + + if (!PCI_FUNC(bdf)) { + header_type = pci_read_config8(bdf, + PCI_HEADER_TYPE); + + found_multi = header_type & 0x80; + } else { + if (!found_multi) + continue; + } + + vendor = pci_read_config16(bdf, PCI_VENDOR_ID); + device = pci_read_config16(bdf, PCI_DEVICE_ID); + + for (i = 0; ids[i].vendor != 0; i++) { + if (vendor == ids[i].vendor && + device == ids[i].device) { + if (index <= 0) + return bdf; + index--; + } + } + } + + return -1; +} diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h index ac1a808..a3550fb 100644 --- a/arch/x86/include/asm/pci.h +++ b/arch/x86/include/asm/pci.h @@ -49,4 +49,6 @@ void pci_write_config8(pci_dev_t dev, unsigned where, unsigned value); void pci_write_config16(pci_dev_t dev, unsigned where, unsigned value); void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value);
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index); + #endif

Hi Bin,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
This new API pci_early_find_devices() is derived from the generic version of pci_find_devices() with modifications required in the early phase (like hose, config space access routines).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 1eee08b..cdfb981 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -111,3 +111,44 @@ void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) { pci_hose_write_config_dword(get_hose(), dev, where, value); }
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index)
Can't we just call the normal function? We have an early 'hose'...
+{
struct pci_controller *hose = gd->arch.hose;
u16 vendor, device;
u8 header_type;
pci_dev_t bdf;
int i, bus, found_multi = 0;
for (bus = hose->first_busno; bus <= hose->last_busno; bus++)
for (bdf = PCI_BDF(bus, 0, 0);
bdf < PCI_BDF(bus + 1, 0, 0);
bdf += PCI_BDF(0, 0, 1)) {
if (pci_skip_dev(hose, bdf))
continue;
if (!PCI_FUNC(bdf)) {
header_type = pci_read_config8(bdf,
PCI_HEADER_TYPE);
found_multi = header_type & 0x80;
} else {
if (!found_multi)
continue;
}
vendor = pci_read_config16(bdf, PCI_VENDOR_ID);
device = pci_read_config16(bdf, PCI_DEVICE_ID);
for (i = 0; ids[i].vendor != 0; i++) {
if (vendor == ids[i].vendor &&
device == ids[i].device) {
if (index <= 0)
return bdf;
index--;
}
}
}
return -1;
+} diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h index ac1a808..a3550fb 100644 --- a/arch/x86/include/asm/pci.h +++ b/arch/x86/include/asm/pci.h @@ -49,4 +49,6 @@ void pci_write_config8(pci_dev_t dev, unsigned where, unsigned value); void pci_write_config16(pci_dev_t dev, unsigned where, unsigned value); void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value);
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index);
#endif
Regards, Simon

Hi Simon,
On Sat, Dec 20, 2014 at 5:53 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
This new API pci_early_find_devices() is derived from the generic version of pci_find_devices() with modifications required in the early phase (like hose, config space access routines).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 1eee08b..cdfb981 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -111,3 +111,44 @@ void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) { pci_hose_write_config_dword(get_hose(), dev, where, value); }
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index)
Can't we just call the normal function? We have an early 'hose'...
We can, but doing this way requires a large update of the drivers/pci/pci.c file, and requires every architecture to have an early hose in its global data. So that early hose becomes a generic feature for every arch we support, not only x86. Do you want to do this?
[snip]
Regards, Bin

Hi Bin,
On 19 December 2014 at 19:37, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:53 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
This new API pci_early_find_devices() is derived from the generic version of pci_find_devices() with modifications required in the early phase (like hose, config space access routines).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 1eee08b..cdfb981 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -111,3 +111,44 @@ void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) { pci_hose_write_config_dword(get_hose(), dev, where, value); }
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index)
Can't we just call the normal function? We have an early 'hose'...
We can, but doing this way requires a large update of the drivers/pci/pci.c file, and requires every architecture to have an early hose in its global data. So that early hose becomes a generic feature for every arch we support, not only x86. Do you want to do this?
Are you sure it requires a large update? The idea with the early PCI support was that it would use the same data structures, etc. and avoid duplicating code. In fact I avoided using the x86-specific config calls because I didn't want to invent a parallel API.
Yes I think early hose should be a generic feature potentially.
Regards, Simon

Hi Simon,
On Sat, Dec 20, 2014 at 12:56 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 19:37, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:53 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
This new API pci_early_find_devices() is derived from the generic version of pci_find_devices() with modifications required in the early phase (like hose, config space access routines).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 1eee08b..cdfb981 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -111,3 +111,44 @@ void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) { pci_hose_write_config_dword(get_hose(), dev, where, value); }
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index)
Can't we just call the normal function? We have an early 'hose'...
We can, but doing this way requires a large update of the drivers/pci/pci.c file, and requires every architecture to have an early hose in its global data. So that early hose becomes a generic feature for every arch we support, not only x86. Do you want to do this?
Are you sure it requires a large update? The idea with the early PCI support was that it would use the same data structures, etc. and avoid duplicating code. In fact I avoided using the x86-specific config calls because I didn't want to invent a parallel API.
Yes I think early hose should be a generic feature potentially.
OK, I will try to make the early hose a generic feature.
Regards, Bin

Hi Bin,
On 19 December 2014 at 22:34, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 12:56 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 19:37, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:53 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
This new API pci_early_find_devices() is derived from the generic version of pci_find_devices() with modifications required in the early phase (like hose, config space access routines).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/pci.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 1eee08b..cdfb981 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -111,3 +111,44 @@ void pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) { pci_hose_write_config_dword(get_hose(), dev, where, value); }
+pci_dev_t pci_early_find_devices(struct pci_device_id *ids, int index)
Can't we just call the normal function? We have an early 'hose'...
We can, but doing this way requires a large update of the drivers/pci/pci.c file, and requires every architecture to have an early hose in its global data. So that early hose becomes a generic feature for every arch we support, not only x86. Do you want to do this?
Are you sure it requires a large update? The idea with the early PCI support was that it would use the same data structures, etc. and avoid duplicating code. In fact I avoided using the x86-specific config calls because I didn't want to invent a parallel API.
Yes I think early hose should be a generic feature potentially.
OK, I will try to make the early hose a generic feature.
OK. Perhaps it just needs to be called from x86_cpu_init_f()?
Regards, Simon

Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h> + +DECLARE_GLOBAL_DATA_PTR; + +#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = { + CONFIG_PCI_UART_DEV, + { } +}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART + pci_dev_t devbusfn; +#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev); @@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART + /* + * Newer x86 Platform Controller Hub chipset starts to integrate + * NS16550 compatible PCI UART devices. The board configuration + * file needs to supply the PCI UART vendor ID and device ID via + * CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot + * serial console. + */ + devbusfn = pci_early_find_devices(uart_supported, 0); + if (devbusfn == -1) + return -ENODEV; + + /* override the register base here for PCI UART */ + plat->base = pci_read_bar32(gd->arch.hose, devbusfn, 0); +#endif + return 0; } U_BOOT_DRIVER(serial_ns16550) = {

Hi BIn,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h>
+DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = {
CONFIG_PCI_UART_DEV,
{ }
+}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART
pci_dev_t devbusfn;
+#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev);
@@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART
/*
* Newer x86 Platform Controller Hub chipset starts to integrate
* NS16550 compatible PCI UART devices. The board configuration
* file needs to supply the PCI UART vendor ID and device ID via
* CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
* serial console.
*/
devbusfn = pci_early_find_devices(uart_supported, 0);
if (devbusfn == -1)
return -ENODEV;
I'm not sure why we need this. Is it because we don't know the device address of the UART?
/* override the register base here for PCI UART */
plat->base = pci_read_bar32(gd->arch.hose, devbusfn, 0);
+#endif
return 0;
} U_BOOT_DRIVER(serial_ns16550) = { -- 1.8.2.1
Regards, Simon

Hi Simon,
On Sat, Dec 20, 2014 at 5:52 AM, Simon Glass sjg@chromium.org wrote:
Hi BIn,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h>
+DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = {
CONFIG_PCI_UART_DEV,
{ }
+}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART
pci_dev_t devbusfn;
+#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev);
@@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART
/*
* Newer x86 Platform Controller Hub chipset starts to integrate
* NS16550 compatible PCI UART devices. The board configuration
* file needs to supply the PCI UART vendor ID and device ID via
* CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
* serial console.
*/
devbusfn = pci_early_find_devices(uart_supported, 0);
if (devbusfn == -1)
return -ENODEV;
I'm not sure why we need this. Is it because we don't know the device address of the UART?
Yes, the UART device is not on the host bridge (bus 0). It is on the PCH which is connected to the one of the downstream PCIe port of the host bridge. Which PCIe port is used is determined by the board designer. So that on my Crown Bay board the UART is on b.d.f 2.10.1 may become b.d.f 3.10.1 on some other queensbay platform board. Actually the pci_find_devices() is a standard way to locate the device's b.d.f, as you see in many PCI ethernet drivers in U-Boot. You have no way to figure out the device will be inserted to which PCI slot on the board.
[snip]
Regards, Bin

Hi Bin,
On 19 December 2014 at 19:43, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:52 AM, Simon Glass sjg@chromium.org wrote:
Hi BIn,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h>
+DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = {
CONFIG_PCI_UART_DEV,
{ }
+}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART
pci_dev_t devbusfn;
+#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev);
@@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART
/*
* Newer x86 Platform Controller Hub chipset starts to integrate
* NS16550 compatible PCI UART devices. The board configuration
* file needs to supply the PCI UART vendor ID and device ID via
* CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
* serial console.
*/
devbusfn = pci_early_find_devices(uart_supported, 0);
if (devbusfn == -1)
return -ENODEV;
I'm not sure why we need this. Is it because we don't know the device address of the UART?
Yes, the UART device is not on the host bridge (bus 0). It is on the PCH which is connected to the one of the downstream PCIe port of the host bridge. Which PCIe port is used is determined by the board designer. So that on my Crown Bay board the UART is on b.d.f 2.10.1 may become b.d.f 3.10.1 on some other queensbay platform board. Actually the pci_find_devices() is a standard way to locate the device's b.d.f, as you see in many PCI ethernet drivers in U-Boot. You have no way to figure out the device will be inserted to which PCI slot on the board.
OK I see, thanks. Is it possible to find the serial port by class rather than having to specify every vendor/ID? If not, is there a binding that allows us to add device tree nodes for the serial ports (even with just vendor/device, not full PCI bus address) and be able to specify which is the console that way? It would be good to avoid a new CONFIG if we can. It feels odd to override an existing device tree node.
Regards, Simon

Hi Simon,
On Sat, Dec 20, 2014 at 1:00 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 19:43, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:52 AM, Simon Glass sjg@chromium.org wrote:
Hi BIn,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h>
+DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = {
CONFIG_PCI_UART_DEV,
{ }
+}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART
pci_dev_t devbusfn;
+#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev);
@@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART
/*
* Newer x86 Platform Controller Hub chipset starts to integrate
* NS16550 compatible PCI UART devices. The board configuration
* file needs to supply the PCI UART vendor ID and device ID via
* CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
* serial console.
*/
devbusfn = pci_early_find_devices(uart_supported, 0);
if (devbusfn == -1)
return -ENODEV;
I'm not sure why we need this. Is it because we don't know the device address of the UART?
Yes, the UART device is not on the host bridge (bus 0). It is on the PCH which is connected to the one of the downstream PCIe port of the host bridge. Which PCIe port is used is determined by the board designer. So that on my Crown Bay board the UART is on b.d.f 2.10.1 may become b.d.f 3.10.1 on some other queensbay platform board. Actually the pci_find_devices() is a standard way to locate the device's b.d.f, as you see in many PCI ethernet drivers in U-Boot. You have no way to figure out the device will be inserted to which PCI slot on the board.
OK I see, thanks. Is it possible to find the serial port by class rather than having to specify every vendor/ID? If not, is there a binding that allows us to add device tree nodes for the serial ports (even with just vendor/device, not full PCI bus address) and be able to specify which is the console that way? It would be good to avoid a new CONFIG if we can. It feels odd to override an existing device tree node.
Unforuntately not like USB OHCI/EHCI, we cannot use class to determine it is a serial port device. Currently there is no device tree bindings for PCI device drivers. Maybe this can be done when driver model supports PCI?
Regards, Bin

HI Bin,
On 19 December 2014 at 22:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 1:00 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 19:43, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:52 AM, Simon Glass sjg@chromium.org wrote:
Hi BIn,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h>
+DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = {
CONFIG_PCI_UART_DEV,
{ }
+}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART
pci_dev_t devbusfn;
+#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev);
@@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART
/*
* Newer x86 Platform Controller Hub chipset starts to integrate
* NS16550 compatible PCI UART devices. The board configuration
* file needs to supply the PCI UART vendor ID and device ID via
* CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
* serial console.
*/
devbusfn = pci_early_find_devices(uart_supported, 0);
if (devbusfn == -1)
return -ENODEV;
I'm not sure why we need this. Is it because we don't know the device address of the UART?
Yes, the UART device is not on the host bridge (bus 0). It is on the PCH which is connected to the one of the downstream PCIe port of the host bridge. Which PCIe port is used is determined by the board designer. So that on my Crown Bay board the UART is on b.d.f 2.10.1 may become b.d.f 3.10.1 on some other queensbay platform board. Actually the pci_find_devices() is a standard way to locate the device's b.d.f, as you see in many PCI ethernet drivers in U-Boot. You have no way to figure out the device will be inserted to which PCI slot on the board.
OK I see, thanks. Is it possible to find the serial port by class rather than having to specify every vendor/ID? If not, is there a binding that allows us to add device tree nodes for the serial ports (even with just vendor/device, not full PCI bus address) and be able to specify which is the console that way? It would be good to avoid a new CONFIG if we can. It feels odd to override an existing device tree node.
Unforuntately not like USB OHCI/EHCI, we cannot use class to determine it is a serial port device. Currently there is no device tree bindings for PCI device drivers. Maybe this can be done when driver model supports PCI?
Sorry for the delay.
If you look here you can see a class called ''16550 compatable serial controller'
http://www.acm.uiuc.edu/sigops/roll_your_own/7.c.1.html
If that means what it says it might be good enough.
I looked through the bindings and it is possible to use PCI with device tree.
http://www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf
So one option would be to actually add your serial ports for this board to the device tree. Then you could use the stdout property to specify which is used for the serial port, and everything should just work.
Let me know what you think about that idea. With this patch I worry that we will end up adding a big long list of PCI IDs plus not have control of the console UART.
I'm not opposed to this patch if it is the best way, I just want to make sure we have explored other options.
Regards, Simon

Hi Simon,
On Tue, Dec 23, 2014 at 8:52 AM, Simon Glass sjg@chromium.org wrote:
HI Bin,
On 19 December 2014 at 22:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 1:00 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 December 2014 at 19:43, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 20, 2014 at 5:52 AM, Simon Glass sjg@chromium.org wrote:
Hi BIn,
On 19 December 2014 at 00:19, Bin Meng bmeng.cn@gmail.com wrote:
Newer x86 Platform Controller Hub chipset starts to integrate NS16550 compatible PCI UART devices. The board configuration file needs to supply the PCI UART vendor ID and device ID via CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot serial console.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/serial/serial_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c index e81e035..7ddd596 100644 --- a/drivers/serial/serial_x86.c +++ b/drivers/serial/serial_x86.c @@ -8,6 +8,17 @@ #include <dm.h> #include <ns16550.h> #include <serial.h> +#include <asm/pci.h> +#include <errno.h>
+DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCI_UART +static struct pci_device_id uart_supported[] = {
CONFIG_PCI_UART_DEV,
{ }
+}; +#endif
static const struct udevice_id x86_serial_ids[] = { { .compatible = "x86-uart" }, @@ -17,6 +28,9 @@ static const struct udevice_id x86_serial_ids[] = { static int x86_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev_get_platdata(dev); +#ifdef CONFIG_PCI_UART
pci_dev_t devbusfn;
+#endif int ret;
ret = ns16550_serial_ofdata_to_platdata(dev);
@@ -24,6 +38,22 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev) return ret; plat->clock = 1843200;
+#ifdef CONFIG_PCI_UART
/*
* Newer x86 Platform Controller Hub chipset starts to integrate
* NS16550 compatible PCI UART devices. The board configuration
* file needs to supply the PCI UART vendor ID and device ID via
* CONFIG_PCI_UART_DEV if we want to use the PCI UART as the U-Boot
* serial console.
*/
devbusfn = pci_early_find_devices(uart_supported, 0);
if (devbusfn == -1)
return -ENODEV;
I'm not sure why we need this. Is it because we don't know the device address of the UART?
Yes, the UART device is not on the host bridge (bus 0). It is on the PCH which is connected to the one of the downstream PCIe port of the host bridge. Which PCIe port is used is determined by the board designer. So that on my Crown Bay board the UART is on b.d.f 2.10.1 may become b.d.f 3.10.1 on some other queensbay platform board. Actually the pci_find_devices() is a standard way to locate the device's b.d.f, as you see in many PCI ethernet drivers in U-Boot. You have no way to figure out the device will be inserted to which PCI slot on the board.
OK I see, thanks. Is it possible to find the serial port by class rather than having to specify every vendor/ID? If not, is there a binding that allows us to add device tree nodes for the serial ports (even with just vendor/device, not full PCI bus address) and be able to specify which is the console that way? It would be good to avoid a new CONFIG if we can. It feels odd to override an existing device tree node.
Unforuntately not like USB OHCI/EHCI, we cannot use class to determine it is a serial port device. Currently there is no device tree bindings for PCI device drivers. Maybe this can be done when driver model supports PCI?
Sorry for the delay.
If you look here you can see a class called ''16550 compatable serial controller'
http://www.acm.uiuc.edu/sigops/roll_your_own/7.c.1.html
If that means what it says it might be good enough.
Yep, I missed this and looks like this should be enough for the ns16550 compatible pci uart devices.
I looked through the bindings and it is possible to use PCI with device tree.
http://www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf
So one option would be to actually add your serial ports for this board to the device tree. Then you could use the stdout property to specify which is used for the serial port, and everything should just work.
OK, let me try it.
Let me know what you think about that idea. With this patch I worry that we will end up adding a big long list of PCI IDs plus not have control of the console UART.
I'm not opposed to this patch if it is the best way, I just want to make sure we have explored other options.
Regards, Bin

The Topcliff PCH has 4 UART devices integrated (Device 10, Funciton 1/2/3/4). Add macros to enable them, but by default the legacy serial port (io addr 0x3f8) is still used on Crown Bay as the console port.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
include/configs/crownbay.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index eadb339..7b0966e 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -23,6 +23,11 @@ #define CONFIG_X86_SERIAL #define CONFIG_SMSC_LPC47M
+/* Turn on this macro if using PCI UART as the U-Boot serial console */ +#undef CONFIG_PCI_UART +#define CONFIG_PCI_UART_DEV \ + {PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_UART_1} + #define CONFIG_PCI_MEM_BUS 0x40000000 #define CONFIG_PCI_MEM_PHYS CONFIG_PCI_MEM_BUS #define CONFIG_PCI_MEM_SIZE 0x80000000
participants (2)
-
Bin Meng
-
Simon Glass