2020-04-23 18:28:55

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi All,

This patch series adds support for endpoint driver for R-Car PCIe controller on
R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
supported by the controller for mapping PCI address locally.

Note:
The cadence/rockchip/designware endpoint drivers are build tested only.

Changes for v9 (Re-spun this series as there were minimal changes requested):
* Rebased patches on top of v5.7.rc1
* Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
* Added a check for max_functions read from DT to restrict with
RCAR_EPC_MAX_FUNCTIONS
* Replaced MSICAP0_MMENUM with MSICAP0_MMESE
* Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr()
* Fixed looping for number windows in pci_epc_mem_exit()
* Set maximum to 1 for max-functions in DT binding (I have restored the acks
from Rob and Shimoda-san)
* Sorted the entry in MAINTAINERS

Changes for v8:
* Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h
* Fixed typo in commit message for patch 2/8
* Reworded commit message for patch 5/8 as suggested by Bjorn
* Split up patch to add pci_epc_mem_init() interface to add page_size argument
as suggested by Bjorn.

Changes for v7:
* Fixed review comments pointed by Shimoda-san
1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed
the example as its built with #{address,size}-cells = <1>. I have still
restored the Ack from Rob and Shimoda-san with these changes.
2] Split up the patches so that they can be picked up by respective subsystem
patches 1/4-9/11 are now part of this series.
3] Dropped altering a comment in pci-epc.h
4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size
variable doesn't get overwritten in the loop.
5] Replaced i-=1 with i--
6] Replaced rcar with R-Car in patch subject and description.
7] Set MACCTLR in init() callback

Changes for v6:
1] Rebased patches on endpoint branch of https://git.kernel.org/pub/
scm/linux/kernel/git/lpieralisi/pci.git/
2] Fixed review comments from Shimoda-san
a] Made sure defconfig changes were in separate patch
b] Created rcar_pcie_host/rcar_pcie_ep structures
c] Added pci-id for R8A774C0
d] Added entry in MAINTAINERS for dt-binding
e] Dropped unnecessary braces
3] Added support for msi.

Changes for v5:
1] Rebased patches on next branch of https://git.kernel.org/pub/scm/
linux/kernel/git/helgaas/pci.git
2] Fixed review comments reported by Kishon while fetching the matching
window in function pci_epc_get_matching_window()
3] Fixed review comments reported by Bjorn
a] Split patch up first patch so that its easier to review and incremental
b] Fixed typos
4] Included Reviewed tag from Rob for the dt-binding patch
5] Fixed issue reported by Nathan for assigning variable to itself

Changes for v4:
1] Fixed dtb_check error reported by Rob
2] Fixed review comments reported by Kishon
a] Dropped pci_epc_find_best_fit_window()
b] Fixed initializing mem ptr in __pci_epc_mem_init()
c] Dropped map_size from pci_epc_mem_window structure

Changes for v3:
1] Fixed review comments from Bjorn and Kishon.
3] Converted to DT schema

Changes for v2:
1] Fixed review comments from Biju for dt-bindings to include an example
for a tested platform.
2] Fixed review comments from Kishon to extend the features of outbound
regions in epf framework.
3] Added support to parse outbound-ranges in OF.

Lad Prabhakar (8):
PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c
PCI: rcar: Move shareable code to a common file
PCI: rcar: Fix calculating mask for PCIEPAMR register
PCI: endpoint: Pass page size as argument to pci_epc_mem_init()
PCI: endpoint: Add support to handle multiple base for mapping
outbound memory
dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint
controller
PCI: rcar: Add endpoint mode support
MAINTAINERS: Add file patterns for rcar PCI device tree bindings

.../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++
MAINTAINERS | 1 +
drivers/pci/controller/Kconfig | 18 +
drivers/pci/controller/Makefile | 3 +-
.../pci/controller/cadence/pcie-cadence-ep.c | 2 +-
.../pci/controller/dwc/pcie-designware-ep.c | 16 +-
drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++
drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++
drivers/pci/controller/pcie-rcar.c | 1206 +----------------
drivers/pci/controller/pcie-rcar.h | 140 ++
drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
drivers/pci/endpoint/pci-epc-mem.c | 204 ++-
include/linux/pci-epc.h | 38 +-
13 files changed, 2078 insertions(+), 1251 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
create mode 100644 drivers/pci/controller/pcie-rcar-ep.c
create mode 100644 drivers/pci/controller/pcie-rcar-host.c
create mode 100644 drivers/pci/controller/pcie-rcar.h

--
2.17.1


2020-04-23 18:29:03

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 6/8] dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint controller

This patch adds the bindings for the R-Car PCIe endpoint driver.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Yoshihiro Shimoda <[email protected]>
---
.../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml

diff --git a/Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
new file mode 100644
index 000000000000..aa483c7f27fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 Renesas Electronics Europe GmbH - https://www.renesas.com/eu/en/
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-pci-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car PCIe Endpoint
+
+maintainers:
+ - Lad Prabhakar <[email protected]>
+ - Yoshihiro Shimoda <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - const: renesas,r8a774c0-pcie-ep
+ - const: renesas,rcar-gen3-pcie-ep
+
+ reg:
+ maxItems: 5
+
+ reg-names:
+ items:
+ - const: apb-base
+ - const: memory0
+ - const: memory1
+ - const: memory2
+ - const: memory3
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: pcie
+
+ max-functions:
+ minimum: 1
+ maximum: 1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - resets
+ - power-domains
+ - clocks
+ - clock-names
+ - max-functions
+
+examples:
+ - |
+ #include <dt-bindings/clock/r8a774c0-cpg-mssr.h>
+ #include <dt-bindings/power/r8a774c0-sysc.h>
+
+ pcie0_ep: pcie-ep@fe000000 {
+ compatible = "renesas,r8a774c0-pcie-ep",
+ "renesas,rcar-gen3-pcie-ep";
+ reg = <0xfe000000 0x80000>,
+ <0xfe100000 0x100000>,
+ <0xfe200000 0x200000>,
+ <0x30000000 0x8000000>,
+ <0x38000000 0x8000000>;
+ reg-names = "apb-base", "memory0", "memory1", "memory2", "memory3";
+ resets = <&cpg 319>;
+ power-domains = <&sysc R8A774C0_PD_ALWAYS_ON>;
+ clocks = <&cpg CPG_MOD 319>;
+ clock-names = "pcie";
+ max-functions = /bits/ 8 <1>;
+ };
--
2.17.1

2020-04-23 18:29:10

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 7/8] PCI: rcar: Add endpoint mode support

Add support for R-Car PCIe controller to work in endpoint mode.

Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/pci/controller/Kconfig | 8 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++++++++++++++++++++
drivers/pci/controller/pcie-rcar.h | 9 +
4 files changed, 575 insertions(+)
create mode 100644 drivers/pci/controller/pcie-rcar-ep.c

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index cfdc898450d0..1f9793467f6d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -71,6 +71,14 @@ config PCIE_RCAR_HOST
Say Y here if you want PCIe controller support on R-Car SoCs in host
mode.

