Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30608
Change subject: [WIP]drivers/cavium: Add UART PCI driver ......................................................................
[WIP]drivers/cavium: Add UART PCI driver
Move PCI handling from cn81xx/soc to its own pci driver.
Change-Id: I0fa2f086aba9b4f9c6dba7a35a84ea61c5fa64e4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/drivers/cavium/pci-cn8xxx/Kconfig A src/drivers/cavium/pci-cn8xxx/Makefile.inc A src/drivers/cavium/pci-cn8xxx/uart.c M src/include/device/pci_ids.h M src/soc/cavium/cn81xx/soc.c 5 files changed, 57 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/30608/1
diff --git a/src/drivers/cavium/pci-cn8xxx/Kconfig b/src/drivers/cavium/pci-cn8xxx/Kconfig new file mode 100644 index 0000000..1de774f --- /dev/null +++ b/src/drivers/cavium/pci-cn8xxx/Kconfig @@ -0,0 +1,7 @@ +config DRIVERS_CAVIUM + bool + depends on SOC_CAVIUM_COMMON + default y if SOC_CAVIUM_COMMON + default n + help + When enabled, adds PCI drivers for Cavium SoCs. diff --git a/src/drivers/cavium/pci-cn8xxx/Makefile.inc b/src/drivers/cavium/pci-cn8xxx/Makefile.inc new file mode 100644 index 0000000..d85fc3e --- /dev/null +++ b/src/drivers/cavium/pci-cn8xxx/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_CAVIUM) += uart.c diff --git a/src/drivers/cavium/pci-cn8xxx/uart.c b/src/drivers/cavium/pci-cn8xxx/uart.c new file mode 100644 index 0000000..2230d5d --- /dev/null +++ b/src/drivers/cavium/pci-cn8xxx/uart.c @@ -0,0 +1,46 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Facebook, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <device/device.h> +#include <console/console.h> +#include <soc/uart.h> +#include <device/pci.h> +#include <device/pci_ids.h> + +static void cavium_uart_init(struct device *dev) +{ + printk(BIOS_ERR, "UART: debug\n"); + + /* using device enable state from devicetree.cb */ + if (dev->enabled) { + const u8 fn = PCI_FUNC(dev->path.pci.devfn); + + /* Calling uart_setup with no baudrate will do minimal HW + * enough for the kernel to not panic */ + if (!uart_is_enabled(fn)) + uart_setup(fn, 0); + } +} + +static struct device_operations device_ops = { + .init = cavium_uart_init, +}; + +static const struct pci_driver soc_cavium_uart __pci_driver = { + .ops = &device_ops, + .vendor = PCI_VENDOR_CAVIUM, + .device = PCI_DEVICE_ID_CAVIUM_THUNDERX_UART, +}; diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 751cca0..ba55f1d 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -132,6 +132,9 @@ #define PCI_DEVICE_ID_BERKOM_A4T 0xffa4 #define PCI_DEVICE_ID_BERKOM_SCITEL_QUADRO 0xffa8
+#define PCI_VENDOR_CAVIUM 0x177d +#define PCI_DEVICE_ID_CAVIUM_THUNDERX_UART 0xa00f + #define PCI_VENDOR_ID_COMPAQ 0x0e11 #define PCI_DEVICE_ID_COMPAQ_TOKENRING 0x0508 #define PCI_DEVICE_ID_COMPAQ_1280 0x3033 diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 2046d21..0748096 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -388,18 +388,6 @@ } }
- /* Init UARTs */ - size_t i; - struct device *uart_dev; - for (i = 0; i <= 3; i++) { - uart_dev = dev_find_slot(1, PCI_DEVFN(8, i)); - /* using device enable state from devicetree.cb */ - if (uart_dev && uart_dev->enabled) { - if (!uart_is_enabled(i)) - uart_setup(i, 0); - } - } - if (IS_ENABLED(CONFIG_ARM64_USE_ARM_TRUSTED_FIRMWARE)) soc_init_atf(); }
Hello Kyösti Mälkki, David Hendricks, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30608
to look at the new patch set (#2).
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
drivers/cavium: Add UART PCI driver
Add UART PCI driver in cavium/common/pci.
Tested on opencellular/elgon. The UART is still initialized and usable in Linux.
Change-Id: I0fa2f086aba9b4f9c6dba7a35a84ea61c5fa64e4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/include/device/pci_ids.h M src/soc/cavium/cn81xx/soc.c M src/soc/cavium/common/Makefile.inc A src/soc/cavium/common/pci/Makefile.inc A src/soc/cavium/common/pci/uart.c 5 files changed, 47 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/30608/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30608 )
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
Patch Set 2: Code-Review+1
On a related topic; I looked at definitions of 'struct cn81xx_uart' and 'struct pl011_uart'. Since those need to match hardware register layout, shouldn't they have attribute __packed applied? Or are we happy with the check_member() asserts here that check overall size.
As for the bitfields in 'union cn81xx_uart_ctl'; from what I remember C standard does not dictate if first field occupies MSB or LSB, and its relevant here as well.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30608 )
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
Patch Set 2:
On a related topic; I looked at definitions of 'struct cn81xx_uart' and 'struct pl011_uart'. Since those need to match hardware register layout, shouldn't they have attribute __packed applied? Or are we happy with the check_member() asserts here that check overall size.
As for the bitfields in 'union cn81xx_uart_ctl'; from what I remember C standard does not dictate if first field occupies MSB or LSB, and its relevant here as well.
Yes, they should be packed and should work for all endianes. ATM the Cavium Soc always operate in LE mode.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30608 )
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/30608/2/src/soc/cavium/common/pci/uart.c File src/soc/cavium/common/pci/uart.c:
https://review.coreboot.org/#/c/30608/2/src/soc/cavium/common/pci/uart.c@18 PS2, Line 18: #include <console/console.h> Doesn't seem used.
Hello Kyösti Mälkki, David Hendricks, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30608
to look at the new patch set (#3).
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
drivers/cavium: Add UART PCI driver
Add UART PCI driver in cavium/common/pci.
Tested on opencellular/elgon. The UART is still initialized and usable in Linux.
Change-Id: I0fa2f086aba9b4f9c6dba7a35a84ea61c5fa64e4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/include/device/pci_ids.h M src/soc/cavium/cn81xx/soc.c M src/soc/cavium/common/Makefile.inc A src/soc/cavium/common/pci/Makefile.inc A src/soc/cavium/common/pci/uart.c 5 files changed, 46 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/30608/3
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30608 )
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/30608/2/src/soc/cavium/common/pci/uart.c File src/soc/cavium/common/pci/uart.c:
https://review.coreboot.org/#/c/30608/2/src/soc/cavium/common/pci/uart.c@18 PS2, Line 18: #include <console/console.h>
Doesn't seem used.
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30608 )
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
Patch Set 3: Code-Review+2
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30608 )
Change subject: drivers/cavium: Add UART PCI driver ......................................................................
drivers/cavium: Add UART PCI driver
Add UART PCI driver in cavium/common/pci.
Tested on opencellular/elgon. The UART is still initialized and usable in Linux.
Change-Id: I0fa2f086aba9b4f9c6dba7a35a84ea61c5fa64e4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/30608 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/include/device/pci_ids.h M src/soc/cavium/cn81xx/soc.c M src/soc/cavium/common/Makefile.inc A src/soc/cavium/common/pci/Makefile.inc A src/soc/cavium/common/pci/uart.c 5 files changed, 46 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 2d19194..9be9759 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -132,6 +132,9 @@ #define PCI_DEVICE_ID_BERKOM_A4T 0xffa4 #define PCI_DEVICE_ID_BERKOM_SCITEL_QUADRO 0xffa8
+#define PCI_VENDOR_CAVIUM 0x177d +#define PCI_DEVICE_ID_CAVIUM_THUNDERX_UART 0xa00f + #define PCI_VENDOR_ID_COMPAQ 0x0e11 #define PCI_DEVICE_ID_COMPAQ_TOKENRING 0x0508 #define PCI_DEVICE_ID_COMPAQ_1280 0x3033 diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 4b265d7..2358d71 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -381,18 +381,6 @@ } }
- /* Init UARTs */ - size_t i; - struct device *uart_dev; - for (i = 0; i <= 3; i++) { - uart_dev = dev_find_slot(1, PCI_DEVFN(8, i)); - /* using device enable state from devicetree.cb */ - if (uart_dev && uart_dev->enabled) { - if (!uart_is_enabled(i)) - uart_setup(i, 0); - } - } - if (IS_ENABLED(CONFIG_ARM64_USE_ARM_TRUSTED_FIRMWARE)) soc_init_atf(); } diff --git a/src/soc/cavium/common/Makefile.inc b/src/soc/cavium/common/Makefile.inc index ada8286..ecde220 100644 --- a/src/soc/cavium/common/Makefile.inc +++ b/src/soc/cavium/common/Makefile.inc @@ -15,6 +15,8 @@
ifeq ($(CONFIG_SOC_CAVIUM_COMMON),y)
+subdirs-y += pci + CFLAGS_arm64 += -Wstack-usage=$(CONFIG_STACK_SIZE)
bootblock-$(CONFIG_BOOTBLOCK_CUSTOM) += bootblock.c diff --git a/src/soc/cavium/common/pci/Makefile.inc b/src/soc/cavium/common/pci/Makefile.inc new file mode 100644 index 0000000..1304052 --- /dev/null +++ b/src/soc/cavium/common/pci/Makefile.inc @@ -0,0 +1 @@ +ramstage-y += uart.c diff --git a/src/soc/cavium/common/pci/uart.c b/src/soc/cavium/common/pci/uart.c new file mode 100644 index 0000000..6e41e1d --- /dev/null +++ b/src/soc/cavium/common/pci/uart.c @@ -0,0 +1,40 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 9Elements GmbH patrick.rudolph@9elements.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <device/device.h> +#include <soc/uart.h> +#include <device/pci.h> +#include <device/pci_ids.h> + +static void cavium_uart_init(struct device *dev) +{ + const u8 fn = PCI_FUNC(dev->path.pci.devfn); + + /* Calling uart_setup with no baudrate will do minimal HW init + * enough for the kernel to not panic */ + if (!uart_is_enabled(fn)) + uart_setup(fn, 0); +} + +static struct device_operations device_ops = { + .init = cavium_uart_init, +}; + +static const struct pci_driver soc_cavium_uart __pci_driver = { + .ops = &device_ops, + .vendor = PCI_VENDOR_CAVIUM, + .device = PCI_DEVICE_ID_CAVIUM_THUNDERX_UART, +};
-
Kyösti Mälkki (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)