Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1326689 > unrolled thread

[RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

Started byGabriele Paoloni <gabriele.paoloni@huawei.com>
First post2016-02-04 12:30 +0100
Last post2016-02-08 17:30 +0100
Articles 12 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-04 12:30 +0100
    [RFC PATCH 1/4] PCI: Designware: Add support for ACPI based controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-04 12:30 +0100
    Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers Arnd Bergmann <arnd@arndb.de> - 2016-02-04 14:20 +0100
      RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host  Controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-04 16:20 +0100
        Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers Arnd Bergmann <arnd@arndb.de> - 2016-02-04 17:10 +0100
          RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host  Controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-04 17:50 +0100
            Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers Arnd Bergmann <arnd@arndb.de> - 2016-02-04 18:10 +0100
              RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host  Controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-04 18:10 +0100
              RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host  Controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-08 14:20 +0100
                Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers Arnd Bergmann <arnd@arndb.de> - 2016-02-08 15:00 +0100
                  Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host  Controllers Sinan Kaya <okaya@codeaurora.org> - 2016-02-08 15:20 +0100
                    RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host  Controllers Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-02-08 17:30 +0100

#1326689 — [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-04 12:30 +0100
Subject[RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qYqgO-7X3-11@gated-at.bofh.it>
From: gabriele paoloni <gabriele.paoloni@huawei.com>

This patchset adds ACPI support for the HiSilicon Hip05/Hip06 SoC PCIe controllers
The four patches respectively:
        - rework PCie Designware to be used for ACPI PCI Host controllers
        - re-architect the current HiSilicon driver to make it scalable to the
          new ACPI quirks
	- rework the current HiSilicon driver to make it ECAM compliant
        - adds the HiSilicon ACPI specific quirks.

   This patchset  is based on 
   https://github.com/semihalf-nowicki-tomasz/linux.git
   branch pci-acpi-v3 + Hip06 support applied on top:
   https://patchwork.ozlabs.org/patch/549198/

gabriele paoloni (4):
  PCI: Designware: Add support for ACPI based controllers
  PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for
    ACPI
  PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
  PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

 .../devicetree/bindings/pci/hisilicon-pcie.txt     |   4 +-
 MAINTAINERS                                        |   3 +
 drivers/pci/host/Kconfig                           |   9 ++
 drivers/pci/host/Makefile                          |   3 +-
 drivers/pci/host/pcie-designware.c                 |  39 +++++-
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-hisi-acpi.c                  | 147 ++++++++++++++++++++
 drivers/pci/host/pcie-hisi-common.c                | 103 ++++++++++++++
 drivers/pci/host/pcie-hisi.c                       | 149 ++++++---------------
 drivers/pci/host/pcie-hisi.h                       |  50 +++++++
 10 files changed, 390 insertions(+), 118 deletions(-)
 create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
 create mode 100644 drivers/pci/host/pcie-hisi-common.c
 create mode 100644 drivers/pci/host/pcie-hisi.h

-- 
1.9.1

[toc] | [next] | [standalone]


#1326690 — [RFC PATCH 1/4] PCI: Designware: Add support for ACPI based controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-04 12:30 +0100
Subject[RFC PATCH 1/4] PCI: Designware: Add support for ACPI based controllers
Message-ID<qYqgQ-7X3-61@gated-at.bofh.it>
In reply to#1326689
From: gabriele paoloni <gabriele.paoloni@huawei.com>

This patch reworks the current Designware framework to
enable ACPI based controllers.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/pci/host/pcie-designware.c | 39 +++++++++++++++++++++++++++++++++-----
 drivers/pci/host/pcie-designware.h |  1 +
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 02a7452..4ced20d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -11,6 +11,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/acpi.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -19,6 +20,7 @@
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
 #include <linux/pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/pci_regs.h>
 #include <linux/platform_device.h>
 #include <linux/types.h>
@@ -46,7 +48,6 @@
 #define PCIE_MSI_INTR0_ENABLE		0x828
 #define PCIE_MSI_INTR0_MASK		0x82C
 #define PCIE_MSI_INTR0_STATUS		0x830
-
 #define PCIE_ATU_VIEWPORT		0x900
 #define PCIE_ATU_REGION_INBOUND		(0x1 << 31)
 #define PCIE_ATU_REGION_OUTBOUND	(0x0 << 31)
@@ -69,7 +70,7 @@
 #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
 #define PCIE_ATU_UPPER_TARGET		0x91C
 
-static struct pci_ops dw_pcie_ops;
+struct pci_ops dw_pcie_ops;
 
 int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
 {
@@ -657,8 +658,22 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
 static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			int size, u32 *val)
 {
-	struct pcie_port *pp = bus->sysdata;
 	int ret;
+	struct pcie_port *pp;
+	struct pci_bus *bridge_bus;
+
+	for (bridge_bus = bus; bridge_bus->parent;
+			bridge_bus = bridge_bus->parent)
+		;
+
+	if (has_acpi_companion(bridge_bus->bridge)) {
+#ifdef CONFIG_ACPI_PCI_HOST_GENERIC
+		struct acpi_pci_root *root = bus->sysdata;
+
+		pp = root->sysdata;
+#endif /* CONFIG_ACPI_PCI_HOST_GENERIC */
+	} else
+		pp = bus->sysdata;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
 		*val = 0xffffffff;
@@ -681,8 +696,22 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			int where, int size, u32 val)
 {
-	struct pcie_port *pp = bus->sysdata;
 	int ret;
+	struct pcie_port *pp;
+	struct pci_bus *bridge_bus;
+
+	for (bridge_bus = bus; bridge_bus->parent;
+			bridge_bus = bridge_bus->parent)
+		;
+
+	if (has_acpi_companion(bridge_bus->bridge)) {
+#ifdef CONFIG_ACPI_PCI_HOST_GENERIC
+		struct acpi_pci_root *root = bus->sysdata;
+
+		pp = root->sysdata;
+#endif /* CONFIG_ACPI_PCI_HOST_GENERIC */
+	} else
+		pp = bus->sysdata;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
 		return PCIBIOS_DEVICE_NOT_FOUND;
@@ -700,7 +729,7 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 	return ret;
 }
 
-static struct pci_ops dw_pcie_ops = {
+struct pci_ops dw_pcie_ops = {
 	.read = dw_pcie_rd_conf,
 	.write = dw_pcie_wr_conf,
 };
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 2356d29..fd27fa4 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -80,4 +80,5 @@ int dw_pcie_link_up(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
 
+extern struct pci_ops dw_pcie_ops;
 #endif /* _PCIE_DESIGNWARE_H */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1326782

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-04 14:20 +0100
Message-ID<qYrZf-FN-1@gated-at.bofh.it>
In reply to#1326689
On Thursday 04 February 2016 19:37:36 Gabriele Paoloni wrote:
> From: gabriele paoloni <gabriele.paoloni@huawei.com>
> 
> This patchset adds ACPI support for the HiSilicon Hip05/Hip06 SoC PCIe controllers
> The four patches respectively:
>         - rework PCie Designware to be used for ACPI PCI Host controllers
>         - re-architect the current HiSilicon driver to make it scalable to the
>           new ACPI quirks
>         - rework the current HiSilicon driver to make it ECAM compliant
>         - adds the HiSilicon ACPI specific quirks.
> 
>    This patchset  is based on 
>    https://github.com/semihalf-nowicki-tomasz/linux.git
>    branch pci-acpi-v3 + Hip06 support applied on top:
>    https://patchwork.ozlabs.org/patch/549198/
> 

ACPI has its own PCI support, and should not need drivers for host
bridges. I don't think we can really mix the two things, as ACPI
needs to have access to things like PCI config space way before
we are probing normal device drivers.

Please put this in drivers/acpi/pci*.c.

	Arnd

[toc] | [prev] | [next] | [standalone]


#1326910 — RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-04 16:20 +0100
SubjectRE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qYtRo-3EV-5@gated-at.bofh.it>
In reply to#1326782
Hi Arnd

Thanks for looking at this

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 04 February 2016 13:12
> To: Gabriele Paoloni
> Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C); Linuxarm;
> qiujiang; bhelgaas@google.com; Lorenzo.Pieralisi@arm.com;
> tn@semihalf.com; zhangjukuo; xuwei (O); Liguozhu (Kenneth); linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; jcm@redhat.com
> Subject: Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host
> Controllers
> 
> On Thursday 04 February 2016 19:37:36 Gabriele Paoloni wrote:
> > From: gabriele paoloni <gabriele.paoloni@huawei.com>
> >
> > This patchset adds ACPI support for the HiSilicon Hip05/Hip06 SoC
> PCIe controllers
> > The four patches respectively:
> >         - rework PCie Designware to be used for ACPI PCI Host
> controllers
> >         - re-architect the current HiSilicon driver to make it
> scalable to the
> >           new ACPI quirks
> >         - rework the current HiSilicon driver to make it ECAM
> compliant
> >         - adds the HiSilicon ACPI specific quirks.
> >
> >    This patchset  is based on
> >    https://github.com/semihalf-nowicki-tomasz/linux.git
> >    branch pci-acpi-v3 + Hip06 support applied on top:
> >    https://patchwork.ozlabs.org/patch/549198/
> >
> 
> ACPI has its own PCI support, and should not need drivers for host
> bridges. I don't think we can really mix the two things, as ACPI
> needs to have access to things like PCI config space way before
> we are probing normal device drivers.
> 
> Please put this in drivers/acpi/pci*.c.

I can put pcie-hisi-acpi.c under drivers/acpi/

However if you look at the driver it is made up of three parts:
pcie-hisi.c --> the DT based driver
pcie-hisi-acpi.c --> the ACPI based hook and ACPI specific init callback
pcie-hisi-common.c --> common functions shared between DT and ACPI versions 
                       of the driver

Now I think that moving pcie-hisi-acpi.c under drivers/acpi/
would make it hard to read as you need to jump across directories
and it seems a bit unnatural...

However it is not a big issue to me...

Cheers

Gab

> 
> 	Arnd

[toc] | [prev] | [next] | [standalone]


#1326947

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-04 17:10 +0100
Message-ID<qYuDN-4en-23@gated-at.bofh.it>
In reply to#1326910
On Thursday 04 February 2016 15:11:18 Gabriele Paoloni wrote:
> > 
> > ACPI has its own PCI support, and should not need drivers for host
> > bridges. I don't think we can really mix the two things, as ACPI
> > needs to have access to things like PCI config space way before
> > we are probing normal device drivers.
> > 
> > Please put this in drivers/acpi/pci*.c.
> 
> I can put pcie-hisi-acpi.c under drivers/acpi/
> 
> However if you look at the driver it is made up of three parts:
> pcie-hisi.c --> the DT based driver
> pcie-hisi-acpi.c --> the ACPI based hook and ACPI specific init callback
> pcie-hisi-common.c --> common functions shared between DT and ACPI versions 
>                        of the driver
> 
> Now I think that moving pcie-hisi-acpi.c under drivers/acpi/
> would make it hard to read as you need to jump across directories
> and it seems a bit unnatural...
> 
> However it is not a big issue to me...

That's not really what I meant though: the pcie-hisi driver uses the
pcie-designware.c library, most of which makes no sense in an environment
where you have ACPI, e.g. link training, custom MSI support, initial
register setup, platform driver hooks, etc.

You should add a very minimal set of hacks for the parts in this driver
that diverge from a standard SBSA compliant PCIe host that ACPI expects.

	Arnd

[toc] | [prev] | [next] | [standalone]


#1327002 — RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-04 17:50 +0100
SubjectRE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qYvgv-4wj-39@gated-at.bofh.it>
In reply to#1326947
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 04 February 2016 16:07
> To: Gabriele Paoloni
> Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C); Linuxarm;
> qiujiang; bhelgaas@google.com; Lorenzo.Pieralisi@arm.com;
> tn@semihalf.com; zhangjukuo; xuwei (O); Liguozhu (Kenneth); linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; jcm@redhat.com
> Subject: Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host
> Controllers
> 
> On Thursday 04 February 2016 15:11:18 Gabriele Paoloni wrote:
> > >
> > > ACPI has its own PCI support, and should not need drivers for host
> > > bridges. I don't think we can really mix the two things, as ACPI
> > > needs to have access to things like PCI config space way before
> > > we are probing normal device drivers.
> > >
> > > Please put this in drivers/acpi/pci*.c.
> >
> > I can put pcie-hisi-acpi.c under drivers/acpi/
> >
> > However if you look at the driver it is made up of three parts:
> > pcie-hisi.c --> the DT based driver
> > pcie-hisi-acpi.c --> the ACPI based hook and ACPI specific init
> callback
> > pcie-hisi-common.c --> common functions shared between DT and ACPI
> versions
> >                        of the driver
> >
> > Now I think that moving pcie-hisi-acpi.c under drivers/acpi/
> > would make it hard to read as you need to jump across directories
> > and it seems a bit unnatural...
> >
> > However it is not a big issue to me...
> 
> That's not really what I meant though: the pcie-hisi driver uses the
> pcie-designware.c library, most of which makes no sense in an
> environment
> where you have ACPI, e.g. link training, custom MSI support, initial
> register setup, platform driver hooks, etc.
> 
> You should add a very minimal set of hacks for the parts in this driver
> that diverge from a standard SBSA compliant PCIe host that ACPI expects.

Effectively the ACPI version of the HiSilicon driver does not rely on
Designware as much as the DT version (that calls dw_pcie_host_init());
however in order to do what you suggest I'd need to copy and paste and
modify dw_pcie_rd_conf and dw_pcie_wr_conf.
Also I'd need to declare duplicate version of the functions in
pcie-hisi-common.c (if I do not want to split the object across
different paths "drivers/pci/host" and "drivers/acpi/")

Now I can do it but I thought it was more correct to pass &dw_pcie_ops
as input pointer in DECLARE_ACPI_MCFG_FIXUP(); this is also because maybe in
future other Designware based controllers may need to support ACPI and it
would be easier for them to reuse their DT based driver functions

Honestly I am a bit confused...

Thanks

Gab

> 
> 	Arnd

[toc] | [prev] | [next] | [standalone]


#1327014

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-04 18:10 +0100
Message-ID<qYvzQ-4U8-9@gated-at.bofh.it>
In reply to#1327002
On Thursday 04 February 2016 16:44:12 Gabriele Paoloni wrote:
> Effectively the ACPI version of the HiSilicon driver does not rely on
> Designware as much as the DT version (that calls dw_pcie_host_init());
> however in order to do what you suggest I'd need to copy and paste and
> modify dw_pcie_rd_conf and dw_pcie_wr_conf.
> Also I'd need to declare duplicate version of the functions in
> pcie-hisi-common.c (if I do not want to split the object across
> different paths "drivers/pci/host" and "drivers/acpi/")
> 
> Now I can do it but I thought it was more correct to pass &dw_pcie_ops
> as input pointer in DECLARE_ACPI_MCFG_FIXUP(); this is also because maybe in
> future other Designware based controllers may need to support ACPI and it
> would be easier for them to reuse their DT based driver functions
> 
> Honestly I am a bit confused...
> 

I don't think you can use the dw_pcie_ops as-is anyway, as they in
turn rely on the device driver structure, and you also don't need
those as they seem to be written for the generic case of a 32-bit
architecture that does not have enough space to map the entire
config space into MMIO. Just set up all those mapping registers from
the firmware and pass the config space base address as you'd normally
do on ACPI.

	Arnd

[toc] | [prev] | [next] | [standalone]


#1327019 — RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-04 18:10 +0100
SubjectRE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qYvzQ-4U8-15@gated-at.bofh.it>
In reply to#1327014
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 04 February 2016 17:02
> To: Gabriele Paoloni
> Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C); Linuxarm;
> qiujiang; bhelgaas@google.com; Lorenzo.Pieralisi@arm.com;
> tn@semihalf.com; zhangjukuo; xuwei (O); Liguozhu (Kenneth); linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; jcm@redhat.com
> Subject: Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host
> Controllers
> 
> On Thursday 04 February 2016 16:44:12 Gabriele Paoloni wrote:
> > Effectively the ACPI version of the HiSilicon driver does not rely on
> > Designware as much as the DT version (that calls
> dw_pcie_host_init());
> > however in order to do what you suggest I'd need to copy and paste
> and
> > modify dw_pcie_rd_conf and dw_pcie_wr_conf.
> > Also I'd need to declare duplicate version of the functions in
> > pcie-hisi-common.c (if I do not want to split the object across
> > different paths "drivers/pci/host" and "drivers/acpi/")
> >
> > Now I can do it but I thought it was more correct to pass
> &dw_pcie_ops
> > as input pointer in DECLARE_ACPI_MCFG_FIXUP(); this is also because
> maybe in
> > future other Designware based controllers may need to support ACPI
> and it
> > would be easier for them to reuse their DT based driver functions
> >
> > Honestly I am a bit confused...
> >
> 
> I don't think you can use the dw_pcie_ops as-is anyway, as they in
> turn rely on the device driver structure, and you also don't need
> those as they seem to be written for the generic case of a 32-bit
> architecture that does not have enough space to map the entire
> config space into MMIO. Just set up all those mapping registers from
> the firmware and pass the config space base address as you'd normally
> do on ACPI.

Ok thanks, I'll wait a bit to see if there is any comment on this
from other people, if not I'll rework the ACPI driver as you suggest
and remove any dependency on Designware.

Cheers

Gab

> 
> 	Arnd

[toc] | [prev] | [next] | [standalone]


#1329025 — RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-08 14:20 +0100
SubjectRE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qZTTs-5KP-5@gated-at.bofh.it>
In reply to#1327014
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 04 February 2016 17:02
> To: Gabriele Paoloni
> Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C); Linuxarm;
> qiujiang; bhelgaas@google.com; Lorenzo.Pieralisi@arm.com;
> tn@semihalf.com; zhangjukuo; xuwei (O); Liguozhu (Kenneth); linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; jcm@redhat.com
> Subject: Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host
> Controllers
> 
> On Thursday 04 February 2016 16:44:12 Gabriele Paoloni wrote:
> > Effectively the ACPI version of the HiSilicon driver does not rely on
> > Designware as much as the DT version (that calls
> dw_pcie_host_init());
> > however in order to do what you suggest I'd need to copy and paste
> and
> > modify dw_pcie_rd_conf and dw_pcie_wr_conf.
> > Also I'd need to declare duplicate version of the functions in
> > pcie-hisi-common.c (if I do not want to split the object across
> > different paths "drivers/pci/host" and "drivers/acpi/")
> >
> > Now I can do it but I thought it was more correct to pass
> &dw_pcie_ops
> > as input pointer in DECLARE_ACPI_MCFG_FIXUP(); this is also because
> maybe in
> > future other Designware based controllers may need to support ACPI
> and it
> > would be easier for them to reuse their DT based driver functions
> >
> > Honestly I am a bit confused...
> >
> 
> I don't think you can use the dw_pcie_ops as-is anyway, as they in
> turn rely on the device driver structure, and you also don't need
> those as they seem to be written for the generic case of a 32-bit
> architecture that does not have enough space to map the entire
> config space into MMIO. Just set up all those mapping registers from
> the firmware and pass the config space base address as you'd normally
> do on ACPI.

Hi Arnd

I have now sent out patchset v2.

Now I have removed the dependency from designware but I still think
the best location for the code is in "drivers/pci/host/":
the ACPI quirks share the config read/write functions with the DT
driver, moreover for other hosts in future I think it makes sense
to stay in "drivers/pci/host/" as they could even share more init
code with their respective DT versions...

Cheers

Gab

> 
> 	Arnd

[toc] | [prev] | [next] | [standalone]


#1329077

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-08 15:00 +0100
Message-ID<qZUwa-63L-31@gated-at.bofh.it>
In reply to#1329025
On Monday 08 February 2016 13:17:58 Gabriele Paoloni wrote:
> 
> Now I have removed the dependency from designware but I still think
> the best location for the code is in "drivers/pci/host/":
> the ACPI quirks share the config read/write functions with the DT
> driver, moreover for other hosts in future I think it makes sense
> to stay in "drivers/pci/host/" as they could even share more init
> code with their respective DT versions...

I haven't really followed what is going on with ACPI. Do you expect
to see future machines come out that are not just implementing SBSA
but that still need to run ACPI? I thought this was just a hack
for some early machines that only run with ACPI but are not actually
compliant.

	Arnd

[toc] | [prev] | [next] | [standalone]


#1329094 — Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromSinan Kaya <okaya@codeaurora.org>
Date2016-02-08 15:20 +0100
SubjectRe: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qZUPv-6rJ-1@gated-at.bofh.it>
In reply to#1329077
On 2/8/2016 8:55 AM, Arnd Bergmann wrote:
> I haven't really followed what is going on with ACPI. Do you expect
> to see future machines come out that are not just implementing SBSA
> but that still need to run ACPI? I thought this was just a hack
> for some early machines that only run with ACPI but are not actually
> compliant.
> 
> 	Arnd

I agree. We shouldn't be playing with half-baked ACPI solutions. We have seen
two variants already that claim to be ACPI compliant yet they do not tie into
anything inside ACPICA. 

The correct route is to use Tomasz's ACPI PCI root bridge driver and use the ACPI
framework.

If a platform has quirks, Tomasz's patches allow vendors add quirks too.

The combination of PCI host bridge driver + ACPI hack is not right.

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]


#1329273 — RE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers

FromGabriele Paoloni <gabriele.paoloni@huawei.com>
Date2016-02-08 17:30 +0100
SubjectRE: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host Controllers
Message-ID<qZWRk-7IX-29@gated-at.bofh.it>
In reply to#1329094
Hi Arnd, Sinan

> -----Original Message-----
> From: Sinan Kaya [mailto:okaya@codeaurora.org]
> Sent: 08 February 2016 14:12
> To: Arnd Bergmann; linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Lorenzo.Pieralisi@arm.com; jcm@redhat.com;
> tn@semihalf.com; linux-pci@vger.kernel.org; Linuxarm; xuwei (O); linux-
> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; Wangzhou (B);
> liudongdong (C); Guohanjun (Hanjun Guo); bhelgaas@google.com;
> zhangjukuo; Liguozhu (Kenneth); qiujiang
> Subject: Re: [RFC PATCH 0/4] Add ACPI support for HiSilicon PCIe Host
> Controllers
> 
> On 2/8/2016 8:55 AM, Arnd Bergmann wrote:
> > I haven't really followed what is going on with ACPI. Do you expect
> > to see future machines come out that are not just implementing SBSA
> > but that still need to run ACPI? I thought this was just a hack
> > for some early machines that only run with ACPI but are not actually
> > compliant.

Well from our side (HiSilicon) we're trying to move away from non fully 
ECAM platforms, so from us in the long term I don't expect too many quirks,
but I don't know about the other vendors.

Obviously the reason why Tomasz implemented the quirks is to fit non 
fully ECAM HW and to allow custom HW init; this is why I thought better
to have the ACPI version in the same dir as the DT (maybe we can create 
an ACPI sub-dir in drivers/pci/host ?)


> >
> > 	Arnd
> 
> I agree. We shouldn't be playing with half-baked ACPI solutions. We
> have seen
> two variants already that claim to be ACPI compliant yet they do not
> tie into
> anything inside ACPICA.
> 
> The correct route is to use Tomasz's ACPI PCI root bridge driver and
> use the ACPI
> framework.
> 
> If a platform has quirks, Tomasz's patches allow vendors add quirks
> too.
> 
> The combination of PCI host bridge driver + ACPI hack is not right.

If you look at my patchset  you can see that I didn't do any hack,

I just used the framework provided by Tomasz patchset.

The discussion here is more about the code location for the quirks.
Since the configuration read/write and the HW init sequences can be
similar between the ACPI variant and DT variant I thought it make 
sense to have them in "drivers/pci/host"

Thanks

Gab

> 
> --
> Sinan Kaya
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center,
> Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web