+config PCIE_RCAR_EP
+ bool "Renesas R-Car PCIe endpoint controller"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ depends on PCI_ENDPOINT
+ help
+ Say Y here if you want PCIe controller support on R-Car SoCs in
+ endpoint mode.
+
config PCI_HOST_COMMON
bool
select PCI_ECAM
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index 39802ee32946..741a5204aa5e 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
obj-$(CONFIG_PCIE_RCAR_HOST) += pcie-rcar.o pcie-rcar-host.o
+obj-$(CONFIG_PCIE_RCAR_EP) += pcie-rcar.o pcie-rcar-ep.o
obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o
obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
diff --git a/drivers/pci/controller/pcie-rcar-ep.c b/drivers/pci/controller/pcie-rcar-ep.c
new file mode 100644
index 000000000000..c8b64277f99b
--- /dev/null
+++ b/drivers/pci/controller/pcie-rcar-ep.c
@@ -0,0 +1,557 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe endpoint driver for Renesas R-Car SoCs
+ * Copyright (c) 2020 Renesas Electronics Europe GmbH
+ *
+ * Author: Lad Prabhakar <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/pci-epc.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar.h"
+
+#define RCAR_EPC_MAX_FUNCTIONS 1
+
+/* Structure representing the PCIe interface */
+struct rcar_pcie_endpoint {
+ struct rcar_pcie pcie;
+ phys_addr_t *ob_mapped_addr;
+ struct pci_epc_mem_window *ob_window;
+ u8 max_functions;
+ unsigned int bar_to_atu[MAX_NR_INBOUND_MAPS];
+ unsigned long *ib_window_map;
+ u32 num_ib_windows;
+ u32 num_ob_windows;
+};
+
+static void rcar_pcie_ep_hw_init(struct rcar_pcie *pcie)
+{
+ u32 val;
+
+ rcar_pci_write_reg(pcie, 0, PCIETCTLR);
+
+ /* Set endpoint mode */
+ rcar_pci_write_reg(pcie, 0, PCIEMSR);
+
+ /* Initialize default capabilities. */
+ rcar_rmw32(pcie, REXPCAP(0), 0xff, PCI_CAP_ID_EXP);
+ rcar_rmw32(pcie, REXPCAP(PCI_EXP_FLAGS),
+ PCI_EXP_FLAGS_TYPE, PCI_EXP_TYPE_ENDPOINT << 4);
+ rcar_rmw32(pcie, RCONF(PCI_HEADER_TYPE), 0x7f,
+ PCI_HEADER_TYPE_NORMAL);
+
+ /* Write out the physical slot number = 0 */
+ rcar_rmw32(pcie, REXPCAP(PCI_EXP_SLTCAP), PCI_EXP_SLTCAP_PSN, 0);
+
+ val = rcar_pci_read_reg(pcie, EXPCAP(1));
+ /* device supports fixed 128 bytes MPSS */
+ val &= ~GENMASK(2, 0);
+ rcar_pci_write_reg(pcie, val, EXPCAP(1));
+
+ val = rcar_pci_read_reg(pcie, EXPCAP(2));
+ /* read requests size 128 bytes */
+ val &= ~GENMASK(14, 12);
+ /* payload size 128 bytes */
+ val &= ~GENMASK(7, 5);
+ rcar_pci_write_reg(pcie, val, EXPCAP(2));
+
+ /* Set target link speed to 5.0 GT/s */
+ rcar_rmw32(pcie, EXPCAP(12), PCI_EXP_LNKSTA_CLS,
+ PCI_EXP_LNKSTA_CLS_5_0GB);
+
+ /* Set the completion timer timeout to the maximum 50ms. */
+ rcar_rmw32(pcie, TLCTLR + 1, 0x3f, 50);
+
+ /* Terminate list of capabilities (Next Capability Offset=0) */
+ rcar_rmw32(pcie, RVCCAP(0), 0xfff00000, 0);
+
+ /* flush modifications */
+ wmb();
+}
+
+static int rcar_pcie_ep_get_window(struct rcar_pcie_endpoint *ep,
+ phys_addr_t addr)
+{
+ int i;
+
+ for (i = 0; i < ep->num_ob_windows; i++)
+ if (ep->ob_window[i].phys_base == addr)
+ return i;
+
+ return -EINVAL;
+}
+
+static int rcar_pcie_parse_outbound_ranges(struct rcar_pcie_endpoint *ep,
+ struct platform_device *pdev)
+{
+ struct rcar_pcie *pcie = &ep->pcie;
+ char outbound_name[10];
+ struct resource *res;
+ unsigned int i = 0;
+
+ ep->num_ob_windows = 0;
+ for (i = 0; i < RCAR_PCI_MAX_RESOURCES; i++) {
+ sprintf(outbound_name, "memory%u", i);
+ res = platform_get_resource_byname(pdev,
+ IORESOURCE_MEM,
+ outbound_name);
+ if (!res) {
+ dev_err(pcie->dev, "missing outbound window %u\n", i);
+ return -EINVAL;
+ }
+ if (!devm_request_mem_region(&pdev->dev, res->start,
+ resource_size(res),
+ outbound_name)) {
+ dev_err(pcie->dev, "Cannot request memory region %s.\n",
+ outbound_name);
+ return -EIO;
+ }
+
+ ep->ob_window[i].phys_base = res->start;
+ ep->ob_window[i].size = resource_size(res);
+ /* controller doesn't support multiple allocation
+ * from same window, so set page_size to window size
+ */
+ ep->ob_window[i].page_size = resource_size(res);
+ }
+ ep->num_ob_windows = i;
+
+ return 0;
+}
+
+static int rcar_pcie_ep_get_pdata(struct rcar_pcie_endpoint *ep,
+ struct platform_device *pdev)
+{
+ struct rcar_pcie *pcie = &ep->pcie;
+ struct pci_epc_mem_window *window;
+ struct device *dev = pcie->dev;
+ struct resource res;
+ int err;
+
+ err = of_address_to_resource(dev->of_node, 0, &res);
+ if (err)
+ return err;
+ pcie->base = devm_ioremap_resource(dev, &res);
+ if (IS_ERR(pcie->base))
+ return PTR_ERR(pcie->base);
+
+ ep->ob_window = devm_kcalloc(dev, RCAR_PCI_MAX_RESOURCES,
+ sizeof(*window), GFP_KERNEL);
+ if (!ep->ob_window)
+ return -ENOMEM;
+
+ rcar_pcie_parse_outbound_ranges(ep, pdev);
+
+ err = of_property_read_u8(dev->of_node, "max-functions",
+ &ep->max_functions);
+ if (err < 0 || ep->max_functions > RCAR_EPC_MAX_FUNCTIONS)
+ ep->max_functions = RCAR_EPC_MAX_FUNCTIONS;
+
+ return 0;
+}
+
+static int rcar_pcie_ep_write_header(struct pci_epc *epc, u8 fn,
+ struct pci_epf_header *hdr)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ struct rcar_pcie *pcie = &ep->pcie;
+ u32 val;
+
+ if (!fn)
+ val = hdr->vendorid;
+ else
+ val = rcar_pci_read_reg(pcie, IDSETR0);
+ val |= hdr->deviceid << 16;
+ rcar_pci_write_reg(pcie, val, IDSETR0);
+
+ val = hdr->revid;
+ val |= hdr->progif_code << 8;
+ val |= hdr->subclass_code << 16;
+ val |= hdr->baseclass_code << 24;
+ rcar_pci_write_reg(pcie, val, IDSETR1);
+
+ if (!fn)
+ val = hdr->subsys_vendor_id;
+ else
+ val = rcar_pci_read_reg(pcie, SUBIDSETR);
+ val |= hdr->subsys_id << 16;
+ rcar_pci_write_reg(pcie, val, SUBIDSETR);
+
+ if (hdr->interrupt_pin > PCI_INTERRUPT_INTA)
+ return -EINVAL;
+ val = rcar_pci_read_reg(pcie, PCICONF(15));
+ val |= (hdr->interrupt_pin << 8);
+ rcar_pci_write_reg(pcie, val, PCICONF(15));
+
+ return 0;
+}
+
+static int rcar_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
+ struct pci_epf_bar *epf_bar)
+{
+ int flags = epf_bar->flags | LAR_ENABLE | LAM_64BIT;
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ u64 size = 1ULL << fls64(epf_bar->size - 1);
+ dma_addr_t cpu_addr = epf_bar->phys_addr;
+ enum pci_barno bar = epf_bar->barno;
+ struct rcar_pcie *pcie = &ep->pcie;
+ u32 mask;
+ int idx;
+ int err;
+
+ idx = find_first_zero_bit(ep->ib_window_map, ep->num_ib_windows);
+ if (idx >= ep->num_ib_windows) {
+ dev_err(pcie->dev, "no free inbound window\n");
+ return -EINVAL;
+ }
+
+ if ((flags & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO)
+ flags |= IO_SPACE;
+
+ ep->bar_to_atu[bar] = idx;
+ /* use 64-bit BARs */
+ set_bit(idx, ep->ib_window_map);
+ set_bit(idx + 1, ep->ib_window_map);
+
+ if (cpu_addr > 0) {
+ unsigned long nr_zeros = __ffs64(cpu_addr);
+ u64 alignment = 1ULL << nr_zeros;
+
+ size = min(size, alignment);
+ }
+
+ size = min(size, 1ULL << 32);
+
+ mask = roundup_pow_of_two(size) - 1;
+ mask &= ~0xf;
+
+ rcar_pcie_set_inbound(pcie, cpu_addr,
+ 0x0, mask | flags, idx, false);
+
+ err = rcar_pcie_wait_for_phyrdy(pcie);
+ if (err) {
+ dev_err(pcie->dev, "phy not ready\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void rcar_pcie_ep_clear_bar(struct pci_epc *epc, u8 fn,
+ struct pci_epf_bar *epf_bar)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ enum pci_barno bar = epf_bar->barno;
+ u32 atu_index = ep->bar_to_atu[bar];
+
+ rcar_pcie_set_inbound(&ep->pcie, 0x0, 0x0, 0x0, bar, false);
+
+ clear_bit(atu_index, ep->ib_window_map);
+ clear_bit(atu_index + 1, ep->ib_window_map);
+}
+
+static int rcar_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 interrupts)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ struct rcar_pcie *pcie = &ep->pcie;
+ u32 flags;
+
+ flags = rcar_pci_read_reg(pcie, MSICAP(fn));
+ flags |= interrupts << MSICAP0_MMESCAP_OFFSET;
+ rcar_pci_write_reg(pcie, flags, MSICAP(fn));
+
+ return 0;
+}
+
+static int rcar_pcie_ep_get_msi(struct pci_epc *epc, u8 fn)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ struct rcar_pcie *pcie = &ep->pcie;
+ u32 flags;
+
+ flags = rcar_pci_read_reg(pcie, MSICAP(fn));
+ if (!(flags & MSICAP0_MSIE))
+ return -EINVAL;
+
+ return ((flags & MSICAP0_MMESE_MASK) >> MSICAP0_MMESE_OFFSET);
+}
+
+static int rcar_pcie_ep_map_addr(struct pci_epc *epc, u8 fn,
+ phys_addr_t addr, u64 pci_addr, size_t size)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ struct rcar_pcie *pcie = &ep->pcie;
+ struct resource res;
+ int window;
+ int err;
+
+ /* check if we have a link. */
+ err = rcar_pcie_wait_for_dl(pcie);
+ if (err) {
+ dev_err(pcie->dev, "link not up\n");
+ return err;
+ }
+
+ window = rcar_pcie_ep_get_window(ep, addr);
+ if (window < 0) {
+ dev_err(pcie->dev, "failed to get corresponding window\n");
+ return -EINVAL;
+ }
+
+ memset(&res, 0x0, sizeof(res));
+ res.start = pci_addr;
+ res.end = pci_addr + size - 1;
+ res.flags = IORESOURCE_MEM;
+
+ rcar_pcie_set_outbound(pcie, window, &res);
+
+ ep->ob_mapped_addr[window] = addr;
+
+ return 0;
+}
+
+static void rcar_pcie_ep_unmap_addr(struct pci_epc *epc, u8 fn,
+ phys_addr_t addr)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+ struct resource res;
+ int idx;
+
+ for (idx = 0; idx < ep->num_ob_windows; idx++)
+ if (ep->ob_mapped_addr[idx] == addr)
+ break;
+
+ if (idx >= ep->num_ob_windows)
+ return;
+
+ memset(&res, 0x0, sizeof(res));
+ rcar_pcie_set_outbound(&ep->pcie, idx, &res);
+
+ ep->ob_mapped_addr[idx] = 0;
+}
+
+static int rcar_pcie_ep_assert_intx(struct rcar_pcie_endpoint *ep,
+ u8 fn, u8 intx)
+{
+ struct rcar_pcie *pcie = &ep->pcie;
+ u32 val;
+
+ val = rcar_pci_read_reg(pcie, PCIEMSITXR);
+ if ((val & PCI_MSI_FLAGS_ENABLE)) {
+ dev_err(pcie->dev, "MSI is enabled, cannot assert INTx\n");
+ return -EINVAL;
+ }
+
+ val = rcar_pci_read_reg(pcie, PCICONF(1));
+ if ((val & INTDIS)) {
+ dev_err(pcie->dev, "INTx message transmission is disabled\n");
+ return -EINVAL;
+ }
+
+ val = rcar_pci_read_reg(pcie, PCIEINTXR);
+ if ((val & ASTINTX)) {
+ dev_err(pcie->dev, "INTx is already asserted\n");
+ return -EINVAL;
+ }
+
+ val |= ASTINTX;
+ rcar_pci_write_reg(pcie, val, PCIEINTXR);
+ usleep_range(1000, 1001);
+ val = rcar_pci_read_reg(pcie, PCIEINTXR);
+ val &= ~ASTINTX;
+ rcar_pci_write_reg(pcie, val, PCIEINTXR);
+
+ return 0;
+}
+
+static int rcar_pcie_ep_assert_msi(struct rcar_pcie *pcie,
+ u8 fn, u8 interrupt_num)
+{
+ u16 msi_count;
+ u32 val;
+
+ /* Check MSI enable bit */
+ val = rcar_pci_read_reg(pcie, MSICAP(fn));
+ if (!(val & MSICAP0_MSIE))
+ return -EINVAL;
+
+ /* Get MSI numbers from MME */
+ msi_count = ((val & MSICAP0_MMESE_MASK) >> MSICAP0_MMESE_OFFSET);
+ msi_count = 1 << msi_count;
+
+ if (!interrupt_num || interrupt_num > msi_count)
+ return -EINVAL;
+
+ val = rcar_pci_read_reg(pcie, PCIEMSITXR);
+ rcar_pci_write_reg(pcie, val | (interrupt_num - 1), PCIEMSITXR);
+
+ return 0;
+}
+
+static int rcar_pcie_ep_raise_irq(struct pci_epc *epc, u8 fn,
+ enum pci_epc_irq_type type,
+ u16 interrupt_num)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+
+ switch (type) {
+ case PCI_EPC_IRQ_LEGACY:
+ return rcar_pcie_ep_assert_intx(ep, fn, 0);
+
+ case PCI_EPC_IRQ_MSI:
+ return rcar_pcie_ep_assert_msi(&ep->pcie, fn, interrupt_num);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int rcar_pcie_ep_start(struct pci_epc *epc)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+
+ rcar_pci_write_reg(&ep->pcie, MACCTLR_INIT_VAL, MACCTLR);
+ rcar_pci_write_reg(&ep->pcie, CFINIT, PCIETCTLR);
+
+ return 0;
+}
+
+static void rcar_pcie_ep_stop(struct pci_epc *epc)
+{
+ struct rcar_pcie_endpoint *ep = epc_get_drvdata(epc);
+
+ rcar_pci_write_reg(&ep->pcie, 0, PCIETCTLR);
+}
+
+static const struct pci_epc_features rcar_pcie_epc_features = {
+ .linkup_notifier = false,
+ .msi_capable = true,
+ .msix_capable = false,
+ /* use 64-bit BARs so mark BAR[1,3,5] as reserved */
+ .reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
+ .bar_fixed_64bit = 1 << BAR_0 | 1 << BAR_2 | 1 << BAR_4,
+ .bar_fixed_size[0] = 128,
+ .bar_fixed_size[2] = 256,
+ .bar_fixed_size[4] = 256,
+};
+
+static const struct pci_epc_features*
+rcar_pcie_ep_get_features(struct pci_epc *epc, u8 func_no)
+{
+ return &rcar_pcie_epc_features;
+}
+
+static const struct pci_epc_ops rcar_pcie_epc_ops = {
+ .write_header = rcar_pcie_ep_write_header,
+ .set_bar = rcar_pcie_ep_set_bar,
+ .clear_bar = rcar_pcie_ep_clear_bar,
+ .set_msi = rcar_pcie_ep_set_msi,
+ .get_msi = rcar_pcie_ep_get_msi,
+ .map_addr = rcar_pcie_ep_map_addr,
+ .unmap_addr = rcar_pcie_ep_unmap_addr,
+ .raise_irq = rcar_pcie_ep_raise_irq,
+ .start = rcar_pcie_ep_start,
+ .stop = rcar_pcie_ep_stop,
+ .get_features = rcar_pcie_ep_get_features,
+};
+
+static const struct of_device_id rcar_pcie_ep_of_match[] = {
+ { .compatible = "renesas,r8a774c0-pcie-ep", },
+ { .compatible = "renesas,rcar-gen3-pcie-ep" },
+ { },
+};
+
+static int rcar_pcie_ep_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rcar_pcie_endpoint *ep;
+ struct rcar_pcie *pcie;
+ struct pci_epc *epc;
+ int err;
+
+ ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
+ if (!ep)
+ return -ENOMEM;
+
+ pcie = &ep->pcie;
+ pcie->dev = dev;
+
+ pm_runtime_enable(dev);
+ err = pm_runtime_get_sync(dev);
+ if (err < 0) {
+ dev_err(dev, "pm_runtime_get_sync failed\n");
+ goto err_pm_disable;
+ }
+
+ err = rcar_pcie_ep_get_pdata(ep, pdev);
+ if (err < 0) {
+ dev_err(dev, "failed to request resources: %d\n", err);
+ goto err_pm_put;
+ }
+
+ ep->num_ib_windows = MAX_NR_INBOUND_MAPS;
+ ep->ib_window_map =
+ devm_kcalloc(dev, BITS_TO_LONGS(ep->num_ib_windows),
+ sizeof(long), GFP_KERNEL);
+ if (!ep->ib_window_map) {
+ err = -ENOMEM;
+ dev_err(dev, "failed to allocate memory for inbound map\n");
+ goto err_pm_put;
+ }
+
+ ep->ob_mapped_addr = devm_kcalloc(dev, ep->num_ob_windows,
+ sizeof(*ep->ob_mapped_addr),
+ GFP_KERNEL);
+ if (!ep->ob_mapped_addr) {
+ err = -ENOMEM;
+ dev_err(dev, "failed to allocate memory for outbound memory pointers\n");
+ goto err_pm_put;
+ }
+
+ epc = devm_pci_epc_create(dev, &rcar_pcie_epc_ops);
+ if (IS_ERR(epc)) {
+ dev_err(dev, "failed to create epc device\n");
+ err = PTR_ERR(epc);
+ goto err_pm_put;
+ }
+
+ epc->max_functions = ep->max_functions;
+ epc_set_drvdata(epc, ep);
+
+ rcar_pcie_ep_hw_init(pcie);
+
+ err = pci_epc_multi_mem_init(epc, ep->ob_window, ep->num_ob_windows);
+ if (err < 0) {
+ dev_err(dev, "failed to initialize the epc memory space\n");
+ goto err_pm_put;
+ }
+
+ return 0;
+
+err_pm_put:
+ pm_runtime_put(dev);
+
+err_pm_disable:
+ pm_runtime_disable(dev);
+
+ return err;
+}
+
+static struct platform_driver rcar_pcie_ep_driver = {
+ .driver = {
+ .name = "rcar-pcie-ep",
+ .of_match_table = rcar_pcie_ep_of_match,
+ .suppress_bind_attrs = true,
+ },
+ .probe = rcar_pcie_ep_probe,
+};
+builtin_platform_driver(rcar_pcie_ep_driver);
diff --git a/drivers/pci/controller/pcie-rcar.h b/drivers/pci/controller/pcie-rcar.h
index cec7768b4725..2f567b690ffd 100644
--- a/drivers/pci/controller/pcie-rcar.h
+++ b/drivers/pci/controller/pcie-rcar.h
@@ -17,6 +17,7 @@
#define PCIECDR 0x000020
#define PCIEMSR 0x000028
#define PCIEINTXR 0x000400
+#define ASTINTX BIT(16)
#define PCIEPHYSR 0x0007f0
#define PHYRDY BIT(0)
#define PCIEMSITXR 0x000840
@@ -55,12 +56,20 @@

/* Configuration */
#define PCICONF(x) (0x010000 + ((x) * 0x4))
+#define INTDIS BIT(10)
#define PMCAP(x) (0x010040 + ((x) * 0x4))
+#define MSICAP(x) (0x010050 + ((x) * 0x4))
+#define MSICAP0_MSIE BIT(16)
+#define MSICAP0_MMESCAP_OFFSET 17
+#define MSICAP0_MMESE_OFFSET 20
+#define MSICAP0_MMESE_MASK GENMASK(22, 20)
#define EXPCAP(x) (0x010070 + ((x) * 0x4))
#define VCCAP(x) (0x010100 + ((x) * 0x4))

/* link layer */
+#define IDSETR0 0x011000
#define IDSETR1 0x011004
+#define SUBIDSETR 0x011024
#define TLCTLR 0x011048
#define MACSR 0x011054
#define SPCHGFIN BIT(4)
--
2.17.1

2020-04-23 18:29:13

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 8/8] MAINTAINERS: Add file patterns for rcar PCI device tree bindings

Add file pattern entry for rcar PCI devicetree binding, so that when
people run ./scripts/get_maintainer.pl the rcar PCI maintainers could also
be listed.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Yoshihiro Shimoda <[email protected]>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c85337a6b85..3e71a3173132 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12958,6 +12958,7 @@ M: Yoshihiro Shimoda <[email protected]>
L: [email protected]
L: [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/pci/*rcar*
F: drivers/pci/controller/*rcar*

PCI DRIVER FOR SAMSUNG EXYNOS
--
2.17.1

2020-04-23 18:30:59

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 5/8] PCI: endpoint: Add support to handle multiple base for mapping outbound memory

R-Car PCIe controller has support to map multiple memory regions for
mapping the outbound memory in local system also the controller limits
single allocation for each region (that is, once a chunk is used from the
region it cannot be used to allocate a new one). This features inspires to
add support for handling multiple memory bases in endpoint framework.

With this patch pci_epc_mem_init() initializes address space for endpoint
controller which support single window and pci_epc_multi_mem_init()
initializes multiple windows supported by endpoint controller.

Signed-off-by: Lad Prabhakar <[email protected]>
---
.../pci/controller/dwc/pcie-designware-ep.c | 16 +-
drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------
include/linux/pci-epc.h | 33 ++-
3 files changed, 170 insertions(+), 78 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 1cdcbd102ce8..a78902cbf2f0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -412,11 +412,11 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
reg = ep->msi_cap + PCI_MSI_DATA_32;
msg_data = dw_pcie_readw_dbi(pci, reg);
}
- aligned_offset = msg_addr_lower & (epc->mem->page_size - 1);
+ aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
msg_addr = ((u64)msg_addr_upper) << 32 |
(msg_addr_lower & ~aligned_offset);
ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
- epc->mem->page_size);
+ epc->mem->window.page_size);
if (ret)
return ret;

@@ -459,9 +459,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
return -EPERM;
}

- aligned_offset = msg_addr & (epc->mem->page_size - 1);
+ aligned_offset = msg_addr & (epc->mem->window.page_size - 1);
ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
- epc->mem->page_size);
+ epc->mem->window.page_size);
if (ret)
return ret;

@@ -477,7 +477,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
struct pci_epc *epc = ep->epc;

pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
- epc->mem->page_size);
+ epc->mem->window.page_size);

pci_epc_mem_exit(epc);
}
@@ -610,15 +610,15 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
if (ret < 0)
epc->max_functions = 1;

- ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
- ep->page_size);
+ ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
+ ep->page_size);
if (ret < 0) {
dev_err(dev, "Failed to initialize address space\n");
return ret;
}

ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
- epc->mem->page_size);
+ epc->mem->window.page_size);
if (!ep->msi_mem) {
dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
return -ENOMEM;
diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
index cdd1d3821249..a3466da2a16f 100644
--- a/drivers/pci/endpoint/pci-epc-mem.c
+++ b/drivers/pci/endpoint/pci-epc-mem.c
@@ -23,7 +23,7 @@
static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
{
int order;
- unsigned int page_shift = ilog2(mem->page_size);
+ unsigned int page_shift = ilog2(mem->window.page_size);

size--;
size >>= page_shift;
@@ -36,67 +36,95 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
}

/**
- * __pci_epc_mem_init() - initialize the pci_epc_mem structure
+ * pci_epc_multi_mem_init() - initialize the pci_epc_mem structure
* @epc: the EPC device that invoked pci_epc_mem_init
- * @phys_base: the physical address of the base
- * @size: the size of the address space
- * @page_size: size of each page
+ * @windows: pointer to windows supported by the device
+ * @num_windows: number of windows device supports
*
* Invoke to initialize the pci_epc_mem structure used by the
* endpoint functions to allocate mapped PCI address.
*/
-int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
- size_t page_size)
+int pci_epc_multi_mem_init(struct pci_epc *epc,
+ struct pci_epc_mem_window *windows,
+ unsigned int num_windows)
{
- int ret;
- struct pci_epc_mem *mem;
- unsigned long *bitmap;
+ struct pci_epc_mem *mem = NULL;
+ unsigned long *bitmap = NULL;
unsigned int page_shift;
- int pages;
+ size_t page_size;
int bitmap_size;
+ int pages;
+ int ret;
+ int i;

- if (page_size < PAGE_SIZE)
- page_size = PAGE_SIZE;
+ epc->num_windows = 0;

- page_shift = ilog2(page_size);
- pages = size >> page_shift;
- bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
+ if (!windows || !num_windows)
+ return -EINVAL;

- mem = kzalloc(sizeof(*mem), GFP_KERNEL);
- if (!mem) {
- ret = -ENOMEM;
- goto err;
- }
+ epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
+ if (!epc->windows)
+ return -ENOMEM;

- bitmap = kzalloc(bitmap_size, GFP_KERNEL);
- if (!bitmap) {
- ret = -ENOMEM;
- goto err_mem;
- }
+ for (i = 0; i < num_windows; i++) {
+ page_size = windows[i].page_size;
+ if (page_size < PAGE_SIZE)
+ page_size = PAGE_SIZE;
+ page_shift = ilog2(page_size);
+ pages = windows[i].size >> page_shift;
+ bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);

- mem->bitmap = bitmap;
- mem->phys_base = phys_base;
- mem->page_size = page_size;
- mem->pages = pages;
- mem->size = size;
- mutex_init(&mem->lock);
+ mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+ if (!mem) {
+ ret = -ENOMEM;
+ i--;
+ goto err_mem;
+ }

- epc->mem = mem;
+ bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ if (!bitmap) {
+ ret = -ENOMEM;
+ kfree(mem);
+ i--;
+ goto err_mem;
+ }
+
+ mem->window.phys_base = windows[i].phys_base;
+ mem->window.size = windows[i].size;
+ mem->window.page_size = page_size;
+ mem->bitmap = bitmap;
+ mem->pages = pages;
+ mutex_init(&mem->lock);
+ epc->windows[i] = mem;
+ }
+
+ epc->mem = epc->windows[0];
+ epc->num_windows = num_windows;

