[U-Boot] [PATCH v1 1/3] drivers: pci_ep: Introduce UCLASS_PCI_EP uclass

Ramon Fried
2019-04-22T16:32:46Z

Hi Simon,
Thanks for the review.
please see inline, I have few questions/suggestions regarding
your notes.

Thanks,
Ramon.
On Mon, Apr 22, 2019 at 5:56 AM Simon Glass <sjg@chromium.org> wrote:

> Hi Ramon,
>
> On Fri, 5 Apr 2019 at 19:12, 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>
> >
> > ---
> >
> > drivers/Kconfig | 2 +
> > drivers/Makefile | 1 +
> > drivers/pci_endpoint/Kconfig | 16 ++
> > drivers/pci_endpoint/Makefile | 6 +
> > drivers/pci_endpoint/pci_ep-uclass.c | 192 ++++++++++++++
> > include/dm/uclass-id.h | 1 +
> > include/pci_ep.h | 375 +++++++++++++++++++++++++++
> > 7 files changed, 593 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
>
> This looks pretty good but I have a lot of nits, sorry.
>
> no worries.

> Please can you add a sandbox driver and a test for this (can be in a
> separate patch).
>
> sure.

> >
> > diff --git a/drivers/Kconfig b/drivers/Kconfig
> > index f24351ac4f..59e2c22cc6 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..2c0a399a08
> > --- /dev/null
> > +++ b/drivers/pci_endpoint/Kconfig
> > @@ -0,0 +1,16 @@
> > +# 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
> > + endpoint. This should be enabled if the platform has a PCI
>
> s/endpoint/endpoints/ I think
>
> Right.

> > + controller that can operate in endpoint mode.
>
Can you explain a bit more about what this means. I understand that
> people can look at the spec, but what is the purpose of 'endpoint
> mode' and what does it enable?
>
> > +
> > +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..06fcfc5d14
> > --- /dev/null
> > +++ b/drivers/pci_endpoint/pci_ep-uclass.c
> > @@ -0,0 +1,192 @@
> > +// 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 dm_pci_ep_write_header(struct udevice *dev, u8 fn,
> > + struct pci_ep_header *hdr)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->write_header)
> > + return -ENODEV;
>
> -ENOSYS (please fix globally)
>
> OK.

> There is a device. The problem here is that the driver does not
> implement the method.
>
> > +
> > + return ops->write_header(dev, fn, hdr);
> > +}
> > +
> > +int dm_pci_ep_read_header(struct udevice *dev, u8 fn,
> > + struct pci_ep_header *hdr)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->read_header)
> > + return -ENODEV;
> > +
> > + return ops->read_header(dev, fn, hdr);
> > +}
> > +
> > +int dm_pci_ep_set_bar(struct udevice *dev, u8 func_no,
>
> Please use uint for func_no. There is no need to limit it to 8 bits,
> and this may not be efficient for non-8-bit machines. Please fix
> globally.
>
I tried to keep the API as similar as it can to the Linux API.
I can change it, no problem, but IMHO I think it's better to keep it
similar.

>
> > + struct pci_bar *ep_bar)
> > +{
> > + struct dm_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) ||
>
> Can you add another set of () in those two lines?
>
yep

>
> > + (upper_32_bits(ep_bar->size) &&
> > + !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64)))
> > + return -EINVAL;
> > +
> > + if (!ops->set_bar)
> > + return -ENODEV;
> > +
> > + return ops->set_bar(dev, func_no, ep_bar);
> > +}
> > +
> > +void pci_ep_clear_bar(struct udevice *dev, u8 func_num, enum pci_barno
> bar)
>
> This should return an error code.
>
ok

>
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->clear_bar)
> > + return;
> > +
> > + ops->clear_bar(dev, func_num, bar);
> > +}
> > +
> > +int dm_pci_ep_map_addr(struct udevice *dev, u8 func_no, phys_addr_t
> addr,
> > + u64 pci_addr, size_t size)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->map_addr)
> > + return -ENODEV;
> > +
> > + return ops->map_addr(dev, func_no, addr, pci_addr, size);
> > +}
> > +
> > +void dm_pci_ep_unmap_addr(struct udevice *dev, u8 func_no, phys_addr_t
> addr)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->unmap_addr)
> > + return;
> > +
> > + ops->unmap_addr(dev, func_no, addr);
> > +}
> > +
> > +int dm_pci_ep_set_msi(struct udevice *dev, u8 func_no, u8 interrupts)
>
> Similar with interrupts (use uint).
>
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > + u8 encode_int;
>
> uint
>
> > +
> > + if (interrupts > 32)
> > + return -EINVAL;
> > +
> > + if (!ops->set_msi)
> > + return -ENODEV;
> > +
> > + encode_int = order_base_2(interrupts);
> > +
> > + return ops->set_msi(dev, func_no, encode_int);
> > +}
> > +
> > +int dm_pci_ep_get_msi(struct udevice *dev, u8 func_no)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > + int interrupt;
> > +
> > + if (!ops->get_msi)
> > + return -ENODEV;
> > +
> > + interrupt = ops->get_msi(dev, func_no);
> > +
> > + if (interrupt < 0)
> > + return 0;
> > +
> > + interrupt = 1 << interrupt;
>
> Perhaps you should declare a 'mask' variable? In any case, it is
> confusing for the same variable to have two different meanings, and it
> does not save code at run-time.
>
as before, this is practically a copy-paste from Linux, I can change it,
but I think it's clearer if the code looks the same as in Linux,
might be easier the port patches like that.

