diff for duplicates of <5AE9C1AB.8020403@kontron.com> diff --git a/a/1.txt b/N1/1.txt index bdbee38..73fc570 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,4 +1,4 @@ -Le 02/05/2018 15:26, Bjorn Helgaas a =E9crit : +Le 02/05/2018 15:26, Bjorn Helgaas a ?crit : > On Wed, May 02, 2018 at 12:57:31PM +0000, Gilles Buloz wrote: >> Hi Bjorn, >> See attached patch (tested ok this morning) @@ -6,113 +6,99 @@ Le 02/05/2018 15:26, Bjorn Helgaas a =E9crit : > > I can fix minor things myself, but I do need a signed-off-by from you > before applying (see -> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Docu= -mentation/process/submitting-patches.rst) +> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst) > > Please add a changelog, too, and include the patch inline (as opposed > to as an attachment) if possible. > ->> --- include/linux/pci.h.orig=092018-03-26 16:51:18.050000000 +0000 ->> +++ include/linux/pci.h=092018-04-30 18:29:14.140000000 +0000 +>> --- include/linux/pci.h.orig 2018-03-26 16:51:18.050000000 +0000 +>> +++ include/linux/pci.h 2018-04-30 18:29:14.140000000 +0000 >> @@ -193,6 +193,7 @@ >> enum pci_bus_flags { ->> =09PCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1, ->> =09PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2, ->> +=09PCI_BUS_FLAGS_NO_EXTCFG =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_NO_EXTCFG = (__force pci_bus_flags_t) 4, > Best if you can rebase this to v4.17-rc1. > >> }; ->> =20 +>> >> /* These values come from the PCI Express Spec */ ->> --- drivers/pci/probe.c.orig=092018-01-22 09:29:52.000000000 +0000 ->> +++ drivers/pci/probe.c=092018-05-02 13:44:35.530000000 +0000 +>> --- drivers/pci/probe.c.orig 2018-01-22 09:29:52.000000000 +0000 +>> +++ drivers/pci/probe.c 2018-05-02 13:44:35.530000000 +0000 >> @@ -664,6 +664,23 @@ ->> =09} +>> } >> } ->> =20 ->> +static bool pci_bridge_child_bus_ext_cfg_accessible(struct pci_dev *bri= -dge) +>> +>> +static bool pci_bridge_child_bus_ext_cfg_accessible(struct pci_dev *bridge) >> +{ ->> +=09int pos; ->> +=09u32 status; +>> + int pos; +>> + u32 status; >> + ->> +=09if (!pci_is_pcie(bridge) || /* PCI/PCI bridge */ ->> +=09 (pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCIE_BRIDGE) || /* PC= -Ie/PCI bridge in forward mode */ ->> +=09 (pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)) { /* PC= -Ie/PCI bridge in reverse mode */ ->> +=09=09pos =3D pci_find_capability(bridge, PCI_CAP_ID_PCIX); ->> +=09=09if (pos) ->> +=09=09=09pci_read_config_dword(bridge, pos + PCI_X_STATUS, &status); ->> +=09=09return pos && (status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MH= -Z)); ->> +=09} +>> + if (!pci_is_pcie(bridge) || /* PCI/PCI bridge */ +>> + (pci_pcie_type(bridge) == PCI_EXP_TYPE_PCIE_BRIDGE) || /* PCIe/PCI bridge in forward mode */ +>> + (pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE)) { /* PCIe/PCI bridge in reverse mode */ +>> + pos = pci_find_capability(bridge, PCI_CAP_ID_PCIX); +>> + if (pos) +>> + pci_read_config_dword(bridge, pos + PCI_X_STATUS, &status); +>> + return pos && (status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ)); +>> + } > Please arrange this so everything fits in 80 columns. > > If you can split it into several simpler "if" statements rather > than one with a complicated expression, that would also be good. > >> + ->> +=09return true; +>> + return true; >> +} >> + >> static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, ->> =09=09=09=09=09 struct pci_dev *bridge, int busnr) +>> struct pci_dev *bridge, int busnr) >> { >> @@ -725,6 +742,19 @@ ->> =09/* Create legacy_io and legacy_mem files for this bus */ ->> =09pci_create_legacy_files(child); ->> =20 ->> +=09/* ->> +=09 * if bus_flags inherited from parent bus do not already report lack= - of extended config ->> +=09 * space support, check if supported by child bus by checking its pa= -rent bridge ->> +=09 */ +>> /* Create legacy_io and legacy_mem files for this bus */ +>> pci_create_legacy_files(child); +>> +>> + /* +>> + * if bus_flags inherited from parent bus do not already report lack of extended config +>> + * space support, check if supported by child bus by checking its parent bridge +>> + */ > Wrap to fit in 80 columns. > ->> +=09if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)) { +>> + if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)) { > The double negative makes this a little bit hard to read. Maybe it > could be improved by reversing the sense of something? > ->> +=09=09if (!pci_bridge_child_bus_ext_cfg_accessible(bridge)) { ->> +=09=09=09child->bus_flags |=3D PCI_BUS_FLAGS_NO_EXTCFG; ->> +=09=09=09dev_info(&child->dev, "extended config space not accessible du= -e to parent bridge\n"); +>> + if (!pci_bridge_child_bus_ext_cfg_accessible(bridge)) { +>> + child->bus_flags |= PCI_BUS_FLAGS_NO_EXTCFG; +>> + dev_info(&child->dev, "extended config space not accessible due to parent bridge\n"); > In v4.17-rc1, there's a pci_info() that should work here (instead of > dev_info()). > ->> +=09=09} ->> +=09} else { ->> +=09=09dev_info(&child->dev, "extended config space not accessible due t= -o parent bus\n"); ->> +=09} +>> + } +>> + } else { +>> + dev_info(&child->dev, "extended config space not accessible due to parent bus\n"); +>> + } >> + ->> =09return child; +>> return child; >> } ->> =20 +>> >> @@ -1084,6 +1114,9 @@ ->> =09u32 status; ->> =09u16 class; ->> =20 ->> +=09if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG) ->> +=09=09return PCI_CFG_SPACE_SIZE; +>> u32 status; +>> u16 class; +>> +>> + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG) +>> + return PCI_CFG_SPACE_SIZE; >> + ->> =09class =3D dev->class >> 8; ->> =09if (class =3D=3D PCI_CLASS_BRIDGE_HOST) ->> =09=09return pci_cfg_space_size_ext(dev); +>> class = dev->class >> 8; +>> if (class == PCI_CLASS_BRIDGE_HOST) +>> return pci_cfg_space_size_ext(dev); > . > -OK I'm going to learn about signing (sorry this is my first "official" patc= -h). -I'll download kernel v4.17-rc1 and write the patch for it; however I hope I= -'ll be able to test it on my platform without the=20 -freescale addons I have on 4.1.35, because I don't want to send an untested= - patch. -For "if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG))", I don't= - understand what you mean with "double negative", as I=20 +OK I'm going to learn about signing (sorry this is my first "official" patch). +I'll download kernel v4.17-rc1 and write the patch for it; however I hope I'll be able to test it on my platform without the +freescale addons I have on 4.1.35, because I don't want to send an untested patch. +For "if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG))", I don't understand what you mean with "double negative", as I only have one "!" -Do you think it's worth keeping the two dev_info() ? The code would be smal= -ler without; however this may help to have it for debug.=20 +Do you think it's worth keeping the two dev_info() ? The code would be smaller without; however this may help to have it for debug. Maybe use _dbg instead of _info ? diff --git a/a/content_digest b/N1/content_digest index 62d0247..53417e5 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -8,18 +8,13 @@ "ref\05AE75809.30701@kontron.com\0" "ref\05AE9B5BB.2080003@kontron.com\0" "ref\020180502132501.GE11698@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\0Wed, 2 May 2018 13:48: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 02/05/2018 15:26, Bjorn Helgaas a =E9crit :\n" + "Le 02/05/2018 15:26, Bjorn Helgaas a ?crit :\n" "> On Wed, May 02, 2018 at 12:57:31PM +0000, Gilles Buloz wrote:\n" ">> Hi Bjorn,\n" ">> See attached patch (tested ok this morning)\n" @@ -27,115 +22,101 @@ ">\n" "> I can fix minor things myself, but I do need a signed-off-by from you\n" "> before applying (see\n" - "> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Docu=\n" - "mentation/process/submitting-patches.rst)\n" + "> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst)\n" ">\n" "> Please add a changelog, too, and include the patch inline (as opposed\n" "> to as an attachment) if possible.\n" ">\n" - ">> --- include/linux/pci.h.orig=092018-03-26 16:51:18.050000000 +0000\n" - ">> +++ include/linux/pci.h=092018-04-30 18:29:14.140000000 +0000\n" + ">> --- include/linux/pci.h.orig\t2018-03-26 16:51:18.050000000 +0000\n" + ">> +++ include/linux/pci.h\t2018-04-30 18:29:14.140000000 +0000\n" ">> @@ -193,6 +193,7 @@\n" ">> enum pci_bus_flags {\n" - ">> =09PCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1,\n" - ">> =09PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2,\n" - ">> +=09PCI_BUS_FLAGS_NO_EXTCFG =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_NO_EXTCFG = (__force pci_bus_flags_t) 4,\n" "> Best if you can rebase this to v4.17-rc1.\n" ">\n" ">> };\n" - ">> =20\n" + ">> \n" ">> /* These values come from the PCI Express Spec */\n" - ">> --- drivers/pci/probe.c.orig=092018-01-22 09:29:52.000000000 +0000\n" - ">> +++ drivers/pci/probe.c=092018-05-02 13:44:35.530000000 +0000\n" + ">> --- drivers/pci/probe.c.orig\t2018-01-22 09:29:52.000000000 +0000\n" + ">> +++ drivers/pci/probe.c\t2018-05-02 13:44:35.530000000 +0000\n" ">> @@ -664,6 +664,23 @@\n" - ">> =09}\n" + ">> \t}\n" ">> }\n" - ">> =20\n" - ">> +static bool pci_bridge_child_bus_ext_cfg_accessible(struct pci_dev *bri=\n" - "dge)\n" + ">> \n" + ">> +static bool pci_bridge_child_bus_ext_cfg_accessible(struct pci_dev *bridge)\n" ">> +{\n" - ">> +=09int pos;\n" - ">> +=09u32 status;\n" + ">> +\tint pos;\n" + ">> +\tu32 status;\n" ">> +\n" - ">> +=09if (!pci_is_pcie(bridge) || /* PCI/PCI bridge */\n" - ">> +=09 (pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCIE_BRIDGE) || /* PC=\n" - "Ie/PCI bridge in forward mode */\n" - ">> +=09 (pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)) { /* PC=\n" - "Ie/PCI bridge in reverse mode */\n" - ">> +=09=09pos =3D pci_find_capability(bridge, PCI_CAP_ID_PCIX);\n" - ">> +=09=09if (pos)\n" - ">> +=09=09=09pci_read_config_dword(bridge, pos + PCI_X_STATUS, &status);\n" - ">> +=09=09return pos && (status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MH=\n" - "Z));\n" - ">> +=09}\n" + ">> +\tif (!pci_is_pcie(bridge) || /* PCI/PCI bridge */\n" + ">> +\t (pci_pcie_type(bridge) == PCI_EXP_TYPE_PCIE_BRIDGE) || /* PCIe/PCI bridge in forward mode */\n" + ">> +\t (pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE)) { /* PCIe/PCI bridge in reverse mode */\n" + ">> +\t\tpos = pci_find_capability(bridge, PCI_CAP_ID_PCIX);\n" + ">> +\t\tif (pos)\n" + ">> +\t\t\tpci_read_config_dword(bridge, pos + PCI_X_STATUS, &status);\n" + ">> +\t\treturn pos && (status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ));\n" + ">> +\t}\n" "> Please arrange this so everything fits in 80 columns.\n" ">\n" "> If you can split it into several simpler \"if\" statements rather\n" "> than one with a complicated expression, that would also be good.\n" ">\n" ">> +\n" - ">> +=09return true;\n" + ">> +\treturn true;\n" ">> +}\n" ">> +\n" ">> static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,\n" - ">> =09=09=09=09=09 struct pci_dev *bridge, int busnr)\n" + ">> \t\t\t\t\t struct pci_dev *bridge, int busnr)\n" ">> {\n" ">> @@ -725,6 +742,19 @@\n" - ">> =09/* Create legacy_io and legacy_mem files for this bus */\n" - ">> =09pci_create_legacy_files(child);\n" - ">> =20\n" - ">> +=09/*\n" - ">> +=09 * if bus_flags inherited from parent bus do not already report lack=\n" - " of extended config\n" - ">> +=09 * space support, check if supported by child bus by checking its pa=\n" - "rent bridge\n" - ">> +=09 */\n" + ">> \t/* Create legacy_io and legacy_mem files for this bus */\n" + ">> \tpci_create_legacy_files(child);\n" + ">> \n" + ">> +\t/*\n" + ">> +\t * if bus_flags inherited from parent bus do not already report lack of extended config\n" + ">> +\t * space support, check if supported by child bus by checking its parent bridge\n" + ">> +\t */\n" "> Wrap to fit in 80 columns.\n" ">\n" - ">> +=09if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)) {\n" + ">> +\tif (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)) {\n" "> The double negative makes this a little bit hard to read. Maybe it\n" "> could be improved by reversing the sense of something?\n" ">\n" - ">> +=09=09if (!pci_bridge_child_bus_ext_cfg_accessible(bridge)) {\n" - ">> +=09=09=09child->bus_flags |=3D PCI_BUS_FLAGS_NO_EXTCFG;\n" - ">> +=09=09=09dev_info(&child->dev, \"extended config space not accessible du=\n" - "e to parent bridge\\n\");\n" + ">> +\t\tif (!pci_bridge_child_bus_ext_cfg_accessible(bridge)) {\n" + ">> +\t\t\tchild->bus_flags |= PCI_BUS_FLAGS_NO_EXTCFG;\n" + ">> +\t\t\tdev_info(&child->dev, \"extended config space not accessible due to parent bridge\\n\");\n" "> In v4.17-rc1, there's a pci_info() that should work here (instead of\n" "> dev_info()).\n" ">\n" - ">> +=09=09}\n" - ">> +=09} else {\n" - ">> +=09=09dev_info(&child->dev, \"extended config space not accessible due t=\n" - "o parent bus\\n\");\n" - ">> +=09}\n" + ">> +\t\t}\n" + ">> +\t} else {\n" + ">> +\t\tdev_info(&child->dev, \"extended config space not accessible due to parent bus\\n\");\n" + ">> +\t}\n" ">> +\n" - ">> =09return child;\n" + ">> \treturn child;\n" ">> }\n" - ">> =20\n" + ">> \n" ">> @@ -1084,6 +1114,9 @@\n" - ">> =09u32 status;\n" - ">> =09u16 class;\n" - ">> =20\n" - ">> +=09if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)\n" - ">> +=09=09return PCI_CFG_SPACE_SIZE;\n" + ">> \tu32 status;\n" + ">> \tu16 class;\n" + ">> \n" + ">> +\tif (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)\n" + ">> +\t\treturn PCI_CFG_SPACE_SIZE;\n" ">> +\n" - ">> =09class =3D dev->class >> 8;\n" - ">> =09if (class =3D=3D PCI_CLASS_BRIDGE_HOST)\n" - ">> =09=09return pci_cfg_space_size_ext(dev);\n" + ">> \tclass = dev->class >> 8;\n" + ">> \tif (class == PCI_CLASS_BRIDGE_HOST)\n" + ">> \t\treturn pci_cfg_space_size_ext(dev);\n" "> .\n" ">\n" - "OK I'm going to learn about signing (sorry this is my first \"official\" patc=\n" - "h).\n" - "I'll download kernel v4.17-rc1 and write the patch for it; however I hope I=\n" - "'ll be able to test it on my platform without the=20\n" - "freescale addons I have on 4.1.35, because I don't want to send an untested=\n" - " patch.\n" - "For \"if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG))\", I don't=\n" - " understand what you mean with \"double negative\", as I=20\n" + "OK I'm going to learn about signing (sorry this is my first \"official\" patch).\n" + "I'll download kernel v4.17-rc1 and write the patch for it; however I hope I'll be able to test it on my platform without the \n" + "freescale addons I have on 4.1.35, because I don't want to send an untested patch.\n" + "For \"if (bridge && !(child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG))\", I don't understand what you mean with \"double negative\", as I \n" "only have one \"!\"\n" "\n" - "Do you think it's worth keeping the two dev_info() ? The code would be smal=\n" - "ler without; however this may help to have it for debug.=20\n" + "Do you think it's worth keeping the two dev_info() ? The code would be smaller without; however this may help to have it for debug. \n" Maybe use _dbg instead of _info ? -dfbef5c1cbfa3a9e3631a290436c1ec5bcb9603e0630ca15c368791ce0b0f574 +4284674a8d72ebd13c82f3a983c877c26cf85e4b3daa115c80074b18ce1300fc
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.