return 0;

err_mem:
- kfree(mem);
+ for (; i >= 0; i--) {
+ mem = epc->windows[i];
+ kfree(mem->bitmap);
+ kfree(mem);
+ }
+ kfree(epc->windows);

-err:
-return ret;
+ return ret;
}
-EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
+EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init);

int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
size_t size, size_t page_size)
{
- return __pci_epc_mem_init(epc, base, size, page_size);
+ struct pci_epc_mem_window mem_window;
+
+ mem_window.phys_base = base;
+ mem_window.size = size;
+ mem_window.page_size = page_size;
+
+ return pci_epc_multi_mem_init(epc, &mem_window, 1);
}
EXPORT_SYMBOL_GPL(pci_epc_mem_init);

@@ -109,11 +137,22 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_init);
*/
void pci_epc_mem_exit(struct pci_epc *epc)
{
- struct pci_epc_mem *mem = epc->mem;
+ struct pci_epc_mem *mem;
+ int i;

+ if (!epc->num_windows)
+ return;
+
+ for (i = 0; i < epc->num_windows; i++) {
+ mem = epc->windows[i];
+ kfree(mem->bitmap);
+ kfree(mem);
+ }
+ kfree(epc->windows);
+
+ epc->windows = NULL;
epc->mem = NULL;
- kfree(mem->bitmap);
- kfree(mem);
+ epc->num_windows = 0;
}
EXPORT_SYMBOL_GPL(pci_epc_mem_exit);

@@ -129,31 +168,60 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
phys_addr_t *phys_addr, size_t size)
{
- int pageno;
void __iomem *virt_addr = NULL;
- struct pci_epc_mem *mem = epc->mem;
- unsigned int page_shift = ilog2(mem->page_size);
+ struct pci_epc_mem *mem;
+ unsigned int page_shift;
+ size_t align_size;
+ int pageno;
int order;
+ int i;

- size = ALIGN(size, mem->page_size);
- order = pci_epc_mem_get_order(mem, size);
-
- mutex_lock(&mem->lock);
- pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
- if (pageno < 0)
- goto ret;
+ for (i = 0; i < epc->num_windows; i++) {
+ mem = epc->windows[i];
+ mutex_lock(&mem->lock);
+ align_size = ALIGN(size, mem->window.page_size);
+ order = pci_epc_mem_get_order(mem, align_size);

- *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
- virt_addr = ioremap(*phys_addr, size);
- if (!virt_addr)
- bitmap_release_region(mem->bitmap, pageno, order);
+ pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
+ order);
+ if (pageno >= 0) {
+ page_shift = ilog2(mem->window.page_size);
+ *phys_addr = mem->window.phys_base +
+ ((phys_addr_t)pageno << page_shift);
+ virt_addr = ioremap(*phys_addr, align_size);
+ if (!virt_addr) {
+ bitmap_release_region(mem->bitmap,
+ pageno, order);
+ mutex_unlock(&mem->lock);
+ continue;
+ }
+ mutex_unlock(&mem->lock);
+ return virt_addr;
+ }
+ mutex_unlock(&mem->lock);
+ }

-ret:
- mutex_unlock(&mem->lock);
return virt_addr;
}
EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);

+struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
+ phys_addr_t phys_addr)
+{
+ struct pci_epc_mem *mem;
+ int i;
+
+ for (i = 0; i < epc->num_windows; i++) {
+ mem = epc->windows[i];
+
+ if (phys_addr >= mem->window.phys_base &&
+ phys_addr < (mem->window.phys_base + mem->window.size))
+ return mem;
+ }
+
+ return NULL;
+}
+
/**
* pci_epc_mem_free_addr() - free the allocated memory address
* @epc: the EPC device on which memory was allocated
@@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
void __iomem *virt_addr, size_t size)
{
+ struct pci_epc_mem *mem;
+ unsigned int page_shift;
+ size_t page_size;
int pageno;
- struct pci_epc_mem *mem = epc->mem;
- unsigned int page_shift = ilog2(mem->page_size);
int order;

+ mem = pci_epc_get_matching_window(epc, phys_addr);
+ if (!mem) {
+ pr_err("failed to get matching window\n");
+ return;
+ }
+
+ page_size = mem->window.page_size;
+ page_shift = ilog2(page_size);
iounmap(virt_addr);
- pageno = (phys_addr - mem->phys_base) >> page_shift;
- size = ALIGN(size, mem->page_size);
+ pageno = (phys_addr - mem->window.phys_base) >> page_shift;
+ size = ALIGN(size, page_size);
order = pci_epc_mem_get_order(mem, size);
mutex_lock(&mem->lock);
bitmap_release_region(mem->bitmap, pageno, order);
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 5bc1de65849e..cc66bec8be90 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -65,20 +65,28 @@ struct pci_epc_ops {
struct module *owner;
};

+/**
+ * struct pci_epc_mem_window - address window of the endpoint controller
+ * @phys_base: physical base address of the PCI address window
+ * @size: the size of the PCI address window
+ * @page_size: size of each page
+ */
+struct pci_epc_mem_window {
+ phys_addr_t phys_base;
+ size_t size;
+ size_t page_size;
+};
+
/**
* struct pci_epc_mem - address space of the endpoint controller
- * @phys_base: physical base address of the PCI address space
- * @size: the size of the PCI address space
+ * @window: address window of the endpoint controller
* @bitmap: bitmap to manage the PCI address space
* @pages: number of bits representing the address region
- * @page_size: size of each page
* @lock: mutex to protect bitmap
*/
struct pci_epc_mem {
- phys_addr_t phys_base;
- size_t size;
+ struct pci_epc_mem_window window;
unsigned long *bitmap;
- size_t page_size;
int pages;
/* mutex to protect against concurrent access for memory allocation*/
struct mutex lock;
@@ -89,7 +97,11 @@ struct pci_epc_mem {
* @dev: PCI EPC device
* @pci_epf: list of endpoint functions present in this EPC device
* @ops: function pointers for performing endpoint operations
- * @mem: address space of the endpoint controller
+ * @windows: array of address space of the endpoint controller
+ * @mem: first window of the endpoint controller, which corresponds to
+ * default address space of the endpoint controller supporting
+ * single window.
+ * @num_windows: number of windows supported by device
* @max_functions: max number of functions that can be configured in this EPC
* @group: configfs group representing the PCI EPC device
* @lock: mutex to protect pci_epc ops
@@ -100,7 +112,9 @@ struct pci_epc {
struct device dev;
struct list_head pci_epf;
const struct pci_epc_ops *ops;
+ struct pci_epc_mem **windows;
struct pci_epc_mem *mem;
+ unsigned int num_windows;
u8 max_functions;
struct config_group *group;
/* mutex to protect against concurrent access of EP controller */
@@ -194,8 +208,9 @@ void pci_epc_put(struct pci_epc *epc);

int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
size_t size, size_t page_size);
-int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_addr, size_t size,
- size_t page_size);
+int pci_epc_multi_mem_init(struct pci_epc *epc,
+ struct pci_epc_mem_window *window,
+ unsigned int num_windows);
void pci_epc_mem_exit(struct pci_epc *epc);
void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
phys_addr_t *phys_addr, size_t size);
--
2.17.1

2020-04-23 18:31:07

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 4/8] PCI: endpoint: Pass page size as argument to pci_epc_mem_init()

pci_epc_mem_init() internally used page size equal to *PAGE_SIZE* to
manage the address space so instead just pass the page size as a
argument to pci_epc_mem_init().

Also make pci_epc_mem_init() as a C function instead of a macro function
in preparation for adding support for pci-epc-mem core to handle multiple
windows.

Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 +-
drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
drivers/pci/endpoint/pci-epc-mem.c | 7 +++++++
include/linux/pci-epc.h | 5 ++---
4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 1c173dad67d1..1c15c8352125 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -450,7 +450,7 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
epc->max_functions = 1;

ret = pci_epc_mem_init(epc, pcie->mem_res->start,
- resource_size(pcie->mem_res));
+ resource_size(pcie->mem_res), PAGE_SIZE);
if (ret < 0) {
dev_err(dev, "failed to initialize the memory space\n");
goto err_init;
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index d743b0a48988..5eaf36629a75 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -615,7 +615,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);

err = pci_epc_mem_init(epc, rockchip->mem_res->start,
- resource_size(rockchip->mem_res));
+ resource_size(rockchip->mem_res), PAGE_SIZE);
if (err < 0) {
dev_err(dev, "failed to initialize the memory space\n");
goto err_uninit_port;
diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
index abfac1109a13..cdd1d3821249 100644
--- a/drivers/pci/endpoint/pci-epc-mem.c
+++ b/drivers/pci/endpoint/pci-epc-mem.c
@@ -93,6 +93,13 @@ return ret;
}
EXPORT_SYMBOL_GPL(__pci_epc_mem_init);

+int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
+ size_t size, size_t page_size)
+{
+ return __pci_epc_mem_init(epc, base, size, page_size);
+}
+EXPORT_SYMBOL_GPL(pci_epc_mem_init);
+
/**
* pci_epc_mem_exit() - cleanup the pci_epc_mem structure
* @epc: the EPC device that invoked pci_epc_mem_exit
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index e0ed9d01f6e5..5bc1de65849e 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -137,9 +137,6 @@ struct pci_epc_features {
#define devm_pci_epc_create(dev, ops) \
__devm_pci_epc_create((dev), (ops), THIS_MODULE)

-#define pci_epc_mem_init(epc, phys_addr, size) \
- __pci_epc_mem_init((epc), (phys_addr), (size), PAGE_SIZE)
-
static inline void epc_set_drvdata(struct pci_epc *epc, void *data)
{
dev_set_drvdata(&epc->dev, data);
@@ -195,6 +192,8 @@ unsigned int pci_epc_get_first_free_bar(const struct pci_epc_features
struct pci_epc *pci_epc_get(const char *epc_name);
void pci_epc_put(struct pci_epc *epc);

+int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
+ size_t size, size_t page_size);
int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_addr, size_t size,
size_t page_size);
void pci_epc_mem_exit(struct pci_epc *epc);
--
2.17.1

2020-04-23 19:37:25

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v9 1/8] PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c

This commit renames pcie-rcar.c to pcie-rcar-host.c in preparation for
adding support for endpoint mode. CONFIG_PCIE_RCAR is kept so that arm64
defconfig change can be a separate patch.

With this patch both config options PCIE_RCAR and PCIE_RCAR_HOST will be
available but PCIE_RCAR internally selects PCIE_RCAR_HOST so that bisect
builds wont be affected.

Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Yoshihiro Shimoda <[email protected]>
---
drivers/pci/controller/Kconfig | 10 ++++++++++
drivers/pci/controller/Makefile | 2 +-
.../pci/controller/{pcie-rcar.c => pcie-rcar-host.c} | 0
3 files changed, 11 insertions(+), 1 deletion(-)
rename drivers/pci/controller/{pcie-rcar.c => pcie-rcar-host.c} (100%)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index af0f0bc11917..cfdc898450d0 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -58,8 +58,18 @@ config PCIE_RCAR
bool "Renesas R-Car PCIe controller"
depends on ARCH_RENESAS || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
+ select PCIE_RCAR_HOST
help
Say Y here if you want PCIe controller support on R-Car SoCs.
+ This option will be removed after arm64 defconfig is updated.
+
+config PCIE_RCAR_HOST
+ bool "Renesas R-Car PCIe host controller"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ depends on PCI_MSI_IRQ_DOMAIN
+ help
+ Say Y here if you want PCIe controller support on R-Car SoCs in host
+ mode.

config PCI_HOST_COMMON
bool
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index 158c59771824..9dbccb5b24e1 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
-obj-$(CONFIG_PCIE_RCAR) += pcie-rcar.o
+obj-$(CONFIG_PCIE_RCAR_HOST) += pcie-rcar-host.o
obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o
obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar-host.c
similarity index 100%
rename from drivers/pci/controller/pcie-rcar.c
rename to drivers/pci/controller/pcie-rcar-host.c
--
2.17.1

2020-04-24 06:01:38

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH v9 4/8] PCI: endpoint: Pass page size as argument to pci_epc_mem_init()

Hi Prabhakar-san,

> From: Lad Prabhakar, Sent: Friday, April 24, 2020 3:23 AM
>
> pci_epc_mem_init() internally used page size equal to *PAGE_SIZE* to
> manage the address space so instead just pass the page size as a
> argument to pci_epc_mem_init().
>
> Also make pci_epc_mem_init() as a C function instead of a macro function
> in preparation for adding support for pci-epc-mem core to handle multiple
> windows.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <[email protected]>

Best regards,
Yoshihiro Shimoda

2020-04-24 06:02:52

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH v9 5/8] PCI: endpoint: Add support to handle multiple base for mapping outbound memory

Hi Prabhakar-san,

> From: Lad Prabhakar, Sent: Friday, April 24, 2020 3:23 AM
>
> R-Car PCIe controller has support to map multiple memory regions for
> mapping the outbound memory in local system also the controller limits
> single allocation for each region (that is, once a chunk is used from the
> region it cannot be used to allocate a new one). This features inspires to
> add support for handling multiple memory bases in endpoint framework.
>
> With this patch pci_epc_mem_init() initializes address space for endpoint
> controller which support single window and pci_epc_multi_mem_init()
> initializes multiple windows supported by endpoint controller.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <[email protected]>

Best regards,
Yoshihiro Shimoda

2020-04-24 06:04:21

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH v9 7/8] PCI: rcar: Add endpoint mode support

Hi Prabhakar-san,

> From: Lad Prabhakar, Sent: Friday, April 24, 2020 3:23 AM
>
> Add support for R-Car PCIe controller to work in endpoint mode.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <[email protected]>

Best regards,
Yoshihiro Shimoda

2020-04-24 06:17:22

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v9 5/8] PCI: endpoint: Add support to handle multiple base for mapping outbound memory

Hi Prabhakar,

On 4/23/2020 11:52 PM, Lad Prabhakar wrote:
> R-Car PCIe controller has support to map multiple memory regions for
> mapping the outbound memory in local system also the controller limits
> single allocation for each region (that is, once a chunk is used from the
> region it cannot be used to allocate a new one). This features inspires to
> add support for handling multiple memory bases in endpoint framework.
>
> With this patch pci_epc_mem_init() initializes address space for endpoint
> controller which support single window and pci_epc_multi_mem_init()
> initializes multiple windows supported by endpoint controller.

Have a couple of clean-up comments. See below.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------
> include/linux/pci-epc.h | 33 ++-
> 3 files changed, 170 insertions(+), 78 deletions(-)
>
.
.
<snip>
.
.
> diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
> index cdd1d3821249..a3466da2a16f 100644
> --- a/drivers/pci/endpoint/pci-epc-mem.c
> +++ b/drivers/pci/endpoint/pci-epc-mem.c
> @@ -23,7 +23,7 @@
> static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> {
> int order;
> - unsigned int page_shift = ilog2(mem->page_size);
> + unsigned int page_shift = ilog2(mem->window.page_size);
>
> size--;
> size >>= page_shift;
> @@ -36,67 +36,95 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> }
>
> /**
> - * __pci_epc_mem_init() - initialize the pci_epc_mem structure
> + * pci_epc_multi_mem_init() - initialize the pci_epc_mem structure
> * @epc: the EPC device that invoked pci_epc_mem_init
> - * @phys_base: the physical address of the base
> - * @size: the size of the address space
> - * @page_size: size of each page
> + * @windows: pointer to windows supported by the device
> + * @num_windows: number of windows device supports
> *
> * Invoke to initialize the pci_epc_mem structure used by the
> * endpoint functions to allocate mapped PCI address.
> */
> -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
> - size_t page_size)
> +int pci_epc_multi_mem_init(struct pci_epc *epc,
> + struct pci_epc_mem_window *windows,
> + unsigned int num_windows)
> {
> - int ret;
> - struct pci_epc_mem *mem;
> - unsigned long *bitmap;
> + struct pci_epc_mem *mem = NULL;
> + unsigned long *bitmap = NULL;
> unsigned int page_shift;
> - int pages;
> + size_t page_size;
> int bitmap_size;
> + int pages;
> + int ret;
> + int i;
>
> - if (page_size < PAGE_SIZE)
> - page_size = PAGE_SIZE;
> + epc->num_windows = 0;
>
> - page_shift = ilog2(page_size);
> - pages = size >> page_shift;
> - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
> + if (!windows || !num_windows)
> + return -EINVAL;
>
> - mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> - if (!mem) {
> - ret = -ENOMEM;
> - goto err;
> - }
> + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
> + if (!epc->windows)
> + return -ENOMEM;
>
> - bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> - if (!bitmap) {
> - ret = -ENOMEM;
> - goto err_mem;
> - }
> + for (i = 0; i < num_windows; i++) {
> + page_size = windows[i].page_size;
> + if (page_size < PAGE_SIZE)
> + page_size = PAGE_SIZE;
> + page_shift = ilog2(page_size);
> + pages = windows[i].size >> page_shift;
> + bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
>
> - mem->bitmap = bitmap;
> - mem->phys_base = phys_base;
> - mem->page_size = page_size;
> - mem->pages = pages;
> - mem->size = size;
> - mutex_init(&mem->lock);
> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> + if (!mem) {
> + ret = -ENOMEM;
> + i--;
> + goto err_mem;
> + }
>
> - epc->mem = mem;
> + bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + if (!bitmap) {
> + ret = -ENOMEM;
> + kfree(mem);
> + i--;
> + goto err_mem;
> + }
> +
> + mem->window.phys_base = windows[i].phys_base;
> + mem->window.size = windows[i].size;
> + mem->window.page_size = page_size;
> + mem->bitmap = bitmap;
> + mem->pages = pages;
> + mutex_init(&mem->lock);
> + epc->windows[i] = mem;
> + }
> +
> + epc->mem = epc->windows[0];

"mem" member of EPC looks unnecessary since that value is available at
epc->windows[0].
> + epc->num_windows = num_windows;
>
> return 0;
>
> err_mem:
> - kfree(mem);
> + for (; i >= 0; i--) {
> + mem = epc->windows[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->windows);
>
> -err:
> -return ret;
> + return ret;
> }
> -EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
> +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init);
>
> int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
> size_t size, size_t page_size)
> {
> - return __pci_epc_mem_init(epc, base, size, page_size);
> + struct pci_epc_mem_window mem_window;
> +
> + mem_window.phys_base = base;
> + mem_window.size = size;
> + mem_window.page_size = page_size;
> +
> + return pci_epc_multi_mem_init(epc, &mem_window, 1);
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_init);
>
> @@ -109,11 +137,22 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_init);
> */
> void pci_epc_mem_exit(struct pci_epc *epc)
> {
> - struct pci_epc_mem *mem = epc->mem;
> + struct pci_epc_mem *mem;
> + int i;
>
> + if (!epc->num_windows)
> + return;
> +
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->windows);
> +
> + epc->windows = NULL;
> epc->mem = NULL;
> - kfree(mem->bitmap);
> - kfree(mem);
> + epc->num_windows = 0;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
>
> @@ -129,31 +168,60 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
> void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size)
> {
> - int pageno;
> void __iomem *virt_addr = NULL;
> - struct pci_epc_mem *mem = epc->mem;
> - unsigned int page_shift = ilog2(mem->page_size);
> + struct pci_epc_mem *mem;
> + unsigned int page_shift;
> + size_t align_size;
> + int pageno;
> int order;
> + int i;
>
> - size = ALIGN(size, mem->page_size);
> - order = pci_epc_mem_get_order(mem, size);
> -
> - mutex_lock(&mem->lock);
> - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
> - if (pageno < 0)
> - goto ret;
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> + mutex_lock(&mem->lock);
> + align_size = ALIGN(size, mem->window.page_size);
> + order = pci_epc_mem_get_order(mem, align_size);
>
> - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
> - virt_addr = ioremap(*phys_addr, size);
> - if (!virt_addr)
> - bitmap_release_region(mem->bitmap, pageno, order);
> + pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
> + order);
> + if (pageno >= 0) {
> + page_shift = ilog2(mem->window.page_size);
> + *phys_addr = mem->window.phys_base +
> + ((phys_addr_t)pageno << page_shift);
> + virt_addr = ioremap(*phys_addr, align_size);
> + if (!virt_addr) {
> + bitmap_release_region(mem->bitmap,
> + pageno, order);
> + mutex_unlock(&mem->lock);
> + continue;
> + }
> + mutex_unlock(&mem->lock);
> + return virt_addr;
> + }
> + mutex_unlock(&mem->lock);
> + }
>
> -ret:
> - mutex_unlock(&mem->lock);
> return virt_addr;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
>
> +struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
> + phys_addr_t phys_addr)
> +{
> + struct pci_epc_mem *mem;
> + int i;
> +
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> +
> + if (phys_addr >= mem->window.phys_base &&
> + phys_addr < (mem->window.phys_base + mem->window.size))
> + return mem;
> + }
> +
> + return NULL;
> +}
> +
> /**
> * pci_epc_mem_free_addr() - free the allocated memory address
> * @epc: the EPC device on which memory was allocated
> @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
> void __iomem *virt_addr, size_t size)
> {
> + struct pci_epc_mem *mem;
> + unsigned int page_shift;
> + size_t page_size;
> int pageno;
> - struct pci_epc_mem *mem = epc->mem;
> - unsigned int page_shift = ilog2(mem->page_size);
> int order;
>
> + mem = pci_epc_get_matching_window(epc, phys_addr);
> + if (!mem) {
> + pr_err("failed to get matching window\n");
> + return;
> + }
> +
> + page_size = mem->window.page_size;
> + page_shift = ilog2(page_size);
> iounmap(virt_addr);
> - pageno = (phys_addr - mem->phys_base) >> page_shift;
> - size = ALIGN(size, mem->page_size);
> + pageno = (phys_addr - mem->window.phys_base) >> page_shift;
> + size = ALIGN(size, page_size);
> order = pci_epc_mem_get_order(mem, size);
> mutex_lock(&mem->lock);
> bitmap_release_region(mem->bitmap, pageno, order);
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 5bc1de65849e..cc66bec8be90 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -65,20 +65,28 @@ struct pci_epc_ops {
> struct module *owner;
> };
>
> +/**
> + * struct pci_epc_mem_window - address window of the endpoint controller
> + * @phys_base: physical base address of the PCI address window
> + * @size: the size of the PCI address window
> + * @page_size: size of each page
> + */
> +struct pci_epc_mem_window {
> + phys_addr_t phys_base;
> + size_t size;
> + size_t page_size;
> +};
> +
> /**
> * struct pci_epc_mem - address space of the endpoint controller
> - * @phys_base: physical base address of the PCI address space
> - * @size: the size of the PCI address space
> + * @window: address window of the endpoint controller
> * @bitmap: bitmap to manage the PCI address space
> * @pages: number of bits representing the address region
> - * @page_size: size of each page
> * @lock: mutex to protect bitmap
> */
> struct pci_epc_mem {
> - phys_addr_t phys_base;
> - size_t size;
> + struct pci_epc_mem_window window;

Don't see any additional value in moving phys_base, size, page_size to a new
structure and again including it here.

Thanks
Kishon

2020-04-24 07:48:44

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v9 5/8] PCI: endpoint: Add support to handle multiple base for mapping outbound memory

Hi Kishon,

Thank you for the review.

