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_ENDPOINThelpSay 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