PCI host drivers have already matched on compatible strings, so checking
device_type is redundant. Also, device_type is considered deprecated for
FDT though we've still been requiring it for PCI hosts as it is useful
for finding PCI buses.
Cc: Will Deacon <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Alan Douglas <[email protected]>
Cc: Subrahmanya Lingappa <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Rob Herring <[email protected]>
---
drivers/pci/controller/pci-host-common.c | 8 --------
drivers/pci/controller/pcie-cadence-host.c | 7 -------
drivers/pci/controller/pcie-mobiveil.c | 7 -------
drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
drivers/pci/controller/pcie-xilinx.c | 7 -------
5 files changed, 38 deletions(-)
diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index d8f10451f273..c742881b5061 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -58,9 +58,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
int pci_host_common_probe(struct platform_device *pdev,
struct pci_ecam_ops *ops)
{
- const char *type;
struct device *dev = &pdev->dev;
- struct device_node *np = dev->of_node;
struct pci_host_bridge *bridge;
struct pci_config_window *cfg;
struct list_head resources;
@@ -70,12 +68,6 @@ int pci_host_common_probe(struct platform_device *pdev,
if (!bridge)
return -ENOMEM;
- type = of_get_property(np, "device_type", NULL);
- if (!type || strcmp(type, "pci")) {
- dev_err(dev, "invalid \"device_type\" %s\n", type);
- return -EINVAL;
- }
-
of_pci_check_probe_only();
/* Parse and map our Configuration Space windows */
diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
index ec394f6a19c8..97e251090b4f 100644
--- a/drivers/pci/controller/pcie-cadence-host.c
+++ b/drivers/pci/controller/pcie-cadence-host.c
@@ -235,7 +235,6 @@ static int cdns_pcie_host_init(struct device *dev,
static int cdns_pcie_host_probe(struct platform_device *pdev)
{
- const char *type;
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct pci_host_bridge *bridge;
@@ -268,12 +267,6 @@ static int cdns_pcie_host_probe(struct platform_device *pdev)
rc->device_id = 0xffff;
of_property_read_u16(np, "device-id", &rc->device_id);
- type = of_get_property(np, "device_type", NULL);
- if (!type || strcmp(type, "pci")) {
- dev_err(dev, "invalid \"device_type\" %s\n", type);
- return -EINVAL;
- }
-
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
pcie->reg_base = devm_ioremap_resource(dev, res);
if (IS_ERR(pcie->reg_base)) {
diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
index a939e8d31735..77052a0712d0 100644
--- a/drivers/pci/controller/pcie-mobiveil.c
+++ b/drivers/pci/controller/pcie-mobiveil.c
@@ -301,13 +301,6 @@ static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
struct platform_device *pdev = pcie->pdev;
struct device_node *node = dev->of_node;
struct resource *res;
- const char *type;
-
- type = of_get_property(node, "device_type", NULL);
- if (!type || strcmp(type, "pci")) {
- dev_err(dev, "invalid \"device_type\" %s\n", type);
- return -EINVAL;
- }
/* map config resource */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index fb32840ce8e6..81538d77f790 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -777,16 +777,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
struct platform_device *pdev)
{
struct device *dev = pcie->dev;
- struct device_node *node = dev->of_node;
struct resource *res;
- const char *type;
-
- /* Check for device type */
- type = of_get_property(node, "device_type", NULL);
- if (!type || strcmp(type, "pci")) {
- dev_err(dev, "invalid \"device_type\" %s\n", type);
- return -EINVAL;
- }
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg");
pcie->breg_base = devm_ioremap_resource(dev, res);
diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index 7b1389d8e2a5..9bd1a35cd5d8 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -574,15 +574,8 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
struct device *dev = port->dev;
struct device_node *node = dev->of_node;
struct resource regs;
- const char *type;
int err;
- type = of_get_property(node, "device_type", NULL);
- if (!type || strcmp(type, "pci")) {
- dev_err(dev, "invalid \"device_type\" %s\n", type);
- return -EINVAL;
- }
-
err = of_address_to_resource(node, 0, ®s);
if (err) {
dev_err(dev, "missing \"reg\" property\n");
--
2.17.1
Hi,
On 29 August 2018 19:35, Rob Herring wrote:
> To: Lorenzo Pieralisi <[email protected]>
> Cc: [email protected]; Will Deacon <[email protected]>; Bjorn Helgaas <[email protected]>; Alan Douglas
> <[email protected]>; Subrahmanya Lingappa <[email protected]>; Michal Simek <[email protected]>; linux-
> [email protected]; [email protected]
> Subject: [PATCH] PCI: remove unnecessary check of device_type == pci
>
> EXTERNAL MAIL
>
>
> PCI host drivers have already matched on compatible strings, so checking
> device_type is redundant. Also, device_type is considered deprecated for
> FDT though we've still been requiring it for PCI hosts as it is useful
> for finding PCI buses.
>
> Cc: Will Deacon <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alan Douglas <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Rob Herring <[email protected]>
> ---
> drivers/pci/controller/pci-host-common.c | 8 --------
> drivers/pci/controller/pcie-cadence-host.c | 7 -------
> drivers/pci/controller/pcie-mobiveil.c | 7 -------
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
> drivers/pci/controller/pcie-xilinx.c | 7 -------
> 5 files changed, 38 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d8f10451f273..c742881b5061 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -58,9 +58,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
> int pci_host_common_probe(struct platform_device *pdev,
> struct pci_ecam_ops *ops)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> struct pci_config_window *cfg;
> struct list_head resources;
> @@ -70,12 +68,6 @@ int pci_host_common_probe(struct platform_device *pdev,
> if (!bridge)
> return -ENOMEM;
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> of_pci_check_probe_only();
>
> /* Parse and map our Configuration Space windows */
> diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
> index ec394f6a19c8..97e251090b4f 100644
> --- a/drivers/pci/controller/pcie-cadence-host.c
> +++ b/drivers/pci/controller/pcie-cadence-host.c
> @@ -235,7 +235,6 @@ static int cdns_pcie_host_init(struct device *dev,
>
> static int cdns_pcie_host_probe(struct platform_device *pdev)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> @@ -268,12 +267,6 @@ static int cdns_pcie_host_probe(struct platform_device *pdev)
> rc->device_id = 0xffff;
> of_property_read_u16(np, "device-id", &rc->device_id);
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> pcie->reg_base = devm_ioremap_resource(dev, res);
> if (IS_ERR(pcie->reg_base)) {
With regards to drivers/pci/controller/pcie-cadence-host.c
Acked-by: Alan Douglas <[email protected]>
Hi,
with regards to : drivers/pci/controller/pcie-mobiveil.c
Acked-by: Subrahmaya Lingappa <[email protected]>
Thanks.
On Thu, Aug 30, 2018 at 12:04 AM Rob Herring <[email protected]> wrote:
>
> PCI host drivers have already matched on compatible strings, so checking
> device_type is redundant. Also, device_type is considered deprecated for
> FDT though we've still been requiring it for PCI hosts as it is useful
> for finding PCI buses.
>
> Cc: Will Deacon <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alan Douglas <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Rob Herring <[email protected]>
> ---
> drivers/pci/controller/pci-host-common.c | 8 --------
> drivers/pci/controller/pcie-cadence-host.c | 7 -------
> drivers/pci/controller/pcie-mobiveil.c | 7 -------
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
> drivers/pci/controller/pcie-xilinx.c | 7 -------
> 5 files changed, 38 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d8f10451f273..c742881b5061 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -58,9 +58,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
> int pci_host_common_probe(struct platform_device *pdev,
> struct pci_ecam_ops *ops)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> struct pci_config_window *cfg;
> struct list_head resources;
> @@ -70,12 +68,6 @@ int pci_host_common_probe(struct platform_device *pdev,
> if (!bridge)
> return -ENOMEM;
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> of_pci_check_probe_only();
>
> /* Parse and map our Configuration Space windows */
> diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
> index ec394f6a19c8..97e251090b4f 100644
> --- a/drivers/pci/controller/pcie-cadence-host.c
> +++ b/drivers/pci/controller/pcie-cadence-host.c
> @@ -235,7 +235,6 @@ static int cdns_pcie_host_init(struct device *dev,
>
> static int cdns_pcie_host_probe(struct platform_device *pdev)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> @@ -268,12 +267,6 @@ static int cdns_pcie_host_probe(struct platform_device *pdev)
> rc->device_id = 0xffff;
> of_property_read_u16(np, "device-id", &rc->device_id);
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> pcie->reg_base = devm_ioremap_resource(dev, res);
> if (IS_ERR(pcie->reg_base)) {
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a939e8d31735..77052a0712d0 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -301,13 +301,6 @@ static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> struct platform_device *pdev = pcie->pdev;
> struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> /* map config resource */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index fb32840ce8e6..81538d77f790 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -777,16 +777,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> struct platform_device *pdev)
> {
> struct device *dev = pcie->dev;
> - struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - /* Check for device type */
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg");
> pcie->breg_base = devm_ioremap_resource(dev, res);
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..9bd1a35cd5d8 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -574,15 +574,8 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
> struct device *dev = port->dev;
> struct device_node *node = dev->of_node;
> struct resource regs;
> - const char *type;
> int err;
>
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> err = of_address_to_resource(node, 0, ®s);
> if (err) {
> dev_err(dev, "missing \"reg\" property\n");
> --
> 2.17.1
>
On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
> PCI host drivers have already matched on compatible strings, so checking
> device_type is redundant. Also, device_type is considered deprecated for
> FDT though we've still been requiring it for PCI hosts as it is useful
> for finding PCI buses.
Hi Rob,
I have no problem with the patch per-se, I can't parse though the second
paragraph, in particular what you mean by "useful for finding PCI
buses".
What about the corresponding dts files ? I suspect we had better leave
them alone lest we can trigger regressions with new dts and older
kernels.
Thanks,
Lorenzo
> Cc: Will Deacon <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alan Douglas <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Rob Herring <[email protected]>
> ---
> drivers/pci/controller/pci-host-common.c | 8 --------
> drivers/pci/controller/pcie-cadence-host.c | 7 -------
> drivers/pci/controller/pcie-mobiveil.c | 7 -------
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
> drivers/pci/controller/pcie-xilinx.c | 7 -------
> 5 files changed, 38 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d8f10451f273..c742881b5061 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -58,9 +58,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
> int pci_host_common_probe(struct platform_device *pdev,
> struct pci_ecam_ops *ops)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> struct pci_config_window *cfg;
> struct list_head resources;
> @@ -70,12 +68,6 @@ int pci_host_common_probe(struct platform_device *pdev,
> if (!bridge)
> return -ENOMEM;
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> of_pci_check_probe_only();
>
> /* Parse and map our Configuration Space windows */
> diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
> index ec394f6a19c8..97e251090b4f 100644
> --- a/drivers/pci/controller/pcie-cadence-host.c
> +++ b/drivers/pci/controller/pcie-cadence-host.c
> @@ -235,7 +235,6 @@ static int cdns_pcie_host_init(struct device *dev,
>
> static int cdns_pcie_host_probe(struct platform_device *pdev)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> @@ -268,12 +267,6 @@ static int cdns_pcie_host_probe(struct platform_device *pdev)
> rc->device_id = 0xffff;
> of_property_read_u16(np, "device-id", &rc->device_id);
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> pcie->reg_base = devm_ioremap_resource(dev, res);
> if (IS_ERR(pcie->reg_base)) {
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a939e8d31735..77052a0712d0 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -301,13 +301,6 @@ static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> struct platform_device *pdev = pcie->pdev;
> struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> /* map config resource */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index fb32840ce8e6..81538d77f790 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -777,16 +777,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> struct platform_device *pdev)
> {
> struct device *dev = pcie->dev;
> - struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - /* Check for device type */
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg");
> pcie->breg_base = devm_ioremap_resource(dev, res);
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..9bd1a35cd5d8 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -574,15 +574,8 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
> struct device *dev = port->dev;
> struct device_node *node = dev->of_node;
> struct resource regs;
> - const char *type;
> int err;
>
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> err = of_address_to_resource(node, 0, ®s);
> if (err) {
> dev_err(dev, "missing \"reg\" property\n");
> --
> 2.17.1
>
On Thu, Sep 13, 2018 at 7:51 AM Lorenzo Pieralisi
<[email protected]> wrote:
>
> On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
> > PCI host drivers have already matched on compatible strings, so checking
> > device_type is redundant. Also, device_type is considered deprecated for
> > FDT though we've still been requiring it for PCI hosts as it is useful
> > for finding PCI buses.
>
> Hi Rob,
>
> I have no problem with the patch per-se, I can't parse though the second
> paragraph, in particular what you mean by "useful for finding PCI
> buses".
Device_type is the only generic way we can identify PCI buses which is
useful for dtc for example to do PCI binding checks. Maybe we can use
the node name which is what I'm doing for cpu nodes, but we've been
less consistent about that for PCI.
>
> What about the corresponding dts files ? I suspect we had better leave
> them alone lest we can trigger regressions with new dts and older
> kernels.
Yes. it will take some time before we can remove from dts files. Or
maybe only new ones change.
Rob
On Tue, Sep 18, 2018 at 12:05:40PM -0700, Rob Herring wrote:
> On Thu, Sep 13, 2018 at 7:51 AM Lorenzo Pieralisi
> <[email protected]> wrote:
> >
> > On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
> > > PCI host drivers have already matched on compatible strings, so checking
> > > device_type is redundant. Also, device_type is considered deprecated for
> > > FDT though we've still been requiring it for PCI hosts as it is useful
> > > for finding PCI buses.
> >
> > Hi Rob,
> >
> > I have no problem with the patch per-se, I can't parse though the second
> > paragraph, in particular what you mean by "useful for finding PCI
> > buses".
>
> Device_type is the only generic way we can identify PCI buses which is
> useful for dtc for example to do PCI binding checks. Maybe we can use
> the node name which is what I'm doing for cpu nodes, but we've been
> less consistent about that for PCI.
>
> >
> > What about the corresponding dts files ? I suspect we had better leave
> > them alone lest we can trigger regressions with new dts and older
> > kernels.
>
> Yes. it will take some time before we can remove from dts files. Or
> maybe only new ones change.
OK, thanks. What about PCI host controllers bindings ? I assume we leave
the bindings (that enforce device_type) unchanged too.
If Will and Michal have no objections I will queue this patch as-is
for v4.20.
Thanks,
Lorenzo
On Wed, Sep 19, 2018 at 05:33:01PM +0100, Lorenzo Pieralisi wrote:
> On Tue, Sep 18, 2018 at 12:05:40PM -0700, Rob Herring wrote:
> > On Thu, Sep 13, 2018 at 7:51 AM Lorenzo Pieralisi
> > <[email protected]> wrote:
> > >
> > > On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
> > > > PCI host drivers have already matched on compatible strings, so checking
> > > > device_type is redundant. Also, device_type is considered deprecated for
> > > > FDT though we've still been requiring it for PCI hosts as it is useful
> > > > for finding PCI buses.
> > >
> > > Hi Rob,
> > >
> > > I have no problem with the patch per-se, I can't parse though the second
> > > paragraph, in particular what you mean by "useful for finding PCI
> > > buses".
> >
> > Device_type is the only generic way we can identify PCI buses which is
> > useful for dtc for example to do PCI binding checks. Maybe we can use
> > the node name which is what I'm doing for cpu nodes, but we've been
> > less consistent about that for PCI.
> >
> > >
> > > What about the corresponding dts files ? I suspect we had better leave
> > > them alone lest we can trigger regressions with new dts and older
> > > kernels.
> >
> > Yes. it will take some time before we can remove from dts files. Or
> > maybe only new ones change.
>
> OK, thanks. What about PCI host controllers bindings ? I assume we leave
> the bindings (that enforce device_type) unchanged too.
>
> If Will and Michal have no objections I will queue this patch as-is
> for v4.20.
Fine by me!
Will
On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
> PCI host drivers have already matched on compatible strings, so checking
> device_type is redundant. Also, device_type is considered deprecated for
> FDT though we've still been requiring it for PCI hosts as it is useful
> for finding PCI buses.
>
> Cc: Will Deacon <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alan Douglas <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Rob Herring <[email protected]>
> ---
> drivers/pci/controller/pci-host-common.c | 8 --------
> drivers/pci/controller/pcie-cadence-host.c | 7 -------
> drivers/pci/controller/pcie-mobiveil.c | 7 -------
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
> drivers/pci/controller/pcie-xilinx.c | 7 -------
> 5 files changed, 38 deletions(-)
Hi Michal,
are you OK with these changes ?
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d8f10451f273..c742881b5061 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -58,9 +58,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
> int pci_host_common_probe(struct platform_device *pdev,
> struct pci_ecam_ops *ops)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> struct pci_config_window *cfg;
> struct list_head resources;
> @@ -70,12 +68,6 @@ int pci_host_common_probe(struct platform_device *pdev,
> if (!bridge)
> return -ENOMEM;
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> of_pci_check_probe_only();
>
> /* Parse and map our Configuration Space windows */
> diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
> index ec394f6a19c8..97e251090b4f 100644
> --- a/drivers/pci/controller/pcie-cadence-host.c
> +++ b/drivers/pci/controller/pcie-cadence-host.c
> @@ -235,7 +235,6 @@ static int cdns_pcie_host_init(struct device *dev,
>
> static int cdns_pcie_host_probe(struct platform_device *pdev)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> @@ -268,12 +267,6 @@ static int cdns_pcie_host_probe(struct platform_device *pdev)
> rc->device_id = 0xffff;
> of_property_read_u16(np, "device-id", &rc->device_id);
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> pcie->reg_base = devm_ioremap_resource(dev, res);
> if (IS_ERR(pcie->reg_base)) {
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a939e8d31735..77052a0712d0 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -301,13 +301,6 @@ static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> struct platform_device *pdev = pcie->pdev;
> struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> /* map config resource */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index fb32840ce8e6..81538d77f790 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -777,16 +777,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> struct platform_device *pdev)
> {
> struct device *dev = pcie->dev;
> - struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - /* Check for device type */
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg");
> pcie->breg_base = devm_ioremap_resource(dev, res);
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..9bd1a35cd5d8 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -574,15 +574,8 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
> struct device *dev = port->dev;
> struct device_node *node = dev->of_node;
> struct resource regs;
> - const char *type;
> int err;
>
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> err = of_address_to_resource(node, 0, ®s);
> if (err) {
> dev_err(dev, "missing \"reg\" property\n");
> --
> 2.17.1
>
Hi Lorenzo,
On 21.9.2018 11:29, Lorenzo Pieralisi wrote:
> On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
>> PCI host drivers have already matched on compatible strings, so checking
>> device_type is redundant. Also, device_type is considered deprecated for
>> FDT though we've still been requiring it for PCI hosts as it is useful
>> for finding PCI buses.
>>
>> Cc: Will Deacon <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Alan Douglas <[email protected]>
>> Cc: Subrahmanya Lingappa <[email protected]>
>> Cc: Michal Simek <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Rob Herring <[email protected]>
>> ---
>> drivers/pci/controller/pci-host-common.c | 8 --------
>> drivers/pci/controller/pcie-cadence-host.c | 7 -------
>> drivers/pci/controller/pcie-mobiveil.c | 7 -------
>> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
>> drivers/pci/controller/pcie-xilinx.c | 7 -------
>> 5 files changed, 38 deletions(-)
>
> Hi Michal,
>
> are you OK with these changes ?
This change is fine.
Acked-by: Michal Simek <[email protected]>
Thanks,
Michal
On Wed, Aug 29, 2018 at 01:34:40PM -0500, Rob Herring wrote:
> PCI host drivers have already matched on compatible strings, so checking
> device_type is redundant. Also, device_type is considered deprecated for
> FDT though we've still been requiring it for PCI hosts as it is useful
> for finding PCI buses.
>
> Cc: Will Deacon <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alan Douglas <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Rob Herring <[email protected]>
> ---
> drivers/pci/controller/pci-host-common.c | 8 --------
> drivers/pci/controller/pcie-cadence-host.c | 7 -------
> drivers/pci/controller/pcie-mobiveil.c | 7 -------
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ---------
> drivers/pci/controller/pcie-xilinx.c | 7 -------
> 5 files changed, 38 deletions(-)
Applied to pci/controller-misc for v4.20, thanks.
Lorenzo
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d8f10451f273..c742881b5061 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -58,9 +58,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
> int pci_host_common_probe(struct platform_device *pdev,
> struct pci_ecam_ops *ops)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> struct pci_config_window *cfg;
> struct list_head resources;
> @@ -70,12 +68,6 @@ int pci_host_common_probe(struct platform_device *pdev,
> if (!bridge)
> return -ENOMEM;
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> of_pci_check_probe_only();
>
> /* Parse and map our Configuration Space windows */
> diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
> index ec394f6a19c8..97e251090b4f 100644
> --- a/drivers/pci/controller/pcie-cadence-host.c
> +++ b/drivers/pci/controller/pcie-cadence-host.c
> @@ -235,7 +235,6 @@ static int cdns_pcie_host_init(struct device *dev,
>
> static int cdns_pcie_host_probe(struct platform_device *pdev)
> {
> - const char *type;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct pci_host_bridge *bridge;
> @@ -268,12 +267,6 @@ static int cdns_pcie_host_probe(struct platform_device *pdev)
> rc->device_id = 0xffff;
> of_property_read_u16(np, "device-id", &rc->device_id);
>
> - type = of_get_property(np, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> pcie->reg_base = devm_ioremap_resource(dev, res);
> if (IS_ERR(pcie->reg_base)) {
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a939e8d31735..77052a0712d0 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -301,13 +301,6 @@ static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> struct platform_device *pdev = pcie->pdev;
> struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> /* map config resource */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index fb32840ce8e6..81538d77f790 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -777,16 +777,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> struct platform_device *pdev)
> {
> struct device *dev = pcie->dev;
> - struct device_node *node = dev->of_node;
> struct resource *res;
> - const char *type;
> -
> - /* Check for device type */
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg");
> pcie->breg_base = devm_ioremap_resource(dev, res);
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..9bd1a35cd5d8 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -574,15 +574,8 @@ static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
> struct device *dev = port->dev;
> struct device_node *node = dev->of_node;
> struct resource regs;
> - const char *type;
> int err;
>
> - type = of_get_property(node, "device_type", NULL);
> - if (!type || strcmp(type, "pci")) {
> - dev_err(dev, "invalid \"device_type\" %s\n", type);
> - return -EINVAL;
> - }
> -
> err = of_address_to_resource(node, 0, ®s);
> if (err) {
> dev_err(dev, "missing \"reg\" property\n");
> --
> 2.17.1
>