On Fri, Apr 24, 2020 at 7:13 AM Kishon Vijay Abraham I <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On 4/23/2020 11:52 PM, Lad Prabhakar wrote:
> > R-Car PCIe controller has support to map multiple memory regions for
> > mapping the outbound memory in local system also the controller limits
> > single allocation for each region (that is, once a chunk is used from the
> > region it cannot be used to allocate a new one). This features inspires to
> > add support for handling multiple memory bases in endpoint framework.
> >
> > With this patch pci_epc_mem_init() initializes address space for endpoint
> > controller which support single window and pci_epc_multi_mem_init()
> > initializes multiple windows supported by endpoint controller.
>
> Have a couple of clean-up comments. See below.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> > drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------
> > include/linux/pci-epc.h | 33 ++-
> > 3 files changed, 170 insertions(+), 78 deletions(-)
> >
> .
> .
> <snip>
> .
> .
> > diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
> > index cdd1d3821249..a3466da2a16f 100644
> > --- a/drivers/pci/endpoint/pci-epc-mem.c
> > +++ b/drivers/pci/endpoint/pci-epc-mem.c
> > @@ -23,7 +23,7 @@
> > static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> > {
> > int order;
> > - unsigned int page_shift = ilog2(mem->page_size);
> > + unsigned int page_shift = ilog2(mem->window.page_size);
> >
> > size--;
> > size >>= page_shift;
> > @@ -36,67 +36,95 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> > }
> >
> > /**
> > - * __pci_epc_mem_init() - initialize the pci_epc_mem structure
> > + * pci_epc_multi_mem_init() - initialize the pci_epc_mem structure
> > * @epc: the EPC device that invoked pci_epc_mem_init
> > - * @phys_base: the physical address of the base
> > - * @size: the size of the address space
> > - * @page_size: size of each page
> > + * @windows: pointer to windows supported by the device
> > + * @num_windows: number of windows device supports
> > *
> > * Invoke to initialize the pci_epc_mem structure used by the
> > * endpoint functions to allocate mapped PCI address.
> > */
> > -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
> > - size_t page_size)
> > +int pci_epc_multi_mem_init(struct pci_epc *epc,
> > + struct pci_epc_mem_window *windows,
> > + unsigned int num_windows)
> > {
> > - int ret;
> > - struct pci_epc_mem *mem;
> > - unsigned long *bitmap;
> > + struct pci_epc_mem *mem = NULL;
> > + unsigned long *bitmap = NULL;
> > unsigned int page_shift;
> > - int pages;
> > + size_t page_size;
> > int bitmap_size;
> > + int pages;
> > + int ret;
> > + int i;
> >
> > - if (page_size < PAGE_SIZE)
> > - page_size = PAGE_SIZE;
> > + epc->num_windows = 0;
> >
> > - page_shift = ilog2(page_size);
> > - pages = size >> page_shift;
> > - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
> > + if (!windows || !num_windows)
> > + return -EINVAL;
> >
> > - mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> > - if (!mem) {
> > - ret = -ENOMEM;
> > - goto err;
> > - }
> > + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
> > + if (!epc->windows)
> > + return -ENOMEM;
> >
> > - bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> > - if (!bitmap) {
> > - ret = -ENOMEM;
> > - goto err_mem;
> > - }
> > + for (i = 0; i < num_windows; i++) {
> > + page_size = windows[i].page_size;
> > + if (page_size < PAGE_SIZE)
> > + page_size = PAGE_SIZE;
> > + page_shift = ilog2(page_size);
> > + pages = windows[i].size >> page_shift;
> > + bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
> >
> > - mem->bitmap = bitmap;
> > - mem->phys_base = phys_base;
> > - mem->page_size = page_size;
> > - mem->pages = pages;
> > - mem->size = size;
> > - mutex_init(&mem->lock);
> > + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> > + if (!mem) {
> > + ret = -ENOMEM;
> > + i--;
> > + goto err_mem;
> > + }
> >
> > - epc->mem = mem;
> > + bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> > + if (!bitmap) {
> > + ret = -ENOMEM;
> > + kfree(mem);
> > + i--;
> > + goto err_mem;
> > + }
> > +
> > + mem->window.phys_base = windows[i].phys_base;
> > + mem->window.size = windows[i].size;
> > + mem->window.page_size = page_size;
> > + mem->bitmap = bitmap;
> > + mem->pages = pages;
> > + mutex_init(&mem->lock);
> > + epc->windows[i] = mem;
> > + }
> > +
> > + epc->mem = epc->windows[0];
>
> "mem" member of EPC looks unnecessary since that value is available at
> epc->windows[0].
This was suggested by Shimoda-san, as most of the current controller
drivers support single region this pointer would be easier to access
the region instead of adding #define EPC_DEFAULT_WINDOW 0 and
accessing as epc->windows[EPC_DEFAULT_WINDOW];

> > + epc->num_windows = num_windows;
> >
> > return 0;
> >
> > err_mem:
> > - kfree(mem);
> > + for (; i >= 0; i--) {
> > + mem = epc->windows[i];
> > + kfree(mem->bitmap);
> > + kfree(mem);
> > + }
> > + kfree(epc->windows);
> >
> > -err:
> > -return ret;
> > + return ret;
> > }
> > -EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
> > +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init);
> >
> > int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
> > size_t size, size_t page_size)
> > {
> > - return __pci_epc_mem_init(epc, base, size, page_size);
> > + struct pci_epc_mem_window mem_window;
> > +
> > + mem_window.phys_base = base;
> > + mem_window.size = size;
> > + mem_window.page_size = page_size;
> > +
> > + return pci_epc_multi_mem_init(epc, &mem_window, 1);
> > }
> > EXPORT_SYMBOL_GPL(pci_epc_mem_init);
> >
> > @@ -109,11 +137,22 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_init);
> > */
> > void pci_epc_mem_exit(struct pci_epc *epc)
> > {
> > - struct pci_epc_mem *mem = epc->mem;
> > + struct pci_epc_mem *mem;
> > + int i;
> >
> > + if (!epc->num_windows)
> > + return;
> > +
> > + for (i = 0; i < epc->num_windows; i++) {
> > + mem = epc->windows[i];
> > + kfree(mem->bitmap);
> > + kfree(mem);
> > + }
> > + kfree(epc->windows);
> > +
> > + epc->windows = NULL;
> > epc->mem = NULL;
> > - kfree(mem->bitmap);
> > - kfree(mem);
> > + epc->num_windows = 0;
> > }
> > EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
> >
> > @@ -129,31 +168,60 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
> > void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> > phys_addr_t *phys_addr, size_t size)
> > {
> > - int pageno;
> > void __iomem *virt_addr = NULL;
> > - struct pci_epc_mem *mem = epc->mem;
> > - unsigned int page_shift = ilog2(mem->page_size);
> > + struct pci_epc_mem *mem;
> > + unsigned int page_shift;
> > + size_t align_size;
> > + int pageno;
> > int order;
> > + int i;
> >
> > - size = ALIGN(size, mem->page_size);
> > - order = pci_epc_mem_get_order(mem, size);
> > -
> > - mutex_lock(&mem->lock);
> > - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
> > - if (pageno < 0)
> > - goto ret;
> > + for (i = 0; i < epc->num_windows; i++) {
> > + mem = epc->windows[i];
> > + mutex_lock(&mem->lock);
> > + align_size = ALIGN(size, mem->window.page_size);
> > + order = pci_epc_mem_get_order(mem, align_size);
> >
> > - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
> > - virt_addr = ioremap(*phys_addr, size);
> > - if (!virt_addr)
> > - bitmap_release_region(mem->bitmap, pageno, order);
> > + pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
> > + order);
> > + if (pageno >= 0) {
> > + page_shift = ilog2(mem->window.page_size);
> > + *phys_addr = mem->window.phys_base +
> > + ((phys_addr_t)pageno << page_shift);
> > + virt_addr = ioremap(*phys_addr, align_size);
> > + if (!virt_addr) {
> > + bitmap_release_region(mem->bitmap,
> > + pageno, order);
> > + mutex_unlock(&mem->lock);
> > + continue;
> > + }
> > + mutex_unlock(&mem->lock);
> > + return virt_addr;
> > + }
> > + mutex_unlock(&mem->lock);
> > + }
> >
> > -ret:
> > - mutex_unlock(&mem->lock);
> > return virt_addr;
> > }
> > EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
> >
> > +struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
> > + phys_addr_t phys_addr)
> > +{
> > + struct pci_epc_mem *mem;
> > + int i;
> > +
> > + for (i = 0; i < epc->num_windows; i++) {
> > + mem = epc->windows[i];
> > +
> > + if (phys_addr >= mem->window.phys_base &&
> > + phys_addr < (mem->window.phys_base + mem->window.size))
> > + return mem;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > /**
> > * pci_epc_mem_free_addr() - free the allocated memory address
> > * @epc: the EPC device on which memory was allocated
> > @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
> > void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
> > void __iomem *virt_addr, size_t size)
> > {
> > + struct pci_epc_mem *mem;
> > + unsigned int page_shift;
> > + size_t page_size;
> > int pageno;
> > - struct pci_epc_mem *mem = epc->mem;
> > - unsigned int page_shift = ilog2(mem->page_size);
> > int order;
> >
> > + mem = pci_epc_get_matching_window(epc, phys_addr);
> > + if (!mem) {
> > + pr_err("failed to get matching window\n");
> > + return;
> > + }
> > +
> > + page_size = mem->window.page_size;
> > + page_shift = ilog2(page_size);
> > iounmap(virt_addr);
> > - pageno = (phys_addr - mem->phys_base) >> page_shift;
> > - size = ALIGN(size, mem->page_size);
> > + pageno = (phys_addr - mem->window.phys_base) >> page_shift;
> > + size = ALIGN(size, page_size);
> > order = pci_epc_mem_get_order(mem, size);
> > mutex_lock(&mem->lock);
> > bitmap_release_region(mem->bitmap, pageno, order);
> > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> > index 5bc1de65849e..cc66bec8be90 100644
> > --- a/include/linux/pci-epc.h
> > +++ b/include/linux/pci-epc.h
> > @@ -65,20 +65,28 @@ struct pci_epc_ops {
> > struct module *owner;
> > };
> >
> > +/**
> > + * struct pci_epc_mem_window - address window of the endpoint controller
> > + * @phys_base: physical base address of the PCI address window
> > + * @size: the size of the PCI address window
> > + * @page_size: size of each page
> > + */
> > +struct pci_epc_mem_window {
> > + phys_addr_t phys_base;
> > + size_t size;
> > + size_t page_size;
> > +};
> > +
> > /**
> > * struct pci_epc_mem - address space of the endpoint controller
> > - * @phys_base: physical base address of the PCI address space
> > - * @size: the size of the PCI address space
> > + * @window: address window of the endpoint controller
> > * @bitmap: bitmap to manage the PCI address space
> > * @pages: number of bits representing the address region
> > - * @page_size: size of each page
> > * @lock: mutex to protect bitmap
> > */
> > struct pci_epc_mem {
> > - phys_addr_t phys_base;
> > - size_t size;
> > + struct pci_epc_mem_window window;
>
> Don't see any additional value in moving phys_base, size, page_size to a new
> structure and again including it here.
>
Controllers supporting multiple windows create a list of supported
regions (struct pci_epc_mem_window ) and pass a pointer to
pci_epc_multi_mem_init(), hence this split.

Cheers,
--Prabhakar

2020-04-24 08:05:34

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v9 5/8] PCI: endpoint: Add support to handle multiple base for mapping outbound memory

Hi Prabhakar,

On 4/24/2020 1:16 PM, Lad, Prabhakar wrote:
> Hi Kishon,
>
> Thank you for the review.
>
> On Fri, Apr 24, 2020 at 7:13 AM Kishon Vijay Abraham I <[email protected]> wrote:
>>
>> Hi Prabhakar,
>>
>> On 4/23/2020 11:52 PM, Lad Prabhakar wrote:
>>> R-Car PCIe controller has support to map multiple memory regions for
>>> mapping the outbound memory in local system also the controller limits
>>> single allocation for each region (that is, once a chunk is used from the
>>> region it cannot be used to allocate a new one). This features inspires to
>>> add support for handling multiple memory bases in endpoint framework.
>>>
>>> With this patch pci_epc_mem_init() initializes address space for endpoint
>>> controller which support single window and pci_epc_multi_mem_init()
>>> initializes multiple windows supported by endpoint controller.
>>
>> Have a couple of clean-up comments. See below.
>>>
>>> Signed-off-by: Lad Prabhakar <[email protected]>
>>> ---
>>> .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
>>> drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------
>>> include/linux/pci-epc.h | 33 ++-
>>> 3 files changed, 170 insertions(+), 78 deletions(-)
>>>
>> .
>> .
>> <snip>
>> .
>> .
>>> diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
>>> index cdd1d3821249..a3466da2a16f 100644
>>> --- a/drivers/pci/endpoint/pci-epc-mem.c
>>> +++ b/drivers/pci/endpoint/pci-epc-mem.c
>>> @@ -23,7 +23,7 @@
>>> static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
>>> {
>>> int order;
>>> - unsigned int page_shift = ilog2(mem->page_size);
>>> + unsigned int page_shift = ilog2(mem->window.page_size);
>>>
>>> size--;
>>> size >>= page_shift;
>>> @@ -36,67 +36,95 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
>>> }
>>>
>>> /**
>>> - * __pci_epc_mem_init() - initialize the pci_epc_mem structure
>>> + * pci_epc_multi_mem_init() - initialize the pci_epc_mem structure
>>> * @epc: the EPC device that invoked pci_epc_mem_init
>>> - * @phys_base: the physical address of the base
>>> - * @size: the size of the address space
>>> - * @page_size: size of each page
>>> + * @windows: pointer to windows supported by the device
>>> + * @num_windows: number of windows device supports
>>> *
>>> * Invoke to initialize the pci_epc_mem structure used by the
>>> * endpoint functions to allocate mapped PCI address.
>>> */
>>> -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
>>> - size_t page_size)
>>> +int pci_epc_multi_mem_init(struct pci_epc *epc,
>>> + struct pci_epc_mem_window *windows,
>>> + unsigned int num_windows)
>>> {
>>> - int ret;
>>> - struct pci_epc_mem *mem;
>>> - unsigned long *bitmap;
>>> + struct pci_epc_mem *mem = NULL;
>>> + unsigned long *bitmap = NULL;
>>> unsigned int page_shift;
>>> - int pages;
>>> + size_t page_size;
>>> int bitmap_size;
>>> + int pages;
>>> + int ret;
>>> + int i;
>>>
>>> - if (page_size < PAGE_SIZE)
>>> - page_size = PAGE_SIZE;
>>> + epc->num_windows = 0;
>>>
>>> - page_shift = ilog2(page_size);
>>> - pages = size >> page_shift;
>>> - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
>>> + if (!windows || !num_windows)
>>> + return -EINVAL;
>>>
>>> - mem = kzalloc(sizeof(*mem), GFP_KERNEL);
>>> - if (!mem) {
>>> - ret = -ENOMEM;
>>> - goto err;
>>> - }
>>> + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
>>> + if (!epc->windows)
>>> + return -ENOMEM;
>>>
>>> - bitmap = kzalloc(bitmap_size, GFP_KERNEL);
>>> - if (!bitmap) {
>>> - ret = -ENOMEM;
>>> - goto err_mem;
>>> - }
>>> + for (i = 0; i < num_windows; i++) {
>>> + page_size = windows[i].page_size;
>>> + if (page_size < PAGE_SIZE)
>>> + page_size = PAGE_SIZE;
>>> + page_shift = ilog2(page_size);
>>> + pages = windows[i].size >> page_shift;
>>> + bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
>>>
>>> - mem->bitmap = bitmap;
>>> - mem->phys_base = phys_base;
>>> - mem->page_size = page_size;
>>> - mem->pages = pages;
>>> - mem->size = size;
>>> - mutex_init(&mem->lock);
>>> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
>>> + if (!mem) {
>>> + ret = -ENOMEM;
>>> + i--;
>>> + goto err_mem;
>>> + }
>>>
>>> - epc->mem = mem;
>>> + bitmap = kzalloc(bitmap_size, GFP_KERNEL);
>>> + if (!bitmap) {
>>> + ret = -ENOMEM;
>>> + kfree(mem);
>>> + i--;
>>> + goto err_mem;
>>> + }
>>> +
>>> + mem->window.phys_base = windows[i].phys_base;
>>> + mem->window.size = windows[i].size;
>>> + mem->window.page_size = page_size;
>>> + mem->bitmap = bitmap;
>>> + mem->pages = pages;
>>> + mutex_init(&mem->lock);
>>> + epc->windows[i] = mem;
>>> + }
>>> +
>>> + epc->mem = epc->windows[0];
>>
>> "mem" member of EPC looks unnecessary since that value is available at
>> epc->windows[0].
> This was suggested by Shimoda-san, as most of the current controller
> drivers support single region this pointer would be easier to access
> the region instead of adding #define EPC_DEFAULT_WINDOW 0 and
> accessing as epc->windows[EPC_DEFAULT_WINDOW];
>
>>> + epc->num_windows = num_windows;
>>>
>>> return 0;
>>>
>>> err_mem:
>>> - kfree(mem);
>>> + for (; i >= 0; i--) {
>>> + mem = epc->windows[i];
>>> + kfree(mem->bitmap);
>>> + kfree(mem);
>>> + }
>>> + kfree(epc->windows);
>>>
>>> -err:
>>> -return ret;
>>> + return ret;
>>> }
>>> -EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
>>> +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init);
>>>
>>> int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
>>> size_t size, size_t page_size)
>>> {
>>> - return __pci_epc_mem_init(epc, base, size, page_size);
>>> + struct pci_epc_mem_window mem_window;
>>> +
>>> + mem_window.phys_base = base;
>>> + mem_window.size = size;
>>> + mem_window.page_size = page_size;
>>> +
>>> + return pci_epc_multi_mem_init(epc, &mem_window, 1);
>>> }
>>> EXPORT_SYMBOL_GPL(pci_epc_mem_init);
>>>
>>> @@ -109,11 +137,22 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_init);
>>> */
>>> void pci_epc_mem_exit(struct pci_epc *epc)
>>> {
>>> - struct pci_epc_mem *mem = epc->mem;
>>> + struct pci_epc_mem *mem;
>>> + int i;
>>>
>>> + if (!epc->num_windows)
>>> + return;
>>> +
>>> + for (i = 0; i < epc->num_windows; i++) {
>>> + mem = epc->windows[i];
>>> + kfree(mem->bitmap);
>>> + kfree(mem);
>>> + }
>>> + kfree(epc->windows);
>>> +
>>> + epc->windows = NULL;
>>> epc->mem = NULL;
>>> - kfree(mem->bitmap);
>>> - kfree(mem);
>>> + epc->num_windows = 0;
>>> }
>>> EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
>>>
>>> @@ -129,31 +168,60 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
>>> void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
>>> phys_addr_t *phys_addr, size_t size)
>>> {
>>> - int pageno;
>>> void __iomem *virt_addr = NULL;
>>> - struct pci_epc_mem *mem = epc->mem;
>>> - unsigned int page_shift = ilog2(mem->page_size);
>>> + struct pci_epc_mem *mem;
>>> + unsigned int page_shift;
>>> + size_t align_size;
>>> + int pageno;
>>> int order;
>>> + int i;
>>>
>>> - size = ALIGN(size, mem->page_size);
>>> - order = pci_epc_mem_get_order(mem, size);
>>> -
>>> - mutex_lock(&mem->lock);
>>> - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
>>> - if (pageno < 0)
>>> - goto ret;
>>> + for (i = 0; i < epc->num_windows; i++) {
>>> + mem = epc->windows[i];
>>> + mutex_lock(&mem->lock);
>>> + align_size = ALIGN(size, mem->window.page_size);
>>> + order = pci_epc_mem_get_order(mem, align_size);
>>>
>>> - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
>>> - virt_addr = ioremap(*phys_addr, size);
>>> - if (!virt_addr)
>>> - bitmap_release_region(mem->bitmap, pageno, order);
>>> + pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
>>> + order);
>>> + if (pageno >= 0) {
>>> + page_shift = ilog2(mem->window.page_size);
>>> + *phys_addr = mem->window.phys_base +
>>> + ((phys_addr_t)pageno << page_shift);
>>> + virt_addr = ioremap(*phys_addr, align_size);
>>> + if (!virt_addr) {
>>> + bitmap_release_region(mem->bitmap,
>>> + pageno, order);
>>> + mutex_unlock(&mem->lock);
>>> + continue;
>>> + }
>>> + mutex_unlock(&mem->lock);
>>> + return virt_addr;
>>> + }
>>> + mutex_unlock(&mem->lock);
>>> + }
>>>
>>> -ret:
>>> - mutex_unlock(&mem->lock);
>>> return virt_addr;
>>> }
>>> EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
>>>
>>> +struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
>>> + phys_addr_t phys_addr)
>>> +{
>>> + struct pci_epc_mem *mem;
>>> + int i;
>>> +
>>> + for (i = 0; i < epc->num_windows; i++) {
>>> + mem = epc->windows[i];
>>> +
>>> + if (phys_addr >= mem->window.phys_base &&
>>> + phys_addr < (mem->window.phys_base + mem->window.size))
>>> + return mem;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> /**
>>> * pci_epc_mem_free_addr() - free the allocated memory address
>>> * @epc: the EPC device on which memory was allocated
>>> @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
>>> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
>>> void __iomem *virt_addr, size_t size)
>>> {
>>> + struct pci_epc_mem *mem;
>>> + unsigned int page_shift;
>>> + size_t page_size;
>>> int pageno;
>>> - struct pci_epc_mem *mem = epc->mem;
>>> - unsigned int page_shift = ilog2(mem->page_size);
>>> int order;
>>>
>>> + mem = pci_epc_get_matching_window(epc, phys_addr);
>>> + if (!mem) {
>>> + pr_err("failed to get matching window\n");
>>> + return;
>>> + }
>>> +
>>> + page_size = mem->window.page_size;
>>> + page_shift = ilog2(page_size);
>>> iounmap(virt_addr);
>>> - pageno = (phys_addr - mem->phys_base) >> page_shift;
>>> - size = ALIGN(size, mem->page_size);
>>> + pageno = (phys_addr - mem->window.phys_base) >> page_shift;
>>> + size = ALIGN(size, page_size);
>>> order = pci_epc_mem_get_order(mem, size);
>>> mutex_lock(&mem->lock);
>>> bitmap_release_region(mem->bitmap, pageno, order);
>>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>>> index 5bc1de65849e..cc66bec8be90 100644
>>> --- a/include/linux/pci-epc.h
>>> +++ b/include/linux/pci-epc.h
>>> @@ -65,20 +65,28 @@ struct pci_epc_ops {
>>> struct module *owner;
>>> };
>>>
>>> +/**
>>> + * struct pci_epc_mem_window - address window of the endpoint controller
>>> + * @phys_base: physical base address of the PCI address window
>>> + * @size: the size of the PCI address window
>>> + * @page_size: size of each page
>>> + */
>>> +struct pci_epc_mem_window {
>>> + phys_addr_t phys_base;
>>> + size_t size;
>>> + size_t page_size;
>>> +};
>>> +
>>> /**
>>> * struct pci_epc_mem - address space of the endpoint controller
>>> - * @phys_base: physical base address of the PCI address space
>>> - * @size: the size of the PCI address space
>>> + * @window: address window of the endpoint controller
>>> * @bitmap: bitmap to manage the PCI address space
>>> * @pages: number of bits representing the address region
>>> - * @page_size: size of each page
>>> * @lock: mutex to protect bitmap
>>> */
>>> struct pci_epc_mem {
>>> - phys_addr_t phys_base;
>>> - size_t size;
>>> + struct pci_epc_mem_window window;
>>
>> Don't see any additional value in moving phys_base, size, page_size to a new
>> structure and again including it here.
>>
> Controllers supporting multiple windows create a list of supported
> regions (struct pci_epc_mem_window ) and pass a pointer to
> pci_epc_multi_mem_init(), hence this split.

Okay, thanks for clarifying.

Regards
Kishon

2020-04-30 08:46:55

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi Kishon,

On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar
<[email protected]> wrote:
>
> Hi All,
>
> This patch series adds support for endpoint driver for R-Car PCIe controller on
> R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
> supported by the controller for mapping PCI address locally.
>
> Note:
> The cadence/rockchip/designware endpoint drivers are build tested only.
>
> Changes for v9 (Re-spun this series as there were minimal changes requested):
> * Rebased patches on top of v5.7.rc1
> * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
> * Added a check for max_functions read from DT to restrict with
> RCAR_EPC_MAX_FUNCTIONS
> * Replaced MSICAP0_MMENUM with MSICAP0_MMESE
> * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr()
> * Fixed looping for number windows in pci_epc_mem_exit()
> * Set maximum to 1 for max-functions in DT binding (I have restored the acks
> from Rob and Shimoda-san)
> * Sorted the entry in MAINTAINERS
>
> Changes for v8:
> * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h
> * Fixed typo in commit message for patch 2/8
> * Reworded commit message for patch 5/8 as suggested by Bjorn
> * Split up patch to add pci_epc_mem_init() interface to add page_size argument
> as suggested by Bjorn.
>
> Changes for v7:
> * Fixed review comments pointed by Shimoda-san
> 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed
> the example as its built with #{address,size}-cells = <1>. I have still
> restored the Ack from Rob and Shimoda-san with these changes.
> 2] Split up the patches so that they can be picked up by respective subsystem
> patches 1/4-9/11 are now part of this series.
> 3] Dropped altering a comment in pci-epc.h
> 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size
> variable doesn't get overwritten in the loop.
> 5] Replaced i-=1 with i--
> 6] Replaced rcar with R-Car in patch subject and description.
> 7] Set MACCTLR in init() callback
>
> Changes for v6:
> 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/
> scm/linux/kernel/git/lpieralisi/pci.git/
> 2] Fixed review comments from Shimoda-san
> a] Made sure defconfig changes were in separate patch
> b] Created rcar_pcie_host/rcar_pcie_ep structures
> c] Added pci-id for R8A774C0
> d] Added entry in MAINTAINERS for dt-binding
> e] Dropped unnecessary braces
> 3] Added support for msi.
>
> Changes for v5:
> 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/
> linux/kernel/git/helgaas/pci.git
> 2] Fixed review comments reported by Kishon while fetching the matching
> window in function pci_epc_get_matching_window()
> 3] Fixed review comments reported by Bjorn
> a] Split patch up first patch so that its easier to review and incremental
> b] Fixed typos
> 4] Included Reviewed tag from Rob for the dt-binding patch
> 5] Fixed issue reported by Nathan for assigning variable to itself
>
> Changes for v4:
> 1] Fixed dtb_check error reported by Rob
> 2] Fixed review comments reported by Kishon
> a] Dropped pci_epc_find_best_fit_window()
> b] Fixed initializing mem ptr in __pci_epc_mem_init()
> c] Dropped map_size from pci_epc_mem_window structure
>
> Changes for v3:
> 1] Fixed review comments from Bjorn and Kishon.
> 3] Converted to DT schema
>
> Changes for v2:
> 1] Fixed review comments from Biju for dt-bindings to include an example
> for a tested platform.
> 2] Fixed review comments from Kishon to extend the features of outbound
> regions in epf framework.
> 3] Added support to parse outbound-ranges in OF.
>
> Lad Prabhakar (8):
> PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c
> PCI: rcar: Move shareable code to a common file
> PCI: rcar: Fix calculating mask for PCIEPAMR register
> PCI: endpoint: Pass page size as argument to pci_epc_mem_init()
> PCI: endpoint: Add support to handle multiple base for mapping
> outbound memory
Could you please do the needy for the above two patches, so that this
can be picked up by Lorenzo.

