diff mbox series

[v2,06/26] PCI: keystone: Move initializations to appropriate places

Message ID 20190325083501.8088-7-kishon@ti.com
State Superseded
Headers show
Series None | expand

Commit Message

Kishon Vijay Abraham I March 25, 2019, 8:34 a.m. UTC 
No functional change. Move host specific platform_get_resource to
ks_add_pcie_port and the common platform_get_resource (applicable
to both host and endpoint) to probe. This is in preparation for
adding endpoint support to pci-keystone driver.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

---
 drivers/pci/controller/dwc/pci-keystone.c | 27 +++++++++++++----------
 1 file changed, 15 insertions(+), 12 deletions(-)

-- 
2.17.1

Comments

Bjorn Helgaas April 13, 2019, 2:30 p.m. UTC | #1 
On Mon, Mar 25, 2019 at 02:04:41PM +0530, Kishon Vijay Abraham I wrote:
> No functional change. Move host specific platform_get_resource to

> ks_add_pcie_port and the common platform_get_resource (applicable

> to both host and endpoint) to probe. This is in preparation for

> adding endpoint support to pci-keystone driver.


This seems to move platform_get_resource() *from* (not *to*)
ks_add_pcie_port().

You seem to be making a distinction in the commit log between (1) a
resource that's only used for host mode, and (2) a common resource
that's used for both host and endpoint mode.  But I don't see that
distinction in the patch, so it's a little confusing to mention it in
the commit log.

It must make endpoint support easier, but I can't quite connect the
dots yet.  Maybe endpoint will also use ks_pcie_add_pcie_port(), but
will have a separate .probe() function that doesn't look up the
resource that's specific to host mode?

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> ---

>  drivers/pci/controller/dwc/pci-keystone.c | 27 +++++++++++++----------

>  1 file changed, 15 insertions(+), 12 deletions(-)

> 

> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c

> index 5eebef9b9ada..95997885a05c 100644

> --- a/drivers/pci/controller/dwc/pci-keystone.c

> +++ b/drivers/pci/controller/dwc/pci-keystone.c

> @@ -806,11 +806,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,

>  	struct resource *res;

>  	int ret;

>  

> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");

> -	pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);

> -	if (IS_ERR(pci->dbi_base))

> -		return PTR_ERR(pci->dbi_base);

> -

>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");

>  	pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);

>  	if (IS_ERR(pp->va_cfg0_base))

> @@ -818,13 +813,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,

>  

>  	pp->va_cfg1_base = pp->va_cfg0_base;

>  

> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");

> -	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);

> -	if (IS_ERR(ks_pcie->va_app_base))

> -		return PTR_ERR(ks_pcie->va_app_base);

> -

> -	ks_pcie->app = *res;

> -

>  	pp->ops = &ks_pcie_host_ops;

>  	ret = dw_pcie_host_init(pp);

>  	if (ret) {

> @@ -895,6 +883,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)

>  	struct dw_pcie *pci;

>  	struct keystone_pcie *ks_pcie;

>  	struct device_link **link;

> +	struct resource *res;

> +	void __iomem *base;

>  	u32 num_viewport;

>  	struct phy **phy;

>  	u32 num_lanes;

> @@ -911,6 +901,19 @@ static int __init ks_pcie_probe(struct platform_device *pdev)

>  	if (!pci)

>  		return -ENOMEM;

>  

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");

> +	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);

> +	if (IS_ERR(ks_pcie->va_app_base))

> +		return PTR_ERR(ks_pcie->va_app_base);

> +

> +	ks_pcie->app = *res;

> +

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");

> +	base = devm_pci_remap_cfg_resource(dev, res);

> +	if (IS_ERR(base))

> +		return PTR_ERR(base);

> +

> +	pci->dbi_base = base;

>  	pci->dev = dev;

>  	pci->ops = &ks_pcie_dw_pcie_ops;

>  

> -- 

> 2.17.1

>
Kishon Vijay Abraham I April 15, 2019, 5:34 a.m. UTC | #2 
Hi Bjorn,

On 13/04/19 8:00 PM, Bjorn Helgaas wrote:
> On Mon, Mar 25, 2019 at 02:04:41PM +0530, Kishon Vijay Abraham I wrote:

>> No functional change. Move host specific platform_get_resource to

>> ks_add_pcie_port and the common platform_get_resource (applicable

>> to both host and endpoint) to probe. This is in preparation for

>> adding endpoint support to pci-keystone driver.

> 

> This seems to move platform_get_resource() *from* (not *to*)

> ks_add_pcie_port().


