* RE: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-08-29 18:34 [PATCH] PCI: remove unnecessary check of device_type == pci Rob Herring
@ 2018-08-30 14:33 ` Alan Douglas
2018-09-11 7:26 ` Subrahmanya Lingappa
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Alan Douglas @ 2018-08-30 14:33 UTC (permalink / raw)
To: Rob Herring, Lorenzo Pieralisi
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Subrahmanya Lingappa,
Michal Simek, linux-pci, linux-arm-kernel
Hi,
On 29 August 2018 19:35, Rob Herring wrote:
> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: linux-kernel@vger.kernel.org; Will Deacon <will.deacon@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; Alan Douglas
> <adouglas@cadence.com>; Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>; Michal Simek <michal.simek@xilinx.com>; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> 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 <will.deacon@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alan Douglas <adouglas@cadence.com>
> Cc: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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 <adouglas@cadence.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-08-29 18:34 [PATCH] PCI: remove unnecessary check of device_type == pci Rob Herring
2018-08-30 14:33 ` Alan Douglas
@ 2018-09-11 7:26 ` Subrahmanya Lingappa
2018-09-13 14:51 ` Lorenzo Pieralisi
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Subrahmanya Lingappa @ 2018-09-11 7:26 UTC (permalink / raw)
To: robh
Cc: Lorenzo Pieralisi, linux-kernel, will.deacon, Bjorn Helgaas,
adouglas, Michal Simek, linux-pci, linux-arm-kernel
Hi,
with regards to : drivers/pci/controller/pcie-mobiveil.c
Acked-by: Subrahmaya Lingappa <l.subrahmanya@mobiveil.co.in>
Thanks.
On Thu, Aug 30, 2018 at 12:04 AM Rob Herring <robh@kernel.org> 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 <will.deacon@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alan Douglas <adouglas@cadence.com>
> Cc: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-08-29 18:34 [PATCH] PCI: remove unnecessary check of device_type == pci Rob Herring
2018-08-30 14:33 ` Alan Douglas
2018-09-11 7:26 ` Subrahmanya Lingappa
@ 2018-09-13 14:51 ` Lorenzo Pieralisi
2018-09-18 19:05 ` Rob Herring
2018-09-21 9:29 ` Lorenzo Pieralisi
2018-09-25 9:11 ` Lorenzo Pieralisi
4 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Pieralisi @ 2018-09-13 14:51 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, Michal Simek, linux-pci, linux-arm-kernel
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 <will.deacon@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alan Douglas <adouglas@cadence.com>
> Cc: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-09-13 14:51 ` Lorenzo Pieralisi
@ 2018-09-18 19:05 ` Rob Herring
2018-09-19 16:33 ` Lorenzo Pieralisi
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2018-09-18 19:05 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, Michal Simek, linux-pci,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
On Thu, Sep 13, 2018 at 7:51 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-09-18 19:05 ` Rob Herring
@ 2018-09-19 16:33 ` Lorenzo Pieralisi
2018-09-20 9:02 ` Will Deacon
0 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Pieralisi @ 2018-09-19 16:33 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, Michal Simek, linux-pci,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
On Tue, Sep 18, 2018 at 12:05:40PM -0700, Rob Herring wrote:
> On Thu, Sep 13, 2018 at 7:51 AM Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-09-19 16:33 ` Lorenzo Pieralisi
@ 2018-09-20 9:02 ` Will Deacon
0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2018-09-20 9:02 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Rob Herring, linux-kernel, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, Michal Simek, linux-pci,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
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
> > <lorenzo.pieralisi@arm.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-08-29 18:34 [PATCH] PCI: remove unnecessary check of device_type == pci Rob Herring
` (2 preceding siblings ...)
2018-09-13 14:51 ` Lorenzo Pieralisi
@ 2018-09-21 9:29 ` Lorenzo Pieralisi
2018-09-25 6:20 ` Michal Simek
2018-09-25 9:11 ` Lorenzo Pieralisi
4 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Pieralisi @ 2018-09-21 9:29 UTC (permalink / raw)
To: Rob Herring, Michal Simek
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, linux-pci, linux-arm-kernel
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 <will.deacon@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alan Douglas <adouglas@cadence.com>
> Cc: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-09-21 9:29 ` Lorenzo Pieralisi
@ 2018-09-25 6:20 ` Michal Simek
0 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2018-09-25 6:20 UTC (permalink / raw)
To: Lorenzo Pieralisi, Rob Herring, Michal Simek
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, linux-pci, linux-arm-kernel
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 <will.deacon@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Alan Douglas <adouglas@cadence.com>
>> Cc: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: linux-pci@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> 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 <michal.simek@xilinx.com>
Thanks,
Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: remove unnecessary check of device_type == pci
2018-08-29 18:34 [PATCH] PCI: remove unnecessary check of device_type == pci Rob Herring
` (3 preceding siblings ...)
2018-09-21 9:29 ` Lorenzo Pieralisi
@ 2018-09-25 9:11 ` Lorenzo Pieralisi
4 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Pieralisi @ 2018-09-25 9:11 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, Will Deacon, Bjorn Helgaas, Alan Douglas,
Subrahmanya Lingappa, Michal Simek, linux-pci, linux-arm-kernel
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 <will.deacon@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alan Douglas <adouglas@cadence.com>
> Cc: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread