PCI: rcar: Driver model and physical address space update

drivers/pci/host/pci-rcar-gen2.c +2 -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;
 
        cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        reg = devm_ioremap_resource(&pdev->dev, cfg_res);

Quoted 13 lines · Magnus Damm

drivers/pci/host/pci-rcar-gen2.c +2 -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;
 
        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;
 

drivers/pci/host/pci-rcar-gen2.c static int __init rcar_pci_probe(struct · line 276

Quoted 4 lines

drivers/pci/host/pci-rcar-gen2.c +3 -1

-       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?

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(-)

drivers/pci/host/pci-rcar-gen2.c +4 -41

@@ -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;
 

drivers/pci/host/pci-rcar-gen2.c +3 -0

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

Quoted 91 lines · Magnus Damm

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(-)

drivers/pci/host/pci-rcar-gen2.c 2014-02-04 renamed from drivers/pci/host/pci-rcar-gen2.c +0 -0

16:38:46.000000000 +0900

drivers/pci/host/pci-rcar-gen2.c 2014-02-04 renamed from drivers/pci/host/pci-rcar-gen2.c +4 -41

@@ -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;
 

drivers/pci/host/pci-rcar-gen2.c 2014-02-04 static int __init rcar_pci_probe(struct · line 282

Quoted 10 lines

drivers/pci/host/pci-rcar-gen2.c 2014-02-04 renamed from drivers/pci/host/pci-rcar-gen2.c +8 -1

-       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?

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

Quoted 42 lines · Magnus Damm

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.

Quoted 2 lines

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.

Hi Ben,

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?

Quoted 2 lines · Magnus Damm

Hi Ben,

[snip]

Quoted 9 lines

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.

Quoted 3 lines

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.

Quoted 21 lines · Ben Dooks

Quoted 2 lines · Magnus Damm

Hi Ben,

[snip]

Quoted 12 lines

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?

Quoted 3 lines

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?

Quoted 4 lines

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... =)

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(-)

Quoted 46 lines · Magnus Damm

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]>

---

Quoted 5 lines

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.