2014-02-05 06:51:51

by Magnus Damm

[permalink] [raw]
Subject: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

PCI: rcar: Driver model and physical address space update

[PATCH 01/04] PCI: rcar: Register each instance independently
[PATCH 02/04] PCI: rcar: Break out window size handling
[PATCH 03/04] PCI: rcar: Add DMABOUNCE support
[PATCH 04/04] PCI: rcar: Enable BOUNCE in case of HIGHMEM

These patches update the pci-rcar-gen2.c driver in various ways
including cleanups for driver model interface (1/4), readability
update (2/4) and also bounce buffer support (3/4, 4/4). Basically
the first two are just cleanups and the rest are fixes.

As it is today without these patches the system memory start address
is hard coded at 0x4000000 and the window is fixed at 1GiB. So we
have board specific code hidden in the driver which is good to avoid.

With these hard coded board specific constants there are some error cases
that are not handled, in particular the issue that only maximum 1GiB of
physical address space can be used for bus mastering with a single window.
The common case of using ARM CONFIG_VMSPLIT_3G results in no visible issues
as long as CONFIG_BOUNCE is used, but other CONFIG_VMSPLIT settings will
break due to the hard coded 1GiB window not being enough. It has been
verified that reducing the window size to 256MB makes the driver behave
the same with VMSPLIT_3G as 1GiB window size and other VMSPLIT settings.

To handle the maximum 1GiB physical address space limitation two types
of bounce buffers are added. The ARM specific DMABOUNCE code is in 3/4
hooked up to a chunk of local memory that is also handed of as coherent
memory to the pci devices hanging off the PCI bridge. The driver makes
sure to set the window so the local memory is always included. When the
PCI devices are operating and in case memory is used outside the window
then the DMABOUNCE buffers kicks in. This makes the driver support all
kinds of VMSPLIT settings and window sizes. The BOUNCE code in 4/4 is
selected to make sure bounce buffers are used for HIGHMEM.

With these patches this driver can be used with or without CMA and
with or without DMA zone. Basically the system wide memory setup is
left to the user. If a DMA zone is used then the PCI window will
be setup to cover that. Same thing with CMA.

Tested with USB storage using LPAE and various VMSPLIT settings together
with renesas git tag renesas-devel-v3.14-rc1-20140204 from kernel.org

Signed-off-by: Magnus Damm <[email protected]>
---

Written against renesas.git tag renesas-devel-v3.14-rc1-20140204

drivers/pci/host/Kconfig | 3
drivers/pci/host/pci-rcar-gen2.c | 367 +++++++++++++++++++++++++++++++-------
2 files changed, 306 insertions(+), 64 deletions(-)


2014-02-05 06:52:24

by Magnus Damm

[permalink] [raw]
Subject: [PATCH 01/04] PCI: rcar: Register each instance independently

From: Magnus Damm <[email protected]>

Convert the code to allow per-device probe() like other device
drivers. This also delays driver registration due to change from
subsys_initcall() to regular module_platform_driver().

Signed-off-by: Magnus Damm <[email protected]>
---

drivers/pci/host/pci-rcar-gen2.c | 74 ++++++++------------------------------
1 file changed, 16 insertions(+), 58 deletions(-)

--- 0001/drivers/pci/host/pci-rcar-gen2.c
+++ work/drivers/pci/host/pci-rcar-gen2.c 2014-02-04 16:38:46.000000000 +0900
@@ -74,9 +74,6 @@

#define RCAR_PCI_UNIT_REV_REG (RCAR_AHBPCI_PCICOM_OFFSET + 0x48)

