| Message ID | 20200724100856.1482324-17-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | arm: Introduce Marvell/Cavium OcteonTX/TX2 | expand |
Hi Stefan, On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr@denx.de> wrote: > > From: Suneel Garapati <sgarapati@marvell.com> > > Adds support for PCI ECAM/PEM controllers found on OcteonTX > or OcteonTX2 SoC platforms. > > Signed-off-by: Suneel Garapati <sgarapati@marvell.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Bin Meng <bmeng.cn@gmail.com> > > Signed-off-by: Stefan Roese <sr@denx.de> > --- > > Changes in v1: > - Change patch subject > - Remove inclusion of common.h > - Remove #ifdef's and use driver specific data instead > - Add comments to struct > - Add some helper functions to reduce code size > - Misc coding style changes (blank lines etc) > - Use debug() instead of printf() in some cases > > drivers/pci/Kconfig | 8 + > drivers/pci/Makefile | 1 + > drivers/pci/pci_octeontx.c | 344 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 353 insertions(+) > create mode 100644 drivers/pci/pci_octeontx.c Reviewed-by: Simon Glass <sjg@chromium.org> > > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig > index bc77c23f89..89cca6ffb3 100644 > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -149,6 +149,14 @@ config PCI_TEGRA > with a total of 5 lanes. Some boards require this for Ethernet > support to work (e.g. beaver, jetson-tk1). > > +config PCI_OCTEONTX > + bool "OcteonTX PCI support" > + depends on (ARCH_OCTEONTX || ARCH_OCTEONTX2) > + help > + Enable support for the OcteonTX/TX2 SoC family ECAM/PEM controllers. > + These controllers provide PCI configuration access to all on-board > + peripherals so it should only be disabled for testing purposes > + > config PCI_XILINX > bool "Xilinx AXI Bridge for PCI Express" > depends on DM_PCI > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile > index 6378821aaf..0529cceee7 100644 > --- a/drivers/pci/Makefile > +++ b/drivers/pci/Makefile > @@ -45,3 +45,4 @@ obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o > obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o > obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o > obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o > +obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o > diff --git a/drivers/pci/pci_octeontx.c b/drivers/pci/pci_octeontx.c > new file mode 100644 > index 0000000000..5c6a6f05f2 > --- /dev/null > +++ b/drivers/pci/pci_octeontx.c > @@ -0,0 +1,344 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Marvell International Ltd. > + * > + * https://spdx.org/licenses > + */ > + > +#include <dm.h> > +#include <errno.h> > +#include <fdtdec.h> > +#include <log.h> > +#include <malloc.h> > +#include <pci.h> > + > +#include <asm/io.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +enum { comment? > + OTX_ECAM, > + OTX_PEM, > + OTX2_PEM, > +}; > + > +/** > + * struct octeontx_pci - Driver private data > + * @type: Device type matched via compatible > + * @cfg: Config resource > + * @bus: Bus resource > + */ > +struct octeontx_pci { > + unsigned int type; Is this the enum above? > + > + struct fdt_resource cfg; > + struct fdt_resource bus; > +}; > + > +static uintptr_t octeontx_cfg_addr(struct octeontx_pci *pcie, > + int bus_offs, int shift_offs, > + pci_dev_t bdf, uint offset) > +{ > + u32 bus, dev, func; > + uintptr_t address; > + > + bus = PCI_BUS(bdf) + bus_offs; > + dev = PCI_DEV(bdf); > + func = PCI_FUNC(bdf); > + > + address = (bus << (20 + shift_offs)) | > + (dev << (15 + shift_offs)) | > + (func << (12 + shift_offs)) | offset; > + address += pcie->cfg.start; > + > + return address; > +} > + > +static ulong readl_size(uintptr_t addr, enum pci_size_t size) > +{ > + ulong val; > + > + switch (size) { > + case PCI_SIZE_8: > + val = readb(addr); > + break; > + case PCI_SIZE_16: > + val = readw(addr); > + break; > + case PCI_SIZE_32: > + val = readl(addr); > + break; > + default: > + printf("Invalid size\n"); return -EINVAL perhaps? Otherwise val is unset. > + }; > + > + return val; > +} > + > +static void writel_size(uintptr_t addr, enum pci_size_t size, ulong valuep) > +{ > + switch (size) { > + case PCI_SIZE_8: > + writeb(valuep, addr); > + break; > + case PCI_SIZE_16: > + writew(valuep, addr); > + break; > + case PCI_SIZE_32: > + writel(valuep, addr); > + break; > + default: > + printf("Invalid size\n"); > + }; > +} > + > +static int octeontx_ecam_read_config(const struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong *valuep, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + struct pci_controller *hose = dev_get_uclass_priv(bus); > + uintptr_t address; > + > + address = octeontx_cfg_addr(pcie, pcie->bus.start - hose->first_busno, > + 0, bdf, offset); > + *valuep = readl_size(address, size); > + > + debug("%02x.%02x.%02x: u%d %x -> %lx\n", > + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, *valuep); > + > + return 0; > +} > + > +static int octeontx_ecam_write_config(struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong value, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + struct pci_controller *hose = dev_get_uclass_priv(bus); > + uintptr_t address; > + > + address = octeontx_cfg_addr(pcie, pcie->bus.start - hose->first_busno, > + 0, bdf, offset); > + writel_size(address, size, value); > + > + debug("%02x.%02x.%02x: u%d %x <- %lx\n", > + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, value); > + > + return 0; > +} > + > +static int octeontx_pem_read_config(const struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong *valuep, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + struct pci_controller *hose = dev_get_uclass_priv(bus); > + uintptr_t address; > + u8 hdrtype; > + u8 pri_bus = pcie->bus.start + 1 - hose->first_busno; > + u32 bus_offs = (pri_bus << 16) | (pri_bus << 8) | (pri_bus << 0); > + > + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 4, > + bdf, 0); > + > + *valuep = pci_conv_32_to_size(~0UL, offset, size); > + > + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) > + return 0; Can you put this check in a function? It seems to appear everywhere. Also, shouldn't you return -EPERM, or similar? > + > + *valuep = readl_size(address + offset, size); > + > + hdrtype = readb(address + PCI_HEADER_TYPE); > + if (hdrtype == PCI_HEADER_TYPE_BRIDGE && > + offset >= PCI_PRIMARY_BUS && > + offset <= PCI_SUBORDINATE_BUS && > + *valuep != pci_conv_32_to_size(~0UL, offset, size)) > + *valuep -= pci_conv_32_to_size(bus_offs, offset, size); > + > + return 0; > +} > + > +static int octeontx_pem_write_config(struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong value, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + struct pci_controller *hose = dev_get_uclass_priv(bus); > + uintptr_t address; > + u8 hdrtype; > + u8 pri_bus = pcie->bus.start + 1 - hose->first_busno; > + u32 bus_offs = (pri_bus << 16) | (pri_bus << 8) | (pri_bus << 0); > + > + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 4, bdf, 0); > + > + hdrtype = readb(address + PCI_HEADER_TYPE); > + if (hdrtype == PCI_HEADER_TYPE_BRIDGE && > + offset >= PCI_PRIMARY_BUS && > + offset <= PCI_SUBORDINATE_BUS && > + value != pci_conv_32_to_size(~0UL, offset, size)) > + value += pci_conv_32_to_size(bus_offs, offset, size); > + > + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) > + return 0; > + > + writel_size(address + offset, size, value); > + > + debug("%02x.%02x.%02x: u%d %x (%lx) <- %lx\n", > + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, > + address, value); > + > + return 0; > +} > + > +static int octeontx2_pem_read_config(const struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong *valuep, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + struct pci_controller *hose = dev_get_uclass_priv(bus); > + uintptr_t address; > + > + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 0, > + bdf, 0); > + > + *valuep = pci_conv_32_to_size(~0UL, offset, size); > + > + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) > + return 0; > + > + *valuep = readl_size(address + offset, size); > + > + debug("%02x.%02x.%02x: u%d %x (%lx) -> %lx\n", > + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, > + address, *valuep); > + > + return 0; > +} > + > +static int octeontx2_pem_write_config(struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong value, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + struct pci_controller *hose = dev_get_uclass_priv(bus); > + uintptr_t address; > + > + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 0, > + bdf, 0); > + > + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) > + return 0; > + > + writel_size(address + offset, size, value); > + > + debug("%02x.%02x.%02x: u%d %x (%lx) <- %lx\n", > + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, > + address, value); > + > + return 0; > +} > + > +int pci_octeontx_read_config(const struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong *valuep, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + int ret = -EIO; > + > + switch (pcie->type) { > + case OTX_ECAM: > + ret = octeontx_ecam_read_config(bus, bdf, offset, valuep, > + size); > + break; > + case OTX_PEM: > + ret = octeontx_pem_read_config(bus, bdf, offset, valuep, > + size); > + break; > + case OTX2_PEM: > + ret = octeontx2_pem_read_config(bus, bdf, offset, valuep, > + size); > + break; > + } > + > + return ret; > +} > + > +int pci_octeontx_write_config(struct udevice *bus, pci_dev_t bdf, > + uint offset, ulong value, > + enum pci_size_t size) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); > + int ret = -EIO; > + > + switch (pcie->type) { > + case OTX_ECAM: > + ret = octeontx_ecam_write_config(bus, bdf, offset, value, > + size); > + break; > + case OTX_PEM: > + ret = octeontx_pem_write_config(bus, bdf, offset, value, > + size); > + break; > + case OTX2_PEM: > + ret = octeontx2_pem_write_config(bus, bdf, offset, value, > + size); > + break; > + } > + > + return ret; > +} > + > +static int pci_octeontx_ofdata_to_platdata(struct udevice *dev) > +{ > + return 0; > +} > + > +static int pci_octeontx_probe(struct udevice *dev) > +{ > + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(dev); > + int err; > + > + pcie->type = dev_get_driver_data(dev); > + > + err = fdt_get_resource(gd->fdt_blob, dev->node.of_offset, "reg", 0, > + &pcie->cfg); We really should have a livetree API for this. > + if (err) { > + debug("Error reading resource: %s\n", fdt_strerror(err)); > + return err; > + } > + > + err = fdtdec_get_pci_bus_range(gd->fdt_blob, dev->node.of_offset, > + &pcie->bus); and this... > + if (err) { > + debug("Error reading resource: %s\n", fdt_strerror(err)); > + return err; > + } > + > + return 0; > +} > + > +static const struct dm_pci_ops pci_octeontx_ops = { > + .read_config = pci_octeontx_read_config, > + .write_config = pci_octeontx_write_config, > +}; > + > +static const struct udevice_id pci_octeontx_ids[] = { > + { .compatible = "cavium,pci-host-thunder-ecam", .data = OTX_ECAM }, > + { .compatible = "cavium,pci-host-octeontx-ecam", .data = OTX_ECAM }, > + { .compatible = "pci-host-ecam-generic", .data = OTX_ECAM }, > + { .compatible = "cavium,pci-host-thunder-pem", .data = OTX_PEM }, > + { .compatible = "marvell,pci-host-octeontx2-pem", .data = OTX2_PEM }, > + { } > +}; > + > +U_BOOT_DRIVER(pci_octeontx) = { > + .name = "pci_octeontx", > + .id = UCLASS_PCI, > + .of_match = pci_octeontx_ids, > + .ops = &pci_octeontx_ops, > + .ofdata_to_platdata = pci_octeontx_ofdata_to_platdata, > + .probe = pci_octeontx_probe, > + .priv_auto_alloc_size = sizeof(struct octeontx_pci), > + .flags = DM_FLAG_PRE_RELOC, > +}; > -- > 2.27.0 > Regards, Simon
Hi Simon, On 28.07.20 21:01, Simon Glass wrote: > Hi Stefan, > > On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr@denx.de> wrote: >> >> From: Suneel Garapati <sgarapati@marvell.com> >> >> Adds support for PCI ECAM/PEM controllers found on OcteonTX >> or OcteonTX2 SoC platforms. >> >> Signed-off-by: Suneel Garapati <sgarapati@marvell.com> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> --- >> >> Changes in v1: >> - Change patch subject >> - Remove inclusion of common.h >> - Remove #ifdef's and use driver specific data instead >> - Add comments to struct >> - Add some helper functions to reduce code size >> - Misc coding style changes (blank lines etc) >> - Use debug() instead of printf() in some cases >> >> drivers/pci/Kconfig | 8 + >> drivers/pci/Makefile | 1 + >> drivers/pci/pci_octeontx.c | 344 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 353 insertions(+) >> create mode 100644 drivers/pci/pci_octeontx.c > > Reviewed-by: Simon Glass <sjg@chromium.org> > >> >> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig >> index bc77c23f89..89cca6ffb3 100644 >> --- a/drivers/pci/Kconfig >> +++ b/drivers/pci/Kconfig >> @@ -149,6 +149,14 @@ config PCI_TEGRA >> with a total of 5 lanes. Some boards require this for Ethernet >> support to work (e.g. beaver, jetson-tk1). >> >> +config PCI_OCTEONTX >> + bool "OcteonTX PCI support" >> + depends on (ARCH_OCTEONTX || ARCH_OCTEONTX2) >> + help >> + Enable support for the OcteonTX/TX2 SoC family ECAM/PEM controllers. >> + These controllers provide PCI configuration access to all on-board >> + peripherals so it should only be disabled for testing purposes >> + >> config PCI_XILINX >> bool "Xilinx AXI Bridge for PCI Express" >> depends on DM_PCI >> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile >> index 6378821aaf..0529cceee7 100644 >> --- a/drivers/pci/Makefile >> +++ b/drivers/pci/Makefile >> @@ -45,3 +45,4 @@ obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o >> obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o >> obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o >> obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o >> +obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o >> diff --git a/drivers/pci/pci_octeontx.c b/drivers/pci/pci_octeontx.c >> new file mode 100644 >> index 0000000000..5c6a6f05f2 >> --- /dev/null >> +++ b/drivers/pci/pci_octeontx.c >> @@ -0,0 +1,344 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2018 Marvell International Ltd. >> + * >> + * https://spdx.org/licenses >> + */ >> + >> +#include <dm.h> >> +#include <errno.h> >> +#include <fdtdec.h> >> +#include <log.h> >> +#include <malloc.h> >> +#include <pci.h> >> + >> +#include <asm/io.h> >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +enum { > > comment? Will add in next version. >> + OTX_ECAM, >> + OTX_PEM, >> + OTX2_PEM, >> +}; >> + >> +/** >> + * struct octeontx_pci - Driver private data >> + * @type: Device type matched via compatible >> + * @cfg: Config resource >> + * @bus: Bus resource >> + */ >> +struct octeontx_pci { >> + unsigned int type; > > Is this the enum above? Yes. Will add some comment here. >> + >> + struct fdt_resource cfg; >> + struct fdt_resource bus; >> +}; >> + >> +static uintptr_t octeontx_cfg_addr(struct octeontx_pci *pcie, >> + int bus_offs, int shift_offs, >> + pci_dev_t bdf, uint offset) >> +{ >> + u32 bus, dev, func; >> + uintptr_t address; >> + >> + bus = PCI_BUS(bdf) + bus_offs; >> + dev = PCI_DEV(bdf); >> + func = PCI_FUNC(bdf); >> + >> + address = (bus << (20 + shift_offs)) | >> + (dev << (15 + shift_offs)) | >> + (func << (12 + shift_offs)) | offset; >> + address += pcie->cfg.start; >> + >> + return address; >> +} >> + >> +static ulong readl_size(uintptr_t addr, enum pci_size_t size) >> +{ >> + ulong val; >> + >> + switch (size) { >> + case PCI_SIZE_8: >> + val = readb(addr); >> + break; >> + case PCI_SIZE_16: >> + val = readw(addr); >> + break; >> + case PCI_SIZE_32: >> + val = readl(addr); >> + break; >> + default: >> + printf("Invalid size\n"); > > return -EINVAL perhaps? Otherwise val is unset. Good idea. Will update. >> + }; >> + >> + return val; >> +} >> + >> +static void writel_size(uintptr_t addr, enum pci_size_t size, ulong valuep) >> +{ >> + switch (size) { >> + case PCI_SIZE_8: >> + writeb(valuep, addr); >> + break; >> + case PCI_SIZE_16: >> + writew(valuep, addr); >> + break; >> + case PCI_SIZE_32: >> + writel(valuep, addr); >> + break; >> + default: >> + printf("Invalid size\n"); >> + }; >> +} >> + >> +static int octeontx_ecam_read_config(const struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong *valuep, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + struct pci_controller *hose = dev_get_uclass_priv(bus); >> + uintptr_t address; >> + >> + address = octeontx_cfg_addr(pcie, pcie->bus.start - hose->first_busno, >> + 0, bdf, offset); >> + *valuep = readl_size(address, size); >> + >> + debug("%02x.%02x.%02x: u%d %x -> %lx\n", >> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, *valuep); >> + >> + return 0; >> +} >> + >> +static int octeontx_ecam_write_config(struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong value, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + struct pci_controller *hose = dev_get_uclass_priv(bus); >> + uintptr_t address; >> + >> + address = octeontx_cfg_addr(pcie, pcie->bus.start - hose->first_busno, >> + 0, bdf, offset); >> + writel_size(address, size, value); >> + >> + debug("%02x.%02x.%02x: u%d %x <- %lx\n", >> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, value); >> + >> + return 0; >> +} >> + >> +static int octeontx_pem_read_config(const struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong *valuep, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + struct pci_controller *hose = dev_get_uclass_priv(bus); >> + uintptr_t address; >> + u8 hdrtype; >> + u8 pri_bus = pcie->bus.start + 1 - hose->first_busno; >> + u32 bus_offs = (pri_bus << 16) | (pri_bus << 8) | (pri_bus << 0); >> + >> + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 4, >> + bdf, 0); >> + >> + *valuep = pci_conv_32_to_size(~0UL, offset, size); >> + >> + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) >> + return 0; > > Can you put this check in a function? It seems to appear everywhere. > Also, shouldn't you return -EPERM, or similar? Will change in next version. >> + >> + *valuep = readl_size(address + offset, size); >> + >> + hdrtype = readb(address + PCI_HEADER_TYPE); >> + if (hdrtype == PCI_HEADER_TYPE_BRIDGE && >> + offset >= PCI_PRIMARY_BUS && >> + offset <= PCI_SUBORDINATE_BUS && >> + *valuep != pci_conv_32_to_size(~0UL, offset, size)) >> + *valuep -= pci_conv_32_to_size(bus_offs, offset, size); >> + >> + return 0; >> +} >> + >> +static int octeontx_pem_write_config(struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong value, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + struct pci_controller *hose = dev_get_uclass_priv(bus); >> + uintptr_t address; >> + u8 hdrtype; >> + u8 pri_bus = pcie->bus.start + 1 - hose->first_busno; >> + u32 bus_offs = (pri_bus << 16) | (pri_bus << 8) | (pri_bus << 0); >> + >> + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 4, bdf, 0); >> + >> + hdrtype = readb(address + PCI_HEADER_TYPE); >> + if (hdrtype == PCI_HEADER_TYPE_BRIDGE && >> + offset >= PCI_PRIMARY_BUS && >> + offset <= PCI_SUBORDINATE_BUS && >> + value != pci_conv_32_to_size(~0UL, offset, size)) >> + value += pci_conv_32_to_size(bus_offs, offset, size); >> + >> + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) >> + return 0; >> + >> + writel_size(address + offset, size, value); >> + >> + debug("%02x.%02x.%02x: u%d %x (%lx) <- %lx\n", >> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, >> + address, value); >> + >> + return 0; >> +} >> + >> +static int octeontx2_pem_read_config(const struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong *valuep, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + struct pci_controller *hose = dev_get_uclass_priv(bus); >> + uintptr_t address; >> + >> + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 0, >> + bdf, 0); >> + >> + *valuep = pci_conv_32_to_size(~0UL, offset, size); >> + >> + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) >> + return 0; >> + >> + *valuep = readl_size(address + offset, size); >> + >> + debug("%02x.%02x.%02x: u%d %x (%lx) -> %lx\n", >> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, >> + address, *valuep); >> + >> + return 0; >> +} >> + >> +static int octeontx2_pem_write_config(struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong value, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + struct pci_controller *hose = dev_get_uclass_priv(bus); >> + uintptr_t address; >> + >> + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 0, >> + bdf, 0); >> + >> + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) >> + return 0; >> + >> + writel_size(address + offset, size, value); >> + >> + debug("%02x.%02x.%02x: u%d %x (%lx) <- %lx\n", >> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, >> + address, value); >> + >> + return 0; >> +} >> + >> +int pci_octeontx_read_config(const struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong *valuep, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + int ret = -EIO; >> + >> + switch (pcie->type) { >> + case OTX_ECAM: >> + ret = octeontx_ecam_read_config(bus, bdf, offset, valuep, >> + size); >> + break; >> + case OTX_PEM: >> + ret = octeontx_pem_read_config(bus, bdf, offset, valuep, >> + size); >> + break; >> + case OTX2_PEM: >> + ret = octeontx2_pem_read_config(bus, bdf, offset, valuep, >> + size); >> + break; >> + } >> + >> + return ret; >> +} >> + >> +int pci_octeontx_write_config(struct udevice *bus, pci_dev_t bdf, >> + uint offset, ulong value, >> + enum pci_size_t size) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); >> + int ret = -EIO; >> + >> + switch (pcie->type) { >> + case OTX_ECAM: >> + ret = octeontx_ecam_write_config(bus, bdf, offset, value, >> + size); >> + break; >> + case OTX_PEM: >> + ret = octeontx_pem_write_config(bus, bdf, offset, value, >> + size); >> + break; >> + case OTX2_PEM: >> + ret = octeontx2_pem_write_config(bus, bdf, offset, value, >> + size); >> + break; >> + } >> + >> + return ret; >> +} >> + >> +static int pci_octeontx_ofdata_to_platdata(struct udevice *dev) >> +{ >> + return 0; >> +} >> + >> +static int pci_octeontx_probe(struct udevice *dev) >> +{ >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(dev); >> + int err; >> + >> + pcie->type = dev_get_driver_data(dev); >> + >> + err = fdt_get_resource(gd->fdt_blob, dev->node.of_offset, "reg", 0, >> + &pcie->cfg); > > We really should have a livetree API for this. Is this mandatory for this patch to get accepted? Or can I add this to my list and post this later and port this driver to using these livetree functions then? BTW: Do you have an example for a similar function added to livetree, so that I can choose the "correct" naming? Thanks, Stefan >> + if (err) { >> + debug("Error reading resource: %s\n", fdt_strerror(err)); >> + return err; >> + } >> + >> + err = fdtdec_get_pci_bus_range(gd->fdt_blob, dev->node.of_offset, >> + &pcie->bus); > > and this... > >> + if (err) { >> + debug("Error reading resource: %s\n", fdt_strerror(err)); >> + return err; >> + } >> + >> + return 0; >> +} >> + >> +static const struct dm_pci_ops pci_octeontx_ops = { >> + .read_config = pci_octeontx_read_config, >> + .write_config = pci_octeontx_write_config, >> +}; >> + >> +static const struct udevice_id pci_octeontx_ids[] = { >> + { .compatible = "cavium,pci-host-thunder-ecam", .data = OTX_ECAM }, >> + { .compatible = "cavium,pci-host-octeontx-ecam", .data = OTX_ECAM }, >> + { .compatible = "pci-host-ecam-generic", .data = OTX_ECAM }, >> + { .compatible = "cavium,pci-host-thunder-pem", .data = OTX_PEM }, >> + { .compatible = "marvell,pci-host-octeontx2-pem", .data = OTX2_PEM }, >> + { } >> +}; >> + >> +U_BOOT_DRIVER(pci_octeontx) = { >> + .name = "pci_octeontx", >> + .id = UCLASS_PCI, >> + .of_match = pci_octeontx_ids, >> + .ops = &pci_octeontx_ops, >> + .ofdata_to_platdata = pci_octeontx_ofdata_to_platdata, >> + .probe = pci_octeontx_probe, >> + .priv_auto_alloc_size = sizeof(struct octeontx_pci), >> + .flags = DM_FLAG_PRE_RELOC, >> +}; >> -- >> 2.27.0 >> > > Regards, > Simon > Viele Grüße, Stefan
Hi Stefan, On Thu, 30 Jul 2020 at 10:26, Stefan Roese <sr@denx.de> wrote: > > Hi Simon, > > On 28.07.20 21:01, Simon Glass wrote: > > Hi Stefan, > > > > On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr@denx.de> wrote: > >> > >> From: Suneel Garapati <sgarapati@marvell.com> > >> > >> Adds support for PCI ECAM/PEM controllers found on OcteonTX > >> or OcteonTX2 SoC platforms. > >> > >> Signed-off-by: Suneel Garapati <sgarapati@marvell.com> > >> Cc: Simon Glass <sjg@chromium.org> > >> Cc: Bin Meng <bmeng.cn@gmail.com> > >> > >> Signed-off-by: Stefan Roese <sr@denx.de> > >> --- > >> > >> Changes in v1: > >> - Change patch subject > >> - Remove inclusion of common.h > >> - Remove #ifdef's and use driver specific data instead > >> - Add comments to struct > >> - Add some helper functions to reduce code size > >> - Misc coding style changes (blank lines etc) > >> - Use debug() instead of printf() in some cases > >> > >> drivers/pci/Kconfig | 8 + > >> drivers/pci/Makefile | 1 + > >> drivers/pci/pci_octeontx.c | 344 +++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 353 insertions(+) > >> create mode 100644 drivers/pci/pci_octeontx.c > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > >> [,,] > >> +static int pci_octeontx_probe(struct udevice *dev) > >> +{ > >> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(dev); > >> + int err; > >> + > >> + pcie->type = dev_get_driver_data(dev); > >> + > >> + err = fdt_get_resource(gd->fdt_blob, dev->node.of_offset, "reg", 0, > >> + &pcie->cfg); > > > > We really should have a livetree API for this. > > Is this mandatory for this patch to get accepted? Or can I add this > to my list and post this later and port this driver to using these > livetree functions then? No, you have my review tag and later is fine. > > BTW: Do you have an example for a similar function added to livetree, > so that I can choose the "correct" naming? Well we have dev_read_resource() so I'm hoping that they are not too far away from what you need. Regards, Simon
Hi Simon, On 31.07.20 20:44, Simon Glass wrote: > Hi Stefan, > > On Thu, 30 Jul 2020 at 10:26, Stefan Roese <sr@denx.de> wrote: >> >> Hi Simon, >> >> On 28.07.20 21:01, Simon Glass wrote: >>> Hi Stefan, >>> >>> On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr@denx.de> wrote: >>>> >>>> From: Suneel Garapati <sgarapati@marvell.com> >>>> >>>> Adds support for PCI ECAM/PEM controllers found on OcteonTX >>>> or OcteonTX2 SoC platforms. >>>> >>>> Signed-off-by: Suneel Garapati <sgarapati@marvell.com> >>>> Cc: Simon Glass <sjg@chromium.org> >>>> Cc: Bin Meng <bmeng.cn@gmail.com> >>>> >>>> Signed-off-by: Stefan Roese <sr@denx.de> >>>> --- >>>> >>>> Changes in v1: >>>> - Change patch subject >>>> - Remove inclusion of common.h >>>> - Remove #ifdef's and use driver specific data instead >>>> - Add comments to struct >>>> - Add some helper functions to reduce code size >>>> - Misc coding style changes (blank lines etc) >>>> - Use debug() instead of printf() in some cases >>>> >>>> drivers/pci/Kconfig | 8 + >>>> drivers/pci/Makefile | 1 + >>>> drivers/pci/pci_octeontx.c | 344 +++++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 353 insertions(+) >>>> create mode 100644 drivers/pci/pci_octeontx.c >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >>> >>>> > > [,,] > >>>> +static int pci_octeontx_probe(struct udevice *dev) >>>> +{ >>>> + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(dev); >>>> + int err; >>>> + >>>> + pcie->type = dev_get_driver_data(dev); >>>> + >>>> + err = fdt_get_resource(gd->fdt_blob, dev->node.of_offset, "reg", 0, >>>> + &pcie->cfg); >>> >>> We really should have a livetree API for this. >> >> Is this mandatory for this patch to get accepted? Or can I add this >> to my list and post this later and port this driver to using these >> livetree functions then? > > No, you have my review tag and later is fine. Okay. >> >> BTW: Do you have an example for a similar function added to livetree, >> so that I can choose the "correct" naming? > > Well we have dev_read_resource() so I'm hoping that they are not too > far away from what you need. I've now added dev_read_pci_bus_range() in the next patchset version and selected OF_LIVE for both new platforms (Octeon TX & TX2). Thanks, Stefan
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index bc77c23f89..89cca6ffb3 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -149,6 +149,14 @@ config PCI_TEGRA with a total of 5 lanes. Some boards require this for Ethernet support to work (e.g. beaver, jetson-tk1). +config PCI_OCTEONTX + bool "OcteonTX PCI support" + depends on (ARCH_OCTEONTX || ARCH_OCTEONTX2) + help + Enable support for the OcteonTX/TX2 SoC family ECAM/PEM controllers. + These controllers provide PCI configuration access to all on-board + peripherals so it should only be disabled for testing purposes + config PCI_XILINX bool "Xilinx AXI Bridge for PCI Express" depends on DM_PCI diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 6378821aaf..0529cceee7 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -45,3 +45,4 @@ obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o +obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o diff --git a/drivers/pci/pci_octeontx.c b/drivers/pci/pci_octeontx.c new file mode 100644 index 0000000000..5c6a6f05f2 --- /dev/null +++ b/drivers/pci/pci_octeontx.c @@ -0,0 +1,344 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Marvell International Ltd. + * + * https://spdx.org/licenses + */ + +#include <dm.h> +#include <errno.h> +#include <fdtdec.h> +#include <log.h> +#include <malloc.h> +#include <pci.h> + +#include <asm/io.h> + +DECLARE_GLOBAL_DATA_PTR; + +enum { + OTX_ECAM, + OTX_PEM, + OTX2_PEM, +}; + +/** + * struct octeontx_pci - Driver private data + * @type: Device type matched via compatible + * @cfg: Config resource + * @bus: Bus resource + */ +struct octeontx_pci { + unsigned int type; + + struct fdt_resource cfg; + struct fdt_resource bus; +}; + +static uintptr_t octeontx_cfg_addr(struct octeontx_pci *pcie, + int bus_offs, int shift_offs, + pci_dev_t bdf, uint offset) +{ + u32 bus, dev, func; + uintptr_t address; + + bus = PCI_BUS(bdf) + bus_offs; + dev = PCI_DEV(bdf); + func = PCI_FUNC(bdf); + + address = (bus << (20 + shift_offs)) | + (dev << (15 + shift_offs)) | + (func << (12 + shift_offs)) | offset; + address += pcie->cfg.start; + + return address; +} + +static ulong readl_size(uintptr_t addr, enum pci_size_t size) +{ + ulong val; + + switch (size) { + case PCI_SIZE_8: + val = readb(addr); + break; + case PCI_SIZE_16: + val = readw(addr); + break; + case PCI_SIZE_32: + val = readl(addr); + break; + default: + printf("Invalid size\n"); + }; + + return val; +} + +static void writel_size(uintptr_t addr, enum pci_size_t size, ulong valuep) +{ + switch (size) { + case PCI_SIZE_8: + writeb(valuep, addr); + break; + case PCI_SIZE_16: + writew(valuep, addr); + break; + case PCI_SIZE_32: + writel(valuep, addr); + break; + default: + printf("Invalid size\n"); + }; +} + +static int octeontx_ecam_read_config(const struct udevice *bus, pci_dev_t bdf, + uint offset, ulong *valuep, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + struct pci_controller *hose = dev_get_uclass_priv(bus); + uintptr_t address; + + address = octeontx_cfg_addr(pcie, pcie->bus.start - hose->first_busno, + 0, bdf, offset); + *valuep = readl_size(address, size); + + debug("%02x.%02x.%02x: u%d %x -> %lx\n", + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, *valuep); + + return 0; +} + +static int octeontx_ecam_write_config(struct udevice *bus, pci_dev_t bdf, + uint offset, ulong value, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + struct pci_controller *hose = dev_get_uclass_priv(bus); + uintptr_t address; + + address = octeontx_cfg_addr(pcie, pcie->bus.start - hose->first_busno, + 0, bdf, offset); + writel_size(address, size, value); + + debug("%02x.%02x.%02x: u%d %x <- %lx\n", + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, value); + + return 0; +} + +static int octeontx_pem_read_config(const struct udevice *bus, pci_dev_t bdf, + uint offset, ulong *valuep, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + struct pci_controller *hose = dev_get_uclass_priv(bus); + uintptr_t address; + u8 hdrtype; + u8 pri_bus = pcie->bus.start + 1 - hose->first_busno; + u32 bus_offs = (pri_bus << 16) | (pri_bus << 8) | (pri_bus << 0); + + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 4, + bdf, 0); + + *valuep = pci_conv_32_to_size(~0UL, offset, size); + + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) + return 0; + + *valuep = readl_size(address + offset, size); + + hdrtype = readb(address + PCI_HEADER_TYPE); + if (hdrtype == PCI_HEADER_TYPE_BRIDGE && + offset >= PCI_PRIMARY_BUS && + offset <= PCI_SUBORDINATE_BUS && + *valuep != pci_conv_32_to_size(~0UL, offset, size)) + *valuep -= pci_conv_32_to_size(bus_offs, offset, size); + + return 0; +} + +static int octeontx_pem_write_config(struct udevice *bus, pci_dev_t bdf, + uint offset, ulong value, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + struct pci_controller *hose = dev_get_uclass_priv(bus); + uintptr_t address; + u8 hdrtype; + u8 pri_bus = pcie->bus.start + 1 - hose->first_busno; + u32 bus_offs = (pri_bus << 16) | (pri_bus << 8) | (pri_bus << 0); + + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 4, bdf, 0); + + hdrtype = readb(address + PCI_HEADER_TYPE); + if (hdrtype == PCI_HEADER_TYPE_BRIDGE && + offset >= PCI_PRIMARY_BUS && + offset <= PCI_SUBORDINATE_BUS && + value != pci_conv_32_to_size(~0UL, offset, size)) + value += pci_conv_32_to_size(bus_offs, offset, size); + + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) + return 0; + + writel_size(address + offset, size, value); + + debug("%02x.%02x.%02x: u%d %x (%lx) <- %lx\n", + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, + address, value); + + return 0; +} + +static int octeontx2_pem_read_config(const struct udevice *bus, pci_dev_t bdf, + uint offset, ulong *valuep, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + struct pci_controller *hose = dev_get_uclass_priv(bus); + uintptr_t address; + + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 0, + bdf, 0); + + *valuep = pci_conv_32_to_size(~0UL, offset, size); + + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) + return 0; + + *valuep = readl_size(address + offset, size); + + debug("%02x.%02x.%02x: u%d %x (%lx) -> %lx\n", + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, + address, *valuep); + + return 0; +} + +static int octeontx2_pem_write_config(struct udevice *bus, pci_dev_t bdf, + uint offset, ulong value, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + struct pci_controller *hose = dev_get_uclass_priv(bus); + uintptr_t address; + + address = octeontx_cfg_addr(pcie, 1 - hose->first_busno, 0, + bdf, 0); + + if (PCI_BUS(bdf) == 1 && PCI_DEV(bdf) > 0) + return 0; + + writel_size(address + offset, size, value); + + debug("%02x.%02x.%02x: u%d %x (%lx) <- %lx\n", + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), size, offset, + address, value); + + return 0; +} + +int pci_octeontx_read_config(const struct udevice *bus, pci_dev_t bdf, + uint offset, ulong *valuep, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + int ret = -EIO; + + switch (pcie->type) { + case OTX_ECAM: + ret = octeontx_ecam_read_config(bus, bdf, offset, valuep, + size); + break; + case OTX_PEM: + ret = octeontx_pem_read_config(bus, bdf, offset, valuep, + size); + break; + case OTX2_PEM: + ret = octeontx2_pem_read_config(bus, bdf, offset, valuep, + size); + break; + } + + return ret; +} + +int pci_octeontx_write_config(struct udevice *bus, pci_dev_t bdf, + uint offset, ulong value, + enum pci_size_t size) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(bus); + int ret = -EIO; + + switch (pcie->type) { + case OTX_ECAM: + ret = octeontx_ecam_write_config(bus, bdf, offset, value, + size); + break; + case OTX_PEM: + ret = octeontx_pem_write_config(bus, bdf, offset, value, + size); + break; + case OTX2_PEM: + ret = octeontx2_pem_write_config(bus, bdf, offset, value, + size); + break; + } + + return ret; +} + +static int pci_octeontx_ofdata_to_platdata(struct udevice *dev) +{ + return 0; +} + +static int pci_octeontx_probe(struct udevice *dev) +{ + struct octeontx_pci *pcie = (struct octeontx_pci *)dev_get_priv(dev); + int err; + + pcie->type = dev_get_driver_data(dev); + + err = fdt_get_resource(gd->fdt_blob, dev->node.of_offset, "reg", 0, + &pcie->cfg); + if (err) { + debug("Error reading resource: %s\n", fdt_strerror(err)); + return err; + } + + err = fdtdec_get_pci_bus_range(gd->fdt_blob, dev->node.of_offset, + &pcie->bus); + if (err) { + debug("Error reading resource: %s\n", fdt_strerror(err)); + return err; + } + + return 0; +} + +static const struct dm_pci_ops pci_octeontx_ops = { + .read_config = pci_octeontx_read_config, + .write_config = pci_octeontx_write_config, +}; + +static const struct udevice_id pci_octeontx_ids[] = { + { .compatible = "cavium,pci-host-thunder-ecam", .data = OTX_ECAM }, + { .compatible = "cavium,pci-host-octeontx-ecam", .data = OTX_ECAM }, + { .compatible = "pci-host-ecam-generic", .data = OTX_ECAM }, + { .compatible = "cavium,pci-host-thunder-pem", .data = OTX_PEM }, + { .compatible = "marvell,pci-host-octeontx2-pem", .data = OTX2_PEM }, + { } +}; + +U_BOOT_DRIVER(pci_octeontx) = { + .name = "pci_octeontx", + .id = UCLASS_PCI, + .of_match = pci_octeontx_ids, + .ops = &pci_octeontx_ops, + .ofdata_to_platdata = pci_octeontx_ofdata_to_platdata, + .probe = pci_octeontx_probe, + .priv_auto_alloc_size = sizeof(struct octeontx_pci), + .flags = DM_FLAG_PRE_RELOC, +};