Maybe I should have mentioned "Keep host specific platform_get_resource() in
ks_add_pcie_port() and move the common platform_get_resource() (applicable
to both host and endpoint) to probe()"
> 

> You seem to be making a distinction in the commit log between (1) a

> resource that's only used for host mode, and (2) a common resource

> that's used for both host and endpoint mode.  But I don't see that

> distinction in the patch, so it's a little confusing to mention it in

> the commit log.

> 

> It must make endpoint support easier, but I can't quite connect the

> dots yet.  Maybe endpoint will also use ks_pcie_add_pcie_port(), but

> will have a separate .probe() function that doesn't look up the

> resource that's specific to host mode?


No ks_pcie_add_pcie_port() is specific to host mode, so "config" resource is
kept there whereas "dbics" and "app" resources are common to both host mode and
device mode, so they are moved to probe().

Thanks
Kishon

> 

>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

>> ---

>>  drivers/pci/controller/dwc/pci-keystone.c | 27 +++++++++++++----------

>>  1 file changed, 15 insertions(+), 12 deletions(-)

>>

>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c

>> index 5eebef9b9ada..95997885a05c 100644

>> --- a/drivers/pci/controller/dwc/pci-keystone.c

>> +++ b/drivers/pci/controller/dwc/pci-keystone.c

>> @@ -806,11 +806,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,

>>  	struct resource *res;

>>  	int ret;

>>  

>> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");

>> -	pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);

>> -	if (IS_ERR(pci->dbi_base))

>> -		return PTR_ERR(pci->dbi_base);

>> -

>>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");

>>  	pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);

>>  	if (IS_ERR(pp->va_cfg0_base))

>> @@ -818,13 +813,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,

>>  

>>  	pp->va_cfg1_base = pp->va_cfg0_base;

>>  

>> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");

>> -	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);

>> -	if (IS_ERR(ks_pcie->va_app_base))

>> -		return PTR_ERR(ks_pcie->va_app_base);

>> -

>> -	ks_pcie->app = *res;

>> -

>>  	pp->ops = &ks_pcie_host_ops;

>>  	ret = dw_pcie_host_init(pp);

>>  	if (ret) {

>> @@ -895,6 +883,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)

>>  	struct dw_pcie *pci;

>>  	struct keystone_pcie *ks_pcie;

>>  	struct device_link **link;

>> +	struct resource *res;

>> +	void __iomem *base;

>>  	u32 num_viewport;

>>  	struct phy **phy;

>>  	u32 num_lanes;

>> @@ -911,6 +901,19 @@ static int __init ks_pcie_probe(struct platform_device *pdev)

>>  	if (!pci)

>>  		return -ENOMEM;

>>  

>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");

>> +	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);

>> +	if (IS_ERR(ks_pcie->va_app_base))

>> +		return PTR_ERR(ks_pcie->va_app_base);

>> +

>> +	ks_pcie->app = *res;

>> +

>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");

>> +	base = devm_pci_remap_cfg_resource(dev, res);

>> +	if (IS_ERR(base))

>> +		return PTR_ERR(base);

>> +

>> +	pci->dbi_base = base;

>>  	pci->dev = dev;

>>  	pci->ops = &ks_pcie_dw_pcie_ops;

>>  

>> -- 

>> 2.17.1

>>
Lorenzo Pieralisi April 15, 2019, 12:25 p.m. UTC | #3 
On Mon, Apr 15, 2019 at 11:04:10AM +0530, Kishon Vijay Abraham I wrote:
> Hi Bjorn,

> 

> On 13/04/19 8:00 PM, Bjorn Helgaas wrote:

> > On Mon, Mar 25, 2019 at 02:04:41PM +0530, Kishon Vijay Abraham I wrote:

> >> No functional change. Move host specific platform_get_resource to

> >> ks_add_pcie_port and the common platform_get_resource (applicable

> >> to both host and endpoint) to probe. This is in preparation for

> >> adding endpoint support to pci-keystone driver.

> > 

> > This seems to move platform_get_resource() *from* (not *to*)

> > ks_add_pcie_port().

> 

> Maybe I should have mentioned "Keep host specific platform_get_resource() in

> ks_add_pcie_port() and move the common platform_get_resource() (applicable

> to both host and endpoint) to probe()"


Commit log updated, pushed out pci/keystone again.

Thanks,
Lorenzo

> > You seem to be making a distinction in the commit log between (1) a

> > resource that's only used for host mode, and (2) a common resource

> > that's used for both host and endpoint mode.  But I don't see that

> > distinction in the patch, so it's a little confusing to mention it in

> > the commit log.

> > 

> > It must make endpoint support easier, but I can't quite connect the

