* [PATCH 1/2] PCI: iproc: directly add pci resources
@ 2015-05-24 20:37 Hauke Mehrtens
2015-05-24 20:37 ` [PATCH 2/2] PCI: iproc: free resource list after registration Hauke Mehrtens
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Hauke Mehrtens @ 2015-05-24 20:37 UTC (permalink / raw)
To: linux-arm-kernel
The resources member in the struct was pointing to a stack variable and
is invalid after the the registration function returned. Remove this
pointer and add it a a parameter to the function.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
drivers/pci/host/pcie-iproc-bcma.c | 4 +---
drivers/pci/host/pcie-iproc-platform.c | 4 +---
drivers/pci/host/pcie-iproc.c | 4 ++--
drivers/pci/host/pcie-iproc.h | 3 +--
4 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
index c318f19..f96b39e 100644
--- a/drivers/pci/host/pcie-iproc-bcma.c
+++ b/drivers/pci/host/pcie-iproc-bcma.c
@@ -62,11 +62,9 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
res_mem.flags = IORESOURCE_MEM;
pci_add_resource(&res, &res_mem);
- pcie->resources = &res;
-
pcie->map_irq = iproc_pcie_bcma_map_irq;
- ret = iproc_pcie_setup(pcie);
+ ret = iproc_pcie_setup(pcie, &res);
if (ret) {
dev_err(pcie->dev, "PCIe controller setup failed\n");
return ret;
diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index c8aa06f..c5fe4c1 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -69,11 +69,9 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
return ret;
}
- pcie->resources = &res;
-
pcie->map_irq = of_irq_parse_and_map_pci;
- ret = iproc_pcie_setup(pcie);
+ ret = iproc_pcie_setup(pcie, &res);
if (ret) {
dev_err(pcie->dev, "PCIe controller setup failed\n");
return ret;
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index cef31f6..d77481e 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -183,7 +183,7 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
writel(SYS_RC_INTX_MASK, pcie->base + SYS_RC_INTX_EN);
}
-int iproc_pcie_setup(struct iproc_pcie *pcie)
+int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
{
int ret;
struct pci_bus *bus;
@@ -211,7 +211,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie)
pcie->sysdata.private_data = pcie;
bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops,
- &pcie->sysdata, pcie->resources);
+ &pcie->sysdata, res);
if (!bus) {
dev_err(pcie->dev, "unable to create PCI root bus\n");
ret = -ENOMEM;
diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
index a333d4b..ba0a108 100644
--- a/drivers/pci/host/pcie-iproc.h
+++ b/drivers/pci/host/pcie-iproc.h
@@ -29,7 +29,6 @@
struct iproc_pcie {
struct device *dev;
void __iomem *base;
- struct list_head *resources;
struct pci_sys_data sysdata;
struct pci_bus *root_bus;
struct phy *phy;
@@ -37,7 +36,7 @@ struct iproc_pcie {
int (*map_irq)(const struct pci_dev *, u8, u8);
};
-int iproc_pcie_setup(struct iproc_pcie *pcie);
+int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);
int iproc_pcie_remove(struct iproc_pcie *pcie);
#endif /* _PCIE_IPROC_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] PCI: iproc: free resource list after registration
2015-05-24 20:37 [PATCH 1/2] PCI: iproc: directly add pci resources Hauke Mehrtens
@ 2015-05-24 20:37 ` Hauke Mehrtens
2015-05-26 0:35 ` Ray Jui
2015-05-25 17:10 ` [PATCH 1/2] PCI: iproc: directly add pci resources Ray Jui
2015-05-27 23:35 ` Bjorn Helgaas
2 siblings, 1 reply; 7+ messages in thread
From: Hauke Mehrtens @ 2015-05-24 20:37 UTC (permalink / raw)
To: linux-arm-kernel
The resource list is only used in the setup process and was never
freed. pci_add_resource() allocates a memory area to store the list
item. This patch fixes the memory leak.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
drivers/pci/host/pcie-iproc-bcma.c | 8 ++++----
drivers/pci/host/pcie-iproc-platform.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
index f96b39e..2e4f2d6 100644
--- a/drivers/pci/host/pcie-iproc-bcma.c
+++ b/drivers/pci/host/pcie-iproc-bcma.c
@@ -65,12 +65,12 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
pcie->map_irq = iproc_pcie_bcma_map_irq;
ret = iproc_pcie_setup(pcie, &res);
- if (ret) {
+ if (ret)
dev_err(pcie->dev, "PCIe controller setup failed\n");
- return ret;
- }
- return 0;
+ pci_free_resource_list(&res);
+
+ return ret;
}
static void iproc_pcie_bcma_remove(struct bcma_device *bdev)
diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index c5fe4c1..9aedc8e 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -72,12 +72,12 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
pcie->map_irq = of_irq_parse_and_map_pci;
ret = iproc_pcie_setup(pcie, &res);
- if (ret) {
+ if (ret)
dev_err(pcie->dev, "PCIe controller setup failed\n");
- return ret;
- }
- return 0;
+ pci_free_resource_list(&res);
+
+ return ret;
}
static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] PCI: iproc: free resource list after registration
2015-05-24 20:37 ` [PATCH 2/2] PCI: iproc: free resource list after registration Hauke Mehrtens
@ 2015-05-26 0:35 ` Ray Jui
0 siblings, 0 replies; 7+ messages in thread
From: Ray Jui @ 2015-05-26 0:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Hauke,
On 15-05-24 01:37 PM, Hauke Mehrtens wrote:
> The resource list is only used in the setup process and was never
> freed. pci_add_resource() allocates a memory area to store the list
> item. This patch fixes the memory leak.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> drivers/pci/host/pcie-iproc-bcma.c | 8 ++++----
> drivers/pci/host/pcie-iproc-platform.c | 8 ++++----
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
> index f96b39e..2e4f2d6 100644
> --- a/drivers/pci/host/pcie-iproc-bcma.c
> +++ b/drivers/pci/host/pcie-iproc-bcma.c
> @@ -65,12 +65,12 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
> pcie->map_irq = iproc_pcie_bcma_map_irq;
>
> ret = iproc_pcie_setup(pcie, &res);
> - if (ret) {
> + if (ret)
> dev_err(pcie->dev, "PCIe controller setup failed\n");
> - return ret;
> - }
>
> - return 0;
> + pci_free_resource_list(&res);
> +
> + return ret;
> }
>
> static void iproc_pcie_bcma_remove(struct bcma_device *bdev)
> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
> index c5fe4c1..9aedc8e 100644
> --- a/drivers/pci/host/pcie-iproc-platform.c
> +++ b/drivers/pci/host/pcie-iproc-platform.c
> @@ -72,12 +72,12 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
> pcie->map_irq = of_irq_parse_and_map_pci;
>
> ret = iproc_pcie_setup(pcie, &res);
> - if (ret) {
> + if (ret)
> dev_err(pcie->dev, "PCIe controller setup failed\n");
> - return ret;
> - }
>
> - return 0;
> + pci_free_resource_list(&res);
> +
> + return ret;
> }
>
> static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
>
This looks good to me and I sanity tested it on a BCM958300K Cygnus
combo board.
Thanks,
Reviewed-by: Ray Jui <rjui@broadcom.com>
Tested-by: Ray Jui <rjui@broadcom.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] PCI: iproc: directly add pci resources
2015-05-24 20:37 [PATCH 1/2] PCI: iproc: directly add pci resources Hauke Mehrtens
2015-05-24 20:37 ` [PATCH 2/2] PCI: iproc: free resource list after registration Hauke Mehrtens
@ 2015-05-25 17:10 ` Ray Jui
2015-05-25 22:48 ` Hauke Mehrtens
2015-05-27 23:35 ` Bjorn Helgaas
2 siblings, 1 reply; 7+ messages in thread
From: Ray Jui @ 2015-05-25 17:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Hauke,
On 5/24/2015 1:37 PM, Hauke Mehrtens wrote:
> The resources member in the struct was pointing to a stack variable and
> is invalid after the the registration function returned. Remove this
> pointer and add it a a parameter to the function.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> drivers/pci/host/pcie-iproc-bcma.c | 4 +---
> drivers/pci/host/pcie-iproc-platform.c | 4 +---
> drivers/pci/host/pcie-iproc.c | 4 ++--
> drivers/pci/host/pcie-iproc.h | 3 +--
> 4 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
> index c318f19..f96b39e 100644
> --- a/drivers/pci/host/pcie-iproc-bcma.c
> +++ b/drivers/pci/host/pcie-iproc-bcma.c
> @@ -62,11 +62,9 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
> res_mem.flags = IORESOURCE_MEM;
> pci_add_resource(&res, &res_mem);
>
> - pcie->resources = &res;
> -
> pcie->map_irq = iproc_pcie_bcma_map_irq;
>
> - ret = iproc_pcie_setup(pcie);
> + ret = iproc_pcie_setup(pcie, &res);
> if (ret) {
> dev_err(pcie->dev, "PCIe controller setup failed\n");
> return ret;
> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
> index c8aa06f..c5fe4c1 100644
> --- a/drivers/pci/host/pcie-iproc-platform.c
> +++ b/drivers/pci/host/pcie-iproc-platform.c
> @@ -69,11 +69,9 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
> return ret;
> }
>
> - pcie->resources = &res;
> -
> pcie->map_irq = of_irq_parse_and_map_pci;
>
> - ret = iproc_pcie_setup(pcie);
> + ret = iproc_pcie_setup(pcie, &res);
> if (ret) {
> dev_err(pcie->dev, "PCIe controller setup failed\n");
> return ret;
> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> index cef31f6..d77481e 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -183,7 +183,7 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
> writel(SYS_RC_INTX_MASK, pcie->base + SYS_RC_INTX_EN);
> }
>
> -int iproc_pcie_setup(struct iproc_pcie *pcie)
> +int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
> {
> int ret;
> struct pci_bus *bus;
> @@ -211,7 +211,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie)
> pcie->sysdata.private_data = pcie;
>
> bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops,
> - &pcie->sysdata, pcie->resources);
> + &pcie->sysdata, res);
> if (!bus) {
> dev_err(pcie->dev, "unable to create PCI root bus\n");
> ret = -ENOMEM;
> diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
> index a333d4b..ba0a108 100644
> --- a/drivers/pci/host/pcie-iproc.h
> +++ b/drivers/pci/host/pcie-iproc.h
> @@ -29,7 +29,6 @@
> struct iproc_pcie {
> struct device *dev;
> void __iomem *base;
> - struct list_head *resources;
This means we do not want to keep a copy of the resources. In the
future, if we need to add support to explicitly set up the
inbound/outbound mapping window, we need to do it in iproc_pcie_setup.
Is that the intention?
Thanks,
Ray
> struct pci_sys_data sysdata;
> struct pci_bus *root_bus;
> struct phy *phy;
> @@ -37,7 +36,7 @@ struct iproc_pcie {
> int (*map_irq)(const struct pci_dev *, u8, u8);
> };
>
> -int iproc_pcie_setup(struct iproc_pcie *pcie);
> +int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);
> int iproc_pcie_remove(struct iproc_pcie *pcie);
>
> #endif /* _PCIE_IPROC_H */
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] PCI: iproc: directly add pci resources
2015-05-25 17:10 ` [PATCH 1/2] PCI: iproc: directly add pci resources Ray Jui
@ 2015-05-25 22:48 ` Hauke Mehrtens
2015-05-26 0:32 ` Ray Jui
0 siblings, 1 reply; 7+ messages in thread
From: Hauke Mehrtens @ 2015-05-25 22:48 UTC (permalink / raw)
To: linux-arm-kernel
On 05/25/2015 07:10 PM, Ray Jui wrote:
> Hi Hauke,
>
> On 5/24/2015 1:37 PM, Hauke Mehrtens wrote:
>> The resources member in the struct was pointing to a stack variable and
>> is invalid after the the registration function returned. Remove this
>> pointer and add it a a parameter to the function.
>>
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>> drivers/pci/host/pcie-iproc-bcma.c | 4 +---
>> drivers/pci/host/pcie-iproc-platform.c | 4 +---
>> drivers/pci/host/pcie-iproc.c | 4 ++--
>> drivers/pci/host/pcie-iproc.h | 3 +--
>> 4 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
>> index c318f19..f96b39e 100644
>> --- a/drivers/pci/host/pcie-iproc-bcma.c
>> +++ b/drivers/pci/host/pcie-iproc-bcma.c
>> @@ -62,11 +62,9 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
>> res_mem.flags = IORESOURCE_MEM;
>> pci_add_resource(&res, &res_mem);
>>
>> - pcie->resources = &res;
>> -
>> pcie->map_irq = iproc_pcie_bcma_map_irq;
>>
>> - ret = iproc_pcie_setup(pcie);
>> + ret = iproc_pcie_setup(pcie, &res);
>> if (ret) {
>> dev_err(pcie->dev, "PCIe controller setup failed\n");
>> return ret;
>> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
>> index c8aa06f..c5fe4c1 100644
>> --- a/drivers/pci/host/pcie-iproc-platform.c
>> +++ b/drivers/pci/host/pcie-iproc-platform.c
>> @@ -69,11 +69,9 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - pcie->resources = &res;
>> -
>> pcie->map_irq = of_irq_parse_and_map_pci;
>>
>> - ret = iproc_pcie_setup(pcie);
>> + ret = iproc_pcie_setup(pcie, &res);
>> if (ret) {
>> dev_err(pcie->dev, "PCIe controller setup failed\n");
>> return ret;
>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
>> index cef31f6..d77481e 100644
>> --- a/drivers/pci/host/pcie-iproc.c
>> +++ b/drivers/pci/host/pcie-iproc.c
>> @@ -183,7 +183,7 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
>> writel(SYS_RC_INTX_MASK, pcie->base + SYS_RC_INTX_EN);
>> }
>>
>> -int iproc_pcie_setup(struct iproc_pcie *pcie)
>> +int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
>> {
>> int ret;
>> struct pci_bus *bus;
>> @@ -211,7 +211,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie)
>> pcie->sysdata.private_data = pcie;
>>
>> bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops,
>> - &pcie->sysdata, pcie->resources);
>> + &pcie->sysdata, res);
>> if (!bus) {
>> dev_err(pcie->dev, "unable to create PCI root bus\n");
>> ret = -ENOMEM;
>> diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
>> index a333d4b..ba0a108 100644
>> --- a/drivers/pci/host/pcie-iproc.h
>> +++ b/drivers/pci/host/pcie-iproc.h
>> @@ -29,7 +29,6 @@
>> struct iproc_pcie {
>> struct device *dev;
>> void __iomem *base;
>> - struct list_head *resources;
>
> This means we do not want to keep a copy of the resources. In the
> future, if we need to add support to explicitly set up the
> inbound/outbound mapping window, we need to do it in iproc_pcie_setup.
> Is that the intention?
I haven't really thought about where to configure the memory mapping,
but I thought it would be somewhere in iproc_pcie_setup() or an method
called from there.
If you need it after iproc_pcie_setup() finished we should embed the
list into the struct iproc_pcie, because currently this pointer points
to the stack.
Hauke
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] PCI: iproc: directly add pci resources
2015-05-25 22:48 ` Hauke Mehrtens
@ 2015-05-26 0:32 ` Ray Jui
0 siblings, 0 replies; 7+ messages in thread
From: Ray Jui @ 2015-05-26 0:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi Hauke,
On 15-05-25 03:48 PM, Hauke Mehrtens wrote:
> On 05/25/2015 07:10 PM, Ray Jui wrote:
>> Hi Hauke,
>>
>> On 5/24/2015 1:37 PM, Hauke Mehrtens wrote:
>>> The resources member in the struct was pointing to a stack variable and
>>> is invalid after the the registration function returned. Remove this
>>> pointer and add it a a parameter to the function.
>>>
>>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>>> ---
>>> drivers/pci/host/pcie-iproc-bcma.c | 4 +---
>>> drivers/pci/host/pcie-iproc-platform.c | 4 +---
>>> drivers/pci/host/pcie-iproc.c | 4 ++--
>>> drivers/pci/host/pcie-iproc.h | 3 +--
>>> 4 files changed, 5 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
>>> index c318f19..f96b39e 100644
>>> --- a/drivers/pci/host/pcie-iproc-bcma.c
>>> +++ b/drivers/pci/host/pcie-iproc-bcma.c
>>> @@ -62,11 +62,9 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
>>> res_mem.flags = IORESOURCE_MEM;
>>> pci_add_resource(&res, &res_mem);
>>>
>>> - pcie->resources = &res;
>>> -
>>> pcie->map_irq = iproc_pcie_bcma_map_irq;
>>>
>>> - ret = iproc_pcie_setup(pcie);
>>> + ret = iproc_pcie_setup(pcie, &res);
>>> if (ret) {
>>> dev_err(pcie->dev, "PCIe controller setup failed\n");
>>> return ret;
>>> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
>>> index c8aa06f..c5fe4c1 100644
>>> --- a/drivers/pci/host/pcie-iproc-platform.c
>>> +++ b/drivers/pci/host/pcie-iproc-platform.c
>>> @@ -69,11 +69,9 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
>>> return ret;
>>> }
>>>
>>> - pcie->resources = &res;
>>> -
>>> pcie->map_irq = of_irq_parse_and_map_pci;
>>>
>>> - ret = iproc_pcie_setup(pcie);
>>> + ret = iproc_pcie_setup(pcie, &res);
>>> if (ret) {
>>> dev_err(pcie->dev, "PCIe controller setup failed\n");
>>> return ret;
>>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
>>> index cef31f6..d77481e 100644
>>> --- a/drivers/pci/host/pcie-iproc.c
>>> +++ b/drivers/pci/host/pcie-iproc.c
>>> @@ -183,7 +183,7 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
>>> writel(SYS_RC_INTX_MASK, pcie->base + SYS_RC_INTX_EN);
>>> }
>>>
>>> -int iproc_pcie_setup(struct iproc_pcie *pcie)
>>> +int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
>>> {
>>> int ret;
>>> struct pci_bus *bus;
>>> @@ -211,7 +211,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie)
>>> pcie->sysdata.private_data = pcie;
>>>
>>> bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops,
>>> - &pcie->sysdata, pcie->resources);
>>> + &pcie->sysdata, res);
>>> if (!bus) {
>>> dev_err(pcie->dev, "unable to create PCI root bus\n");
>>> ret = -ENOMEM;
>>> diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
>>> index a333d4b..ba0a108 100644
>>> --- a/drivers/pci/host/pcie-iproc.h
>>> +++ b/drivers/pci/host/pcie-iproc.h
>>> @@ -29,7 +29,6 @@
>>> struct iproc_pcie {
>>> struct device *dev;
>>> void __iomem *base;
>>> - struct list_head *resources;
>>
>> This means we do not want to keep a copy of the resources. In the
>> future, if we need to add support to explicitly set up the
>> inbound/outbound mapping window, we need to do it in iproc_pcie_setup.
>> Is that the intention?
>
> I haven't really thought about where to configure the memory mapping,
> but I thought it would be somewhere in iproc_pcie_setup() or an method
> called from there.
> If you need it after iproc_pcie_setup() finished we should embed the
> list into the struct iproc_pcie, because currently this pointer points
> to the stack.
>
> Hauke
>
I think most likely I'll be adding the memory mapping logic in
iproc_pcie_setup(), so your current patch should be fine.
Reviewed-by: Ray Jui <rjui@broadcom.com>
Tested-by: Ray Jui <rjui@broadcom.com>
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] PCI: iproc: directly add pci resources
2015-05-24 20:37 [PATCH 1/2] PCI: iproc: directly add pci resources Hauke Mehrtens
2015-05-24 20:37 ` [PATCH 2/2] PCI: iproc: free resource list after registration Hauke Mehrtens
2015-05-25 17:10 ` [PATCH 1/2] PCI: iproc: directly add pci resources Ray Jui
@ 2015-05-27 23:35 ` Bjorn Helgaas
2 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2015-05-27 23:35 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, May 24, 2015 at 10:37:02PM +0200, Hauke Mehrtens wrote:
> The resources member in the struct was pointing to a stack variable and
> is invalid after the the registration function returned. Remove this
> pointer and add it a a parameter to the function.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied both patches with Ray's Reviewed-by to pci/host-iproc for v4.2,
thanks!
> ---
> drivers/pci/host/pcie-iproc-bcma.c | 4 +---
> drivers/pci/host/pcie-iproc-platform.c | 4 +---
> drivers/pci/host/pcie-iproc.c | 4 ++--
> drivers/pci/host/pcie-iproc.h | 3 +--
> 4 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
> index c318f19..f96b39e 100644
> --- a/drivers/pci/host/pcie-iproc-bcma.c
> +++ b/drivers/pci/host/pcie-iproc-bcma.c
> @@ -62,11 +62,9 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
> res_mem.flags = IORESOURCE_MEM;
> pci_add_resource(&res, &res_mem);
>
> - pcie->resources = &res;
> -
> pcie->map_irq = iproc_pcie_bcma_map_irq;
>
> - ret = iproc_pcie_setup(pcie);
> + ret = iproc_pcie_setup(pcie, &res);
> if (ret) {
> dev_err(pcie->dev, "PCIe controller setup failed\n");
> return ret;
> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
> index c8aa06f..c5fe4c1 100644
> --- a/drivers/pci/host/pcie-iproc-platform.c
> +++ b/drivers/pci/host/pcie-iproc-platform.c
> @@ -69,11 +69,9 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
> return ret;
> }
>
> - pcie->resources = &res;
> -
> pcie->map_irq = of_irq_parse_and_map_pci;
>
> - ret = iproc_pcie_setup(pcie);
> + ret = iproc_pcie_setup(pcie, &res);
> if (ret) {
> dev_err(pcie->dev, "PCIe controller setup failed\n");
> return ret;
> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> index cef31f6..d77481e 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -183,7 +183,7 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
> writel(SYS_RC_INTX_MASK, pcie->base + SYS_RC_INTX_EN);
> }
>
> -int iproc_pcie_setup(struct iproc_pcie *pcie)
> +int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
> {
> int ret;
> struct pci_bus *bus;
> @@ -211,7 +211,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie)
> pcie->sysdata.private_data = pcie;
>
> bus = pci_create_root_bus(pcie->dev, 0, &iproc_pcie_ops,
> - &pcie->sysdata, pcie->resources);
> + &pcie->sysdata, res);
> if (!bus) {
> dev_err(pcie->dev, "unable to create PCI root bus\n");
> ret = -ENOMEM;
> diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
> index a333d4b..ba0a108 100644
> --- a/drivers/pci/host/pcie-iproc.h
> +++ b/drivers/pci/host/pcie-iproc.h
> @@ -29,7 +29,6 @@
> struct iproc_pcie {
> struct device *dev;
> void __iomem *base;
> - struct list_head *resources;
> struct pci_sys_data sysdata;
> struct pci_bus *root_bus;
> struct phy *phy;
> @@ -37,7 +36,7 @@ struct iproc_pcie {
> int (*map_irq)(const struct pci_dev *, u8, u8);
> };
>
> -int iproc_pcie_setup(struct iproc_pcie *pcie);
> +int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);
> int iproc_pcie_remove(struct iproc_pcie *pcie);
>
> #endif /* _PCIE_IPROC_H */
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-05-27 23:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-24 20:37 [PATCH 1/2] PCI: iproc: directly add pci resources Hauke Mehrtens
2015-05-24 20:37 ` [PATCH 2/2] PCI: iproc: free resource list after registration Hauke Mehrtens
2015-05-26 0:35 ` Ray Jui
2015-05-25 17:10 ` [PATCH 1/2] PCI: iproc: directly add pci resources Ray Jui
2015-05-25 22:48 ` Hauke Mehrtens
2015-05-26 0:32 ` Ray Jui
2015-05-27 23:35 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).