>
> > +
> > + return interrupt;
> > +}
> > +
> > +int dm_pci_ep_set_msix(struct udevice *dev, u8 func_no, u16 interrupts)
>
> s/u16/uint/
>
> There is no saving by using u16, only potential cost. You range-check
> it below anyway. Please fix globally.
>
ok

>
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (interrupts < 1 || interrupts > 2048)
> > + return -EINVAL;
> > +
> > + if (!ops->set_msix)
> > + return -ENODEV;
> > +
> > + return ops->set_msix(dev, func_no, interrupts);
> > +}
> > +
> > +int dm_pci_ep_get_msix(struct udevice *dev, u8 func_no)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > + int interrupt;
> > +
> > + if (!ops->get_msix)
> > + return -ENODEV;
> > +
> > + interrupt = ops->get_msix(dev, func_no);
> > +
> > + if (interrupt < 0)
> > + return 0;
> > +
> > + return interrupt + 1;
>
> It's odd that your uclass function returns a different value from your
> driver function. I think that will be confusing. Is it necessary?
>
As you can understand from the code, 0 means 1 interrupt. basically the
driver functions just return the msix field in the PCI registers,
The translation to real number is done by the uclass wrapper.

>
> > +}
> > +
> > +int dm_pci_ep_raise_irq(struct udevice *dev, u8 func_no,
> > + enum pci_ep_irq_type type, u16 interrupt_num)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->raise_irq)
> > + return -ENODEV;
> > +
> > + return ops->raise_irq(dev, func_no, type, interrupt_num);
> > +}
> > +
> > +int dm_pci_ep_start(struct udevice *dev)
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->start)
> > + return -ENODEV;
> > +
> > + return ops->start(dev);
> > +}
> > +
> > +void dm_pci_ep_stop(struct udevice *dev)
>
> Needs a return value.
>
ok