> > dots yet.  Maybe endpoint will also use ks_pcie_add_pcie_port(), but

> > will have a separate .probe() function that doesn't look up the

> > resource that's specific to host mode?

> 

> No ks_pcie_add_pcie_port() is specific to host mode, so "config" resource is

> kept there whereas "dbics" and "app" resources are common to both host mode and

> device mode, so they are moved to probe().

> 

> Thanks

> Kishon

> 

> > 

> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> >> ---

> >>  drivers/pci/controller/dwc/pci-keystone.c | 27 +++++++++++++----------

> >>  1 file changed, 15 insertions(+), 12 deletions(-)

> >>

> >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c

> >> index 5eebef9b9ada..95997885a05c 100644

> >> --- a/drivers/pci/controller/dwc/pci-keystone.c

> >> +++ b/drivers/pci/controller/dwc/pci-keystone.c

> >> @@ -806,11 +806,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,

> >>  	struct resource *res;

> >>  	int ret;

> >>  

> >> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");

> >> -	pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);

> >> -	if (IS_ERR(pci->dbi_base))

> >> -		return PTR_ERR(pci->dbi_base);

> >> -

> >>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");

> >>  	pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);

> >>  	if (IS_ERR(pp->va_cfg0_base))

> >> @@ -818,13 +813,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,

> >>  

> >>  	pp->va_cfg1_base = pp->va_cfg0_base;

> >>  

> >> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");

> >> -	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);

> >> -	if (IS_ERR(ks_pcie->va_app_base))

> >> -		return PTR_ERR(ks_pcie->va_app_base);

> >> -

> >> -	ks_pcie->app = *res;

> >> -

> >>  	pp->ops = &ks_pcie_host_ops;

> >>  	ret = dw_pcie_host_init(pp);

> >>  	if (ret) {

> >> @@ -895,6 +883,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)

> >>  	struct dw_pcie *pci;

> >>  	struct keystone_pcie *ks_pcie;

> >>  	struct device_link **link;

> >> +	struct resource *res;

> >> +	void __iomem *base;

> >>  	u32 num_viewport;

> >>  	struct phy **phy;

> >>  	u32 num_lanes;

> >> @@ -911,6 +901,19 @@ static int __init ks_pcie_probe(struct platform_device *pdev)

> >>  	if (!pci)

> >>  		return -ENOMEM;

> >>  

> >> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");

> >> +	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);

> >> +	if (IS_ERR(ks_pcie->va_app_base))

> >> +		return PTR_ERR(ks_pcie->va_app_base);

> >> +

> >> +	ks_pcie->app = *res;

> >> +

> >> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");

> >> +	base = devm_pci_remap_cfg_resource(dev, res);

> >> +	if (IS_ERR(base))

> >> +		return PTR_ERR(base);

> >> +

> >> +	pci->dbi_base = base;

> >>  	pci->dev = dev;

> >>  	pci->ops = &ks_pcie_dw_pcie_ops;

> >>  

> >> -- 

> >> 2.17.1

> >>
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 5eebef9b9ada..95997885a05c 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -806,11 +806,6 @@  static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,
 	struct resource *res;
 	int ret;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");
-	pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
-	if (IS_ERR(pci->dbi_base))
-		return PTR_ERR(pci->dbi_base);
-
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
 	pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
 	if (IS_ERR(pp->va_cfg0_base))
@@ -818,13 +813,6 @@  static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,
 
 	pp->va_cfg1_base = pp->va_cfg0_base;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
-	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(ks_pcie->va_app_base))
-		return PTR_ERR(ks_pcie->va_app_base);
-
-	ks_pcie->app = *res;
-
 	pp->ops = &ks_pcie_host_ops;
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
@@ -895,6 +883,8 @@  static int __init ks_pcie_probe(struct platform_device *pdev)
 	struct dw_pcie *pci;
 	struct keystone_pcie *ks_pcie;
 	struct device_link **link;
+	struct resource *res;
+	void __iomem *base;
 	u32 num_viewport;
 	struct phy **phy;
 	u32 num_lanes;
@@ -911,6 +901,19 @@  static int __init ks_pcie_probe(struct platform_device *pdev)
 	if (!pci)
 		return -ENOMEM;
 
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
+	ks_pcie->va_app_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ks_pcie->va_app_base))
+		return PTR_ERR(ks_pcie->va_app_base);
+
+	ks_pcie->app = *res;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbics");
+	base = devm_pci_remap_cfg_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	pci->dbi_base = base;
 	pci->dev = dev;
 	pci->ops = &ks_pcie_dw_pcie_ops;