Cheers,
--Prabhakar

> dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint
> controller
> PCI: rcar: Add endpoint mode support
> MAINTAINERS: Add file patterns for rcar PCI device tree bindings
>
> .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++
> MAINTAINERS | 1 +
> drivers/pci/controller/Kconfig | 18 +
> drivers/pci/controller/Makefile | 3 +-
> .../pci/controller/cadence/pcie-cadence-ep.c | 2 +-
> .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++
> drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++
> drivers/pci/controller/pcie-rcar.c | 1206 +----------------
> drivers/pci/controller/pcie-rcar.h | 140 ++
> drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
> drivers/pci/endpoint/pci-epc-mem.c | 204 ++-
> include/linux/pci-epc.h | 38 +-
> 13 files changed, 2078 insertions(+), 1251 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
> create mode 100644 drivers/pci/controller/pcie-rcar-ep.c
> create mode 100644 drivers/pci/controller/pcie-rcar-host.c
> create mode 100644 drivers/pci/controller/pcie-rcar.h
>
> --
> 2.17.1
>

2020-05-05 09:46:28

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote:
> Hi Kishon,
>
> On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar
> <[email protected]> wrote:
> >
> > Hi All,
> >
> > This patch series adds support for endpoint driver for R-Car PCIe controller on
> > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
> > supported by the controller for mapping PCI address locally.
> >
> > Note:
> > The cadence/rockchip/designware endpoint drivers are build tested only.
> >
> > Changes for v9 (Re-spun this series as there were minimal changes requested):
> > * Rebased patches on top of v5.7.rc1
> > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
> > * Added a check for max_functions read from DT to restrict with
> > RCAR_EPC_MAX_FUNCTIONS
> > * Replaced MSICAP0_MMENUM with MSICAP0_MMESE
> > * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr()
> > * Fixed looping for number windows in pci_epc_mem_exit()
> > * Set maximum to 1 for max-functions in DT binding (I have restored the acks
> > from Rob and Shimoda-san)
> > * Sorted the entry in MAINTAINERS
> >
> > Changes for v8:
> > * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h
> > * Fixed typo in commit message for patch 2/8
> > * Reworded commit message for patch 5/8 as suggested by Bjorn
> > * Split up patch to add pci_epc_mem_init() interface to add page_size argument
> > as suggested by Bjorn.
> >
> > Changes for v7:
> > * Fixed review comments pointed by Shimoda-san
> > 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed
> > the example as its built with #{address,size}-cells = <1>. I have still
> > restored the Ack from Rob and Shimoda-san with these changes.
> > 2] Split up the patches so that they can be picked up by respective subsystem
> > patches 1/4-9/11 are now part of this series.
> > 3] Dropped altering a comment in pci-epc.h
> > 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size
> > variable doesn't get overwritten in the loop.
> > 5] Replaced i-=1 with i--
> > 6] Replaced rcar with R-Car in patch subject and description.
> > 7] Set MACCTLR in init() callback
> >
> > Changes for v6:
> > 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/
> > scm/linux/kernel/git/lpieralisi/pci.git/
> > 2] Fixed review comments from Shimoda-san
> > a] Made sure defconfig changes were in separate patch
> > b] Created rcar_pcie_host/rcar_pcie_ep structures
> > c] Added pci-id for R8A774C0
> > d] Added entry in MAINTAINERS for dt-binding
> > e] Dropped unnecessary braces
> > 3] Added support for msi.
> >
> > Changes for v5:
> > 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/
> > linux/kernel/git/helgaas/pci.git
> > 2] Fixed review comments reported by Kishon while fetching the matching
> > window in function pci_epc_get_matching_window()
> > 3] Fixed review comments reported by Bjorn
> > a] Split patch up first patch so that its easier to review and incremental
> > b] Fixed typos
> > 4] Included Reviewed tag from Rob for the dt-binding patch
> > 5] Fixed issue reported by Nathan for assigning variable to itself
> >
> > Changes for v4:
> > 1] Fixed dtb_check error reported by Rob
> > 2] Fixed review comments reported by Kishon
> > a] Dropped pci_epc_find_best_fit_window()
> > b] Fixed initializing mem ptr in __pci_epc_mem_init()
> > c] Dropped map_size from pci_epc_mem_window structure
> >
> > Changes for v3:
> > 1] Fixed review comments from Bjorn and Kishon.
> > 3] Converted to DT schema
> >
> > Changes for v2:
> > 1] Fixed review comments from Biju for dt-bindings to include an example
> > for a tested platform.
> > 2] Fixed review comments from Kishon to extend the features of outbound
> > regions in epf framework.
> > 3] Added support to parse outbound-ranges in OF.
> >
> > Lad Prabhakar (8):
> > PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c
> > PCI: rcar: Move shareable code to a common file
> > PCI: rcar: Fix calculating mask for PCIEPAMR register
> > PCI: endpoint: Pass page size as argument to pci_epc_mem_init()
> > PCI: endpoint: Add support to handle multiple base for mapping
> > outbound memory
> Could you please do the needy for the above two patches, so that this
> can be picked up by Lorenzo.

Yes please. I would kindly ask you to rebase it on top of my
pci/rcar branch - with Kishon's ACK when provided.

Thanks,
Lorenzo

> Cheers,
> --Prabhakar
>
> > dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint
> > controller
> > PCI: rcar: Add endpoint mode support
> > MAINTAINERS: Add file patterns for rcar PCI device tree bindings
> >
> > .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++
> > MAINTAINERS | 1 +
> > drivers/pci/controller/Kconfig | 18 +
> > drivers/pci/controller/Makefile | 3 +-
> > .../pci/controller/cadence/pcie-cadence-ep.c | 2 +-
> > .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> > drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++
> > drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++
> > drivers/pci/controller/pcie-rcar.c | 1206 +----------------
> > drivers/pci/controller/pcie-rcar.h | 140 ++
> > drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
> > drivers/pci/endpoint/pci-epc-mem.c | 204 ++-
> > include/linux/pci-epc.h | 38 +-
> > 13 files changed, 2078 insertions(+), 1251 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
> > create mode 100644 drivers/pci/controller/pcie-rcar-ep.c
> > create mode 100644 drivers/pci/controller/pcie-rcar-host.c
> > create mode 100644 drivers/pci/controller/pcie-rcar.h
> >
> > --
> > 2.17.1
> >

2020-05-05 09:49:48

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi Lorenzo,

On Tue, May 5, 2020 at 10:44 AM Lorenzo Pieralisi
<[email protected]> wrote:
>
> On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote:
> > Hi Kishon,
> >
> > On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar
> > <[email protected]> wrote:
> > >
> > > Hi All,
> > >
> > > This patch series adds support for endpoint driver for R-Car PCIe controller on
> > > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
> > > supported by the controller for mapping PCI address locally.
> > >
> > > Note:
> > > The cadence/rockchip/designware endpoint drivers are build tested only.
> > >
> > > Changes for v9 (Re-spun this series as there were minimal changes requested):
> > > * Rebased patches on top of v5.7.rc1
> > > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
> > > * Added a check for max_functions read from DT to restrict with
> > > RCAR_EPC_MAX_FUNCTIONS
> > > * Replaced MSICAP0_MMENUM with MSICAP0_MMESE
> > > * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr()
> > > * Fixed looping for number windows in pci_epc_mem_exit()
> > > * Set maximum to 1 for max-functions in DT binding (I have restored the acks
> > > from Rob and Shimoda-san)
> > > * Sorted the entry in MAINTAINERS
> > >
> > > Changes for v8:
> > > * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h
> > > * Fixed typo in commit message for patch 2/8
> > > * Reworded commit message for patch 5/8 as suggested by Bjorn
> > > * Split up patch to add pci_epc_mem_init() interface to add page_size argument
> > > as suggested by Bjorn.
> > >
> > > Changes for v7:
> > > * Fixed review comments pointed by Shimoda-san
> > > 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed
> > > the example as its built with #{address,size}-cells = <1>. I have still
> > > restored the Ack from Rob and Shimoda-san with these changes.
> > > 2] Split up the patches so that they can be picked up by respective subsystem
> > > patches 1/4-9/11 are now part of this series.
> > > 3] Dropped altering a comment in pci-epc.h
> > > 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size
> > > variable doesn't get overwritten in the loop.
> > > 5] Replaced i-=1 with i--
> > > 6] Replaced rcar with R-Car in patch subject and description.
> > > 7] Set MACCTLR in init() callback
> > >
> > > Changes for v6:
> > > 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/
> > > scm/linux/kernel/git/lpieralisi/pci.git/
> > > 2] Fixed review comments from Shimoda-san
> > > a] Made sure defconfig changes were in separate patch
> > > b] Created rcar_pcie_host/rcar_pcie_ep structures
> > > c] Added pci-id for R8A774C0
> > > d] Added entry in MAINTAINERS for dt-binding
> > > e] Dropped unnecessary braces
> > > 3] Added support for msi.
> > >
> > > Changes for v5:
> > > 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/
> > > linux/kernel/git/helgaas/pci.git
> > > 2] Fixed review comments reported by Kishon while fetching the matching
> > > window in function pci_epc_get_matching_window()
> > > 3] Fixed review comments reported by Bjorn
> > > a] Split patch up first patch so that its easier to review and incremental
> > > b] Fixed typos
> > > 4] Included Reviewed tag from Rob for the dt-binding patch
> > > 5] Fixed issue reported by Nathan for assigning variable to itself
> > >
> > > Changes for v4:
> > > 1] Fixed dtb_check error reported by Rob
> > > 2] Fixed review comments reported by Kishon
> > > a] Dropped pci_epc_find_best_fit_window()
> > > b] Fixed initializing mem ptr in __pci_epc_mem_init()
> > > c] Dropped map_size from pci_epc_mem_window structure
> > >
> > > Changes for v3:
> > > 1] Fixed review comments from Bjorn and Kishon.
> > > 3] Converted to DT schema
> > >
> > > Changes for v2:
> > > 1] Fixed review comments from Biju for dt-bindings to include an example
> > > for a tested platform.
> > > 2] Fixed review comments from Kishon to extend the features of outbound
> > > regions in epf framework.
> > > 3] Added support to parse outbound-ranges in OF.
> > >
> > > Lad Prabhakar (8):
> > > PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c
> > > PCI: rcar: Move shareable code to a common file
> > > PCI: rcar: Fix calculating mask for PCIEPAMR register
> > > PCI: endpoint: Pass page size as argument to pci_epc_mem_init()
> > > PCI: endpoint: Add support to handle multiple base for mapping
> > > outbound memory
> > Could you please do the needy for the above two patches, so that this
> > can be picked up by Lorenzo.
>
> Yes please. I would kindly ask you to rebase it on top of my
> pci/rcar branch - with Kishon's ACK when provided.
>
Sure will do that as soon as I get Kishon's Ack.