-/* Number of internal PCI controllers */
-#define RCAR_PCI_NR_CONTROLLERS 3
-
struct rcar_pci_priv {
struct device *dev;
void __iomem *reg;
@@ -228,6 +225,8 @@ static int __init rcar_pci_setup(int nr,
pci_add_resource(&sys->resources, &priv->io_res);
pci_add_resource(&sys->resources, &priv->mem_res);

+ /* Setup bus number based on platform device id */
+ sys->busnr = to_platform_device(priv->dev)->id;
return 1;
}

@@ -236,48 +235,12 @@ static struct pci_ops rcar_pci_ops = {
.write = rcar_pci_write_config,
};

-static struct hw_pci rcar_hw_pci __initdata = {
- .map_irq = rcar_pci_map_irq,
- .ops = &rcar_pci_ops,
- .setup = rcar_pci_setup,
-};
-
-static int rcar_pci_count __initdata;
-
-static int __init rcar_pci_add_controller(struct rcar_pci_priv *priv)
-{
- void **private_data;
- int count;
-
- if (rcar_hw_pci.nr_controllers < rcar_pci_count)
- goto add_priv;
-
- /* (Re)allocate private data pointer array if needed */
- count = rcar_pci_count + RCAR_PCI_NR_CONTROLLERS;
- private_data = kzalloc(count * sizeof(void *), GFP_KERNEL);
- if (!private_data)
- return -ENOMEM;
-
- rcar_pci_count = count;
- if (rcar_hw_pci.private_data) {
- memcpy(private_data, rcar_hw_pci.private_data,
- rcar_hw_pci.nr_controllers * sizeof(void *));
- kfree(rcar_hw_pci.private_data);
- }
-
- rcar_hw_pci.private_data = private_data;
-
-add_priv:
- /* Add private data pointer to the array */
- rcar_hw_pci.private_data[rcar_hw_pci.nr_controllers++] = priv;
- return 0;
-}
-
-static int __init rcar_pci_probe(struct platform_device *pdev)
+static int rcar_pci_probe(struct platform_device *pdev)
{
struct resource *cfg_res, *mem_res;
struct rcar_pci_priv *priv;
void __iomem *reg;
+ struct hw_pci hw;

cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
reg = devm_ioremap_resource(&pdev->dev, cfg_res);
@@ -308,31 +271,26 @@ static int __init rcar_pci_probe(struct
priv->reg = reg;
priv->dev = &pdev->dev;

- return rcar_pci_add_controller(priv);
+ memset(&hw, 0, sizeof(hw));
+ hw.nr_controllers = 1;
+ hw.private_data = (void **)&priv;
+ hw.map_irq = rcar_pci_map_irq;
+ hw.ops = &rcar_pci_ops;
+ hw.setup = rcar_pci_setup;
+ pci_common_init_dev(&pdev->dev, &hw);
+ return 0;
}

static struct platform_driver rcar_pci_driver = {
.driver = {
.name = "pci-rcar-gen2",
+ .owner = THIS_MODULE,
+ .suppress_bind_attrs = true,
},
+ .probe = rcar_pci_probe,
};

-static int __init rcar_pci_init(void)
-{
- int retval;
-
- retval = platform_driver_probe(&rcar_pci_driver, rcar_pci_probe);
- if (!retval)
- pci_common_init(&rcar_hw_pci);
-
- /* Private data pointer array is not needed any more */
- kfree(rcar_hw_pci.private_data);
- rcar_hw_pci.private_data = NULL;
-
- return retval;
-}
-
-subsys_initcall(rcar_pci_init);
+module_platform_driver(rcar_pci_driver);

MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("Renesas R-Car Gen2 internal PCI");

2014-02-05 06:52:29

by Magnus Damm

[permalink] [raw]
Subject: [PATCH 03/04] PCI: rcar: Add DMABOUNCE support

From: Magnus Damm <[email protected]>

Add DMABOUNCE support together with a local memory area for
all PCI devices hanging off this bridge. 4MiB of memory is
set aside for coherent allocations which is shared between
the on-chip OHCI and EHCI devices that are hanging off the
PCI bridge. With this patch the driver will work for all
CONFIG_VMSPLIT settings.

Signed-off-by: Magnus Damm <[email protected]>
---

drivers/pci/host/Kconfig | 2
drivers/pci/host/pci-rcar-gen2.c | 262 +++++++++++++++++++++++++++++++++++++-
2 files changed, 261 insertions(+), 3 deletions(-)

--- 0001/drivers/pci/host/Kconfig
+++ work/drivers/pci/host/Kconfig 2014-02-04 17:23:57.000000000 +0900
@@ -28,6 +28,8 @@ config PCI_TEGRA
config PCI_RCAR_GEN2
bool "Renesas R-Car Gen2 Internal PCI controller"
depends on ARM && (ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST)
+ select GENERIC_ALLOCATOR
+ select DMABOUNCE
help
Say Y here if you want internal PCI support on R-Car Gen2 SoC.
There are 3 internal PCI controllers available with a single
--- 0006/drivers/pci/host/pci-rcar-gen2.c
+++ work/drivers/pci/host/pci-rcar-gen2.c 2014-02-04 17:26:10.000000000 +0900
@@ -15,11 +15,14 @@
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/notifier.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/sizes.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/genalloc.h>

/* AHB-PCI Bridge PCI communication registers */
#define RCAR_AHBPCI_PCICOM_OFFSET 0x800
@@ -83,6 +86,11 @@ struct rcar_pci_priv {
struct resource *cfg_res;
int irq;
unsigned long window_size;
+ u32 window_base;
+ struct notifier_block bus_notifier;
+ struct gen_pool *dma_pool;
+ unsigned long dma_size;
+ dma_addr_t dma_phys;
};

/* PCI configuration space operations */
@@ -217,8 +225,8 @@ static int __init rcar_pci_setup(int nr,
RCAR_PCI_ARBITER_PCIBP_MODE;
iowrite32(val, reg + RCAR_PCI_ARBITER_CTR_REG);

- /* PCI-AHB mapping: 0x40000000 base */
- iowrite32(0x40000000 | RCAR_PCIAHB_PREFETCH16,
+ /* PCI-AHB mapping: dynamic base */
+ iowrite32(priv->window_base | RCAR_PCIAHB_PREFETCH16,
reg + RCAR_PCIAHB_WIN1_CTR_REG);

/* AHB-PCI mapping: OHCI/EHCI registers */
@@ -229,7 +237,7 @@ static int __init rcar_pci_setup(int nr,
iowrite32(RCAR_AHBPCI_WIN1_HOST | RCAR_AHBPCI_WIN_CTR_CFG,
reg + RCAR_AHBPCI_WIN1_CTR_REG);
/* Set PCI-AHB Window1 address */
- iowrite32(0x40000000 | PCI_BASE_ADDRESS_MEM_PREFETCH,
+ iowrite32(priv->window_base | PCI_BASE_ADDRESS_MEM_PREFETCH,
reg + PCI_BASE_ADDRESS_1);
/* Set AHB-PCI bridge PCI communication area address */
val = priv->cfg_res->start + RCAR_AHBPCI_PCICOM_OFFSET;
@@ -258,12 +266,231 @@ static struct pci_ops rcar_pci_ops = {
.write = rcar_pci_write_config,
};

+static struct rcar_pci_priv *rcar_pci_dev_to_priv(struct device *dev)
+{
+ struct pci_sys_data *sys = to_pci_dev(dev)->sysdata;
+
+ return sys->private_data;
+}
+
+static void *rcar_pci_dma_alloc(struct device *dev, size_t size,
+ dma_addr_t *handle, gfp_t gfp,
+ struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return (void *)gen_pool_dma_alloc(p->dma_pool, size, handle);
+}
+
+static void rcar_pci_dma_free(struct device *dev, size_t size, void *cpu_addr,
+ dma_addr_t handle, struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ gen_pool_free(p->dma_pool, (unsigned long)cpu_addr, size);
+}
+
+static struct dma_map_ops rcar_pci_dma_ops_init = {
+ .alloc = rcar_pci_dma_alloc,
+ .free = rcar_pci_dma_free,
+};
+
+static int rcar_pci_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+ void *cpu_addr, dma_addr_t dma_addr, size_t size,
+ struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->mmap(p->dev, vma, cpu_addr,
+ dma_addr, size, attrs);
+}
+
+static int rcar_pci_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
+ void *cpu_addr, dma_addr_t handle,
+ size_t size, struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->get_sgtable(p->dev, sgt, cpu_addr,
+ handle, size, attrs);
+}
+
+static dma_addr_t rcar_pci_dma_map_page(struct device *dev, struct page *page,
+ unsigned long offset, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->map_page(p->dev, page, offset,
+ size, dir, attrs);
+}
+
+static void rcar_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
+ size_t size, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ get_dma_ops(p->dev)->unmap_page(p->dev, dma_handle, size, dir, attrs);
+}
+
+static int rcar_pci_dma_map_sg(struct device *dev, struct scatterlist *sg,
+ int nents, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->map_sg(p->dev, sg, nents, dir, attrs);
+}
+
+static void rcar_pci_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
+ int nents, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ get_dma_ops(p->dev)->unmap_sg(p->dev, sg, nents, dir, attrs);
+}
+
+static void rcar_pci_dma_sync_single_for_cpu(struct device *dev,
+ dma_addr_t dma_handle,
+ size_t size,
+ enum dma_data_direction dir)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ get_dma_ops(p->dev)->sync_single_for_cpu(p->dev, dma_handle,
+ size, dir);
+}
+
+static void rcar_pci_dma_sync_single_for_device(struct device *dev,
+ dma_addr_t dma_handle,
+ size_t size,
+ enum dma_data_direction dir)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ get_dma_ops(p->dev)->sync_single_for_device(p->dev, dma_handle,
+ size, dir);
+}
+
+static void rcar_pci_dma_sync_sg_for_cpu(struct device *dev,
+ struct scatterlist *sg,
+ int nents,
+ enum dma_data_direction dir)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ get_dma_ops(p->dev)->sync_sg_for_cpu(p->dev, sg, nents, dir);
+}
+
+static void rcar_pci_dma_sync_sg_for_device(struct device *dev,
+ struct scatterlist *sg,
+ int nents,
+ enum dma_data_direction dir)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ get_dma_ops(p->dev)->sync_sg_for_device(p->dev, sg, nents, dir);
+}
+
+static int rcar_pci_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->mapping_error(p->dev, dma_addr);
+}
+
+static int rcar_pci_dma_supported(struct device *dev, u64 mask)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->dma_supported(p->dev, mask);
+}
+
+static int rcar_pci_dma_set_dma_mask(struct device *dev, u64 mask)
+{
+ struct rcar_pci_priv *p = rcar_pci_dev_to_priv(dev);
+
+ return get_dma_ops(p->dev)->set_dma_mask(p->dev, mask);
+}
+
+static struct dma_map_ops rcar_pci_dma_ops = {
+ .alloc = rcar_pci_dma_alloc,
+ .free = rcar_pci_dma_free,
+ .mmap = rcar_pci_dma_mmap,
+ .get_sgtable = rcar_pci_dma_get_sgtable,
+ .map_page = rcar_pci_dma_map_page,
+ .unmap_page = rcar_pci_dma_unmap_page,
+ .map_sg = rcar_pci_dma_map_sg,
+ .unmap_sg = rcar_pci_dma_unmap_sg,
+ .sync_single_for_cpu = rcar_pci_dma_sync_single_for_cpu,
+ .sync_single_for_device = rcar_pci_dma_sync_single_for_device,
+ .sync_sg_for_cpu = rcar_pci_dma_sync_sg_for_cpu,
+ .sync_sg_for_device = rcar_pci_dma_sync_sg_for_device,
+ .mapping_error = rcar_pci_dma_mapping_error,
+ .dma_supported = rcar_pci_dma_supported,
+ .set_dma_mask = rcar_pci_dma_set_dma_mask,
+};
+
+static int rcar_pci_bus_notify(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct rcar_pci_priv *priv;
+ struct device *dev = data;
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_sys_data *sys = pci_dev->sysdata;
+
+ priv = container_of(nb, struct rcar_pci_priv, bus_notifier);
+ if (priv != sys->private_data)
+ return 0;
+
+ if (action == BUS_NOTIFY_BIND_DRIVER)
+ set_dma_ops(dev, &rcar_pci_dma_ops);
+
+ return 0;
+}
+
+static void rcar_pci_add_bus(struct pci_bus *bus)
+{
+ struct pci_sys_data *sys = bus->sysdata;
+ struct rcar_pci_priv *priv = sys->private_data;
+
+ bus_register_notifier(&pci_bus_type, &priv->bus_notifier);
+}
+
+static void rcar_pci_remove_bus(struct pci_bus *bus)
+{
+ struct pci_sys_data *sys = bus->sysdata;
+ struct rcar_pci_priv *priv = sys->private_data;
+
+ bus_unregister_notifier(&pci_bus_type, &priv->bus_notifier);
+}
+
+static int rcar_pci_needs_bounce(struct device *dev,
+ dma_addr_t dma_addr, size_t size)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct rcar_pci_priv *priv = platform_get_drvdata(pdev);
+
+ if (dma_addr < priv->window_base)
+ return 1;
+
+ if (dma_addr >= (priv->window_base + priv->window_size))
+ return 1;
+
+ return 0;
+}
+
static int rcar_pci_probe(struct platform_device *pdev)
{
struct resource *cfg_res, *mem_res;
struct rcar_pci_priv *priv;
void __iomem *reg;
struct hw_pci hw;
+ void *dma_virt;
+ int ret;

cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
reg = devm_ioremap_resource(&pdev->dev, cfg_res);
@@ -293,8 +520,35 @@ static int rcar_pci_probe(struct platfor
priv->irq = platform_get_irq(pdev, 0);
priv->reg = reg;
priv->dev = &pdev->dev;
+ platform_set_drvdata(pdev, priv);
+ priv->bus_notifier.notifier_call = rcar_pci_bus_notify;

priv->window_size = SZ_1G;
+ priv->dma_size = SZ_4M;
+
+ /* allocate pool of memory guaranteed to be inside window */
+ priv->dma_pool = devm_gen_pool_create(&pdev->dev, 7, -1);
+ if (!priv->dma_pool)
+ return -ENOMEM;
+
+ dma_virt = dmam_alloc_coherent(&pdev->dev, priv->dma_size,
+ &priv->dma_phys, GFP_KERNEL);
+ if (!dma_virt)
+ return -ENOMEM;
+
+ ret = gen_pool_add_virt(priv->dma_pool, (unsigned long)dma_virt,
+ priv->dma_phys, priv->dma_size, -1);
+ if (ret)
+ return ret;
+
+ /* select window base address based on physical address for memory */
+ priv->window_base = priv->dma_phys & ~(priv->window_size - 1);
+
+ /* setup alloc()/free() to let dmabounce allocate from our pool */
+ set_dma_ops(&pdev->dev, &rcar_pci_dma_ops_init);
+
+ /* shared dmabounce for the PCI bridge */
+ dmabounce_register_dev(&pdev->dev, 2048, 4096, rcar_pci_needs_bounce);

memset(&hw, 0, sizeof(hw));
hw.nr_controllers = 1;
@@ -302,6 +556,8 @@ static int rcar_pci_probe(struct platfor
hw.map_irq = rcar_pci_map_irq;
hw.ops = &rcar_pci_ops;
hw.setup = rcar_pci_setup;
+ hw.add_bus = rcar_pci_add_bus;
+ hw.remove_bus = rcar_pci_remove_bus;
pci_common_init_dev(&pdev->dev, &hw);
return 0;
}

2014-02-05 06:52:27

by Magnus Damm

[permalink] [raw]
Subject: [PATCH 02/04] PCI: rcar: Break out window size handling

From: Magnus Damm <[email protected]>

Break out the hard coded window size code to allow
dynamic setup. The window size is still left at 1GiB
but with this patch changing window size is easy for
testing.

Signed-off-by: Magnus Damm <[email protected]>
---

drivers/pci/host/pci-rcar-gen2.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)

--- 0005/drivers/pci/host/pci-rcar-gen2.c
+++ work/drivers/pci/host/pci-rcar-gen2.c 2014-02-04 17:22:41.000000000 +0900
@@ -18,6 +18,7 @@
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/sizes.h>
#include <linux/slab.h>

/* AHB-PCI Bridge PCI communication registers */
@@ -81,6 +82,7 @@ struct rcar_pci_priv {
struct resource mem_res;
struct resource *cfg_res;
int irq;
+ unsigned long window_size;
};

/* PCI configuration space operations */
@@ -180,10 +182,31 @@ static int __init rcar_pci_setup(int nr,
iowrite32(val, reg + RCAR_USBCTR_REG);
udelay(4);

- /* De-assert reset and set PCIAHB window1 size to 1GB */
+ /* De-assert reset and reset PCIAHB window1 size */
val &= ~(RCAR_USBCTR_PCIAHB_WIN1_MASK | RCAR_USBCTR_PCICLK_MASK |
RCAR_USBCTR_USBH_RST | RCAR_USBCTR_PLL_RST);
- iowrite32(val | RCAR_USBCTR_PCIAHB_WIN1_1G, reg + RCAR_USBCTR_REG);
+
+ /* Setup PCIAHB window1 size */
+ switch (priv->window_size) {
+ case SZ_2G:
+ val |= RCAR_USBCTR_PCIAHB_WIN1_2G;
+ break;
+ case SZ_1G:
+ val |= RCAR_USBCTR_PCIAHB_WIN1_1G;
+ break;
+ case SZ_512M:
+ val |= RCAR_USBCTR_PCIAHB_WIN1_512M;
+ break;
+ default:
+ pr_warn("unknown window size %ld - defaulting to 256M\n",
+ priv->window_size);
+ priv->window_size = SZ_256M;
+ /* fall-through */
+ case SZ_256M:
+ val |= RCAR_USBCTR_PCIAHB_WIN1_256M;
+ break;
+ }
+ iowrite32(val, reg + RCAR_USBCTR_REG);

/* Configure AHB master and slave modes */
iowrite32(RCAR_AHB_BUS_MODE, reg + RCAR_AHB_BUS_CTR_REG);
@@ -194,7 +217,7 @@ static int __init rcar_pci_setup(int nr,
RCAR_PCI_ARBITER_PCIBP_MODE;
iowrite32(val, reg + RCAR_PCI_ARBITER_CTR_REG);

- /* PCI-AHB mapping: 0x40000000-0x80000000 */
+ /* PCI-AHB mapping: 0x40000000 base */
iowrite32(0x40000000 | RCAR_PCIAHB_PREFETCH16,
reg + RCAR_PCIAHB_WIN1_CTR_REG);

@@ -271,6 +294,8 @@ static int rcar_pci_probe(struct platfor
priv->reg = reg;
priv->dev = &pdev->dev;

+ priv->window_size = SZ_1G;
+
memset(&hw, 0, sizeof(hw));
hw.nr_controllers = 1;
hw.private_data = (void **)&priv;

2014-02-05 06:53:00

by Magnus Damm

[permalink] [raw]
Subject: [PATCH 04/04] PCI: rcar: Enable BOUNCE in case of HIGHMEM

From: Magnus Damm <[email protected]>

Select BOUNCE in case of HIGHMEM to enable bounce buffers in
the block layer. Without this patch the DMABOUNCE code will
error out due to lack of HIGHMEM support, and without DMABOUNCE
there will be silent errors.

Signed-off-by: Magnus Damm <[email protected]>
---

drivers/pci/host/Kconfig | 1 +
1 file changed, 1 insertion(+)

--- 0009/drivers/pci/host/Kconfig
+++ work/drivers/pci/host/Kconfig 2014-02-04 16:11:27.000000000 +0900
@@ -30,6 +30,7 @@ config PCI_RCAR_GEN2
depends on ARM && (ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST)
select GENERIC_ALLOCATOR
select DMABOUNCE
+ select BOUNCE if HIGHMEM
help
Say Y here if you want internal PCI support on R-Car Gen2 SoC.
There are 3 internal PCI controllers available with a single

2014-02-05 08:02:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 01/04] PCI: rcar: Register each instance independently

On Wed, Feb 5, 2014 at 7:52 AM, Magnus Damm <[email protected]> wrote:
> +static int rcar_pci_probe(struct platform_device *pdev)
> {
> struct resource *cfg_res, *mem_res;
> struct rcar_pci_priv *priv;
> void __iomem *reg;
> + struct hw_pci hw;
>
> cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> reg = devm_ioremap_resource(&pdev->dev, cfg_res);
> @@ -308,31 +271,26 @@ static int __init rcar_pci_probe(struct
> priv->reg = reg;
> priv->dev = &pdev->dev;
>
> - return rcar_pci_add_controller(priv);
> + memset(&hw, 0, sizeof(hw));
> + hw.nr_controllers = 1;
> + hw.private_data = (void **)&priv;

This raised a red flag: taking the address of a variable on the stack.
I know it's correct, as hw.private_data is an array of pointers copied
by pci_common_init_dev() below.
But perhaps it's a good idea to turn priv into an array with one element,
to make this clearer, and avoid the ugly cast?

> + hw.map_irq = rcar_pci_map_irq;
> + hw.ops = &rcar_pci_ops;
> + hw.setup = rcar_pci_setup;
> + pci_common_init_dev(&pdev->dev, &hw);
> + return 0;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-02-05 08:29:52

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 01/04] PCI: rcar: Register each instance independently

On 05/02/14 06:52, Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Convert the code to allow per-device probe() like other device
> drivers. This also delays driver registration due to change from
> subsys_initcall() to regular module_platform_driver().
>
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> drivers/pci/host/pci-rcar-gen2.c | 74 ++++++++------------------------------
> 1 file changed, 16 insertions(+), 58 deletions(-)
>
> --- 0001/drivers/pci/host/pci-rcar-gen2.c
> +++ work/drivers/pci/host/pci-rcar-gen2.c 2014-02-04 16:38:46.000000000 +0900
> @@ -74,9 +74,6 @@
>
> #define RCAR_PCI_UNIT_REV_REG (RCAR_AHBPCI_PCICOM_OFFSET + 0x48)
>
> -/* Number of internal PCI controllers */
> -#define RCAR_PCI_NR_CONTROLLERS 3
> -
> struct rcar_pci_priv {
> struct device *dev;
> void __iomem *reg;
> @@ -228,6 +225,8 @@ static int __init rcar_pci_setup(int nr,
> pci_add_resource(&sys->resources, &priv->io_res);
> pci_add_resource(&sys->resources, &priv->mem_res);
>
> + /* Setup bus number based on platform device id */
> + sys->busnr = to_platform_device(priv->dev)->id;
> return 1;
> }
>
> @@ -236,48 +235,12 @@ static struct pci_ops rcar_pci_ops = {
> .write = rcar_pci_write_config,
> };
>
> -static struct hw_pci rcar_hw_pci __initdata = {
> - .map_irq = rcar_pci_map_irq,
> - .ops = &rcar_pci_ops,
> - .setup = rcar_pci_setup,
> -};
> -
> -static int rcar_pci_count __initdata;
> -
> -static int __init rcar_pci_add_controller(struct rcar_pci_priv *priv)
> -{
> - void **private_data;
> - int count;
> -
> - if (rcar_hw_pci.nr_controllers < rcar_pci_count)
> - goto add_priv;
> -
> - /* (Re)allocate private data pointer array if needed */
> - count = rcar_pci_count + RCAR_PCI_NR_CONTROLLERS;
> - private_data = kzalloc(count * sizeof(void *), GFP_KERNEL);
> - if (!private_data)
> - return -ENOMEM;
> -
> - rcar_pci_count = count;
> - if (rcar_hw_pci.private_data) {
> - memcpy(private_data, rcar_hw_pci.private_data,
> - rcar_hw_pci.nr_controllers * sizeof(void *));
> - kfree(rcar_hw_pci.private_data);
> - }
> -
> - rcar_hw_pci.private_data = private_data;
> -
> -add_priv:
> - /* Add private data pointer to the array */
> - rcar_hw_pci.private_data[rcar_hw_pci.nr_controllers++] = priv;
> - return 0;
> -}
> -
> -static int __init rcar_pci_probe(struct platform_device *pdev)
> +static int rcar_pci_probe(struct platform_device *pdev)
> {
> struct resource *cfg_res, *mem_res;
> struct rcar_pci_priv *priv;
> void __iomem *reg;
> + struct hw_pci hw;
>
> cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> reg = devm_ioremap_resource(&pdev->dev, cfg_res);
> @@ -308,31 +271,26 @@ static int __init rcar_pci_probe(struct
> priv->reg = reg;
> priv->dev = &pdev->dev;
>
> - return rcar_pci_add_controller(priv);
> + memset(&hw, 0, sizeof(hw));
> + hw.nr_controllers = 1;
> + hw.private_data = (void **)&priv;
> + hw.map_irq = rcar_pci_map_irq;
> + hw.ops = &rcar_pci_ops;
> + hw.setup = rcar_pci_setup;
> + pci_common_init_dev(&pdev->dev, &hw);
> + return 0;
> }

Do we really want to explicitly set the bus number here?

>
> static struct platform_driver rcar_pci_driver = {
> .driver = {
> .name = "pci-rcar-gen2",
> + .owner = THIS_MODULE,
> + .suppress_bind_attrs = true,
> },
> + .probe = rcar_pci_probe,
> };


--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2014-02-05 08:33:21

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

On 05/02/14 06:52, Magnus Damm wrote:
> PCI: rcar: Driver model and physical address space update
>
> [PATCH 01/04] PCI: rcar: Register each instance independently
> [PATCH 02/04] PCI: rcar: Break out window size handling
> [PATCH 03/04] PCI: rcar: Add DMABOUNCE support
> [PATCH 04/04] PCI: rcar: Enable BOUNCE in case of HIGHMEM
>
> These patches update the pci-rcar-gen2.c driver in various ways
> including cleanups for driver model interface (1/4), readability
> update (2/4) and also bounce buffer support (3/4, 4/4). Basically
> the first two are just cleanups and the rest are fixes.
>
> As it is today without these patches the system memory start address
> is hard coded at 0x4000000 and the window is fixed at 1GiB. So we
> have board specific code hidden in the driver which is good to avoid.
>
> With these hard coded board specific constants there are some error cases
> that are not handled, in particular the issue that only maximum 1GiB of
> physical address space can be used for bus mastering with a single window.
> The common case of using ARM CONFIG_VMSPLIT_3G results in no visible issues
> as long as CONFIG_BOUNCE is used, but other CONFIG_VMSPLIT settings will
> break due to the hard coded 1GiB window not being enough. It has been
> verified that reducing the window size to 256MB makes the driver behave
> the same with VMSPLIT_3G as 1GiB window size and other VMSPLIT settings.
>
> To handle the maximum 1GiB physical address space limitation two types
> of bounce buffers are added. The ARM specific DMABOUNCE code is in 3/4
> hooked up to a chunk of local memory that is also handed of as coherent
> memory to the pci devices hanging off the PCI bridge. The driver makes
> sure to set the window so the local memory is always included. When the
> PCI devices are operating and in case memory is used outside the window
> then the DMABOUNCE buffers kicks in. This makes the driver support all
> kinds of VMSPLIT settings and window sizes. The BOUNCE code in 4/4 is
> selected to make sure bounce buffers are used for HIGHMEM.
>
> With these patches this driver can be used with or without CMA and
> with or without DMA zone. Basically the system wide memory setup is
> left to the user. If a DMA zone is used then the PCI window will
> be setup to cover that. Same thing with CMA.
>
> Tested with USB storage using LPAE and various VMSPLIT settings together
> with renesas git tag renesas-devel-v3.14-rc1-20140204 from kernel.org

Ok, is it possible to build a set with my of updates ready for next
merge window? I can update the set based on this if you like and try
and make the necessary changes to deal with the modifications from this
series.

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2014-02-05 08:39:17

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 01/04] PCI: rcar: Register each instance independently

On Wed, Feb 5, 2014 at 5:02 PM, Geert Uytterhoeven <[email protected]> wrote:
> On Wed, Feb 5, 2014 at 7:52 AM, Magnus Damm <[email protected]> wrote:
>> +static int rcar_pci_probe(struct platform_device *pdev)
>> {
>> struct resource *cfg_res, *mem_res;
>> struct rcar_pci_priv *priv;
>> void __iomem *reg;
>> + struct hw_pci hw;
>>
>> cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> reg = devm_ioremap_resource(&pdev->dev, cfg_res);
>> @@ -308,31 +271,26 @@ static int __init rcar_pci_probe(struct
>> priv->reg = reg;
>> priv->dev = &pdev->dev;
>>
>> - return rcar_pci_add_controller(priv);
>> + memset(&hw, 0, sizeof(hw));
>> + hw.nr_controllers = 1;
>> + hw.private_data = (void **)&priv;
>
> This raised a red flag: taking the address of a variable on the stack.
> I know it's correct, as hw.private_data is an array of pointers copied
> by pci_common_init_dev() below.
> But perhaps it's a good idea to turn priv into an array with one element,
> to make this clearer, and avoid the ugly cast?

I simply followed the same style as pci-tegra.c, but I agree that it
can be made prettier. Also, there may be some better way how to
register independent instances, not sure.

Thanks,

/ magnus

2014-02-05 08:44:03

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 01/04] PCI: rcar: Register each instance independently

On Wed, Feb 5, 2014 at 5:29 PM, Ben Dooks <[email protected]> wrote:
> On 05/02/14 06:52, Magnus Damm wrote:
>>
>> From: Magnus Damm <[email protected]>
>>
>> Convert the code to allow per-device probe() like other device
>> drivers. This also delays driver registration due to change from
>> subsys_initcall() to regular module_platform_driver().
>>
>> Signed-off-by: Magnus Damm <[email protected]>
>> ---
>>
>> drivers/pci/host/pci-rcar-gen2.c | 74
>> ++++++++------------------------------
>> 1 file changed, 16 insertions(+), 58 deletions(-)
>>
>> --- 0001/drivers/pci/host/pci-rcar-gen2.c
>> +++ work/drivers/pci/host/pci-rcar-gen2.c 2014-02-04
>> 16:38:46.000000000 +0900
>> @@ -74,9 +74,6 @@
>>
>> #define RCAR_PCI_UNIT_REV_REG (RCAR_AHBPCI_PCICOM_OFFSET + 0x48)
>>
>> -/* Number of internal PCI controllers */
>> -#define RCAR_PCI_NR_CONTROLLERS 3
>> -
>> struct rcar_pci_priv {
>> struct device *dev;
>> void __iomem *reg;
>> @@ -228,6 +225,8 @@ static int __init rcar_pci_setup(int nr,
>> pci_add_resource(&sys->resources, &priv->io_res);
>> pci_add_resource(&sys->resources, &priv->mem_res);
>>
>> + /* Setup bus number based on platform device id */
>> + sys->busnr = to_platform_device(priv->dev)->id;
>> return 1;
>> }
>>
>> @@ -236,48 +235,12 @@ static struct pci_ops rcar_pci_ops = {
>> .write = rcar_pci_write_config,
>> };
>>
>> -static struct hw_pci rcar_hw_pci __initdata = {
>> - .map_irq = rcar_pci_map_irq,
>> - .ops = &rcar_pci_ops,
>> - .setup = rcar_pci_setup,
>> -};
>> -
>> -static int rcar_pci_count __initdata;
>> -
>> -static int __init rcar_pci_add_controller(struct rcar_pci_priv *priv)
>> -{
>> - void **private_data;
>> - int count;
>> -
>> - if (rcar_hw_pci.nr_controllers < rcar_pci_count)
>> - goto add_priv;
>> -
>> - /* (Re)allocate private data pointer array if needed */
>> - count = rcar_pci_count + RCAR_PCI_NR_CONTROLLERS;
>> - private_data = kzalloc(count * sizeof(void *), GFP_KERNEL);
>> - if (!private_data)
>> - return -ENOMEM;
>> -
>> - rcar_pci_count = count;
>> - if (rcar_hw_pci.private_data) {
>> - memcpy(private_data, rcar_hw_pci.private_data,
>> - rcar_hw_pci.nr_controllers * sizeof(void *));
>> - kfree(rcar_hw_pci.private_data);
>> - }
>> -
>> - rcar_hw_pci.private_data = private_data;
>> -
>> -add_priv:
>> - /* Add private data pointer to the array */
>> - rcar_hw_pci.private_data[rcar_hw_pci.nr_controllers++] = priv;
>> - return 0;
>> -}
>> -
>> -static int __init rcar_pci_probe(struct platform_device *pdev)
>> +static int rcar_pci_probe(struct platform_device *pdev)
>> {
>> struct resource *cfg_res, *mem_res;
>> struct rcar_pci_priv *priv;
>> void __iomem *reg;
>> + struct hw_pci hw;
>>
>> cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> reg = devm_ioremap_resource(&pdev->dev, cfg_res);
>> @@ -308,31 +271,26 @@ static int __init rcar_pci_probe(struct
>> priv->reg = reg;
>> priv->dev = &pdev->dev;
>>
>> - return rcar_pci_add_controller(priv);
>> + memset(&hw, 0, sizeof(hw));
>> + hw.nr_controllers = 1;
>> + hw.private_data = (void **)&priv;
>> + hw.map_irq = rcar_pci_map_irq;
>> + hw.ops = &rcar_pci_ops;
>> + hw.setup = rcar_pci_setup;
>> + pci_common_init_dev(&pdev->dev, &hw);
>> + return 0;
>> }
>
>
> Do we really want to explicitly set the bus number here?

Right now the code in rcar_pci_setup() sets up the bus number. Ugly
but better than before IMO. If you have any suggestions how to make
this prettier then please let me know. =)

/ magnus

2014-02-05 09:00:29

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

Hi Ben,

On Wed, Feb 5, 2014 at 5:33 PM, Ben Dooks <[email protected]> wrote:
> On 05/02/14 06:52, Magnus Damm wrote:
>>
>> PCI: rcar: Driver model and physical address space update
>>
>> [PATCH 01/04] PCI: rcar: Register each instance independently
>> [PATCH 02/04] PCI: rcar: Break out window size handling
>> [PATCH 03/04] PCI: rcar: Add DMABOUNCE support
>> [PATCH 04/04] PCI: rcar: Enable BOUNCE in case of HIGHMEM
>>
>> These patches update the pci-rcar-gen2.c driver in various ways
>> including cleanups for driver model interface (1/4), readability
>> update (2/4) and also bounce buffer support (3/4, 4/4). Basically
>> the first two are just cleanups and the rest are fixes.
>>
>> As it is today without these patches the system memory start address
>> is hard coded at 0x4000000 and the window is fixed at 1GiB. So we
>> have board specific code hidden in the driver which is good to avoid.
>>
>> With these hard coded board specific constants there are some error cases
>> that are not handled, in particular the issue that only maximum 1GiB of
>> physical address space can be used for bus mastering with a single window.
>> The common case of using ARM CONFIG_VMSPLIT_3G results in no visible
>> issues
>> as long as CONFIG_BOUNCE is used, but other CONFIG_VMSPLIT settings will
>> break due to the hard coded 1GiB window not being enough. It has been
>> verified that reducing the window size to 256MB makes the driver behave
>> the same with VMSPLIT_3G as 1GiB window size and other VMSPLIT settings.
>>
>> To handle the maximum 1GiB physical address space limitation two types
>> of bounce buffers are added. The ARM specific DMABOUNCE code is in 3/4
>> hooked up to a chunk of local memory that is also handed of as coherent
>> memory to the pci devices hanging off the PCI bridge. The driver makes
>> sure to set the window so the local memory is always included. When the
>> PCI devices are operating and in case memory is used outside the window
>> then the DMABOUNCE buffers kicks in. This makes the driver support all
>> kinds of VMSPLIT settings and window sizes. The BOUNCE code in 4/4 is
>> selected to make sure bounce buffers are used for HIGHMEM.
>>
>> With these patches this driver can be used with or without CMA and
>> with or without DMA zone. Basically the system wide memory setup is
>> left to the user. If a DMA zone is used then the PCI window will
>> be setup to cover that. Same thing with CMA.
>>
>> Tested with USB storage using LPAE and various VMSPLIT settings together
>> with renesas git tag renesas-devel-v3.14-rc1-20140204 from kernel.org
>
>
> Ok, is it possible to build a set with my of updates ready for next
> merge window? I can update the set based on this if you like and try
> and make the necessary changes to deal with the modifications from this
> series.

I think we should try to pick out the stuff that is ready to be merged
first. I think these patches may require a bit of time before people
start looking at them. I don't mind resending in the future.

To be honest, I have not been paying too much attention to other
patches including yours - been focused trying to get the memory
management part right.. So I'd like to focus on correctness over DT
for now if possible. Of course we it all.

Would it be possible for you to provide a list of pci-rcar-gen2.c
patches that you posted? Or perhaps you can resend your series and
include acks that you received?

Thanks,

/ magnus

2014-02-05 09:25:35

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

On 05/02/14 09:00, Magnus Damm wrote:
> Hi Ben,

[snip]
> I think we should try to pick out the stuff that is ready to be merged
> first. I think these patches may require a bit of time before people
> start looking at them. I don't mind resending in the future.
>
> To be honest, I have not been paying too much attention to other
> patches including yours - been focused trying to get the memory
> management part right.. So I'd like to focus on correctness over DT
> for now if possible. Of course we it all.
>
> Would it be possible for you to provide a list of pci-rcar-gen2.c
> patches that you posted? Or perhaps you can resend your series and
> include acks that you received?

I don't think I have any acks, only review comments. I think the code
itself is pretty much ready to be merged and has been tested here with
the Lager board.

The big issue for us is that we /must/ boot with fdt, which means the
less fdt support that is in the kernel then the more patches we end up
carrying out of tree. This is why we have been pushing patches out to
try and get the support in.

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2014-02-05 09:40:08

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

On Wed, Feb 5, 2014 at 6:25 PM, Ben Dooks <[email protected]> wrote:
> On 05/02/14 09:00, Magnus Damm wrote:
>>
>> Hi Ben,
>
>
> [snip]
>
>> I think we should try to pick out the stuff that is ready to be merged
>> first. I think these patches may require a bit of time before people
>> start looking at them. I don't mind resending in the future.
>>
>> To be honest, I have not been paying too much attention to other
>> patches including yours - been focused trying to get the memory
>> management part right.. So I'd like to focus on correctness over DT
>> for now if possible. Of course we it all.
>>
>> Would it be possible for you to provide a list of pci-rcar-gen2.c
>> patches that you posted? Or perhaps you can resend your series and
>> include acks that you received?
>
>
> I don't think I have any acks, only review comments. I think the code
> itself is pretty much ready to be merged and has been tested here with
> the Lager board.

Ok, thanks. I'd like to go through them myself if possible. I was
looking for a cover letter or something that listed the patches but I
can't seem to find it. Can you please provide a list?

> The big issue for us is that we /must/ boot with fdt, which means the
> less fdt support that is in the kernel then the more patches we end up
> carrying out of tree. This is why we have been pushing patches out to
> try and get the support in.

Can you please define "boot with fdt"? As you probably know, both
board-lager.c and board-lager-reference.c boot with DT. I suppose you
mean that you want DT to describe the entire system, with no C code
for the board? Please note that some devices like timers still don't
use DT, so if you can live without those then... =)

Thanks,

/ magnus

2014-02-05 10:12:34

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

On 05/02/14 09:40, Magnus Damm wrote:
> On Wed, Feb 5, 2014 at 6:25 PM, Ben Dooks <[email protected]> wrote:
>> On 05/02/14 09:00, Magnus Damm wrote:
>>>
>>> Hi Ben,
>>
>>
>> [snip]
>>
>>> I think we should try to pick out the stuff that is ready to be merged
>>> first. I think these patches may require a bit of time before people
>>> start looking at them. I don't mind resending in the future.
>>>
>>> To be honest, I have not been paying too much attention to other
>>> patches including yours - been focused trying to get the memory
>>> management part right.. So I'd like to focus on correctness over DT
>>> for now if possible. Of course we it all.
>>>
>>> Would it be possible for you to provide a list of pci-rcar-gen2.c
>>> patches that you posted? Or perhaps you can resend your series and
>>> include acks that you received?
>>
>>
>> I don't think I have any acks, only review comments. I think the code
>> itself is pretty much ready to be merged and has been tested here with
>> the Lager board.
>
> Ok, thanks. I'd like to go through them myself if possible. I was
> looking for a cover letter or something that listed the patches but I
> can't seem to find it. Can you please provide a list?

I will re-send the patches once I've applied the relevant fixups from
the last round, hopefully within the next few hours.

>> The big issue for us is that we /must/ boot with fdt, which means the
>> less fdt support that is in the kernel then the more patches we end up
>> carrying out of tree. This is why we have been pushing patches out to
>> try and get the support in.
>
> Can you please define "boot with fdt"? As you probably know, both
> board-lager.c and board-lager-reference.c boot with DT. I suppose you
> mean that you want DT to describe the entire system, with no C code
> for the board? Please note that some devices like timers still don't
> use DT, so if you can live without those then... =)

We currently have enough to use the system. And yes, we boot with
board-lager-reference.c which current seems to be turning up bugs
in the up-stream (see the recent new bug with cpg clock code reported
by my colleague).

Due to issues out of our control we can test with board-lager.c but
we must use the board-lager-reference.c for actual product test. The
only way we found around this is ugly hacking of the core driver code
to link platform and fdt devices...

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2014-02-12 20:59:19

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

On Wed, Feb 05, 2014 at 03:52:43PM +0900, Magnus Damm wrote:
> PCI: rcar: Driver model and physical address space update
>
> [PATCH 01/04] PCI: rcar: Register each instance independently
> [PATCH 02/04] PCI: rcar: Break out window size handling
> [PATCH 03/04] PCI: rcar: Add DMABOUNCE support
> [PATCH 04/04] PCI: rcar: Enable BOUNCE in case of HIGHMEM
>
> These patches update the pci-rcar-gen2.c driver in various ways
> including cleanups for driver model interface (1/4), readability
> update (2/4) and also bounce buffer support (3/4, 4/4). Basically
> the first two are just cleanups and the rest are fixes.
>
> As it is today without these patches the system memory start address
> is hard coded at 0x4000000 and the window is fixed at 1GiB. So we
> have board specific code hidden in the driver which is good to avoid.
>
> With these hard coded board specific constants there are some error cases
> that are not handled, in particular the issue that only maximum 1GiB of
> physical address space can be used for bus mastering with a single window.
> The common case of using ARM CONFIG_VMSPLIT_3G results in no visible issues
> as long as CONFIG_BOUNCE is used, but other CONFIG_VMSPLIT settings will
> break due to the hard coded 1GiB window not being enough. It has been
> verified that reducing the window size to 256MB makes the driver behave
> the same with VMSPLIT_3G as 1GiB window size and other VMSPLIT settings.
>
> To handle the maximum 1GiB physical address space limitation two types
> of bounce buffers are added. The ARM specific DMABOUNCE code is in 3/4
> hooked up to a chunk of local memory that is also handed of as coherent
> memory to the pci devices hanging off the PCI bridge. The driver makes
> sure to set the window so the local memory is always included. When the
> PCI devices are operating and in case memory is used outside the window
> then the DMABOUNCE buffers kicks in. This makes the driver support all
> kinds of VMSPLIT settings and window sizes. The BOUNCE code in 4/4 is
> selected to make sure bounce buffers are used for HIGHMEM.
>
> With these patches this driver can be used with or without CMA and
> with or without DMA zone. Basically the system wide memory setup is
> left to the user. If a DMA zone is used then the PCI window will
> be setup to cover that. Same thing with CMA.
>
> Tested with USB storage using LPAE and various VMSPLIT settings together
> with renesas git tag renesas-devel-v3.14-rc1-20140204 from kernel.org
>
> Signed-off-by: Magnus Damm <[email protected]>
> ---
>
> Written against renesas.git tag renesas-devel-v3.14-rc1-20140204
>
> drivers/pci/host/Kconfig | 3
> drivers/pci/host/pci-rcar-gen2.c | 367 +++++++++++++++++++++++++++++++-------
> 2 files changed, 306 insertions(+), 64 deletions(-)

Simon, if you want to ack these, I'll be happy to merge them for v3.15.
I'm not really qualified to review them myself.

Bjorn

2014-02-13 04:37:56

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 00/04] PCI: rcar: Driver model and physical address space update

On Wed, Feb 12, 2014 at 01:59:11PM -0700, Bjorn Helgaas wrote:
> On Wed, Feb 05, 2014 at 03:52:43PM +0900, Magnus Damm wrote:
> > PCI: rcar: Driver model and physical address space update
> >
> > [PATCH 01/04] PCI: rcar: Register each instance independently
> > [PATCH 02/04] PCI: rcar: Break out window size handling
> > [PATCH 03/04] PCI: rcar: Add DMABOUNCE support
> > [PATCH 04/04] PCI: rcar: Enable BOUNCE in case of HIGHMEM
> >
> > These patches update the pci-rcar-gen2.c driver in various ways
> > including cleanups for driver model interface (1/4), readability
> > update (2/4) and also bounce buffer support (3/4, 4/4). Basically
> > the first two are just cleanups and the rest are fixes.
> >
> > As it is today without these patches the system memory start address
> > is hard coded at 0x4000000 and the window is fixed at 1GiB. So we
> > have board specific code hidden in the driver which is good to avoid.
> >
> > With these hard coded board specific constants there are some error cases
> > that are not handled, in particular the issue that only maximum 1GiB of
> > physical address space can be used for bus mastering with a single window.
> > The common case of using ARM CONFIG_VMSPLIT_3G results in no visible issues
> > as long as CONFIG_BOUNCE is used, but other CONFIG_VMSPLIT settings will
> > break due to the hard coded 1GiB window not being enough. It has been
> > verified that reducing the window size to 256MB makes the driver behave
> > the same with VMSPLIT_3G as 1GiB window size and other VMSPLIT settings.
> >
> > To handle the maximum 1GiB physical address space limitation two types
> > of bounce buffers are added. The ARM specific DMABOUNCE code is in 3/4
> > hooked up to a chunk of local memory that is also handed of as coherent
> > memory to the pci devices hanging off the PCI bridge. The driver makes
> > sure to set the window so the local memory is always included. When the
> > PCI devices are operating and in case memory is used outside the window
> > then the DMABOUNCE buffers kicks in. This makes the driver support all
> > kinds of VMSPLIT settings and window sizes. The BOUNCE code in 4/4 is
> > selected to make sure bounce buffers are used for HIGHMEM.
> >
> > With these patches this driver can be used with or without CMA and
> > with or without DMA zone. Basically the system wide memory setup is
> > left to the user. If a DMA zone is used then the PCI window will
> > be setup to cover that. Same thing with CMA.
> >
> > Tested with USB storage using LPAE and various VMSPLIT settings together
> > with renesas git tag renesas-devel-v3.14-rc1-20140204 from kernel.org
> >
> > Signed-off-by: Magnus Damm <[email protected]>
> > ---
> >
> > Written against renesas.git tag renesas-devel-v3.14-rc1-20140204
> >
> > drivers/pci/host/Kconfig | 3
> > drivers/pci/host/pci-rcar-gen2.c | 367 +++++++++++++++++++++++++++++++-------
> > 2 files changed, 306 insertions(+), 64 deletions(-)
>
> Simon, if you want to ack these, I'll be happy to merge them for v3.15.
> I'm not really qualified to review them myself.

Hi Bjorn,

Magnus has reposted these patches as part of a larger series
which includes some patches of his own. That series is
"[PATCH 00/08] PCI: rcar: Recent driver patches from Ben Dooks and me".
I am happy with it and will Ack it. Could you consider merging it?