* [U-Boot] [PATCH v2 1/4] drivers: pci_ep: Introduce UCLASS_PCI_EP uclass
2019-04-24 18:53 [U-Boot] [PATCH v2 0/4] Add Cadence PCIe endpoint driver with new uclass Ramon Fried
@ 2019-04-24 18:53 ` Ramon Fried
2019-04-24 23:33 ` Simon Glass
2019-04-24 18:53 ` [U-Boot] [PATCH v2 2/4] pci_ep: add Cadence PCIe endpoint driver Ramon Fried
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Ramon Fried @ 2019-04-24 18:53 UTC (permalink / raw)
To: u-boot
Introduce new UCLASS_PCI_EP class for handling PCI endpoint
devices, allowing to set various attributes of the PCI endpoint
device, such as:
* configuration space header
* BAR definitions
* outband memory mapping
* start/stop PCI link
Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---
Changes in V2:
- Changed u8/u16... to uint
- Changed EINVAL to ENOSYS
- Changed void funcs to int
- Added a bit more info in comments
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/pci_endpoint/Kconfig | 17 ++
drivers/pci_endpoint/Makefile | 6 +
drivers/pci_endpoint/pci_ep-uclass.c | 189 +++++++++++++
include/dm/uclass-id.h | 1 +
include/pci_ep.h | 388 +++++++++++++++++++++++++++
7 files changed, 604 insertions(+)
create mode 100644 drivers/pci_endpoint/Kconfig
create mode 100644 drivers/pci_endpoint/Makefile
create mode 100644 drivers/pci_endpoint/pci_ep-uclass.c
create mode 100644 include/pci_ep.h
diff --git a/drivers/Kconfig b/drivers/Kconfig
index e6702eced4..258dfa19e8 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -64,6 +64,8 @@ source "drivers/nvme/Kconfig"
source "drivers/pci/Kconfig"
+source "drivers/pci_endpoint/Kconfig"
+
source "drivers/pch/Kconfig"
source "drivers/pcmcia/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index a7bba3ed56..480b97ef58 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_FPGA) += fpga/
obj-y += misc/
obj-$(CONFIG_MMC) += mmc/
obj-$(CONFIG_NVME) += nvme/
+obj-$(CONFIG_PCI_ENDPOINT) += pci_endpoint/
obj-y += pcmcia/
obj-y += dfu/
obj-$(CONFIG_PCH) += pch/
diff --git a/drivers/pci_endpoint/Kconfig b/drivers/pci_endpoint/Kconfig
new file mode 100644
index 0000000000..ac4f43d1ab
--- /dev/null
+++ b/drivers/pci_endpoint/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# PCI Endpoint Support
+#
+
+menu "PCI Endpoint"
+
+config PCI_ENDPOINT
+ bool "PCI Endpoint Support"
+ depends on DM
+ help
+ Enable this configuration option to support configurable PCI
+ endpoints. This should be enabled if the platform has a PCI
+ controllers that can operate in endpoint mode (as a device
+ connected to PCI host or bridge).
+
+endmenu
diff --git a/drivers/pci_endpoint/Makefile b/drivers/pci_endpoint/Makefile
new file mode 100644
index 0000000000..80a1066925
--- /dev/null
+++ b/drivers/pci_endpoint/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2019
+# Ramon Fried <ramon.fried@gmail.com>
+
+obj-y += pci_ep-uclass.o
diff --git a/drivers/pci_endpoint/pci_ep-uclass.c b/drivers/pci_endpoint/pci_ep-uclass.c
new file mode 100644
index 0000000000..403441cf02
--- /dev/null
+++ b/drivers/pci_endpoint/pci_ep-uclass.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PCI Endpoint uclass
+ *
+ * Based on Linux PCI-EP driver written by
+ * Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * Copyright (c) 2019
+ * Written by Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <linux/log2.h>
+#include <pci_ep.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int pci_ep_write_header(struct udevice *dev, uint fn, struct pci_ep_header *hdr)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->write_header)
+ return -ENOSYS;
+
+ return ops->write_header(dev, fn, hdr);
+}
+
+int pci_ep_read_header(struct udevice *dev, uint fn, struct pci_ep_header *hdr)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->read_header)
+ return -ENOSYS;
+
+ return ops->read_header(dev, fn, hdr);
+}
+
+int pci_ep_set_bar(struct udevice *dev, uint func_no, struct pci_bar *ep_bar)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+ int flags = ep_bar->flags;
+
+ /* Some basic bar validity checks */
+ if (((ep_bar->barno == BAR_5) &&
+ (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) ||
+ ((flags & PCI_BASE_ADDRESS_SPACE_IO) &&
+ (flags & PCI_BASE_ADDRESS_IO_MASK)) ||
+ (upper_32_bits(ep_bar->size) &&
+ !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64)))
+ return -EINVAL;
+
+ if (!ops->set_bar)
+ return -ENOSYS;
+
+ return ops->set_bar(dev, func_no, ep_bar);
+}
+
+int pci_ep_clear_bar(struct udevice *dev, uint func_num, enum pci_barno bar)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->clear_bar)
+ return -ENOSYS;
+
+ return ops->clear_bar(dev, func_num, bar);
+}
+
+int pci_ep_map_addr(struct udevice *dev, uint func_no, phys_addr_t addr,
+ u64 pci_addr, size_t size)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->map_addr)
+ return -ENOSYS;
+
+ return ops->map_addr(dev, func_no, addr, pci_addr, size);
+}
+
+void pci_ep_unmap_addr(struct udevice *dev, uint func_no, phys_addr_t addr)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->unmap_addr)
+ return;
+
+ ops->unmap_addr(dev, func_no, addr);
+}
+
+int pci_ep_set_msi(struct udevice *dev, uint func_no, uint interrupts)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+ u8 encode_int;
+
+ if (interrupts > 32)
+ return -EINVAL;
+
+ if (!ops->set_msi)
+ return -ENOSYS;
+
+ encode_int = order_base_2(interrupts);
+
+ return ops->set_msi(dev, func_no, encode_int);
+}
+
+int pci_ep_get_msi(struct udevice *dev, uint func_no)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+ int interrupt;
+
+ if (!ops->get_msi)
+ return -ENOSYS;
+
+ interrupt = ops->get_msi(dev, func_no);
+
+ if (interrupt < 0)
+ return 0;
+
+ interrupt = 1 << interrupt;
+
+ return interrupt;
+}
+
+int pci_ep_set_msix(struct udevice *dev, uint func_no, uint interrupts)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (interrupts < 1 || interrupts > 2048)
+ return -EINVAL;
+
+ if (!ops->set_msix)
+ return -ENOSYS;
+
+ return ops->set_msix(dev, func_no, interrupts);
+}
+
+int pci_ep_get_msix(struct udevice *dev, uint func_no)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+ int interrupt;
+
+ if (!ops->get_msix)
+ return -ENOSYS;
+
+ interrupt = ops->get_msix(dev, func_no);
+
+ if (interrupt < 0)
+ return 0;
+
+ return interrupt + 1;
+}
+
+int pci_ep_raise_irq(struct udevice *dev, uint func_no,
+ enum pci_ep_irq_type type, uint interrupt_num)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->raise_irq)
+ return -ENOSYS;
+
+ return ops->raise_irq(dev, func_no, type, interrupt_num);
+}
+
+int pci_ep_start(struct udevice *dev)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->start)
+ return -ENOSYS;
+
+ return ops->start(dev);
+}
+
+int pci_ep_stop(struct udevice *dev)
+{
+ struct pci_ep_ops *ops = pci_ep_get_ops(dev);
+
+ if (!ops->stop)
+ return -ENOSYS;
+
+ return ops->stop(dev);
+}
+
+UCLASS_DRIVER(pci_ep) = {
+ .id = UCLASS_PCI_EP,
+ .name = "pci_ep",
+ .flags = DM_UC_FLAG_SEQ_ALIAS,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 86e59781b0..d29f7d5a34 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -67,6 +67,7 @@ enum uclass_id {
UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */
UCLASS_PCH, /* x86 platform controller hub */
UCLASS_PCI, /* PCI bus */
+ UCLASS_PCI_EP, /* PCI endpoint device */
UCLASS_PCI_GENERIC, /* Generic PCI bus device */
UCLASS_PHY, /* Physical Layer (PHY) device */
UCLASS_PINCONFIG, /* Pin configuration node device */
diff --git a/include/pci_ep.h b/include/pci_ep.h
new file mode 100644
index 0000000000..183db59f59
--- /dev/null
+++ b/include/pci_ep.h
@@ -0,0 +1,388 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Adapted from Linux kernel driver
+ * Copyright (C) 2017 Texas Instruments
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * (C) Copyright 2019
+ * Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#ifndef _PCI_EP_H
+#define _PCI_EP_H
+
+#include <pci.h>
+
+/**
+ * enum pci_interrupt_pin - PCI INTx interrupt values
+ * @PCI_INTERRUPT_UNKNOWN: Unknown or unassigned interrupt
+ * @PCI_INTERRUPT_INTA: PCI INTA pin
+ * @PCI_INTERRUPT_INTB: PCI INTB pin
+ * @PCI_INTERRUPT_INTC: PCI INTC pin
+ * @PCI_INTERRUPT_INTD: PCI INTD pin
+ *
+ * Corresponds to values for legacy PCI INTx interrupts, as can be found in the
+ * PCI_INTERRUPT_PIN register.
+ */
+enum pci_interrupt_pin {
+ PCI_INTERRUPT_UNKNOWN,
+ PCI_INTERRUPT_INTA,
+ PCI_INTERRUPT_INTB,
+ PCI_INTERRUPT_INTC,
+ PCI_INTERRUPT_INTD,
+};
+
+enum pci_barno {
+ BAR_0,
+ BAR_1,
+ BAR_2,
+ BAR_3,
+ BAR_4,
+ BAR_5,
+};
+
+enum pci_ep_irq_type {
+ PCI_EP_IRQ_UNKNOWN,
+ PCI_EP_IRQ_LEGACY,
+ PCI_EP_IRQ_MSI,
+ PCI_EP_IRQ_MSIX,
+};
+
+/**
+ * struct pci_bar - represents the BAR (Base Address Register) of EP device
+ * @phys_addr: physical address that should be mapped to the BAR
+ * @size: the size of the address space present in BAR
+ * pci_barno: number of pci BAR to set (0..5)
+ * @flags: BAR access flags
+ */
+struct pci_bar {
+ dma_addr_t phys_addr;
+ size_t size;
+ enum pci_barno barno;
+ int flags;
+};
+
+/**
+ * struct pci_ep_header - represents standard configuration header
+ * @vendorid: identifies device manufacturer
+ * @deviceid: identifies a particular device
+ * @revid: specifies a device-specific revision identifier
+ * @progif_code: identifies a specific register-level programming interface
+ * @subclass_code: identifies more specifically the function of the device
+ * @baseclass_code: broadly classifies the type of function the device performs
+ * @cache_line_size: specifies the system cacheline size in units of DWORDs
+ * @subsys_vendor_id: vendor of the add-in card or subsystem
+ * @subsys_id: id specific to vendor
+ * @interrupt_pin: interrupt pin the device (or device function) uses
+ */
+struct pci_ep_header {
+ u16 vendorid;
+ u16 deviceid;
+ u8 revid;
+ u8 progif_code;
+ u8 subclass_code;
+ u8 baseclass_code;
+ u8 cache_line_size;
+ u16 subsys_vendor_id;
+ u16 subsys_id;
+ enum pci_interrupt_pin interrupt_pin;
+};
+
+/* PCI endpoint operations */
+struct pci_ep_ops {
+ /**
+ * write_header() - Write a PCI configuration space header
+ *
+ * @dev: device to write to
+ * @func_num: EP function to fill
+ * @hdr: header to write
+ * @return 0 if OK, -ve on error
+ */
+ int (*write_header)(struct udevice *dev, uint func_num,
+ struct pci_ep_header *hdr);
+ /**
+ * read_header() - Read a PCI configuration space header
+ *
+ * @dev: device to write to
+ * @func_num: EP function to fill
+ * @hdr: header to read to
+ * @return 0 if OK, -ve on error
+ */
+ int (*read_header)(struct udevice *dev, uint func_num,
+ struct pci_ep_header *hdr);
+ /**
+ * set_bar() - set BAR (Base Address Register)
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @bar: bar data
+ * @return 0 if OK, -ve on error
+ */
+ int (*set_bar)(struct udevice *dev, uint func_num,
+ struct pci_bar *bar);
+ /**
+ * clear_bar() - clear BAR (Base Address Register)
+ *
+ * @dev: device to clear
+ * @func_num: EP function to clear
+ * @bar: bar number
+ * @return 0 if OK, -ve on error
+ */
+ int (*clear_bar)(struct udevice *dev, uint func_num,
+ enum pci_barno bar);
+ /**
+ * map_addr() - map CPU address to PCI address
+ *
+ * outband region is used in order to generate PCI read/write
+ * transaction from local memory/write.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @addr: local physical address base
+ * @pci_addr: pci address to translate to
+ * @size: region size
+ * @return 0 if OK, -ve on error
+ */
+ int (*map_addr)(struct udevice *dev, uint func_num,
+ phys_addr_t addr, u64 pci_addr, size_t size);
+ /**
+ * unmap_addr() - unmap CPU address to PCI address
+ *
+ * unmap previously mapped region.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @addr: local physical address base
+ */
+ void (*unmap_addr)(struct udevice *dev, uint func_num,
+ phys_addr_t addr);
+ /**
+ * set_msi() - set msi capability property
+ *
+ * set the number of required MSI vectors the device
+ * needs for operation.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @interrupts: required interrupts count
+ * @return 0 if OK, -ve on error
+ */
+ int (*set_msi)(struct udevice *dev, uint func_num, uint interrupts);
+
+ /**
+ * get_msi() - get the number of MSI interrupts allocated by the host.
+ *
+ * Read the Multiple Message Enable bitfield from
+ * Message control register.
+ *
+ * @dev: device to use
+ * @func_num: EP function to use
+ * @return msi count if OK, -EINVAL if msi were not enabled at host.
+ */
+ int (*get_msi)(struct udevice *dev, uint func_num);
+
+ /**
+ * set_msix() - set msix capability property
+ *
+ * set the number of required MSIx vectors the device
+ * needs for operation.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @interrupts: required interrupts count
+ * @return 0 if OK, -ve on error
+ */
+ int (*set_msix)(struct udevice *dev, uint func_num,
+ uint interrupts);
+
+ /**
+ * get_msix() - get the number of MSIx interrupts allocated by the host.
+ *
+ * Read the Multiple Message Enable bitfield from
+ * Message control register.
+ *
+ * @dev: device to use
+ * @func_num: EP function to use
+ * @return msi count if OK, -EINVAL if msi were not enabled at host.
+ */
+ int (*get_msix)(struct udevice *dev, uint func_num);
+
+ /**
+ * raise_irq() - raise a legacy, MSI or MSI-X interrupt
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @type: type of irq to send
+ * @interrupt_num: interrupt vector to use
+ * @return 0 if OK, -ve on error
+ */
+ int (*raise_irq)(struct udevice *dev, uint func_num,
+ enum pci_ep_irq_type type, uint interrupt_num);
+ /**
+ * start() - start the PCI link
+ *
+ * @dev: device to set
+ * @return 0 if OK, -ve on error
+ */
+ int (*start)(struct udevice *dev);
+
+ /**
+ * stop() - stop the PCI link
+ *
+ * @dev: device to set
+ * @return 0 if OK, -ve on error
+ */
+ int (*stop)(struct udevice *dev);
+};
+
+#define pci_ep_get_ops(dev) ((struct pci_ep_ops *)(dev)->driver->ops)
+
+/**
+ * pci_ep_write_header() - Write a PCI configuration space header
+ *
+ * @dev: device to write to
+ * @func_num: EP function to fill
+ * @hdr: header to write
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_write_header(struct udevice *dev, uint func_num,
+ struct pci_ep_header *hdr);
+
+/**
+ * dm_pci_ep_read_header() - Read a PCI configuration space header
+ *
+ * @dev: device to write to
+ * @func_num: EP function to fill
+ * @hdr: header to read to
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_read_header(struct udevice *dev, uint func_num,
+ struct pci_ep_header *hdr);
+/**
+ * pci_ep_set_bar() - set BAR (Base Address Register)
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @bar: bar data
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_set_bar(struct udevice *dev, uint func_num, struct pci_bar *bar);
+
+/**
+ * pci_ep_clear_bar() - clear BAR (Base Address Register)
+ *
+ * @dev: device to clear
+ * @func_num: EP function to clear
+ * @bar: bar number
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_clear_bar(struct udevice *dev, uint func_num, enum pci_barno bar);
+/**
+ * pci_ep_map_addr() - map CPU address to PCI address
+ *
+ * outband region is used in order to generate PCI read/write
+ * transaction from local memory/write.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @addr: local physical address base
+ * @pci_addr: pci address to translate to
+ * @size: region size
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_map_addr(struct udevice *dev, uint func_num, phys_addr_t addr,
+ u64 pci_addr, size_t size);
+/**
+ * pci_ep_unmap_addr() - unmap CPU address to PCI address
+ *
+ * unmap previously mapped region.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @addr: local physical address base
+ */
+void pci_ep_unmap_addr(struct udevice *dev, uint func_num, phys_addr_t addr);
+/**
+ * pci_ep_set_msi() - set msi capability property
+ *
+ * set the number of required MSI vectors the device
+ * needs for operation.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @interrupts: required interrupts count
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_set_msi(struct udevice *dev, uint func_num, uint interrupts);
+
+/**
+ * pci_ep_get_msi() - get the number of MSI interrupts allocated by the host.
+ *
+ * Read the Multiple Message Enable bitfield from
+ * Message control register.
+ *
+ * @dev: device to use
+ * @func_num: EP function to use
+ * @return msi count if OK, -EINVAL if msi were not enabled at host.
+ */
+int pci_ep_get_msi(struct udevice *dev, uint func_num);
+
+/**
+ * pci_ep_set_msix() - set msi capability property
+ *
+ * set the number of required MSIx vectors the device
+ * needs for operation.
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @interrupts: required interrupts count
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_set_msix(struct udevice *dev, uint func_num, uint interrupts);
+
+/**
+ * pci_ep_get_msix() - get the number of MSIx interrupts allocated by the host.
+ *
+ * Read the Multiple Message Enable bitfield from
+ * Message control register.
+ *
+ * @dev: device to use
+ * @func_num: EP function to use
+ * @return msi count if OK, -EINVAL if msi were not enabled at host.
+ */
+int pci_ep_get_msix(struct udevice *dev, uint func_num);
+
+/**
+ * pci_ep_raise_irq() - raise a legacy, MSI or MSI-X interrupt
+ *
+ * @dev: device to set
+ * @func_num: EP function to set
+ * @type: type of irq to send
+ * @interrupt_num: interrupt vector to use
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_raise_irq(struct udevice *dev, uint func_num,
+ enum pci_ep_irq_type type, uint interrupt_num);
+/**
+ * pci_ep_start() - start the PCI link
+ *
+ * Enable PCI endpoint device and start link
+ * process.
+ *
+ * @dev: device to set
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_start(struct udevice *dev);
+
+/**
+ * pci_ep_stop() - stop the PCI link
+ *
+ * Disable PCI endpoint device and stop
+ * link.
+ *
+ * @dev: device to set
+ * @return 0 if OK, -ve on error
+ */
+int pci_ep_stop(struct udevice *dev);
+
+#endif
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 1/4] drivers: pci_ep: Introduce UCLASS_PCI_EP uclass
2019-04-24 18:53 ` [U-Boot] [PATCH v2 1/4] drivers: pci_ep: Introduce UCLASS_PCI_EP uclass Ramon Fried
@ 2019-04-24 23:33 ` Simon Glass
0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2019-04-24 23:33 UTC (permalink / raw)
To: u-boot
Hi Ramon,
On Wed, 24 Apr 2019 at 12:54, Ramon Fried <ramon.fried@gmail.com> wrote:
>
> Introduce new UCLASS_PCI_EP class for handling PCI endpoint
> devices, allowing to set various attributes of the PCI endpoint
> device, such as:
> * configuration space header
> * BAR definitions
> * outband memory mapping
> * start/stop PCI link
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>
> ---
> Changes in V2:
> - Changed u8/u16... to uint
> - Changed EINVAL to ENOSYS
> - Changed void funcs to int
> - Added a bit more info in comments
The only thing I see here is that unmap_addr should return an int, and
encode_int should be uint, not u8.
>
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/pci_endpoint/Kconfig | 17 ++
> drivers/pci_endpoint/Makefile | 6 +
> drivers/pci_endpoint/pci_ep-uclass.c | 189 +++++++++++++
> include/dm/uclass-id.h | 1 +
> include/pci_ep.h | 388 +++++++++++++++++++++++++++
> 7 files changed, 604 insertions(+)
> create mode 100644 drivers/pci_endpoint/Kconfig
> create mode 100644 drivers/pci_endpoint/Makefile
> create mode 100644 drivers/pci_endpoint/pci_ep-uclass.c
> create mode 100644 include/pci_ep.h
>
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 2/4] pci_ep: add Cadence PCIe endpoint driver
2019-04-24 18:53 [U-Boot] [PATCH v2 0/4] Add Cadence PCIe endpoint driver with new uclass Ramon Fried
2019-04-24 18:53 ` [U-Boot] [PATCH v2 1/4] drivers: pci_ep: Introduce UCLASS_PCI_EP uclass Ramon Fried
@ 2019-04-24 18:53 ` Ramon Fried
2019-04-24 23:35 ` Simon Glass
2019-04-24 18:53 ` [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver Ramon Fried
2019-04-24 18:53 ` [U-Boot] [PATCH v2 4/4] test: pci_ep: add basic pci_ep tests Ramon Fried
3 siblings, 1 reply; 10+ messages in thread
From: Ramon Fried @ 2019-04-24 18:53 UTC (permalink / raw)
To: u-boot
Add Cadence PCIe endpoint driver supporting configuration
of header, bars and MSI for device.
Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---
.../pci_endpoint/cdns,cdns-pcie-ep.txt | 18 +
drivers/pci_endpoint/Kconfig | 8 +
drivers/pci_endpoint/Makefile | 1 +
drivers/pci_endpoint/pcie-cadence-ep.c | 177 ++++++++++
drivers/pci_endpoint/pcie-cadence.h | 309 ++++++++++++++++++
5 files changed, 513 insertions(+)
create mode 100644 doc/device-tree-bindings/pci_endpoint/cdns,cdns-pcie-ep.txt
create mode 100644 drivers/pci_endpoint/pcie-cadence-ep.c
create mode 100644 drivers/pci_endpoint/pcie-cadence.h
diff --git a/doc/device-tree-bindings/pci_endpoint/cdns,cdns-pcie-ep.txt b/doc/device-tree-bindings/pci_endpoint/cdns,cdns-pcie-ep.txt
new file mode 100644
index 0000000000..7705430559
--- /dev/null
+++ b/doc/device-tree-bindings/pci_endpoint/cdns,cdns-pcie-ep.txt
@@ -0,0 +1,18 @@
+* Cadence PCIe endpoint controller
+
+Required properties:
+- compatible: Should contain "cdns,cdns-pcie-ep" to identify the IP used.
+- reg: Should contain the controller register base address.
+
+Optional properties:
+- max-functions: Maximum number of functions that can be configured (default 1).
+- cdns,max-outbound-regions: Set to maximum number of outbound regions (default 8)
+
+Example:
+
+pcie_ep at fc000000 {
+ compatible = "cdns,cdns-pcie-ep";
+ reg = <0x0 0xfc000000 0x0 0x01000000>;
+ cdns,max-outbound-regions = <16>;
+ max-functions = /bits/ 8 <8>;
+};
diff --git a/drivers/pci_endpoint/Kconfig b/drivers/pci_endpoint/Kconfig
index ac4f43d1ab..c54bd2a9ac 100644
--- a/drivers/pci_endpoint/Kconfig
+++ b/drivers/pci_endpoint/Kconfig
@@ -14,4 +14,12 @@ config PCI_ENDPOINT
controllers that can operate in endpoint mode (as a device
connected to PCI host or bridge).
+config PCIE_CADENCE_EP
+ bool "Cadence PCIe endpoint controller"
+ depends on PCI_ENDPOINT
+ help
+ Say Y here if you want to support the Cadence PCIe controller in
+ endpoint mode. This PCIe controller may be embedded into many
+ different vendors SoCs.
+
endmenu
diff --git a/drivers/pci_endpoint/Makefile b/drivers/pci_endpoint/Makefile
index 80a1066925..0a849deb19 100644
--- a/drivers/pci_endpoint/Makefile
+++ b/drivers/pci_endpoint/Makefile
@@ -4,3 +4,4 @@
# Ramon Fried <ramon.fried@gmail.com>
obj-y += pci_ep-uclass.o
+obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
diff --git a/drivers/pci_endpoint/pcie-cadence-ep.c b/drivers/pci_endpoint/pcie-cadence-ep.c
new file mode 100644
index 0000000000..60032fc724
--- /dev/null
+++ b/drivers/pci_endpoint/pcie-cadence-ep.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019
+ * Written by Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <pci_ep.h>
+#include <linux/sizes.h>
+#include <linux/log2.h>
+#include "pcie-cadence.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int cdns_write_header(struct udevice *dev, uint fn,
+ struct pci_ep_header *hdr)
+{
+ struct cdns_pcie *pcie = dev_get_priv(dev);
+
+ cdns_pcie_ep_fn_writew(pcie, fn, PCI_DEVICE_ID, hdr->deviceid);
+ cdns_pcie_ep_fn_writeb(pcie, fn, PCI_REVISION_ID, hdr->revid);
+ cdns_pcie_ep_fn_writeb(pcie, fn, PCI_CLASS_PROG,
+ hdr->progif_code);
+ cdns_pcie_ep_fn_writew(pcie, fn, PCI_CLASS_DEVICE,
+ hdr->subclass_code |
+ hdr->baseclass_code << 8);
+ cdns_pcie_ep_fn_writeb(pcie, fn, PCI_CACHE_LINE_SIZE,
+ hdr->cache_line_size);
+ cdns_pcie_ep_fn_writew(pcie, fn, PCI_SUBSYSTEM_ID,
+ hdr->subsys_id);
+ cdns_pcie_ep_fn_writeb(pcie, fn, PCI_INTERRUPT_PIN,
+ hdr->interrupt_pin);
+
+ /*
+ * Vendor ID can only be modified from function 0, all other functions
+ * use the same vendor ID as function 0.
+ */
+ if (fn == 0) {
+ /* Update the vendor IDs. */
+ u32 id = CDNS_PCIE_LM_ID_VENDOR(hdr->vendorid) |
+ CDNS_PCIE_LM_ID_SUBSYS(hdr->subsys_vendor_id);
+
+ cdns_pcie_writel(pcie, CDNS_PCIE_LM_ID, id);
+ }
+
+ return 0;
+}
+
+static int cdns_set_bar(struct udevice *dev, uint fn, struct pci_bar *ep_bar)
+{
+ struct cdns_pcie *pcie = dev_get_priv(dev);
+ dma_addr_t bar_phys = ep_bar->phys_addr;
+ enum pci_barno bar = ep_bar->barno;
+ int flags = ep_bar->flags;
+ u32 addr0, addr1, reg, cfg, b, aperture, ctrl;
+ u64 sz;
+
+ /* BAR size is 2^(aperture + 7) */
+ sz = max_t(size_t, ep_bar->size, CDNS_PCIE_EP_MIN_APERTURE);
+ /*
+ * roundup_pow_of_two() returns an unsigned long, which is not suited
+ * for 64bit values.
+ */
+ sz = 1ULL << fls64(sz - 1);
+ aperture = ilog2(sz) - 7; /* 128B -> 0, 256B -> 1, 512B -> 2, ... */
+
+ if ((flags & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
+ ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS;
+ } else {
+ bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH);
+ bool is_64bits = (sz > SZ_2G) |
+ !!(ep_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64);
+
+ if (is_64bits && (bar & 1))
+ return -EINVAL;
+
+ if (is_64bits && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))
+ ep_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+
+ if (is_64bits && is_prefetch)
+ ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS;
+ else if (is_prefetch)
+ ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_32BITS;
+ else if (is_64bits)
+ ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_64BITS;
+ else
+ ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_32BITS;
+ }
+
+ addr0 = lower_32_bits(bar_phys);
+ addr1 = upper_32_bits(bar_phys);
+ cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar),
+ addr0);
+ cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar),
+ addr1);
+
+ if (bar < BAR_4) {
+ reg = CDNS_PCIE_LM_EP_FUNC_BAR_CFG0(fn);
+ b = bar;
+ } else {
+ reg = CDNS_PCIE_LM_EP_FUNC_BAR_CFG1(fn);
+ b = bar - BAR_4;
+ }
+
+ cfg = cdns_pcie_readl(pcie, reg);
+ cfg &= ~(CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b) |
+ CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b));
+ cfg |= (CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE(b, aperture) |
+ CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL(b, ctrl));
+ cdns_pcie_writel(pcie, reg, cfg);
+
+ return 0;
+}
+
+static int cdns_set_msi(struct udevice *dev, uint fn, uint mmc)
+{
+ struct cdns_pcie *pcie = dev_get_priv(dev);
+ u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
+
+ /*
+ * Set the Multiple Message Capable bitfield into the Message Control
+ * register.
+ */
+ u16 flags;
+
+ flags = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_MSI_FLAGS);
+ flags = (flags & ~PCI_MSI_FLAGS_QMASK) | (mmc << 1);
+ flags |= PCI_MSI_FLAGS_64BIT;
+ flags &= ~PCI_MSI_FLAGS_MASKBIT;
+ cdns_pcie_ep_fn_writew(pcie, fn, cap + PCI_MSI_FLAGS, flags);
+
+ return 0;
+}
+
+static struct pci_ep_ops cdns_pci_ep_ops = {
+ .write_header = cdns_write_header,
+ .set_bar = cdns_set_bar,
+ .set_msi = cdns_set_msi,
+};
+
+static int cdns_pci_ep_probe(struct udevice *dev)
+{
+ struct cdns_pcie *pdata = dev_get_priv(dev);
+
+ pdata->reg_base = (void *)devfdt_get_addr(dev);
+ if (!pdata->reg_base)
+ return -ENOMEM;
+
+ pdata->max_functions = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
+ "max-functions", 1);
+ pdata->max_regions = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
+ "cdns,max-outbound-regions", 8);
+
+ return 0;
+}
+
+static int cdns_pci_ep_remove(struct udevice *dev)
+{
+ return 0;
+}
+
+const struct udevice_id cadence_pci_ep_of_match[] = {
+ { .compatible = "cdns,cdns-pcie-ep" },
+ { }
+};
+
+U_BOOT_DRIVER(cdns_pcie) = {
+ .name = "cdns,pcie-ep",
+ .id = UCLASS_PCI_EP,
+ .of_match = cadence_pci_ep_of_match,
+ .ops = &cdns_pci_ep_ops,
+ .probe = cdns_pci_ep_probe,
+ .remove = cdns_pci_ep_remove,
+ .priv_auto_alloc_size = sizeof(struct cdns_pcie),
+};
diff --git a/drivers/pci_endpoint/pcie-cadence.h b/drivers/pci_endpoint/pcie-cadence.h
new file mode 100644
index 0000000000..91630d35c3
--- /dev/null
+++ b/drivers/pci_endpoint/pcie-cadence.h
@@ -0,0 +1,309 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Cadence PCIe controlloer definitions
+ * Adapted from linux kernel driver.
+ * Copyright (c) 2017 Cadence
+ *
+ * Copyright (c) 2019
+ * Written by Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#ifndef PCIE_CADENCE_H
+#define PCIE_CADENCE_H
+
+#include <common.h>
+#include <pci_ep.h>
+#include <asm/io.h>
+
+/*
+ * Local Management Registers
+ */
+#define CDNS_PCIE_LM_BASE 0x00100000
+
+/* Vendor ID Register */
+#define CDNS_PCIE_LM_ID (CDNS_PCIE_LM_BASE + 0x0044)
+#define CDNS_PCIE_LM_ID_VENDOR_MASK GENMASK(15, 0)
+#define CDNS_PCIE_LM_ID_VENDOR_SHIFT 0
+#define CDNS_PCIE_LM_ID_VENDOR(vid) \
+ (((vid) << CDNS_PCIE_LM_ID_VENDOR_SHIFT) & CDNS_PCIE_LM_ID_VENDOR_MASK)
+#define CDNS_PCIE_LM_ID_SUBSYS_MASK GENMASK(31, 16)
+#define CDNS_PCIE_LM_ID_SUBSYS_SHIFT 16
+#define CDNS_PCIE_LM_ID_SUBSYS(sub) \
+ (((sub) << CDNS_PCIE_LM_ID_SUBSYS_SHIFT) & CDNS_PCIE_LM_ID_SUBSYS_MASK)
+
+/* Root Port Requestor ID Register */
+#define CDNS_PCIE_LM_RP_RID (CDNS_PCIE_LM_BASE + 0x0228)
+#define CDNS_PCIE_LM_RP_RID_MASK GENMASK(15, 0)
+#define CDNS_PCIE_LM_RP_RID_SHIFT 0
+#define CDNS_PCIE_LM_RP_RID_(rid) \
+ (((rid) << CDNS_PCIE_LM_RP_RID_SHIFT) & CDNS_PCIE_LM_RP_RID_MASK)
+
+/* Endpoint Bus and Device Number Register */
+#define CDNS_PCIE_LM_EP_ID (CDNS_PCIE_LM_BASE + 0x022c)
+#define CDNS_PCIE_LM_EP_ID_DEV_MASK GENMASK(4, 0)
+#define CDNS_PCIE_LM_EP_ID_DEV_SHIFT 0
+#define CDNS_PCIE_LM_EP_ID_BUS_MASK GENMASK(15, 8)
+#define CDNS_PCIE_LM_EP_ID_BUS_SHIFT 8
+
+/* Endpoint Function f BAR b Configuration Registers */
+#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG0(fn) \
+ (CDNS_PCIE_LM_BASE + 0x0240 + (fn) * 0x0008)
+#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG1(fn) \
+ (CDNS_PCIE_LM_BASE + 0x0244 + (fn) * 0x0008)
+#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b) \
+ (GENMASK(4, 0) << ((b) * 8))
+#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE(b, a) \
+ (((a) << ((b) * 8)) & CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b))
+#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b) \
+ (GENMASK(7, 5) << ((b) * 8))
+#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL(b, c) \
+ (((c) << ((b) * 8 + 5)) & CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b))
+
+/* Endpoint Function Configuration Register */
+#define CDNS_PCIE_LM_EP_FUNC_CFG (CDNS_PCIE_LM_BASE + 0x02c0)
+
+/* Root Complex BAR Configuration Register */
+#define CDNS_PCIE_LM_RC_BAR_CFG (CDNS_PCIE_LM_BASE + 0x0300)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK GENMASK(5, 0)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE(a) \
+ (((a) << 0) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK GENMASK(8, 6)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL(c) \
+ (((c) << 6) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK GENMASK(13, 9)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE(a) \
+ (((a) << 9) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK GENMASK(16, 14)
+#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL(c) \
+ (((c) << 14) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK)
+#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_ENABLE BIT(17)
+#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_32BITS 0
+#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS BIT(18)
+#define CDNS_PCIE_LM_RC_BAR_CFG_IO_ENABLE BIT(19)
+#define CDNS_PCIE_LM_RC_BAR_CFG_IO_16BITS 0
+#define CDNS_PCIE_LM_RC_BAR_CFG_IO_32BITS BIT(20)
+#define CDNS_PCIE_LM_RC_BAR_CFG_CHECK_ENABLE BIT(31)
+
+/* BAR control values applicable to both Endpoint Function and Root Complex */
+#define CDNS_PCIE_LM_BAR_CFG_CTRL_DISABLED 0x0
+#define CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS 0x1
+#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_32BITS 0x4
+#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_32BITS 0x5
+#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_64BITS 0x6
+#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS 0x7
+
+/*
+ * Endpoint Function Registers (PCI configuration space for endpoint functions)
+ */
+#define CDNS_PCIE_EP_FUNC_BASE(fn) (((fn) << 12) & GENMASK(19, 12))
+
+#define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90
+
+/*
+ * Root Port Registers (PCI configuration space for the root port function)
+ */
+#define CDNS_PCIE_RP_BASE 0x00200000
+
+/*
+ * Address Translation Registers
+ */
+#define CDNS_PCIE_AT_BASE 0x00400000
+
+/* Region r Outbound AXI to PCIe Address Translation Register 0 */
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r) \
+ (CDNS_PCIE_AT_BASE + 0x0000 + ((r) & 0x1f) * 0x0020)
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK GENMASK(5, 0)
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(nbits) \
+ (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK)
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK GENMASK(19, 12)
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) \
+ (((devfn) << 12) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK)
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK GENMASK(27, 20)
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(bus) \
+ (((bus) << 20) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK)
+
+/* Region r Outbound AXI to PCIe Address Translation Register 1 */
+#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r) \
+ (CDNS_PCIE_AT_BASE + 0x0004 + ((r) & 0x1f) * 0x0020)
+
+/* Region r Outbound PCIe Descriptor Register 0 */
+#define CDNS_PCIE_AT_OB_REGION_DESC0(r) \
+ (CDNS_PCIE_AT_BASE + 0x0008 + ((r) & 0x1f) * 0x0020)
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MASK GENMASK(3, 0)
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MEM 0x2
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_IO 0x6
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE0 0xa
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE1 0xb
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_NORMAL_MSG 0xc
+#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_VENDOR_MSG 0xd
+/* Bit 23 MUST be set in RC mode. */
+#define CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID BIT(23)
+#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK GENMASK(31, 24)
+#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(devfn) \
+ (((devfn) << 24) & CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK)
+
+/* Region r Outbound PCIe Descriptor Register 1 */
+#define CDNS_PCIE_AT_OB_REGION_DESC1(r) \
+ (CDNS_PCIE_AT_BASE + 0x000c + ((r) & 0x1f) * 0x0020)
+#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK GENMASK(7, 0)
+#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS(bus) \
+ ((bus) & CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK)
+
+/* Region r AXI Region Base Address Register 0 */
+#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r) \
+ (CDNS_PCIE_AT_BASE + 0x0018 + ((r) & 0x1f) * 0x0020)
+#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK GENMASK(5, 0)
+#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) \
+ (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK)
+
+/* Region r AXI Region Base Address Register 1 */
+#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r) \
+ (CDNS_PCIE_AT_BASE + 0x001c + ((r) & 0x1f) * 0x0020)
+
+/* Root Port BAR Inbound PCIe to AXI Address Translation Register */
+#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0(bar) \
+ (CDNS_PCIE_AT_BASE + 0x0800 + (bar) * 0x0008)
+#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK GENMASK(5, 0)
+#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(nbits) \
+ (((nbits) - 1) & CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK)
+#define CDNS_PCIE_AT_IB_RP_BAR_ADDR1(bar) \
+ (CDNS_PCIE_AT_BASE + 0x0804 + (bar) * 0x0008)
+
+/* AXI link down register */
+#define CDNS_PCIE_AT_LINKDOWN (CDNS_PCIE_AT_BASE + 0x0824)
+
+enum cdns_pcie_rp_bar {
+ RP_BAR0,
+ RP_BAR1,
+ RP_NO_BAR
+};
+
+/* Endpoint Function BAR Inbound PCIe to AXI Address Translation Register */
+#define CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
+ (CDNS_PCIE_AT_BASE + 0x0840 + (fn) * 0x0040 + (bar) * 0x0008)
+#define CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar) \
+ (CDNS_PCIE_AT_BASE + 0x0844 + (fn) * 0x0040 + (bar) * 0x0008)
+
+/* Normal/Vendor specific message access: offset inside some outbound region */
+#define CDNS_PCIE_NORMAL_MSG_ROUTING_MASK GENMASK(7, 5)
+#define CDNS_PCIE_NORMAL_MSG_ROUTING(route) \
+ (((route) << 5) & CDNS_PCIE_NORMAL_MSG_ROUTING_MASK)
+#define CDNS_PCIE_NORMAL_MSG_CODE_MASK GENMASK(15, 8)
+#define CDNS_PCIE_NORMAL_MSG_CODE(code) \
+ (((code) << 8) & CDNS_PCIE_NORMAL_MSG_CODE_MASK)
+#define CDNS_PCIE_MSG_NO_DATA BIT(16)
+
+#define CDNS_PCIE_EP_MIN_APERTURE 128 /* 128 bytes */
+
+enum cdns_pcie_msg_code {
+ MSG_CODE_ASSERT_INTA = 0x20,
+ MSG_CODE_ASSERT_INTB = 0x21,
+ MSG_CODE_ASSERT_INTC = 0x22,
+ MSG_CODE_ASSERT_INTD = 0x23,
+ MSG_CODE_DEASSERT_INTA = 0x24,
+ MSG_CODE_DEASSERT_INTB = 0x25,
+ MSG_CODE_DEASSERT_INTC = 0x26,
+ MSG_CODE_DEASSERT_INTD = 0x27,
+};
+
+enum cdns_pcie_msg_routing {
+ /* Route to Root Complex */
+ MSG_ROUTING_TO_RC,
+
+ /* Use Address Routing */
+ MSG_ROUTING_BY_ADDR,
+
+ /* Use ID Routing */
+ MSG_ROUTING_BY_ID,
+
+ /* Route as Broadcast Message from Root Complex */
+ MSG_ROUTING_BCAST,
+
+ /* Local message; terminate@receiver (INTx messages) */
+ MSG_ROUTING_LOCAL,
+
+ /* Gather & route to Root Complex (PME_TO_Ack message) */
+ MSG_ROUTING_GATHER,
+};
+
+struct cdns_pcie {
+ void __iomem *reg_base;
+ u32 max_functions;
+ u32 max_regions;
+};
+
+/* Register access */
+static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
+{
+ writeb(value, pcie->reg_base + reg);
+}
+
+static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
+{
+ writew(value, pcie->reg_base + reg);
+}
+
+static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
+{
+ writel(value, pcie->reg_base + reg);
+}
+
+static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
+{
+ return readl(pcie->reg_base + reg);
+}
+
+/* Root Port register access */
+static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
+ u32 reg, u8 value)
+{
+ writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
+}
+
+static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
+ u32 reg, u16 value)
+{
+ writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
+}
+
+static inline void cdns_pcie_rp_writel(struct cdns_pcie *pcie,
+ u32 reg, u32 value)
+{
+ writel(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
+}
+
+/* Endpoint Function register access */
+static inline void cdns_pcie_ep_fn_writeb(struct cdns_pcie *pcie, u8 fn,
+ u32 reg, u8 value)
+{
+ writeb(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+}
+
+static inline void cdns_pcie_ep_fn_writew(struct cdns_pcie *pcie, u8 fn,
+ u32 reg, u16 value)
+{
+ writew(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+}
+
+static inline void cdns_pcie_ep_fn_writel(struct cdns_pcie *pcie, u8 fn,
+ u32 reg, u32 value)
+{
+ writel(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+}
+
+static inline u8 cdns_pcie_ep_fn_readb(struct cdns_pcie *pcie, u8 fn, u32 reg)
+{
+ return readb(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+}
+
+static inline u16 cdns_pcie_ep_fn_readw(struct cdns_pcie *pcie, u8 fn, u32 reg)
+{
+ return readw(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+}
+
+static inline u32 cdns_pcie_ep_fn_readl(struct cdns_pcie *pcie, u8 fn, u32 reg)
+{
+ return readl(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+}
+
+#endif /* end of include guard: PCIE_CADENCE_H */
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 2/4] pci_ep: add Cadence PCIe endpoint driver
2019-04-24 18:53 ` [U-Boot] [PATCH v2 2/4] pci_ep: add Cadence PCIe endpoint driver Ramon Fried
@ 2019-04-24 23:35 ` Simon Glass
0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2019-04-24 23:35 UTC (permalink / raw)
To: u-boot
Hi Ramon,
On Wed, 24 Apr 2019 at 12:54, Ramon Fried <ramon.fried@gmail.com> wrote:
>
> Add Cadence PCIe endpoint driver supporting configuration
> of header, bars and MSI for device.
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> ---
>
> .../pci_endpoint/cdns,cdns-pcie-ep.txt | 18 +
> drivers/pci_endpoint/Kconfig | 8 +
> drivers/pci_endpoint/Makefile | 1 +
> drivers/pci_endpoint/pcie-cadence-ep.c | 177 ++++++++++
> drivers/pci_endpoint/pcie-cadence.h | 309 ++++++++++++++++++
> 5 files changed, 513 insertions(+)
> create mode 100644 doc/device-tree-bindings/pci_endpoint/cdns,cdns-pcie-ep.txt
> create mode 100644 drivers/pci_endpoint/pcie-cadence-ep.c
> create mode 100644 drivers/pci_endpoint/pcie-cadence.h
This looks OK to me.
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver
2019-04-24 18:53 [U-Boot] [PATCH v2 0/4] Add Cadence PCIe endpoint driver with new uclass Ramon Fried
2019-04-24 18:53 ` [U-Boot] [PATCH v2 1/4] drivers: pci_ep: Introduce UCLASS_PCI_EP uclass Ramon Fried
2019-04-24 18:53 ` [U-Boot] [PATCH v2 2/4] pci_ep: add Cadence PCIe endpoint driver Ramon Fried
@ 2019-04-24 18:53 ` Ramon Fried
2019-04-24 23:29 ` Simon Glass
2019-04-24 18:53 ` [U-Boot] [PATCH v2 4/4] test: pci_ep: add basic pci_ep tests Ramon Fried
3 siblings, 1 reply; 10+ messages in thread
From: Ramon Fried @ 2019-04-24 18:53 UTC (permalink / raw)
To: u-boot
Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---
arch/sandbox/dts/test.dts | 4 +
configs/sandbox64_defconfig | 2 +
configs/sandbox_defconfig | 2 +
drivers/pci_endpoint/Kconfig | 7 +
drivers/pci_endpoint/Makefile | 1 +
drivers/pci_endpoint/sandbox-pci_ep.c | 176 ++++++++++++++++++++++++++
6 files changed, 192 insertions(+)
create mode 100644 drivers/pci_endpoint/sandbox-pci_ep.c
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 8b2d6451c6..001dc302ed 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -475,6 +475,10 @@
};
};
+ pci_ep: pci_ep {
+ compatible = "sandbox,pci_ep";
+ };
+
probing {
compatible = "simple-bus";
test1 {
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index c04ecd915a..7137ea481c 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -127,9 +127,11 @@ CONFIG_SPI_FLASH_WINBOND=y
CONFIG_DM_ETH=y
CONFIG_NVME=y
CONFIG_PCI=y
+CONFIG_PCI_ENDPOINT=y
CONFIG_DM_PCI=y
CONFIG_DM_PCI_COMPAT=y
CONFIG_PCI_SANDBOX=y
+CONFIG_PCI_SANDBOX_EP=y
CONFIG_PHY=y
CONFIG_PHY_SANDBOX=y
CONFIG_PINCTRL=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index bb508a8d02..04ba9a3ba1 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -142,9 +142,11 @@ CONFIG_SPI_FLASH_WINBOND=y
CONFIG_DM_ETH=y
CONFIG_NVME=y
CONFIG_PCI=y
+CONFIG_PCI_ENDPOINT=y
CONFIG_DM_PCI=y
CONFIG_DM_PCI_COMPAT=y
CONFIG_PCI_SANDBOX=y
+CONFIG_PCI_SANDBOX_EP=y
CONFIG_PHY=y
CONFIG_PHY_SANDBOX=y
CONFIG_PINCTRL=y
diff --git a/drivers/pci_endpoint/Kconfig b/drivers/pci_endpoint/Kconfig
index c54bd2a9ac..e529e560fc 100644
--- a/drivers/pci_endpoint/Kconfig
+++ b/drivers/pci_endpoint/Kconfig
@@ -22,4 +22,11 @@ config PCIE_CADENCE_EP
endpoint mode. This PCIe controller may be embedded into many
different vendors SoCs.
+config PCI_SANDBOX_EP
+ bool "Sandbox PCIe endpoint controller"
+ depends on PCI_ENDPOINT
+ help
+ Say Y here if you want to support the Sandbox PCIe controller in
+ endpoint mode.
+
endmenu
diff --git a/drivers/pci_endpoint/Makefile b/drivers/pci_endpoint/Makefile
index 0a849deb19..3cd987259d 100644
--- a/drivers/pci_endpoint/Makefile
+++ b/drivers/pci_endpoint/Makefile
@@ -5,3 +5,4 @@
obj-y += pci_ep-uclass.o
obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
+obj-$(CONFIG_PCI_SANDBOX_EP) += sandbox-pci_ep.o
diff --git a/drivers/pci_endpoint/sandbox-pci_ep.c b/drivers/pci_endpoint/sandbox-pci_ep.c
new file mode 100644
index 0000000000..eb19c6da81
--- /dev/null
+++ b/drivers/pci_endpoint/sandbox-pci_ep.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019 Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <pci.h>
+#include <pci_ep.h>
+#include <asm/test.h>
+
+struct sandbox_pci_ep_priv {
+ struct pci_ep_header hdr;
+ int msix;
+ int msi;
+};
+
+static const struct udevice_id sandbox_pci_ep_ids[] = {
+ { .compatible = "sandbox,pci_ep" },
+ { }
+};
+
+static int sandbox_write_header(struct udevice *dev, uint fn,
+ struct pci_ep_header *hdr)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ if (fn > 0)
+ return -ENODEV;
+
+ memcpy(&priv->hdr, hdr, sizeof(*hdr));
+
+ return 0;
+}
+
+static int sandbox_read_header(struct udevice *dev, uint fn,
+ struct pci_ep_header *hdr)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ if (fn > 0)
+ return -ENODEV;
+
+ memcpy(hdr, &priv->hdr, sizeof(*hdr));
+
+ return 0;
+}
+
+static int sandbox_set_bar(struct udevice *dev, uint fn,
+ struct pci_bar *ep_bar)
+{
+ if (fn > 0)
+ return -ENODEV;
+ return 0;
+}
+
+int sandbox_clear_bar(struct udevice *dev, uint fn,
+ enum pci_barno bar)
+{
+ if (fn > 0)
+ return -ENODEV;
+ return 0;
+}
+
+static int sandbox_map_addr(struct udevice *dev, uint fn, phys_addr_t addr,
+ u64 pci_addr, size_t size)
+{
+ if (fn > 0)
+ return -ENODEV;
+
+ return 0;
+}
+
+static void sandbox_unmap_addr(struct udevice *dev, uint fn, phys_addr_t addr)
+{
+ if (fn > 0)
+ return;
+}
+
+static int sandbox_set_msi(struct udevice *dev, uint fn, uint interrupts)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ if (fn > 0)
+ return -ENODEV;
+
+ priv->msi = interrupts;
+
+ return 0;
+}
+
+static int sandbox_get_msi(struct udevice *dev, uint fn)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ if (fn > 0)
+ return -ENODEV;
+
+ return priv->msi;
+}
+
+static int sandbox_set_msix(struct udevice *dev, uint fn, uint interrupts)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ if (fn > 0)
+ return -ENODEV;
+
+ priv->msix = interrupts;
+
+ return 0;
+}
+
+static int sandbox_get_msix(struct udevice *dev, uint fn)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ if (fn > 0)
+ return -ENODEV;
+
+ return priv->msix;
+}
+
+static int sandbox_raise_irq(struct udevice *dev, uint fn,
+ enum pci_ep_irq_type type, uint interrupt_num)
+{
+ if (fn > 0)
+ return -ENODEV;
+
+ return 0;
+}
+
+static int sandbox_start(struct udevice *dev)
+{
+ return 0;
+}
+
+static int sandbox_stop(struct udevice *dev)
+{
+ return 0;
+}
+
+static int sandbox_pci_ep_probe(struct udevice *dev)
+{
+ struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
+
+ memset(priv, 0, sizeof(*priv));
+
+ return 0;
+}
+
+static struct pci_ep_ops sandbox_pci_ep_ops = {
+ .write_header = sandbox_write_header,
+ .read_header = sandbox_read_header,
+ .set_bar = sandbox_set_bar,
+ .clear_bar = sandbox_clear_bar,
+ .map_addr = sandbox_map_addr,
+ .unmap_addr = sandbox_unmap_addr,
+ .set_msi = sandbox_set_msi,
+ .get_msi = sandbox_get_msi,
+ .set_msix = sandbox_set_msix,
+ .get_msix = sandbox_get_msix,
+ .raise_irq = sandbox_raise_irq,
+ .start = sandbox_start,
+ .stop = sandbox_stop,
+};
+
+U_BOOT_DRIVER(pci_ep_sandbox) = {
+ .name = "pci_ep_sandbox",
+ .id = UCLASS_PCI_EP,
+ .of_match = sandbox_pci_ep_ids,
+ .probe = sandbox_pci_ep_probe,
+ .ops = &sandbox_pci_ep_ops,
+ .priv_auto_alloc_size = sizeof(struct sandbox_pci_ep_priv),
+};
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver
2019-04-24 18:53 ` [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver Ramon Fried
@ 2019-04-24 23:29 ` Simon Glass
2019-04-25 11:35 ` Ramon Fried
0 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2019-04-24 23:29 UTC (permalink / raw)
To: u-boot
Hi Ramon,
On Wed, 24 Apr 2019 at 12:54, Ramon Fried <ramon.fried@gmail.com> wrote:
Please add a commit message.
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> ---
>
> arch/sandbox/dts/test.dts | 4 +
> configs/sandbox64_defconfig | 2 +
> configs/sandbox_defconfig | 2 +
> drivers/pci_endpoint/Kconfig | 7 +
> drivers/pci_endpoint/Makefile | 1 +
> drivers/pci_endpoint/sandbox-pci_ep.c | 176 ++++++++++++++++++++++++++
> 6 files changed, 192 insertions(+)
> create mode 100644 drivers/pci_endpoint/sandbox-pci_ep.c
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 8b2d6451c6..001dc302ed 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -475,6 +475,10 @@
> };
> };
>
> + pci_ep: pci_ep {
> + compatible = "sandbox,pci_ep";
> + };
> +
> probing {
> compatible = "simple-bus";
> test1 {
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index c04ecd915a..7137ea481c 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -127,9 +127,11 @@ CONFIG_SPI_FLASH_WINBOND=y
> CONFIG_DM_ETH=y
> CONFIG_NVME=y
> CONFIG_PCI=y
> +CONFIG_PCI_ENDPOINT=y
It might be better to 'imply' this in the sandbox Kconfig file.
> CONFIG_DM_PCI=y
> CONFIG_DM_PCI_COMPAT=y
> CONFIG_PCI_SANDBOX=y
> +CONFIG_PCI_SANDBOX_EP=y
> CONFIG_PHY=y
> CONFIG_PHY_SANDBOX=y
> CONFIG_PINCTRL=y
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index bb508a8d02..04ba9a3ba1 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -142,9 +142,11 @@ CONFIG_SPI_FLASH_WINBOND=y
> CONFIG_DM_ETH=y
> CONFIG_NVME=y
> CONFIG_PCI=y
> +CONFIG_PCI_ENDPOINT=y
> CONFIG_DM_PCI=y
> CONFIG_DM_PCI_COMPAT=y
> CONFIG_PCI_SANDBOX=y
> +CONFIG_PCI_SANDBOX_EP=y
> CONFIG_PHY=y
> CONFIG_PHY_SANDBOX=y
> CONFIG_PINCTRL=y
> diff --git a/drivers/pci_endpoint/Kconfig b/drivers/pci_endpoint/Kconfig
> index c54bd2a9ac..e529e560fc 100644
> --- a/drivers/pci_endpoint/Kconfig
> +++ b/drivers/pci_endpoint/Kconfig
> @@ -22,4 +22,11 @@ config PCIE_CADENCE_EP
> endpoint mode. This PCIe controller may be embedded into many
> different vendors SoCs.
>
> +config PCI_SANDBOX_EP
> + bool "Sandbox PCIe endpoint controller"
> + depends on PCI_ENDPOINT
> + help
> + Say Y here if you want to support the Sandbox PCIe controller in
Use single space after PCIe
> + endpoint mode.
How about another few sentences saying what the driver does?
> +
> endmenu
> diff --git a/drivers/pci_endpoint/Makefile b/drivers/pci_endpoint/Makefile
> index 0a849deb19..3cd987259d 100644
> --- a/drivers/pci_endpoint/Makefile
> +++ b/drivers/pci_endpoint/Makefile
> @@ -5,3 +5,4 @@
>
> obj-y += pci_ep-uclass.o
> obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
> +obj-$(CONFIG_PCI_SANDBOX_EP) += sandbox-pci_ep.o
> diff --git a/drivers/pci_endpoint/sandbox-pci_ep.c b/drivers/pci_endpoint/sandbox-pci_ep.c
> new file mode 100644
> index 0000000000..eb19c6da81
> --- /dev/null
> +++ b/drivers/pci_endpoint/sandbox-pci_ep.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2019 Ramon Fried <ramon.fried@gmail.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
I think you can drop this
> +#include <pci.h>
> +#include <pci_ep.h>
> +#include <asm/test.h>
> +
struct comment, explaining each member.
> +struct sandbox_pci_ep_priv {
> + struct pci_ep_header hdr;
> + int msix;
> + int msi;
> +};
> +
> +static const struct udevice_id sandbox_pci_ep_ids[] = {
> + { .compatible = "sandbox,pci_ep" },
> + { }
> +};
> +
> +static int sandbox_write_header(struct udevice *dev, uint fn,
> + struct pci_ep_header *hdr)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + if (fn > 0)
> + return -ENODEV;
> +
> + memcpy(&priv->hdr, hdr, sizeof(*hdr));
> +
> + return 0;
> +}
> +
> +static int sandbox_read_header(struct udevice *dev, uint fn,
> + struct pci_ep_header *hdr)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + if (fn > 0)
> + return -ENODEV;
> +
> + memcpy(hdr, &priv->hdr, sizeof(*hdr));
> +
> + return 0;
> +}
> +
> +static int sandbox_set_bar(struct udevice *dev, uint fn,
> + struct pci_bar *ep_bar)
> +{
> + if (fn > 0)
> + return -ENODEV;
blank line before return. Please fix globally.
> + return 0;
> +}
> +
> +int sandbox_clear_bar(struct udevice *dev, uint fn,
static int?
> + enum pci_barno bar)
> +{
> + if (fn > 0)
> + return -ENODEV;
> + return 0;
> +}
> +
> +static int sandbox_map_addr(struct udevice *dev, uint fn, phys_addr_t addr,
> + u64 pci_addr, size_t size)
> +{
> + if (fn > 0)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static void sandbox_unmap_addr(struct udevice *dev, uint fn, phys_addr_t addr)
This should have a return value
> +{
> + if (fn > 0)
> + return;
> +}
> +
> +static int sandbox_set_msi(struct udevice *dev, uint fn, uint interrupts)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + if (fn > 0)
> + return -ENODEV;
> +
> + priv->msi = interrupts;
> +
> + return 0;
> +}
> +
> +static int sandbox_get_msi(struct udevice *dev, uint fn)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + if (fn > 0)
> + return -ENODEV;
> +
> + return priv->msi;
> +}
> +
> +static int sandbox_set_msix(struct udevice *dev, uint fn, uint interrupts)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + if (fn > 0)
> + return -ENODEV;
> +
> + priv->msix = interrupts;
> +
> + return 0;
> +}
> +
> +static int sandbox_get_msix(struct udevice *dev, uint fn)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + if (fn > 0)
> + return -ENODEV;
> +
> + return priv->msix;
> +}
> +
> +static int sandbox_raise_irq(struct udevice *dev, uint fn,
> + enum pci_ep_irq_type type, uint interrupt_num)
> +{
> + if (fn > 0)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int sandbox_start(struct udevice *dev)
> +{
> + return 0;
> +}
> +
> +static int sandbox_stop(struct udevice *dev)
> +{
> + return 0;
> +}
> +
> +static int sandbox_pci_ep_probe(struct udevice *dev)
> +{
> + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> +
> + memset(priv, 0, sizeof(*priv));
> +
> + return 0;
> +}
> +
> +static struct pci_ep_ops sandbox_pci_ep_ops = {
> + .write_header = sandbox_write_header,
> + .read_header = sandbox_read_header,
> + .set_bar = sandbox_set_bar,
> + .clear_bar = sandbox_clear_bar,
> + .map_addr = sandbox_map_addr,
> + .unmap_addr = sandbox_unmap_addr,
> + .set_msi = sandbox_set_msi,
> + .get_msi = sandbox_get_msi,
> + .set_msix = sandbox_set_msix,
> + .get_msix = sandbox_get_msix,
> + .raise_irq = sandbox_raise_irq,
> + .start = sandbox_start,
> + .stop = sandbox_stop,
> +};
> +
> +U_BOOT_DRIVER(pci_ep_sandbox) = {
> + .name = "pci_ep_sandbox",
> + .id = UCLASS_PCI_EP,
> + .of_match = sandbox_pci_ep_ids,
> + .probe = sandbox_pci_ep_probe,
> + .ops = &sandbox_pci_ep_ops,
> + .priv_auto_alloc_size = sizeof(struct sandbox_pci_ep_priv),
> +};
> --
You also need an actual test here, perhaps in test/dm/pci.c, which
calls each of these methods and checks (perhaps via a back-channel
direct call into the driver) that they work. You don't have to be
exhaustive, just a basic test for each method.
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver
2019-04-24 23:29 ` Simon Glass
@ 2019-04-25 11:35 ` Ramon Fried
2019-04-26 1:11 ` Simon Glass
0 siblings, 1 reply; 10+ messages in thread
From: Ramon Fried @ 2019-04-25 11:35 UTC (permalink / raw)
To: u-boot
On Thu, Apr 25, 2019 at 2:29 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ramon,
>
> On Wed, 24 Apr 2019 at 12:54, Ramon Fried <ramon.fried@gmail.com> wrote:
>
> Please add a commit message.
>
> >
> > Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> > ---
> >
> > arch/sandbox/dts/test.dts | 4 +
> > configs/sandbox64_defconfig | 2 +
> > configs/sandbox_defconfig | 2 +
> > drivers/pci_endpoint/Kconfig | 7 +
> > drivers/pci_endpoint/Makefile | 1 +
> > drivers/pci_endpoint/sandbox-pci_ep.c | 176 ++++++++++++++++++++++++++
> > 6 files changed, 192 insertions(+)
> > create mode 100644 drivers/pci_endpoint/sandbox-pci_ep.c
> >
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index 8b2d6451c6..001dc302ed 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -475,6 +475,10 @@
> > };
> > };
> >
> > + pci_ep: pci_ep {
> > + compatible = "sandbox,pci_ep";
> > + };
> > +
> > probing {
> > compatible = "simple-bus";
> > test1 {
> > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > index c04ecd915a..7137ea481c 100644
> > --- a/configs/sandbox64_defconfig
> > +++ b/configs/sandbox64_defconfig
> > @@ -127,9 +127,11 @@ CONFIG_SPI_FLASH_WINBOND=y
> > CONFIG_DM_ETH=y
> > CONFIG_NVME=y
> > CONFIG_PCI=y
> > +CONFIG_PCI_ENDPOINT=y
>
> It might be better to 'imply' this in the sandbox Kconfig file.
arch/sandbox/Kconfig doesn't include any implies or selects for drivers,
am I looking at the wrong file ?
>
>
> > CONFIG_DM_PCI=y
> > CONFIG_DM_PCI_COMPAT=y
> > CONFIG_PCI_SANDBOX=y
> > +CONFIG_PCI_SANDBOX_EP=y
> > CONFIG_PHY=y
> > CONFIG_PHY_SANDBOX=y
> > CONFIG_PINCTRL=y
> > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > index bb508a8d02..04ba9a3ba1 100644
> > --- a/configs/sandbox_defconfig
> > +++ b/configs/sandbox_defconfig
> > @@ -142,9 +142,11 @@ CONFIG_SPI_FLASH_WINBOND=y
> > CONFIG_DM_ETH=y
> > CONFIG_NVME=y
> > CONFIG_PCI=y
> > +CONFIG_PCI_ENDPOINT=y
> > CONFIG_DM_PCI=y
> > CONFIG_DM_PCI_COMPAT=y
> > CONFIG_PCI_SANDBOX=y
> > +CONFIG_PCI_SANDBOX_EP=y
> > CONFIG_PHY=y
> > CONFIG_PHY_SANDBOX=y
> > CONFIG_PINCTRL=y
> > diff --git a/drivers/pci_endpoint/Kconfig b/drivers/pci_endpoint/Kconfig
> > index c54bd2a9ac..e529e560fc 100644
> > --- a/drivers/pci_endpoint/Kconfig
> > +++ b/drivers/pci_endpoint/Kconfig
> > @@ -22,4 +22,11 @@ config PCIE_CADENCE_EP
> > endpoint mode. This PCIe controller may be embedded into many
> > different vendors SoCs.
> >
> > +config PCI_SANDBOX_EP
> > + bool "Sandbox PCIe endpoint controller"
> > + depends on PCI_ENDPOINT
> > + help
> > + Say Y here if you want to support the Sandbox PCIe controller in
>
> Use single space after PCIe
>
> > + endpoint mode.
>
> How about another few sentences saying what the driver does?
>
> > +
> > endmenu
> > diff --git a/drivers/pci_endpoint/Makefile b/drivers/pci_endpoint/Makefile
> > index 0a849deb19..3cd987259d 100644
> > --- a/drivers/pci_endpoint/Makefile
> > +++ b/drivers/pci_endpoint/Makefile
> > @@ -5,3 +5,4 @@
> >
> > obj-y += pci_ep-uclass.o
> > obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
> > +obj-$(CONFIG_PCI_SANDBOX_EP) += sandbox-pci_ep.o
> > diff --git a/drivers/pci_endpoint/sandbox-pci_ep.c b/drivers/pci_endpoint/sandbox-pci_ep.c
> > new file mode 100644
> > index 0000000000..eb19c6da81
> > --- /dev/null
> > +++ b/drivers/pci_endpoint/sandbox-pci_ep.c
> > @@ -0,0 +1,176 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2019 Ramon Fried <ramon.fried@gmail.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <errno.h>
>
> I think you can drop this
>
> > +#include <pci.h>
> > +#include <pci_ep.h>
> > +#include <asm/test.h>
> > +
>
> struct comment, explaining each member.
>
> > +struct sandbox_pci_ep_priv {
> > + struct pci_ep_header hdr;
> > + int msix;
> > + int msi;
> > +};
> > +
> > +static const struct udevice_id sandbox_pci_ep_ids[] = {
> > + { .compatible = "sandbox,pci_ep" },
> > + { }
> > +};
> > +
> > +static int sandbox_write_header(struct udevice *dev, uint fn,
> > + struct pci_ep_header *hdr)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + memcpy(&priv->hdr, hdr, sizeof(*hdr));
> > +
> > + return 0;
> > +}
> > +
> > +static int sandbox_read_header(struct udevice *dev, uint fn,
> > + struct pci_ep_header *hdr)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + memcpy(hdr, &priv->hdr, sizeof(*hdr));
> > +
> > + return 0;
> > +}
> > +
> > +static int sandbox_set_bar(struct udevice *dev, uint fn,
> > + struct pci_bar *ep_bar)
> > +{
> > + if (fn > 0)
> > + return -ENODEV;
>
> blank line before return. Please fix globally.
>
> > + return 0;
> > +}
> > +
> > +int sandbox_clear_bar(struct udevice *dev, uint fn,
>
> static int?
>
> > + enum pci_barno bar)
> > +{
> > + if (fn > 0)
> > + return -ENODEV;
> > + return 0;
> > +}
> > +
> > +static int sandbox_map_addr(struct udevice *dev, uint fn, phys_addr_t addr,
> > + u64 pci_addr, size_t size)
> > +{
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + return 0;
> > +}
> > +
> > +static void sandbox_unmap_addr(struct udevice *dev, uint fn, phys_addr_t addr)
>
> This should have a return value
this practically behaves like free(), why should it have a return value ?
>
> > +{
> > + if (fn > 0)
> > + return;
> > +}
> > +
> > +static int sandbox_set_msi(struct udevice *dev, uint fn, uint interrupts)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + priv->msi = interrupts;
> > +
> > + return 0;
> > +}
> > +
> > +static int sandbox_get_msi(struct udevice *dev, uint fn)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + return priv->msi;
> > +}
> > +
> > +static int sandbox_set_msix(struct udevice *dev, uint fn, uint interrupts)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + priv->msix = interrupts;
> > +
> > + return 0;
> > +}
> > +
> > +static int sandbox_get_msix(struct udevice *dev, uint fn)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + return priv->msix;
> > +}
> > +
> > +static int sandbox_raise_irq(struct udevice *dev, uint fn,
> > + enum pci_ep_irq_type type, uint interrupt_num)
> > +{
> > + if (fn > 0)
> > + return -ENODEV;
> > +
> > + return 0;
> > +}
> > +
> > +static int sandbox_start(struct udevice *dev)
> > +{
> > + return 0;
> > +}
> > +
> > +static int sandbox_stop(struct udevice *dev)
> > +{
> > + return 0;
> > +}
> > +
> > +static int sandbox_pci_ep_probe(struct udevice *dev)
> > +{
> > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > +
> > + memset(priv, 0, sizeof(*priv));
> > +
> > + return 0;
> > +}
> > +
> > +static struct pci_ep_ops sandbox_pci_ep_ops = {
> > + .write_header = sandbox_write_header,
> > + .read_header = sandbox_read_header,
> > + .set_bar = sandbox_set_bar,
> > + .clear_bar = sandbox_clear_bar,
> > + .map_addr = sandbox_map_addr,
> > + .unmap_addr = sandbox_unmap_addr,
> > + .set_msi = sandbox_set_msi,
> > + .get_msi = sandbox_get_msi,
> > + .set_msix = sandbox_set_msix,
> > + .get_msix = sandbox_get_msix,
> > + .raise_irq = sandbox_raise_irq,
> > + .start = sandbox_start,
> > + .stop = sandbox_stop,
> > +};
> > +
> > +U_BOOT_DRIVER(pci_ep_sandbox) = {
> > + .name = "pci_ep_sandbox",
> > + .id = UCLASS_PCI_EP,
> > + .of_match = sandbox_pci_ep_ids,
> > + .probe = sandbox_pci_ep_probe,
> > + .ops = &sandbox_pci_ep_ops,
> > + .priv_auto_alloc_size = sizeof(struct sandbox_pci_ep_priv),
> > +};
> > --
>
> You also need an actual test here, perhaps in test/dm/pci.c, which
> calls each of these methods and checks (perhaps via a back-channel
> direct call into the driver) that they work. You don't have to be
> exhaustive, just a basic test for each method.
Not sure I understand, how do you back-channel a driver (removing static ?).
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver
2019-04-25 11:35 ` Ramon Fried
@ 2019-04-26 1:11 ` Simon Glass
0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2019-04-26 1:11 UTC (permalink / raw)
To: u-boot
Hi Ramon,
On Thu, 25 Apr 2019 at 05:35, Ramon Fried <ramon.fried@gmail.com> wrote:
>
> On Thu, Apr 25, 2019 at 2:29 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ramon,
> >
> > On Wed, 24 Apr 2019 at 12:54, Ramon Fried <ramon.fried@gmail.com> wrote:
> >
> > Please add a commit message.
> >
> > >
> > > Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> > > ---
> > >
> > > arch/sandbox/dts/test.dts | 4 +
> > > configs/sandbox64_defconfig | 2 +
> > > configs/sandbox_defconfig | 2 +
> > > drivers/pci_endpoint/Kconfig | 7 +
> > > drivers/pci_endpoint/Makefile | 1 +
> > > drivers/pci_endpoint/sandbox-pci_ep.c | 176 ++++++++++++++++++++++++++
> > > 6 files changed, 192 insertions(+)
> > > create mode 100644 drivers/pci_endpoint/sandbox-pci_ep.c
> > >
> > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > > index 8b2d6451c6..001dc302ed 100644
> > > --- a/arch/sandbox/dts/test.dts
> > > +++ b/arch/sandbox/dts/test.dts
> > > @@ -475,6 +475,10 @@
> > > };
> > > };
> > >
> > > + pci_ep: pci_ep {
> > > + compatible = "sandbox,pci_ep";
> > > + };
> > > +
> > > probing {
> > > compatible = "simple-bus";
> > > test1 {
> > > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > > index c04ecd915a..7137ea481c 100644
> > > --- a/configs/sandbox64_defconfig
> > > +++ b/configs/sandbox64_defconfig
> > > @@ -127,9 +127,11 @@ CONFIG_SPI_FLASH_WINBOND=y
> > > CONFIG_DM_ETH=y
> > > CONFIG_NVME=y
> > > CONFIG_PCI=y
> > > +CONFIG_PCI_ENDPOINT=y
> >
> > It might be better to 'imply' this in the sandbox Kconfig file.
>
> arch/sandbox/Kconfig doesn't include any implies or selects for drivers,
> am I looking at the wrong file ?
See arch/Kconfig
>
> >
> >
> > > CONFIG_DM_PCI=y
> > > CONFIG_DM_PCI_COMPAT=y
> > > CONFIG_PCI_SANDBOX=y
> > > +CONFIG_PCI_SANDBOX_EP=y
> > > CONFIG_PHY=y
> > > CONFIG_PHY_SANDBOX=y
> > > CONFIG_PINCTRL=y
> > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > index bb508a8d02..04ba9a3ba1 100644
> > > --- a/configs/sandbox_defconfig
> > > +++ b/configs/sandbox_defconfig
> > > @@ -142,9 +142,11 @@ CONFIG_SPI_FLASH_WINBOND=y
> > > CONFIG_DM_ETH=y
> > > CONFIG_NVME=y
> > > CONFIG_PCI=y
> > > +CONFIG_PCI_ENDPOINT=y
> > > CONFIG_DM_PCI=y
> > > CONFIG_DM_PCI_COMPAT=y
> > > CONFIG_PCI_SANDBOX=y
> > > +CONFIG_PCI_SANDBOX_EP=y
> > > CONFIG_PHY=y
> > > CONFIG_PHY_SANDBOX=y
> > > CONFIG_PINCTRL=y
> > > diff --git a/drivers/pci_endpoint/Kconfig b/drivers/pci_endpoint/Kconfig
> > > index c54bd2a9ac..e529e560fc 100644
> > > --- a/drivers/pci_endpoint/Kconfig
> > > +++ b/drivers/pci_endpoint/Kconfig
> > > @@ -22,4 +22,11 @@ config PCIE_CADENCE_EP
> > > endpoint mode. This PCIe controller may be embedded into many
> > > different vendors SoCs.
> > >
> > > +config PCI_SANDBOX_EP
> > > + bool "Sandbox PCIe endpoint controller"
> > > + depends on PCI_ENDPOINT
> > > + help
> > > + Say Y here if you want to support the Sandbox PCIe controller in
> >
> > Use single space after PCIe
> >
> > > + endpoint mode.
> >
> > How about another few sentences saying what the driver does?
> >
> > > +
> > > endmenu
> > > diff --git a/drivers/pci_endpoint/Makefile b/drivers/pci_endpoint/Makefile
> > > index 0a849deb19..3cd987259d 100644
> > > --- a/drivers/pci_endpoint/Makefile
> > > +++ b/drivers/pci_endpoint/Makefile
> > > @@ -5,3 +5,4 @@
> > >
> > > obj-y += pci_ep-uclass.o
> > > obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
> > > +obj-$(CONFIG_PCI_SANDBOX_EP) += sandbox-pci_ep.o
> > > diff --git a/drivers/pci_endpoint/sandbox-pci_ep.c b/drivers/pci_endpoint/sandbox-pci_ep.c
> > > new file mode 100644
> > > index 0000000000..eb19c6da81
> > > --- /dev/null
> > > +++ b/drivers/pci_endpoint/sandbox-pci_ep.c
> > > @@ -0,0 +1,176 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2019 Ramon Fried <ramon.fried@gmail.com>
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <dm.h>
> > > +#include <errno.h>
> >
> > I think you can drop this
> >
> > > +#include <pci.h>
> > > +#include <pci_ep.h>
> > > +#include <asm/test.h>
> > > +
> >
> > struct comment, explaining each member.
> >
> > > +struct sandbox_pci_ep_priv {
> > > + struct pci_ep_header hdr;
> > > + int msix;
> > > + int msi;
> > > +};
> > > +
> > > +static const struct udevice_id sandbox_pci_ep_ids[] = {
> > > + { .compatible = "sandbox,pci_ep" },
> > > + { }
> > > +};
> > > +
> > > +static int sandbox_write_header(struct udevice *dev, uint fn,
> > > + struct pci_ep_header *hdr)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + memcpy(&priv->hdr, hdr, sizeof(*hdr));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_read_header(struct udevice *dev, uint fn,
> > > + struct pci_ep_header *hdr)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + memcpy(hdr, &priv->hdr, sizeof(*hdr));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_set_bar(struct udevice *dev, uint fn,
> > > + struct pci_bar *ep_bar)
> > > +{
> > > + if (fn > 0)
> > > + return -ENODEV;
> >
> > blank line before return. Please fix globally.
> >
> > > + return 0;
> > > +}
> > > +
> > > +int sandbox_clear_bar(struct udevice *dev, uint fn,
> >
> > static int?
> >
> > > + enum pci_barno bar)
> > > +{
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_map_addr(struct udevice *dev, uint fn, phys_addr_t addr,
> > > + u64 pci_addr, size_t size)
> > > +{
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void sandbox_unmap_addr(struct udevice *dev, uint fn, phys_addr_t addr)
> >
> > This should have a return value
> this practically behaves like free(), why should it have a return value ?
Well you have the answer two lines below. It should return -EINVAL, not void:
> >
> > > +{
> > > + if (fn > 0)
> > > + return;
> > > +}
> > > +
> > > +static int sandbox_set_msi(struct udevice *dev, uint fn, uint interrupts)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + priv->msi = interrupts;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_get_msi(struct udevice *dev, uint fn)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + return priv->msi;
> > > +}
> > > +
> > > +static int sandbox_set_msix(struct udevice *dev, uint fn, uint interrupts)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + priv->msix = interrupts;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_get_msix(struct udevice *dev, uint fn)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + return priv->msix;
> > > +}
> > > +
> > > +static int sandbox_raise_irq(struct udevice *dev, uint fn,
> > > + enum pci_ep_irq_type type, uint interrupt_num)
> > > +{
> > > + if (fn > 0)
> > > + return -ENODEV;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_start(struct udevice *dev)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_stop(struct udevice *dev)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static int sandbox_pci_ep_probe(struct udevice *dev)
> > > +{
> > > + struct sandbox_pci_ep_priv *priv = dev_get_priv(dev);
> > > +
> > > + memset(priv, 0, sizeof(*priv));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct pci_ep_ops sandbox_pci_ep_ops = {
> > > + .write_header = sandbox_write_header,
> > > + .read_header = sandbox_read_header,
> > > + .set_bar = sandbox_set_bar,
> > > + .clear_bar = sandbox_clear_bar,
> > > + .map_addr = sandbox_map_addr,
> > > + .unmap_addr = sandbox_unmap_addr,
> > > + .set_msi = sandbox_set_msi,
> > > + .get_msi = sandbox_get_msi,
> > > + .set_msix = sandbox_set_msix,
> > > + .get_msix = sandbox_get_msix,
> > > + .raise_irq = sandbox_raise_irq,
> > > + .start = sandbox_start,
> > > + .stop = sandbox_stop,
> > > +};
> > > +
> > > +U_BOOT_DRIVER(pci_ep_sandbox) = {
> > > + .name = "pci_ep_sandbox",
> > > + .id = UCLASS_PCI_EP,
> > > + .of_match = sandbox_pci_ep_ids,
> > > + .probe = sandbox_pci_ep_probe,
> > > + .ops = &sandbox_pci_ep_ops,
> > > + .priv_auto_alloc_size = sizeof(struct sandbox_pci_ep_priv),
> > > +};
> > > --
> >
> > You also need an actual test here, perhaps in test/dm/pci.c, which
> > calls each of these methods and checks (perhaps via a back-channel
> > direct call into the driver) that they work. You don't have to be
> > exhaustive, just a basic test for each method.
> Not sure I understand, how do you back-channel a driver (removing static ?).
Yes that's right. Because it is sandbox this is acceptable, and you
can add a prototype to arch/sandbox/include/asm/test.h
See test/dm/sound.c for an example.
It is also possible to add the platdata struct to that header, include
it in the test and access the platdata (or priv data) from the test to
check that things are ending up their correctly. Another way that I'm
trying to get away from is to put things in struct sandbox_state.
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 4/4] test: pci_ep: add basic pci_ep tests
2019-04-24 18:53 [U-Boot] [PATCH v2 0/4] Add Cadence PCIe endpoint driver with new uclass Ramon Fried
` (2 preceding siblings ...)
2019-04-24 18:53 ` [U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver Ramon Fried
@ 2019-04-24 18:53 ` Ramon Fried
3 siblings, 0 replies; 10+ messages in thread
From: Ramon Fried @ 2019-04-24 18:53 UTC (permalink / raw)
To: u-boot
Add basic PCI endpoint sandbox testing.
Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---
test/dm/Makefile | 1 +
test/dm/pci_ep.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
create mode 100644 test/dm/pci_ep.c
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 49857c5092..fe36f8df47 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -30,6 +30,7 @@ obj-y += ofnode.o
obj-$(CONFIG_OSD) += osd.o
obj-$(CONFIG_DM_VIDEO) += panel.o
obj-$(CONFIG_DM_PCI) += pci.o
+obj-$(CONFIG_PCI_ENDPOINT) += pci_ep.o
obj-$(CONFIG_PCH) += pch.o
obj-$(CONFIG_PHY) += phy.o
obj-$(CONFIG_POWER_DOMAIN) += power-domain.o
diff --git a/test/dm/pci_ep.c b/test/dm/pci_ep.c
new file mode 100644
index 0000000000..5f2153e393
--- /dev/null
+++ b/test/dm/pci_ep.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Ramon Fried
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <asm/io.h>
+#include <asm/test.h>
+#include <dm/test.h>
+#include <test/ut.h>
+#include <pci_ep.h>
+
+/* Test that sandbox PCI EP works correctly */
+static int dm_test_pci_ep_base(struct unit_test_state *uts)
+{
+ struct udevice *bus;
+ struct pci_ep_header ep_header = {
+ .vendorid = 0x1234,
+ .deviceid = 0x2020,
+ .revid = 1,
+ .interrupt_pin = PCI_INTERRUPT_INTA,
+ };
+
+ struct pci_bar bar = {
+ .phys_addr = 0x80000000,
+ .size = 0x100000,
+ .barno = BAR_0,
+ .flags = PCI_BASE_ADDRESS_MEM_TYPE_64 |
+ PCI_BASE_ADDRESS_MEM_PREFETCH,
+ };
+
+ ut_assertok(uclass_get_device(UCLASS_PCI_EP, 0, &bus));
+ ut_assertnonnull(bus);
+
+ ut_assertok(pci_ep_write_header(bus, 0, &ep_header));
+ ut_assertok(pci_ep_set_msi(bus, 0, 4));
+ ut_assertok(pci_ep_set_msix(bus, 0, 360));
+ ut_assertok(pci_ep_set_bar(bus, 0, &bar));
+
+ return 0;
+}
+
+DM_TEST(dm_test_pci_ep_base, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread