From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933025AbbCPSBG (ORCPT ); Mon, 16 Mar 2015 14:01:06 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:45868 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932899AbbCPSBC convert rfc822-to-8bit (ORCPT ); Mon, 16 Mar 2015 14:01:02 -0400 Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: [PATCH v2 4/5] PCI: designware: Add disable IO support From: Kumar Gala In-Reply-To: <1426515635-9466-5-git-send-email-gabriel.fernandez@linaro.org> Date: Mon, 16 Mar 2015 13:00:51 -0500 Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Srinivas Kandagatla , Maxime Coquelin , Patrice Chotard , Russell King , Bjorn Helgaas , Mohit Kumar , Jingoo Han , Lucas Stach , Fabrice Gasnier , Kishon Vijay Abraham I , Andrew Morton , "David S. Miller" , Greg KH , Mauro Carvalho Chehab , Joe Perches , Tejun Heo , Arnd Bergmann , Viresh Kumar , Thierry Reding , Phil Edworthy , Minghuan Lian , Tanmay Inamdar , m-karicheri2@ti.com, Sachin Kamat , Andrew Lunn , Liviu Dudau , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@stlinux.com, linux-pci@vger.kernel.org, Lee Jones , Gabriel Fernandez Content-Transfer-Encoding: 8BIT Message-Id: <582D947C-1229-4DD9-BC82-812D1560C49E@codeaurora.org> References: <1426515635-9466-1-git-send-email-gabriel.fernandez@linaro.org> <1426515635-9466-5-git-send-email-gabriel.fernandez@linaro.org> To: Gabriel FERNANDEZ X-Mailer: Apple Mail (2.1878.6) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mar 16, 2015, at 9:20 AM, Gabriel FERNANDEZ wrote: > ST sti SoCs PCIe IPs are built around DesignWare IP Core. > But in these SoCs PCIe IP doesn't support IO. > > This patch adds the possibility to disable it through > a DT property, by creating an empty IO window and by > removing PCI_COMMAND_IO from the setup register. > > Signed-off-by: Fabrice Gasnier > Signed-off-by: Gabriel Fernandez > --- > .../devicetree/bindings/pci/designware-pcie.txt | 2 ++ > drivers/pci/host/pcie-designware.c | 24 ++++++++++++++++++++-- > drivers/pci/host/pcie-designware.h | 1 + > 3 files changed, 25 insertions(+), 2 deletions(-) Why not just update the code such that if the ranges doesn’t have an IO space rather than introducing a new DT property? - k > > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt > index 9f4faa8..40544d4 100644 > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt > @@ -26,3 +26,5 @@ Optional properties: > - bus-range: PCI bus numbers covered (it is recommended for new devicetrees to > specify this property, to keep backwards compatibility a range of 0x00-0xff > is assumed if not present) > +- disable_io_support: set this property for PCIe host controller without IO > + port access > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 1f4ea6f..f9d70f5 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -471,6 +471,9 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > return -EINVAL; > } > > + pp->disable_io_support = of_property_read_bool(np, > + "disable_io_support"); > + > if (IS_ENABLED(CONFIG_PCI_MSI)) { > if (!pp->ops->msi_host_init) { > pp->irq_domain = irq_domain_add_linear(pp->dev->of_node, > @@ -704,6 +707,7 @@ static struct pci_ops dw_pcie_ops = { > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > { > struct pcie_port *pp; > + struct resource *res; > > pp = sys_to_pcie(sys); > > @@ -719,6 +723,18 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset); > pci_add_resource(&sys->resources, &pp->busn); > > + if (pp->disable_io_support) { > + /* This PCIe controller does not support IO, set an empty one */ > + res = devm_kzalloc(pp->dev, sizeof(*res), GFP_KERNEL); > + if (res) { > + res->start = 0; > + res->end = 0; > + res->name = "PCIe empty IO space"; > + res->flags = IORESOURCE_IO; > + pci_add_resource(&sys->resources, res); Do we really need an empty resource? What happens if we just dont have one? > + } > + } > + > return 1; > } > > @@ -822,8 +838,12 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > /* setup command register */ > dw_pcie_readl_rc(pp, PCI_COMMAND, &val); > val &= 0xffff0000; > - val |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | > - PCI_COMMAND_MASTER | PCI_COMMAND_SERR; > + > + if (!pp->disable_io_support) > + val |= PCI_COMMAND_IO; > + > + val |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_SERR; > + > dw_pcie_writel_rc(pp, val, PCI_COMMAND); > } > > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > index d0bbd27..027045d 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -52,6 +52,7 @@ struct pcie_port { > int msi_irq; > struct irq_domain *irq_domain; > unsigned long msi_data; > + bool disable_io_support; > DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > }; > > -- > 1.9.1 > -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kumar Gala Subject: Re: [PATCH v2 4/5] PCI: designware: Add disable IO support Date: Mon, 16 Mar 2015 13:00:51 -0500 Message-ID: <582D947C-1229-4DD9-BC82-812D1560C49E@codeaurora.org> References: <1426515635-9466-1-git-send-email-gabriel.fernandez@linaro.org> <1426515635-9466-5-git-send-email-gabriel.fernandez@linaro.org> Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1426515635-9466-5-git-send-email-gabriel.fernandez@linaro.org> Sender: linux-pci-owner@vger.kernel.org To: Gabriel FERNANDEZ Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Srinivas Kandagatla , Maxime Coquelin , Patrice Chotard , Russell King , Bjorn Helgaas , Mohit Kumar , Jingoo Han , Lucas Stach , Fabrice Gasnier , Kishon Vijay Abraham I , Andrew Morton , "David S. Miller" , Greg KH , Mauro Carvalho Chehab , Joe Perches , Tejun Heo , Arnd Bergmann , Viresh Kumar , Thierry Reding List-Id: devicetree@vger.kernel.org On Mar 16, 2015, at 9:20 AM, Gabriel FERNANDEZ wrote: > ST sti SoCs PCIe IPs are built around DesignWare IP Core. > But in these SoCs PCIe IP doesn't support IO. >=20 > This patch adds the possibility to disable it through > a DT property, by creating an empty IO window and by > removing PCI_COMMAND_IO from the setup register. >=20 > Signed-off-by: Fabrice Gasnier > Signed-off-by: Gabriel Fernandez > --- > .../devicetree/bindings/pci/designware-pcie.txt | 2 ++ > drivers/pci/host/pcie-designware.c | 24 +++++++++++++= +++++++-- > drivers/pci/host/pcie-designware.h | 1 + > 3 files changed, 25 insertions(+), 2 deletions(-) Why not just update the code such that if the ranges doesn=92t have an = IO space rather than introducing a new DT property? - k >=20 > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.tx= t b/Documentation/devicetree/bindings/pci/designware-pcie.txt > index 9f4faa8..40544d4 100644 > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt > @@ -26,3 +26,5 @@ Optional properties: > - bus-range: PCI bus numbers covered (it is recommended for new devic= etrees to > specify this property, to keep backwards compatibility a range of 0= x00-0xff > is assumed if not present) > +- disable_io_support: set this property for PCIe host controller wit= hout IO > + port access > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pc= ie-designware.c > index 1f4ea6f..f9d70f5 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -471,6 +471,9 @@ int __init dw_pcie_host_init(struct pcie_port *pp= ) > return -EINVAL; > } >=20 > + pp->disable_io_support =3D of_property_read_bool(np, > + "disable_io_support"); > + > if (IS_ENABLED(CONFIG_PCI_MSI)) { > if (!pp->ops->msi_host_init) { > pp->irq_domain =3D irq_domain_add_linear(pp->dev->of_node, > @@ -704,6 +707,7 @@ static struct pci_ops dw_pcie_ops =3D { > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > { > struct pcie_port *pp; > + struct resource *res; >=20 > pp =3D sys_to_pcie(sys); >=20 > @@ -719,6 +723,18 @@ static int dw_pcie_setup(int nr, struct pci_sys_= data *sys) > pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset); > pci_add_resource(&sys->resources, &pp->busn); >=20 > + if (pp->disable_io_support) { > + /* This PCIe controller does not support IO, set an empty one */ > + res =3D devm_kzalloc(pp->dev, sizeof(*res), GFP_KERNEL); > + if (res) { > + res->start =3D 0; > + res->end =3D 0; > + res->name =3D "PCIe empty IO space"; > + res->flags =3D IORESOURCE_IO; > + pci_add_resource(&sys->resources, res); Do we really need an empty resource? What happens if we just dont have= one? > + } > + } > + > return 1; > } >=20 > @@ -822,8 +838,12 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > /* setup command register */ > dw_pcie_readl_rc(pp, PCI_COMMAND, &val); > val &=3D 0xffff0000; > - val |=3D PCI_COMMAND_IO | PCI_COMMAND_MEMORY | > - PCI_COMMAND_MASTER | PCI_COMMAND_SERR; > + > + if (!pp->disable_io_support) > + val |=3D PCI_COMMAND_IO; > + > + val |=3D PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_SERR= ; > + > dw_pcie_writel_rc(pp, val, PCI_COMMAND); > } >=20 > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pc= ie-designware.h > index d0bbd27..027045d 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -52,6 +52,7 @@ struct pcie_port { > int msi_irq; > struct irq_domain *irq_domain; > unsigned long msi_data; > + bool disable_io_support; > DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > }; >=20 > --=20 > 1.9.1 >=20 --=20 Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora For= um, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: galak@codeaurora.org (Kumar Gala) Date: Mon, 16 Mar 2015 13:00:51 -0500 Subject: [PATCH v2 4/5] PCI: designware: Add disable IO support In-Reply-To: <1426515635-9466-5-git-send-email-gabriel.fernandez@linaro.org> References: <1426515635-9466-1-git-send-email-gabriel.fernandez@linaro.org> <1426515635-9466-5-git-send-email-gabriel.fernandez@linaro.org> Message-ID: <582D947C-1229-4DD9-BC82-812D1560C49E@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mar 16, 2015, at 9:20 AM, Gabriel FERNANDEZ wrote: > ST sti SoCs PCIe IPs are built around DesignWare IP Core. > But in these SoCs PCIe IP doesn't support IO. > > This patch adds the possibility to disable it through > a DT property, by creating an empty IO window and by > removing PCI_COMMAND_IO from the setup register. > > Signed-off-by: Fabrice Gasnier > Signed-off-by: Gabriel Fernandez > --- > .../devicetree/bindings/pci/designware-pcie.txt | 2 ++ > drivers/pci/host/pcie-designware.c | 24 ++++++++++++++++++++-- > drivers/pci/host/pcie-designware.h | 1 + > 3 files changed, 25 insertions(+), 2 deletions(-) Why not just update the code such that if the ranges doesn?t have an IO space rather than introducing a new DT property? - k > > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt > index 9f4faa8..40544d4 100644 > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt > @@ -26,3 +26,5 @@ Optional properties: > - bus-range: PCI bus numbers covered (it is recommended for new devicetrees to > specify this property, to keep backwards compatibility a range of 0x00-0xff > is assumed if not present) > +- disable_io_support: set this property for PCIe host controller without IO > + port access > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 1f4ea6f..f9d70f5 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -471,6 +471,9 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > return -EINVAL; > } > > + pp->disable_io_support = of_property_read_bool(np, > + "disable_io_support"); > + > if (IS_ENABLED(CONFIG_PCI_MSI)) { > if (!pp->ops->msi_host_init) { > pp->irq_domain = irq_domain_add_linear(pp->dev->of_node, > @@ -704,6 +707,7 @@ static struct pci_ops dw_pcie_ops = { > static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > { > struct pcie_port *pp; > + struct resource *res; > > pp = sys_to_pcie(sys); > > @@ -719,6 +723,18 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys) > pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset); > pci_add_resource(&sys->resources, &pp->busn); > > + if (pp->disable_io_support) { > + /* This PCIe controller does not support IO, set an empty one */ > + res = devm_kzalloc(pp->dev, sizeof(*res), GFP_KERNEL); > + if (res) { > + res->start = 0; > + res->end = 0; > + res->name = "PCIe empty IO space"; > + res->flags = IORESOURCE_IO; > + pci_add_resource(&sys->resources, res); Do we really need an empty resource? What happens if we just dont have one? > + } > + } > + > return 1; > } > > @@ -822,8 +838,12 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > /* setup command register */ > dw_pcie_readl_rc(pp, PCI_COMMAND, &val); > val &= 0xffff0000; > - val |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | > - PCI_COMMAND_MASTER | PCI_COMMAND_SERR; > + > + if (!pp->disable_io_support) > + val |= PCI_COMMAND_IO; > + > + val |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_SERR; > + > dw_pcie_writel_rc(pp, val, PCI_COMMAND); > } > > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > index d0bbd27..027045d 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -52,6 +52,7 @@ struct pcie_port { > int msi_irq; > struct irq_domain *irq_domain; > unsigned long msi_data; > + bool disable_io_support; > DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > }; > > -- > 1.9.1 > -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project