Hi Ramon, On Wed, 24 Apr 2019 at 12:54, Ramon Fried <[email protected]> wrote:
Please add a commit message. > > Signed-off-by: Ramon Fried <[email protected]> > --- > > 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 <[email protected]> > + */ > + > +#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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