>
> > +{
> > + struct dm_pci_ep_ops *ops = pci_ep_get_ops(dev);
> > +
> > + if (!ops->stop)
> > + 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..c98453ccfa
> > --- /dev/null
> > +++ b/include/pci_ep.h
> > @@ -0,0 +1,375 @@
> > +/* 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 of EP device
>
> What is a BAR and what is an EP? Please spell things out in comments,
> at least ones per file:
>
> base-address register (BAR)
>
> > + * @phys_addr: physical address that should be mapped to the BAR
> > + * @size: the size of the address space present in BAR
>
> Please add the other two also
>
> > + */
> > +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
>
> What is a DWORD? I think i is 32-bits, so just say that, since WORD is
> a confusing term on a non-16-bit machine.
>
hehe, I presume it's just a copy-paste from the PCI spec. I'll check, if
so, I'll prefer to keep the original naming.

>
> > + * @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;
>
> Shouldn't that be a u16 or something? The enum does not have a defined
> size, I think.
>
well, there can be only 4 different interrupt pins, so it doesn't matter as
long as the rage
is checked.

>
> > +};
> > +
> > +/* PCI endpoint operations */
> > +struct dm_pci_ep_ops {
> > + /**
> > + * write_header() - Write a PCI configuration space header
> > + *
> > + * @dev: device to write to
> > + * @func_num: Function to fill
>
> Is this the function number within the device? If so, how about
> 'Function number (within device) to write to'? Similarly below.
>
OK, I concur.

>
> > + * @hdr: header to write
> > + * @return 0 if OK, -ve on error
> > + */
> > + int (*write_header)(struct udevice *dev, u8 func_num,
> > + struct pci_ep_header *hdr);
> > + /**
> > + * read_header() - Read a PCI configuration space header
> > + *
> > + * @dev: device to write to
> > + * @func_num: Function to fill
> > + * @hdr: header to read to
> > + * @return 0 if OK, -ve on error
> > + */
> > + int (*read_header)(struct udevice *dev, u8 func_num,
> > + struct pci_ep_header *hdr);
> > + /**
> > + * set_bar() - set BAR (Base Address Register)
>
> Good to expand BAR as you have here
>
> > + *
> > + * @dev: device to set
> > + * @func_num: Function to set
> > + * @bar: bar data
> > + * @return 0 if OK, -ve on error
> > + */
> > + int (*set_bar)(struct udevice *dev, u8 func_num,
> > + struct pci_bar *bar);
> > + /**
> > + * clear_bar() - clear BAR (Base Address Register)
> > + *
> > + * @dev: device to clear
> > + * @func_num: Function to clear
> > + * @bar: bar number
> > + */
> > + void (*clear_bar)(struct udevice *dev, u8 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: 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, u8 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: Function to set
> > + * @addr: local physical address base
> > + */
> > + void (*unmap_addr)(struct udevice *dev, u8 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: Function to set
> > + * @interrupts: required interrupts count
> > + * @return 0 if OK, -ve on error
> > + */
> > + int (*set_msi)(struct udevice *dev, u8 func_num, u8
> 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: Function to use
> > + * @return msi count if OK, -EINVAL if msi were not enabled at
> host.
> > + */
> > + int (*get_msi)(struct udevice *dev, u8 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: Function to set
> > + * @interrupts: required interrupts count
>
> This is too vague, please expand it.
>
You're referring to the set_msix() or the whole section,
can you elaborate ?

> > + * @return 0 if OK, -ve on error
> > + */
> > + int (*set_msix)(struct udevice *dev, u8 func_num, u16
> 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: Function to use
> > + * @return msi count if OK, -EINVAL if msi were not enabled at
> host.
> > + */
> > + int (*get_msix)(struct udevice *dev, u8 func_num);
> > +
> > + /**
> > + * raise_irq() - raise a legacy, MSI or MSI-X interrupt
> > + *
> > + * @dev: device to set
> > + * @func_num: 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, u8 func_num,
> > + enum pci_ep_irq_type type, u16
> interrupt_num);
> > + /**
> > + * start() - start the PCI link
> > + *
>
> Needs more docs. What is the effect of this?
>
OK, I'll elaborate more.

> > + * @dev: device to set
> > + * @return 0 if OK, -ve on error
> > + */
> > + int (*start)(struct udevice *dev);
> > +
> > + /**
> > + * stop() - stop the PCI link
>
> Needs more docs. What is the effect of this?
>
> > + *
> > + * @dev: device to set
> > + */
> > + void (*stop)(struct udevice *dev);
> > +};
> > +
> > +#define pci_ep_get_ops(dev) ((struct dm_pci_ep_ops
> *)(dev)->driver->ops)
> > +
> > +/**
> > + * pci_ep_write_header() - Write a PCI configuration space header
> > + *
> > + * @dev: device to write to
> > + * @func_num: Function to fill
> > + * @hdr: header to write
> > + * @return 0 if OK, -ve on error
> > + */
> > +int pci_ep_write_header(struct udevice *dev, u8 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: Function to fill
> > + * @hdr: header to read to
> > + * @return 0 if OK, -ve on error
> > + */
> > +int pci_ep_read_header(struct udevice *dev, u8 func_num,
> > + struct pci_ep_header *hdr);
> > +/**
> > + * pci_ep_set_bar() - set BAR (Base Address Register)
> > + *
> > + * @dev: device to set
> > + * @func_num: Function to set
> > + * @bar: bar data
> > + * @return 0 if OK, -ve on error
> > + */
> > +int pci_ep_set_bar(struct udevice *dev, u8 func_num, struct pci_bar
> *bar);
> > +
> > +/**
> > + * pci_ep_clear_bar() - clear BAR (Base Address Register)
> > + *
> > + * @dev: device to clear
> > + * @func_num: Function to clear
> > + * @bar: bar number
> > + */
> > +void pci_ep_clear_bar(struct udevice *dev, u8 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: 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, u8 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: Function to set
> > + * @addr: local physical address base
> > + */
> > +void pci_ep_unmap_addr(struct udevice *dev, u8 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: Function to set
> > + * @interrupts: required interrupts count
> > + * @return 0 if OK, -ve on error
> > + */
> > +int pci_ep_set_msi(struct udevice *dev, u8 func_num, u8 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: Function to use
> > + * @return msi count if OK, -EINVAL if msi were not enabled at host.
> > + */
> > +int pci_ep_get_msi(struct udevice *dev, u8 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: Function to set
> > + * @interrupts: required interrupts count
> > + * @return 0 if OK, -ve on error
> > + */
> > +int pci_ep_set_msix(struct udevice *dev, u8 func_num, u16 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: Function to use
> > + * @return msi count if OK, -EINVAL if msi were not enabled at host.
> > + */
> > +int pci_ep_get_msix(struct udevice *dev, u8 func_num);
> > +
> > +/**
> > + * pci_ep_raise_irq() - raise a legacy, MSI or MSI-X interrupt
> > + *
> > + * @dev: device to set
> > + * @func_num: 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, u8 func_num,
> > + enum pci_ep_irq_type type, u16 interrupt_num);
> > +/**
> > + * pci_ep_start() - start the PCI link
> > + *
> > + * @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
> > + *
> > + * @dev: device to set
> > + */
> > +void pci_ep_stop(struct udevice *dev);
> > +
> > +#endif
> > --
> > 2.21.0
> >
>
> Regards,
> Simon
>