diff for duplicates of <5AE6D7E2.9030506@kontron.com> diff --git a/a/1.txt b/N1/1.txt index 6b68712..91a707c 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,10 +1,9 @@ -Le 27/04/2018 18:56, Bjorn Helgaas a =E9crit : +Le 27/04/2018 18:56, Bjorn Helgaas a ?crit : > On Fri, Apr 27, 2018 at 12:29:32PM +0000, Gilles Buloz wrote: ->> Le 27/04/2018 10:43, Ard Biesheuvel a =E9crit : +>> Le 27/04/2018 10:43, Ard Biesheuvel a ?crit : >>> (add Bjorn and linux-pci) >>> ->>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> wrot= -e: +>>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> wrote: >>>> Dear developers, >>>> >>>> I currently have two functional workarounds for this issue but @@ -63,7 +62,7 @@ e: >> The PEX8112 PCIe-to-PCI bridge has capability PCI_CAP_ID_EXP, but >> has no PCI_CAP_ID_PCIX capability. As I understand the lack of >> PCI_CAP_ID_PCIX is advertising this limitation on the PCI side (no ->> support for PCI config offset >=3D0x100). +>> support for PCI config offset >=0x100). > Sounds right to me. > >> Also I guess in the case of a bridge having PCI_CAP_ID_PCIX, this @@ -77,7 +76,7 @@ e: >> I'm currently using the attached patch (for kernel 4.1.35-rt41 from >> NXP Yocto BSP). It uses bus_flags to remember if a bus is behind a >> bridge without extended address capability to avoid PCi config ->> accesses at offset >=3D 0x100. Thanks to this patch I now have a +>> accesses at offset >= 0x100. Thanks to this patch I now have a >> functional system with functional PCI/PCIe devices. > The patch seems like it's looking at the right things, but I don't > want to build it into pci_scan_bridge_extend() because that function @@ -92,146 +91,103 @@ e: > index ac91b6fd0bcd..d8b091f0bcd1 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c -> @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *= -dev) -> * pci_cfg_space_size - Get the configuration space size of the PCI dev= -ice +> @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev) +> * pci_cfg_space_size - Get the configuration space size of the PCI device > * @dev: PCI device > * -> - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express devic= -es -> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Express = -devices -> * have 4096 bytes. Even if the device is capable, that doesn't mean w= -e can -> * access it. Maybe we don't have a way to generate extended config sp= -ace -> * accesses, or the device is behind a reverse Express bridge. So we t= -ry -> @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev = -*dev) +> - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express devices +> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Express devices +> * have 4096 bytes. Even if the device is capable, that doesn't mean we can +> * access it. Maybe we don't have a way to generate extended config space +> * accesses, or the device is behind a reverse Express bridge. So we try +> @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev) > */ > static int pci_cfg_space_size_ext(struct pci_dev *dev) > { -> + struct pci_dev *bridge =3D pci_upstream_bridge(dev); +> + struct pci_dev *bridge = pci_upstream_bridge(dev); > u32 status; -> int pos =3D PCI_CFG_SPACE_SIZE; -> = - +> int pos = PCI_CFG_SPACE_SIZE; +> > + if (bridge && pci_is_pcie(bridge) && -> + pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE) +> + pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE) > + return PCI_CFG_SPACE_SIZE; > + -> if (pci_read_config_dword(dev, pos, &status) !=3D PCIBIOS_SUCCESSFUL) +> if (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL) > return PCI_CFG_SPACE_SIZE; -> if (status =3D=3D 0xffffffff || pci_ext_cfg_is_aliased(dev)) +> if (status == 0xffffffff || pci_ext_cfg_is_aliased(dev)) > >> --- include/linux/pci.h.orig 2018-03-26 16:51:18.050000000 +0000 >> +++ include/linux/pci.h 2018-03-26 16:51:27.660000000 +0000 >> @@ -193,6 +193,7 @@ >> enum pci_bus_flags { ->> PCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1, ->> PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2, ->> + PCI_BUS_FLAGS_COMPAT_CFG_SPACE =3D (__force pci_bus_flags_t) 4, +>> PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, +>> PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, +>> + PCI_BUS_FLAGS_COMPAT_CFG_SPACE = (__force pci_bus_flags_t) 4, >> }; ->> = - +>> >> /* These values come from the PCI Express Spec */ >> --- drivers/pci/probe.c.orig 2018-01-22 09:29:52.000000000 +0000 >> +++ drivers/pci/probe.c 2018-03-26 16:54:30.830000000 +0000 >> @@ -827,6 +827,28 @@ ->> child->primary =3D primary; +>> child->primary = primary; >> pci_bus_insert_busn_res(child, secondary, subordinate); ->> child->bridge_ctl =3D bctl; +>> child->bridge_ctl = bctl; >> + >> + { >> + int pos; >> + u32 status; ->> + bool pci_compat_cfg_space =3D false; +>> + bool pci_compat_cfg_space = false; >> + ->> + if (!pci_is_pcie(dev) || (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PC= -IE_BRIDGE) || (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)) { ->> + /* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse m= -ode, we have to check for PCI-X capabilities */ ->> + pos =3D pci_find_capability(dev, PCI_CAP_ID_PCIX); +>> + if (!pci_is_pcie(dev) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCIE_BRIDGE) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)) { +>> + /* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse mode, we have to check for PCI-X capabilities */ +>> + pos = pci_find_capability(dev, PCI_CAP_ID_PCIX); >> + if (pos) { >> + pci_read_config_dword(dev, pos + PCI_X_STATUS, &status); >> + if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ))) ->> + pci_compat_cfg_space =3D true; +>> + pci_compat_cfg_space = true; >> + } else { ->> + pci_compat_cfg_space =3D true; +>> + pci_compat_cfg_space = true; >> + } >> + if (pci_compat_cfg_space) { ->> + dev_info(&dev->dev, "[%04x:%04x] Child bus limited to PCI-Compati= -ble config space\n", dev->vendor, dev->device); ->> + child->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_CFG_SPACE; +>> + dev_info(&dev->dev, "[%04x:%04x] Child bus limited to PCI-Compatible config space\n", dev->vendor, dev->device); +>> + child->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE; >> + } >> + } >> + } >> } ->> = - ->> cmax =3D pci_scan_child_bus(child); +>> +>> cmax = pci_scan_child_bus(child); >> @@ -1098,6 +1120,11 @@ >> goto fail; >> } ->> = - +>> >> + if (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) { ->> + dev_info(&dev->dev, "[%04x:%04x] PCI-Compatible config space only due= - to parent bus(es)\n", dev->vendor, dev->device); +>> + dev_info(&dev->dev, "[%04x:%04x] PCI-Compatible config space only due to parent bus(es)\n", dev->vendor, dev->device); >> + return PCI_CFG_SPACE_SIZE; >> + } >> + >> return pci_cfg_space_size_ext(dev); ->> = - +>> >> fail: Bjorn, -If I'm right about your proposed patch to pci_cfg_space_size_ext(), *bridge= - is pointing to the upper device of device *dev being = - -checked. I understand the purpose, but I think this fails for my config tha= -t is : - -LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector -> PCI= --to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe = +If I'm right about your proposed patch to pci_cfg_space_size_ext(), *bridge is pointing to the upper device of device *dev being +checked. I understand the purpose, but I think this fails for my config that is : +LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector -> PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe devices (one on each port) because : -- when pci_cfg_space_size_ext() is run on the 4 PCIe devices, *bridge is th= -e PCIe switch which is not matching = - -PCI_EXP_TYPE_PCI_BRIDGE. In this case *bridge should also be checked for th= -e parent bus of the PCIe switch, and so on. -- when pci_cfg_space_size_ext() is run for the PCI-to-PCIe bridge, *bridge = -is the PEX8112 that is also not matching = - -PCI_EXP_TYPE_PCI_BRIDGE but PCI_EXP_TYPE_PCIE_BRIDGE. This leads to a confi= -g access at offset 0x100 to the PCI-to-PCIe bridge, so a = - +- when pci_cfg_space_size_ext() is run on the 4 PCIe devices, *bridge is the PCIe switch which is not matching +PCI_EXP_TYPE_PCI_BRIDGE. In this case *bridge should also be checked for the parent bus of the PCIe switch, and so on. +- when pci_cfg_space_size_ext() is run for the PCI-to-PCIe bridge, *bridge is the PEX8112 that is also not matching +PCI_EXP_TYPE_PCI_BRIDGE but PCI_EXP_TYPE_PCIE_BRIDGE. This leads to a config access at offset 0x100 to the PCI-to-PCIe bridge, so a crash (because of the PEX8112) -I think setting a bit in bus_flags when creating a child bus is very effici= -ent because once set it is automatically inherited by all = - -child buses and then the only thing that pci_cfg_space_size() has to do for= - each device is to check for this bit. Also this = +I think setting a bit in bus_flags when creating a child bus is very efficient because once set it is automatically inherited by all +child buses and then the only thing that pci_cfg_space_size() has to do for each device is to check for this bit. Also this +PCI_BUS_FLAGS_COMPAT_CFG_SPACE flag is actually a bus property that is compliant with the purpose of bus_flags. -PCI_BUS_FLAGS_COMPAT_CFG_SPACE flag is actually a bus property that is comp= -liant with the purpose of bus_flags. - -I agree that pci_scan_bridge_extend() is already too complicated, so would = -you be okay to only add one line to it : +I agree that pci_scan_bridge_extend() is already too complicated, so would you be okay to only add one line to it : pci_bus_set_compat_cfg_space(child); -and put all the code I suggested in this new function pci_bus_set_compat_cf= -g_space() ? (also supporting PCI-X Mode 2 devices) -Improvement : this function can return immediately if the child bus has alr= -eady inherited the flag from its parent. - - -_______________________________________________ -linux-arm-kernel mailing list -linux-arm-kernel@lists.infradead.org -http://lists.infradead.org/mailman/listinfo/linux-arm-kernel +and put all the code I suggested in this new function pci_bus_set_compat_cfg_space() ? (also supporting PCI-X Mode 2 devices) +Improvement : this function can return immediately if the child bus has already inherited the flag from its parent. diff --git a/a/content_digest b/N1/content_digest index 0ee6e69..6d8b40c 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,24 +2,18 @@ "ref\0CAKv+Gu_v4V8DUa6f4WxvjZHoJbbqT0mKpOwYVmaLaE20CB3U_g@mail.gmail.com\0" "ref\05AE317AB.4020404@kontron.com\0" "ref\020180427165627.GA8199@bhelgaas-glaptop.roam.corp.google.com\0" - "From\0Gilles Buloz <Gilles.Buloz@kontron.com>\0" - "Subject\0Re: LS1043A : \"synchronous abort\" at boot due to PCI config read\0" + "From\0Gilles.Buloz@kontron.com (Gilles Buloz)\0" + "Subject\0LS1043A : \"synchronous abort\" at boot due to PCI config read\0" "Date\0Mon, 30 Apr 2018 08:46:27 +0000\0" - "To\0Bjorn Helgaas <helgaas@kernel.org>\0" - "Cc\0Bjorn Helgaas <bhelgaas@google.com>" - linux-pci <linux-pci@vger.kernel.org> - Minghuan.Lian@nxp.com <Minghuan.Lian@nxp.com> - linux-arm-kernel@lists.infradead.org <linux-arm-kernel@lists.infradead.org> - " Ard Biesheuvel <ard.biesheuvel@linaro.org>\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" - "Le 27/04/2018 18:56, Bjorn Helgaas a =E9crit :\n" + "Le 27/04/2018 18:56, Bjorn Helgaas a ?crit :\n" "> On Fri, Apr 27, 2018 at 12:29:32PM +0000, Gilles Buloz wrote:\n" - ">> Le 27/04/2018 10:43, Ard Biesheuvel a =E9crit :\n" + ">> Le 27/04/2018 10:43, Ard Biesheuvel a ?crit :\n" ">>> (add Bjorn and linux-pci)\n" ">>>\n" - ">>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> wrot=\n" - "e:\n" + ">>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> wrote:\n" ">>>> Dear developers,\n" ">>>>\n" ">>>> I currently have two functional workarounds for this issue but\n" @@ -78,7 +72,7 @@ ">> The PEX8112 PCIe-to-PCI bridge has capability PCI_CAP_ID_EXP, but\n" ">> has no PCI_CAP_ID_PCIX capability. As I understand the lack of\n" ">> PCI_CAP_ID_PCIX is advertising this limitation on the PCI side (no\n" - ">> support for PCI config offset >=3D0x100).\n" + ">> support for PCI config offset >=0x100).\n" "> Sounds right to me.\n" ">\n" ">> Also I guess in the case of a bridge having PCI_CAP_ID_PCIX, this\n" @@ -92,7 +86,7 @@ ">> I'm currently using the attached patch (for kernel 4.1.35-rt41 from\n" ">> NXP Yocto BSP). It uses bus_flags to remember if a bus is behind a\n" ">> bridge without extended address capability to avoid PCi config\n" - ">> accesses at offset >=3D 0x100. Thanks to this patch I now have a\n" + ">> accesses at offset >= 0x100. Thanks to this patch I now have a\n" ">> functional system with functional PCI/PCIe devices.\n" "> The patch seems like it's looking at the right things, but I don't\n" "> want to build it into pci_scan_bridge_extend() because that function\n" @@ -107,148 +101,105 @@ "> index ac91b6fd0bcd..d8b091f0bcd1 100644\n" "> --- a/drivers/pci/probe.c\n" "> +++ b/drivers/pci/probe.c\n" - "> @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *=\n" - "dev)\n" - "> * pci_cfg_space_size - Get the configuration space size of the PCI dev=\n" - "ice\n" + "> @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev)\n" + "> * pci_cfg_space_size - Get the configuration space size of the PCI device\n" "> * @dev: PCI device\n" "> *\n" - "> - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express devic=\n" - "es\n" - "> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Express =\n" - "devices\n" - "> * have 4096 bytes. Even if the device is capable, that doesn't mean w=\n" - "e can\n" - "> * access it. Maybe we don't have a way to generate extended config sp=\n" - "ace\n" - "> * accesses, or the device is behind a reverse Express bridge. So we t=\n" - "ry\n" - "> @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev =\n" - "*dev)\n" + "> - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express devices\n" + "> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Express devices\n" + "> * have 4096 bytes. Even if the device is capable, that doesn't mean we can\n" + "> * access it. Maybe we don't have a way to generate extended config space\n" + "> * accesses, or the device is behind a reverse Express bridge. So we try\n" + "> @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev)\n" "> */\n" "> static int pci_cfg_space_size_ext(struct pci_dev *dev)\n" "> {\n" - "> +\tstruct pci_dev *bridge =3D pci_upstream_bridge(dev);\n" + "> +\tstruct pci_dev *bridge = pci_upstream_bridge(dev);\n" "> \tu32 status;\n" - "> \tint pos =3D PCI_CFG_SPACE_SIZE;\n" - "> =\n" - "\n" + "> \tint pos = PCI_CFG_SPACE_SIZE;\n" + "> \n" "> +\tif (bridge && pci_is_pcie(bridge) &&\n" - "> +\t pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)\n" + "> +\t pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE)\n" "> +\t\treturn PCI_CFG_SPACE_SIZE;\n" "> +\n" - "> \tif (pci_read_config_dword(dev, pos, &status) !=3D PCIBIOS_SUCCESSFUL)\n" + "> \tif (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL)\n" "> \t\treturn PCI_CFG_SPACE_SIZE;\n" - "> \tif (status =3D=3D 0xffffffff || pci_ext_cfg_is_aliased(dev))\n" + "> \tif (status == 0xffffffff || pci_ext_cfg_is_aliased(dev))\n" ">\n" ">> --- include/linux/pci.h.orig\t2018-03-26 16:51:18.050000000 +0000\n" ">> +++ include/linux/pci.h\t2018-03-26 16:51:27.660000000 +0000\n" ">> @@ -193,6 +193,7 @@\n" ">> enum pci_bus_flags {\n" - ">> \tPCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1,\n" - ">> \tPCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2,\n" - ">> +\tPCI_BUS_FLAGS_COMPAT_CFG_SPACE =3D (__force pci_bus_flags_t) 4,\n" + ">> \tPCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,\n" + ">> \tPCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,\n" + ">> +\tPCI_BUS_FLAGS_COMPAT_CFG_SPACE = (__force pci_bus_flags_t) 4,\n" ">> };\n" - ">> =\n" - "\n" + ">> \n" ">> /* These values come from the PCI Express Spec */\n" ">> --- drivers/pci/probe.c.orig\t2018-01-22 09:29:52.000000000 +0000\n" ">> +++ drivers/pci/probe.c\t2018-03-26 16:54:30.830000000 +0000\n" ">> @@ -827,6 +827,28 @@\n" - ">> \t\t\tchild->primary =3D primary;\n" + ">> \t\t\tchild->primary = primary;\n" ">> \t\t\tpci_bus_insert_busn_res(child, secondary, subordinate);\n" - ">> \t\t\tchild->bridge_ctl =3D bctl;\n" + ">> \t\t\tchild->bridge_ctl = bctl;\n" ">> +\n" ">> +\t\t\t{\n" ">> +\t\t\t\tint pos;\n" ">> +\t\t\t\tu32 status;\n" - ">> +\t\t\t\tbool pci_compat_cfg_space =3D false;\n" + ">> +\t\t\t\tbool pci_compat_cfg_space = false;\n" ">> +\n" - ">> +\t\t\t\tif (!pci_is_pcie(dev) || (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PC=\n" - "IE_BRIDGE) || (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)) {\n" - ">> +\t\t\t\t\t/* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse m=\n" - "ode, we have to check for PCI-X capabilities */\n" - ">> +\t\t\t\t\tpos =3D pci_find_capability(dev, PCI_CAP_ID_PCIX);\n" + ">> +\t\t\t\tif (!pci_is_pcie(dev) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCIE_BRIDGE) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)) {\n" + ">> +\t\t\t\t\t/* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse mode, we have to check for PCI-X capabilities */\n" + ">> +\t\t\t\t\tpos = pci_find_capability(dev, PCI_CAP_ID_PCIX);\n" ">> +\t\t\t\t\tif (pos) {\n" ">> +\t\t\t\t\t\tpci_read_config_dword(dev, pos + PCI_X_STATUS, &status);\n" ">> +\t\t\t\t\t\tif (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ)))\n" - ">> +\t\t\t\t\t\t\tpci_compat_cfg_space =3D true;\n" + ">> +\t\t\t\t\t\t\tpci_compat_cfg_space = true;\n" ">> +\t\t\t\t\t} else {\n" - ">> +\t\t\t\t\t\tpci_compat_cfg_space =3D true;\n" + ">> +\t\t\t\t\t\tpci_compat_cfg_space = true;\n" ">> +\t\t\t\t\t}\n" ">> +\t\t\t\t\tif (pci_compat_cfg_space) {\n" - ">> +\t\t\t\t\t\tdev_info(&dev->dev, \"[%04x:%04x] Child bus limited to PCI-Compati=\n" - "ble config space\\n\", dev->vendor, dev->device);\n" - ">> +\t\t\t\t\t\tchild->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" + ">> +\t\t\t\t\t\tdev_info(&dev->dev, \"[%04x:%04x] Child bus limited to PCI-Compatible config space\\n\", dev->vendor, dev->device);\n" + ">> +\t\t\t\t\t\tchild->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" ">> +\t\t\t\t\t}\n" ">> +\t\t\t\t}\n" ">> +\t\t\t}\n" ">> \t\t}\n" - ">> =\n" - "\n" - ">> \t\tcmax =3D pci_scan_child_bus(child);\n" + ">> \n" + ">> \t\tcmax = pci_scan_child_bus(child);\n" ">> @@ -1098,6 +1120,11 @@\n" ">> \t\t\tgoto fail;\n" ">> \t}\n" - ">> =\n" - "\n" + ">> \n" ">> +\tif (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) {\n" - ">> +\t\tdev_info(&dev->dev, \"[%04x:%04x] PCI-Compatible config space only due=\n" - " to parent bus(es)\\n\", dev->vendor, dev->device);\n" + ">> +\t\tdev_info(&dev->dev, \"[%04x:%04x] PCI-Compatible config space only due to parent bus(es)\\n\", dev->vendor, dev->device);\n" ">> +\t\treturn PCI_CFG_SPACE_SIZE;\n" ">> +\t}\n" ">> +\n" ">> \treturn pci_cfg_space_size_ext(dev);\n" - ">> =\n" - "\n" + ">> \n" ">> fail:\n" "Bjorn,\n" - "If I'm right about your proposed patch to pci_cfg_space_size_ext(), *bridge=\n" - " is pointing to the upper device of device *dev being =\n" - "\n" - "checked. I understand the purpose, but I think this fails for my config tha=\n" - "t is :\n" - "\n" - "LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector -> PCI=\n" - "-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe =\n" + "If I'm right about your proposed patch to pci_cfg_space_size_ext(), *bridge is pointing to the upper device of device *dev being \n" + "checked. I understand the purpose, but I think this fails for my config that is :\n" "\n" + "LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector -> PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe \n" "devices (one on each port)\n" "\n" "because :\n" - "- when pci_cfg_space_size_ext() is run on the 4 PCIe devices, *bridge is th=\n" - "e PCIe switch which is not matching =\n" - "\n" - "PCI_EXP_TYPE_PCI_BRIDGE. In this case *bridge should also be checked for th=\n" - "e parent bus of the PCIe switch, and so on.\n" - "- when pci_cfg_space_size_ext() is run for the PCI-to-PCIe bridge, *bridge =\n" - "is the PEX8112 that is also not matching =\n" - "\n" - "PCI_EXP_TYPE_PCI_BRIDGE but PCI_EXP_TYPE_PCIE_BRIDGE. This leads to a confi=\n" - "g access at offset 0x100 to the PCI-to-PCIe bridge, so a =\n" - "\n" + "- when pci_cfg_space_size_ext() is run on the 4 PCIe devices, *bridge is the PCIe switch which is not matching \n" + "PCI_EXP_TYPE_PCI_BRIDGE. In this case *bridge should also be checked for the parent bus of the PCIe switch, and so on.\n" + "- when pci_cfg_space_size_ext() is run for the PCI-to-PCIe bridge, *bridge is the PEX8112 that is also not matching \n" + "PCI_EXP_TYPE_PCI_BRIDGE but PCI_EXP_TYPE_PCIE_BRIDGE. This leads to a config access at offset 0x100 to the PCI-to-PCIe bridge, so a \n" "crash (because of the PEX8112)\n" "\n" - "I think setting a bit in bus_flags when creating a child bus is very effici=\n" - "ent because once set it is automatically inherited by all =\n" - "\n" - "child buses and then the only thing that pci_cfg_space_size() has to do for=\n" - " each device is to check for this bit. Also this =\n" + "I think setting a bit in bus_flags when creating a child bus is very efficient because once set it is automatically inherited by all \n" + "child buses and then the only thing that pci_cfg_space_size() has to do for each device is to check for this bit. Also this \n" + "PCI_BUS_FLAGS_COMPAT_CFG_SPACE flag is actually a bus property that is compliant with the purpose of bus_flags.\n" "\n" - "PCI_BUS_FLAGS_COMPAT_CFG_SPACE flag is actually a bus property that is comp=\n" - "liant with the purpose of bus_flags.\n" - "\n" - "I agree that pci_scan_bridge_extend() is already too complicated, so would =\n" - "you be okay to only add one line to it :\n" + "I agree that pci_scan_bridge_extend() is already too complicated, so would you be okay to only add one line to it :\n" " pci_bus_set_compat_cfg_space(child);\n" - "and put all the code I suggested in this new function pci_bus_set_compat_cf=\n" - "g_space() ? (also supporting PCI-X Mode 2 devices)\n" - "Improvement : this function can return immediately if the child bus has alr=\n" - "eady inherited the flag from its parent.\n" - "\n" - "\n" - "_______________________________________________\n" - "linux-arm-kernel mailing list\n" - "linux-arm-kernel@lists.infradead.org\n" - http://lists.infradead.org/mailman/listinfo/linux-arm-kernel + "and put all the code I suggested in this new function pci_bus_set_compat_cfg_space() ? (also supporting PCI-X Mode 2 devices)\n" + Improvement : this function can return immediately if the child bus has already inherited the flag from its parent. -fb40fbd5413f206562904b211c9b28492faa72e69d8b46ad53136b18b06e24ab +2b7d722738d5b8bdbc68ad78fa958c5860f19c471eafe896349b1de48fae9e1a
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.