[U-Boot] [PATCH v2 3/4] pci_ep: add pci_ep sandbox driver
Simon Glass
sjg at chromium.org
Wed Apr 24 23:29:32 UTC 2019
Hi Ramon,
On Wed, 24 Apr 2019 at 12:54, Ramon Fried <ramon.fried at gmail.com> wrote:
Please add a commit message.
>
> Signed-off-by: Ramon Fried <ramon.fried at 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 at 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
More information about the U-Boot
mailing list