Cheers,
--Prabhakar

> Thanks,
> Lorenzo
>
> > Cheers,
> > --Prabhakar
> >
> > > dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint
> > > controller
> > > PCI: rcar: Add endpoint mode support
> > > MAINTAINERS: Add file patterns for rcar PCI device tree bindings
> > >
> > > .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++
> > > MAINTAINERS | 1 +
> > > drivers/pci/controller/Kconfig | 18 +
> > > drivers/pci/controller/Makefile | 3 +-
> > > .../pci/controller/cadence/pcie-cadence-ep.c | 2 +-
> > > .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> > > drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++
> > > drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++
> > > drivers/pci/controller/pcie-rcar.c | 1206 +----------------
> > > drivers/pci/controller/pcie-rcar.h | 140 ++
> > > drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
> > > drivers/pci/endpoint/pci-epc-mem.c | 204 ++-
> > > include/linux/pci-epc.h | 38 +-
> > > 13 files changed, 2078 insertions(+), 1251 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
> > > create mode 100644 drivers/pci/controller/pcie-rcar-ep.c
> > > create mode 100644 drivers/pci/controller/pcie-rcar-host.c
> > > create mode 100644 drivers/pci/controller/pcie-rcar.h
> > >
> > > --
> > > 2.17.1
> > >

2020-05-07 03:25:30

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v9 4/8] PCI: endpoint: Pass page size as argument to pci_epc_mem_init()



On 4/23/2020 11:52 PM, Lad Prabhakar wrote:
> pci_epc_mem_init() internally used page size equal to *PAGE_SIZE* to
> manage the address space so instead just pass the page size as a
> argument to pci_epc_mem_init().
>
> Also make pci_epc_mem_init() as a C function instead of a macro function
> in preparation for adding support for pci-epc-mem core to handle multiple
> windows.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Acked-by: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 +-
> drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
> drivers/pci/endpoint/pci-epc-mem.c | 7 +++++++
> include/linux/pci-epc.h | 5 ++---
> 4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> index 1c173dad67d1..1c15c8352125 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> @@ -450,7 +450,7 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
> epc->max_functions = 1;
>
> ret = pci_epc_mem_init(epc, pcie->mem_res->start,
> - resource_size(pcie->mem_res));
> + resource_size(pcie->mem_res), PAGE_SIZE);
> if (ret < 0) {
> dev_err(dev, "failed to initialize the memory space\n");
> goto err_init;
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index d743b0a48988..5eaf36629a75 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -615,7 +615,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
> rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
>
> err = pci_epc_mem_init(epc, rockchip->mem_res->start,
> - resource_size(rockchip->mem_res));
> + resource_size(rockchip->mem_res), PAGE_SIZE);
> if (err < 0) {
> dev_err(dev, "failed to initialize the memory space\n");
> goto err_uninit_port;
> diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
> index abfac1109a13..cdd1d3821249 100644
> --- a/drivers/pci/endpoint/pci-epc-mem.c
> +++ b/drivers/pci/endpoint/pci-epc-mem.c
> @@ -93,6 +93,13 @@ return ret;
> }
> EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
>
> +int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
> + size_t size, size_t page_size)
> +{
> + return __pci_epc_mem_init(epc, base, size, page_size);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_mem_init);
> +
> /**
> * pci_epc_mem_exit() - cleanup the pci_epc_mem structure
> * @epc: the EPC device that invoked pci_epc_mem_exit
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index e0ed9d01f6e5..5bc1de65849e 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -137,9 +137,6 @@ struct pci_epc_features {
> #define devm_pci_epc_create(dev, ops) \
> __devm_pci_epc_create((dev), (ops), THIS_MODULE)
>
> -#define pci_epc_mem_init(epc, phys_addr, size) \
> - __pci_epc_mem_init((epc), (phys_addr), (size), PAGE_SIZE)
> -
> static inline void epc_set_drvdata(struct pci_epc *epc, void *data)
> {
> dev_set_drvdata(&epc->dev, data);
> @@ -195,6 +192,8 @@ unsigned int pci_epc_get_first_free_bar(const struct pci_epc_features
> struct pci_epc *pci_epc_get(const char *epc_name);
> void pci_epc_put(struct pci_epc *epc);
>
> +int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
> + size_t size, size_t page_size);
> int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_addr, size_t size,
> size_t page_size);
> void pci_epc_mem_exit(struct pci_epc *epc);
>

2020-05-07 03:26:15

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v9 5/8] PCI: endpoint: Add support to handle multiple base for mapping outbound memory



On 4/23/2020 11:52 PM, Lad Prabhakar wrote:
> R-Car PCIe controller has support to map multiple memory regions for
> mapping the outbound memory in local system also the controller limits
> single allocation for each region (that is, once a chunk is used from the
> region it cannot be used to allocate a new one). This features inspires to
> add support for handling multiple memory bases in endpoint framework.
>
> With this patch pci_epc_mem_init() initializes address space for endpoint
> controller which support single window and pci_epc_multi_mem_init()
> initializes multiple windows supported by endpoint controller.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Acked-by: Kishon Vijay Abraham I <[email protected]>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------
> include/linux/pci-epc.h | 33 ++-
> 3 files changed, 170 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1cdcbd102ce8..a78902cbf2f0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -412,11 +412,11 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> reg = ep->msi_cap + PCI_MSI_DATA_32;
> msg_data = dw_pcie_readw_dbi(pci, reg);
> }
> - aligned_offset = msg_addr_lower & (epc->mem->page_size - 1);
> + aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
> msg_addr = ((u64)msg_addr_upper) << 32 |
> (msg_addr_lower & ~aligned_offset);
> ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
> - epc->mem->page_size);
> + epc->mem->window.page_size);
> if (ret)
> return ret;
>
> @@ -459,9 +459,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> return -EPERM;
> }
>
> - aligned_offset = msg_addr & (epc->mem->page_size - 1);
> + aligned_offset = msg_addr & (epc->mem->window.page_size - 1);
> ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
> - epc->mem->page_size);
> + epc->mem->window.page_size);
> if (ret)
> return ret;
>
> @@ -477,7 +477,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> struct pci_epc *epc = ep->epc;
>
> pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> - epc->mem->page_size);
> + epc->mem->window.page_size);
>
> pci_epc_mem_exit(epc);
> }
> @@ -610,15 +610,15 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ret < 0)
> epc->max_functions = 1;
>
> - ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> - ep->page_size);
> + ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> + ep->page_size);
> if (ret < 0) {
> dev_err(dev, "Failed to initialize address space\n");
> return ret;
> }
>
> ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> - epc->mem->page_size);
> + epc->mem->window.page_size);
> if (!ep->msi_mem) {
> dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
> return -ENOMEM;
> diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
> index cdd1d3821249..a3466da2a16f 100644
> --- a/drivers/pci/endpoint/pci-epc-mem.c
> +++ b/drivers/pci/endpoint/pci-epc-mem.c
> @@ -23,7 +23,7 @@
> static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> {
> int order;
> - unsigned int page_shift = ilog2(mem->page_size);
> + unsigned int page_shift = ilog2(mem->window.page_size);
>
> size--;
> size >>= page_shift;
> @@ -36,67 +36,95 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> }
>
> /**
> - * __pci_epc_mem_init() - initialize the pci_epc_mem structure
> + * pci_epc_multi_mem_init() - initialize the pci_epc_mem structure
> * @epc: the EPC device that invoked pci_epc_mem_init
> - * @phys_base: the physical address of the base
> - * @size: the size of the address space
> - * @page_size: size of each page
> + * @windows: pointer to windows supported by the device
> + * @num_windows: number of windows device supports
> *
> * Invoke to initialize the pci_epc_mem structure used by the
> * endpoint functions to allocate mapped PCI address.
> */
> -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
> - size_t page_size)
> +int pci_epc_multi_mem_init(struct pci_epc *epc,
> + struct pci_epc_mem_window *windows,
> + unsigned int num_windows)
> {
> - int ret;
> - struct pci_epc_mem *mem;
> - unsigned long *bitmap;
> + struct pci_epc_mem *mem = NULL;
> + unsigned long *bitmap = NULL;
> unsigned int page_shift;
> - int pages;
> + size_t page_size;
> int bitmap_size;
> + int pages;
> + int ret;
> + int i;
>
> - if (page_size < PAGE_SIZE)
> - page_size = PAGE_SIZE;
> + epc->num_windows = 0;
>
> - page_shift = ilog2(page_size);
> - pages = size >> page_shift;
> - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
> + if (!windows || !num_windows)
> + return -EINVAL;
>
> - mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> - if (!mem) {
> - ret = -ENOMEM;
> - goto err;
> - }
> + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
> + if (!epc->windows)
> + return -ENOMEM;
>
> - bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> - if (!bitmap) {
> - ret = -ENOMEM;
> - goto err_mem;
> - }
> + for (i = 0; i < num_windows; i++) {
> + page_size = windows[i].page_size;
> + if (page_size < PAGE_SIZE)
> + page_size = PAGE_SIZE;
> + page_shift = ilog2(page_size);
> + pages = windows[i].size >> page_shift;
> + bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
>
> - mem->bitmap = bitmap;
> - mem->phys_base = phys_base;
> - mem->page_size = page_size;
> - mem->pages = pages;
> - mem->size = size;
> - mutex_init(&mem->lock);
> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> + if (!mem) {
> + ret = -ENOMEM;
> + i--;
> + goto err_mem;
> + }
>
> - epc->mem = mem;
> + bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + if (!bitmap) {
> + ret = -ENOMEM;
> + kfree(mem);
> + i--;
> + goto err_mem;
> + }
> +
> + mem->window.phys_base = windows[i].phys_base;
> + mem->window.size = windows[i].size;
> + mem->window.page_size = page_size;
> + mem->bitmap = bitmap;
> + mem->pages = pages;
> + mutex_init(&mem->lock);
> + epc->windows[i] = mem;
> + }
> +
> + epc->mem = epc->windows[0];
> + epc->num_windows = num_windows;
>
> return 0;
>
> err_mem:
> - kfree(mem);
> + for (; i >= 0; i--) {
> + mem = epc->windows[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->windows);
>
> -err:
> -return ret;
> + return ret;
> }
> -EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
> +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init);
>
> int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
> size_t size, size_t page_size)
> {
> - return __pci_epc_mem_init(epc, base, size, page_size);
> + struct pci_epc_mem_window mem_window;
> +
> + mem_window.phys_base = base;
> + mem_window.size = size;
> + mem_window.page_size = page_size;
> +
> + return pci_epc_multi_mem_init(epc, &mem_window, 1);
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_init);
>
> @@ -109,11 +137,22 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_init);
> */
> void pci_epc_mem_exit(struct pci_epc *epc)
> {
> - struct pci_epc_mem *mem = epc->mem;
> + struct pci_epc_mem *mem;
> + int i;
>
> + if (!epc->num_windows)
> + return;
> +
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->windows);
> +
> + epc->windows = NULL;
> epc->mem = NULL;
> - kfree(mem->bitmap);
> - kfree(mem);
> + epc->num_windows = 0;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
>
> @@ -129,31 +168,60 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
> void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size)
> {
> - int pageno;
> void __iomem *virt_addr = NULL;
> - struct pci_epc_mem *mem = epc->mem;
> - unsigned int page_shift = ilog2(mem->page_size);
> + struct pci_epc_mem *mem;
> + unsigned int page_shift;
> + size_t align_size;
> + int pageno;
> int order;
> + int i;
>
> - size = ALIGN(size, mem->page_size);
> - order = pci_epc_mem_get_order(mem, size);
> -
> - mutex_lock(&mem->lock);
> - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
> - if (pageno < 0)
> - goto ret;
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> + mutex_lock(&mem->lock);
> + align_size = ALIGN(size, mem->window.page_size);
> + order = pci_epc_mem_get_order(mem, align_size);
>
> - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
> - virt_addr = ioremap(*phys_addr, size);
> - if (!virt_addr)
> - bitmap_release_region(mem->bitmap, pageno, order);
> + pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
> + order);
> + if (pageno >= 0) {
> + page_shift = ilog2(mem->window.page_size);
> + *phys_addr = mem->window.phys_base +
> + ((phys_addr_t)pageno << page_shift);
> + virt_addr = ioremap(*phys_addr, align_size);
> + if (!virt_addr) {
> + bitmap_release_region(mem->bitmap,
> + pageno, order);
> + mutex_unlock(&mem->lock);
> + continue;
> + }
> + mutex_unlock(&mem->lock);
> + return virt_addr;
> + }
> + mutex_unlock(&mem->lock);
> + }
>
> -ret:
> - mutex_unlock(&mem->lock);
> return virt_addr;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
>
> +struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
> + phys_addr_t phys_addr)
> +{
> + struct pci_epc_mem *mem;
> + int i;
> +
> + for (i = 0; i < epc->num_windows; i++) {
> + mem = epc->windows[i];
> +
> + if (phys_addr >= mem->window.phys_base &&
> + phys_addr < (mem->window.phys_base + mem->window.size))
> + return mem;
> + }
> +
> + return NULL;
> +}
> +
> /**
> * pci_epc_mem_free_addr() - free the allocated memory address
> * @epc: the EPC device on which memory was allocated
> @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
> void __iomem *virt_addr, size_t size)
> {
> + struct pci_epc_mem *mem;
> + unsigned int page_shift;
> + size_t page_size;
> int pageno;
> - struct pci_epc_mem *mem = epc->mem;
> - unsigned int page_shift = ilog2(mem->page_size);
> int order;
>
> + mem = pci_epc_get_matching_window(epc, phys_addr);
> + if (!mem) {
> + pr_err("failed to get matching window\n");
> + return;
> + }
> +
> + page_size = mem->window.page_size;
> + page_shift = ilog2(page_size);
> iounmap(virt_addr);
> - pageno = (phys_addr - mem->phys_base) >> page_shift;
> - size = ALIGN(size, mem->page_size);
> + pageno = (phys_addr - mem->window.phys_base) >> page_shift;
> + size = ALIGN(size, page_size);
> order = pci_epc_mem_get_order(mem, size);
> mutex_lock(&mem->lock);
> bitmap_release_region(mem->bitmap, pageno, order);
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 5bc1de65849e..cc66bec8be90 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -65,20 +65,28 @@ struct pci_epc_ops {
> struct module *owner;
> };
>
> +/**
> + * struct pci_epc_mem_window - address window of the endpoint controller
> + * @phys_base: physical base address of the PCI address window
> + * @size: the size of the PCI address window
> + * @page_size: size of each page
> + */
> +struct pci_epc_mem_window {
> + phys_addr_t phys_base;
> + size_t size;
> + size_t page_size;
> +};
> +
> /**
> * struct pci_epc_mem - address space of the endpoint controller
> - * @phys_base: physical base address of the PCI address space
> - * @size: the size of the PCI address space
> + * @window: address window of the endpoint controller
> * @bitmap: bitmap to manage the PCI address space
> * @pages: number of bits representing the address region
> - * @page_size: size of each page
> * @lock: mutex to protect bitmap
> */
> struct pci_epc_mem {
> - phys_addr_t phys_base;
> - size_t size;
> + struct pci_epc_mem_window window;
> unsigned long *bitmap;
> - size_t page_size;
> int pages;
> /* mutex to protect against concurrent access for memory allocation*/
> struct mutex lock;
> @@ -89,7 +97,11 @@ struct pci_epc_mem {
> * @dev: PCI EPC device
> * @pci_epf: list of endpoint functions present in this EPC device
> * @ops: function pointers for performing endpoint operations
> - * @mem: address space of the endpoint controller
> + * @windows: array of address space of the endpoint controller
> + * @mem: first window of the endpoint controller, which corresponds to
> + * default address space of the endpoint controller supporting
> + * single window.
> + * @num_windows: number of windows supported by device
> * @max_functions: max number of functions that can be configured in this EPC
> * @group: configfs group representing the PCI EPC device
> * @lock: mutex to protect pci_epc ops
> @@ -100,7 +112,9 @@ struct pci_epc {
> struct device dev;
> struct list_head pci_epf;
> const struct pci_epc_ops *ops;
> + struct pci_epc_mem **windows;
> struct pci_epc_mem *mem;
> + unsigned int num_windows;
> u8 max_functions;
> struct config_group *group;
> /* mutex to protect against concurrent access of EP controller */
> @@ -194,8 +208,9 @@ void pci_epc_put(struct pci_epc *epc);
>
> int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base,
> size_t size, size_t page_size);
> -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_addr, size_t size,
> - size_t page_size);
> +int pci_epc_multi_mem_init(struct pci_epc *epc,
> + struct pci_epc_mem_window *window,
> + unsigned int num_windows);
> void pci_epc_mem_exit(struct pci_epc *epc);
> void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size);
>

2020-05-07 03:29:40

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi Prabhakar,

On 5/5/2020 3:17 PM, Lad, Prabhakar wrote:
> Hi Lorenzo,
>
> On Tue, May 5, 2020 at 10:44 AM Lorenzo Pieralisi
> <[email protected]> wrote:
>>
>> On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote:
>>> Hi Kishon,
>>>
>>> On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar
>>> <[email protected]> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> This patch series adds support for endpoint driver for R-Car PCIe controller on
>>>> R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
>>>> supported by the controller for mapping PCI address locally.
>>>>
>>>> Note:
>>>> The cadence/rockchip/designware endpoint drivers are build tested only.
>>>>
>>>> Changes for v9 (Re-spun this series as there were minimal changes requested):
>>>> * Rebased patches on top of v5.7.rc1
>>>> * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
>>>> * Added a check for max_functions read from DT to restrict with
>>>> RCAR_EPC_MAX_FUNCTIONS
>>>> * Replaced MSICAP0_MMENUM with MSICAP0_MMESE
>>>> * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr()
>>>> * Fixed looping for number windows in pci_epc_mem_exit()
>>>> * Set maximum to 1 for max-functions in DT binding (I have restored the acks
>>>> from Rob and Shimoda-san)
>>>> * Sorted the entry in MAINTAINERS
>>>>
>>>> Changes for v8:
>>>> * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h
>>>> * Fixed typo in commit message for patch 2/8
>>>> * Reworded commit message for patch 5/8 as suggested by Bjorn
>>>> * Split up patch to add pci_epc_mem_init() interface to add page_size argument
>>>> as suggested by Bjorn.
>>>>
>>>> Changes for v7:
>>>> * Fixed review comments pointed by Shimoda-san
>>>> 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed
>>>> the example as its built with #{address,size}-cells = <1>. I have still
>>>> restored the Ack from Rob and Shimoda-san with these changes.
>>>> 2] Split up the patches so that they can be picked up by respective subsystem
>>>> patches 1/4-9/11 are now part of this series.
>>>> 3] Dropped altering a comment in pci-epc.h
>>>> 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size
>>>> variable doesn't get overwritten in the loop.
>>>> 5] Replaced i-=1 with i--
>>>> 6] Replaced rcar with R-Car in patch subject and description.
>>>> 7] Set MACCTLR in init() callback
>>>>
>>>> Changes for v6:
>>>> 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/
>>>> scm/linux/kernel/git/lpieralisi/pci.git/
>>>> 2] Fixed review comments from Shimoda-san
>>>> a] Made sure defconfig changes were in separate patch
>>>> b] Created rcar_pcie_host/rcar_pcie_ep structures
>>>> c] Added pci-id for R8A774C0
>>>> d] Added entry in MAINTAINERS for dt-binding
>>>> e] Dropped unnecessary braces
>>>> 3] Added support for msi.
>>>>
>>>> Changes for v5:
>>>> 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/
>>>> linux/kernel/git/helgaas/pci.git
>>>> 2] Fixed review comments reported by Kishon while fetching the matching
>>>> window in function pci_epc_get_matching_window()
>>>> 3] Fixed review comments reported by Bjorn
>>>> a] Split patch up first patch so that its easier to review and incremental
>>>> b] Fixed typos
>>>> 4] Included Reviewed tag from Rob for the dt-binding patch
>>>> 5] Fixed issue reported by Nathan for assigning variable to itself
>>>>
>>>> Changes for v4:
>>>> 1] Fixed dtb_check error reported by Rob
>>>> 2] Fixed review comments reported by Kishon
>>>> a] Dropped pci_epc_find_best_fit_window()
>>>> b] Fixed initializing mem ptr in __pci_epc_mem_init()
>>>> c] Dropped map_size from pci_epc_mem_window structure
>>>>
>>>> Changes for v3:
>>>> 1] Fixed review comments from Bjorn and Kishon.
>>>> 3] Converted to DT schema
>>>>
>>>> Changes for v2:
>>>> 1] Fixed review comments from Biju for dt-bindings to include an example
>>>> for a tested platform.
>>>> 2] Fixed review comments from Kishon to extend the features of outbound
>>>> regions in epf framework.
>>>> 3] Added support to parse outbound-ranges in OF.
>>>>
>>>> Lad Prabhakar (8):
>>>> PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c
>>>> PCI: rcar: Move shareable code to a common file
>>>> PCI: rcar: Fix calculating mask for PCIEPAMR register
>>>> PCI: endpoint: Pass page size as argument to pci_epc_mem_init()
>>>> PCI: endpoint: Add support to handle multiple base for mapping
>>>> outbound memory
>>> Could you please do the needy for the above two patches, so that this
>>> can be picked up by Lorenzo.
>>
>> Yes please. I would kindly ask you to rebase it on top of my
>> pci/rcar branch - with Kishon's ACK when provided.
>>
> Sure will do that as soon as I get Kishon's Ack.

I've given my Acked by on the two endpoint core patches. I've also tested my
endpoint series [1] after applying this series.

Thanks
Kishon

[1] -> http://lore.kernel.org/r/[email protected]
>
> Cheers,
> --Prabhakar
>
>> Thanks,
>> Lorenzo
>>
>>> Cheers,
>>> --Prabhakar
>>>
>>>> dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint
>>>> controller
>>>> PCI: rcar: Add endpoint mode support
>>>> MAINTAINERS: Add file patterns for rcar PCI device tree bindings
>>>>
>>>> .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++
>>>> MAINTAINERS | 1 +
>>>> drivers/pci/controller/Kconfig | 18 +
>>>> drivers/pci/controller/Makefile | 3 +-
>>>> .../pci/controller/cadence/pcie-cadence-ep.c | 2 +-
>>>> .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
>>>> drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++
>>>> drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++
>>>> drivers/pci/controller/pcie-rcar.c | 1206 +----------------
>>>> drivers/pci/controller/pcie-rcar.h | 140 ++
>>>> drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
>>>> drivers/pci/endpoint/pci-epc-mem.c | 204 ++-
>>>> include/linux/pci-epc.h | 38 +-
>>>> 13 files changed, 2078 insertions(+), 1251 deletions(-)
>>>> create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
>>>> create mode 100644 drivers/pci/controller/pcie-rcar-ep.c
>>>> create mode 100644 drivers/pci/controller/pcie-rcar-host.c
>>>> create mode 100644 drivers/pci/controller/pcie-rcar.h
>>>>
>>>> --
>>>> 2.17.1
>>>>

2020-05-07 07:36:55

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi Kishon,

On Thu, May 7, 2020 at 4:13 AM Kishon Vijay Abraham I <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On 5/5/2020 3:17 PM, Lad, Prabhakar wrote:
> > Hi Lorenzo,
> >
> > On Tue, May 5, 2020 at 10:44 AM Lorenzo Pieralisi
> > <[email protected]> wrote:
> >>
> >> On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote:
> >>> Hi Kishon,
> >>>
> >>> On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar
> >>> <[email protected]> wrote:
> >>>>
> >>>> Hi All,
> >>>>
> >>>> This patch series adds support for endpoint driver for R-Car PCIe controller on
> >>>> R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
> >>>> supported by the controller for mapping PCI address locally.
> >>>>
> >>>> Note:
> >>>> The cadence/rockchip/designware endpoint drivers are build tested only.
> >>>>
> >>>> Changes for v9 (Re-spun this series as there were minimal changes requested):
> >>>> * Rebased patches on top of v5.7.rc1
> >>>> * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
> >>>> * Added a check for max_functions read from DT to restrict with
> >>>> RCAR_EPC_MAX_FUNCTIONS
> >>>> * Replaced MSICAP0_MMENUM with MSICAP0_MMESE
> >>>> * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr()
> >>>> * Fixed looping for number windows in pci_epc_mem_exit()
> >>>> * Set maximum to 1 for max-functions in DT binding (I have restored the acks
> >>>> from Rob and Shimoda-san)
> >>>> * Sorted the entry in MAINTAINERS
> >>>>
> >>>> Changes for v8:
> >>>> * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h
> >>>> * Fixed typo in commit message for patch 2/8
> >>>> * Reworded commit message for patch 5/8 as suggested by Bjorn
> >>>> * Split up patch to add pci_epc_mem_init() interface to add page_size argument
> >>>> as suggested by Bjorn.
> >>>>
> >>>> Changes for v7:
> >>>> * Fixed review comments pointed by Shimoda-san
> >>>> 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed
> >>>> the example as its built with #{address,size}-cells = <1>. I have still
> >>>> restored the Ack from Rob and Shimoda-san with these changes.
> >>>> 2] Split up the patches so that they can be picked up by respective subsystem
> >>>> patches 1/4-9/11 are now part of this series.
> >>>> 3] Dropped altering a comment in pci-epc.h
> >>>> 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size
> >>>> variable doesn't get overwritten in the loop.
> >>>> 5] Replaced i-=1 with i--
> >>>> 6] Replaced rcar with R-Car in patch subject and description.
> >>>> 7] Set MACCTLR in init() callback
> >>>>
> >>>> Changes for v6:
> >>>> 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/
> >>>> scm/linux/kernel/git/lpieralisi/pci.git/
> >>>> 2] Fixed review comments from Shimoda-san
> >>>> a] Made sure defconfig changes were in separate patch
> >>>> b] Created rcar_pcie_host/rcar_pcie_ep structures
> >>>> c] Added pci-id for R8A774C0
> >>>> d] Added entry in MAINTAINERS for dt-binding
> >>>> e] Dropped unnecessary braces
> >>>> 3] Added support for msi.
> >>>>
> >>>> Changes for v5:
> >>>> 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/
> >>>> linux/kernel/git/helgaas/pci.git
> >>>> 2] Fixed review comments reported by Kishon while fetching the matching
> >>>> window in function pci_epc_get_matching_window()
> >>>> 3] Fixed review comments reported by Bjorn
> >>>> a] Split patch up first patch so that its easier to review and incremental
> >>>> b] Fixed typos
> >>>> 4] Included Reviewed tag from Rob for the dt-binding patch
> >>>> 5] Fixed issue reported by Nathan for assigning variable to itself
> >>>>
> >>>> Changes for v4:
> >>>> 1] Fixed dtb_check error reported by Rob
> >>>> 2] Fixed review comments reported by Kishon
> >>>> a] Dropped pci_epc_find_best_fit_window()
> >>>> b] Fixed initializing mem ptr in __pci_epc_mem_init()
> >>>> c] Dropped map_size from pci_epc_mem_window structure
> >>>>
> >>>> Changes for v3:
> >>>> 1] Fixed review comments from Bjorn and Kishon.
> >>>> 3] Converted to DT schema
> >>>>
> >>>> Changes for v2:
> >>>> 1] Fixed review comments from Biju for dt-bindings to include an example
> >>>> for a tested platform.
> >>>> 2] Fixed review comments from Kishon to extend the features of outbound
> >>>> regions in epf framework.
> >>>> 3] Added support to parse outbound-ranges in OF.
> >>>>
> >>>> Lad Prabhakar (8):
> >>>> PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c
> >>>> PCI: rcar: Move shareable code to a common file
> >>>> PCI: rcar: Fix calculating mask for PCIEPAMR register
> >>>> PCI: endpoint: Pass page size as argument to pci_epc_mem_init()
> >>>> PCI: endpoint: Add support to handle multiple base for mapping
> >>>> outbound memory
> >>> Could you please do the needy for the above two patches, so that this
> >>> can be picked up by Lorenzo.
> >>
> >> Yes please. I would kindly ask you to rebase it on top of my
> >> pci/rcar branch - with Kishon's ACK when provided.
> >>
> > Sure will do that as soon as I get Kishon's Ack.
>
> I've given my Acked by on the two endpoint core patches. I've also tested my
> endpoint series [1] after applying this series.
>
Thank you for testing and the Ack's.

Cheers,
--Prabhakar

> Thanks
> Kishon
>
> [1] -> http://lore.kernel.org/r/[email protected]
> >
> > Cheers,
> > --Prabhakar
> >
> >> Thanks,
> >> Lorenzo
> >>
> >>> Cheers,
> >>> --Prabhakar
> >>>
> >>>> dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint
> >>>> controller
> >>>> PCI: rcar: Add endpoint mode support
> >>>> MAINTAINERS: Add file patterns for rcar PCI device tree bindings
> >>>>
> >>>> .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++
> >>>> MAINTAINERS | 1 +
> >>>> drivers/pci/controller/Kconfig | 18 +
> >>>> drivers/pci/controller/Makefile | 3 +-
> >>>> .../pci/controller/cadence/pcie-cadence-ep.c | 2 +-
> >>>> .../pci/controller/dwc/pcie-designware-ep.c | 16 +-
> >>>> drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++
> >>>> drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++
> >>>> drivers/pci/controller/pcie-rcar.c | 1206 +----------------
> >>>> drivers/pci/controller/pcie-rcar.h | 140 ++
> >>>> drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
> >>>> drivers/pci/endpoint/pci-epc-mem.c | 204 ++-
> >>>> include/linux/pci-epc.h | 38 +-
> >>>> 13 files changed, 2078 insertions(+), 1251 deletions(-)
> >>>> create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml
> >>>> create mode 100644 drivers/pci/controller/pcie-rcar-ep.c
> >>>> create mode 100644 drivers/pci/controller/pcie-rcar-host.c
> >>>> create mode 100644 drivers/pci/controller/pcie-rcar.h
> >>>>
> >>>> --
> >>>> 2.17.1
> >>>>

2020-05-07 17:49:00

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi!


> R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
> supported by the controller for mapping PCI address locally.
>
> Note:
> The cadence/rockchip/designware endpoint drivers are build tested only.
>
> Changes for v9 (Re-spun this series as there were minimal changes requested):
...
> * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()

Are you sure that is good idea? You are requesting 1ms sleep time with 1us tolerance,
I dont believe common systems can do that.

Best regards,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2020-05-07 17:56:20

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v9 0/8] Add endpoint driver for R-Car PCIe controller

Hi Pavel,

On Thu, May 7, 2020 at 6:44 PM Pavel Machek <[email protected]> wrote:
>
> Hi!
>
>
> > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows
> > supported by the controller for mapping PCI address locally.
> >
> > Note:
> > The cadence/rockchip/designware endpoint drivers are build tested only.
> >
> > Changes for v9 (Re-spun this series as there were minimal changes requested):
> ...
> > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx()
>
> Are you sure that is good idea? You are requesting 1ms sleep time with 1us tolerance,
> I dont believe common systems can do that.
>
Agreed the systems cannot do that, but the main reason of replacing
mdelay(1) with usleep_range(1000, 1001) was since pci_epc_raise_irq()
calls mutex_lock() and then this function rcar_pcie_ep_assert_intx(),
so we can assume this function also can sleep. And, according to
Documentation/timers/timers-howto.rst, we should use usleep_range()
instead of mdelay().

Cheers,
--Prabhakar



> Best regards,
> Pavel
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html