Follow the reasoning from commit 842de40d93e0 ("PCI: add generic
pci_enable_resources()"):
The only functional difference from the MIPS version is that the
generic one uses "!r->parent" to check for resource collisions
instead of "!r->start && r->end".
That should have no effect on any pci-legacy driver.
Suggested-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Ilya Lipnitskiy <[email protected]>
---
arch/mips/pci/pci-legacy.c | 40 ++------------------------------------
1 file changed, 2 insertions(+), 38 deletions(-)
diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 78c22987bef0..c24226ea0a6e 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -241,47 +241,11 @@ static int __init pcibios_init(void)
subsys_initcall(pcibios_init);
-static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1<<idx)))
- continue;
-
- r = &dev->resource[idx];
- if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
- continue;
- if ((idx == PCI_ROM_RESOURCE) &&
- (!(r->flags & IORESOURCE_ROM_ENABLE)))
- continue;
- if (!r->start && r->end) {
- pci_err(dev,
- "can't enable device: resource collisions\n");
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
- int err;
+ int err = pci_enable_resources(dev, mask);
- if ((err = pcibios_enable_resources(dev, mask)) < 0)
+ if (err < 0)
return err;
return pcibios_plat_dev_init(dev);
--
2.31.1
On Tue, Apr 13, 2021 at 08:12:40PM -0700, Ilya Lipnitskiy wrote:
> Follow the reasoning from commit 842de40d93e0 ("PCI: add generic
> pci_enable_resources()"):
>
> The only functional difference from the MIPS version is that the
> generic one uses "!r->parent" to check for resource collisions
> instead of "!r->start && r->end".
>
> That should have no effect on any pci-legacy driver.
>
Unfortunately it does. With this patch in place, all my mips qemu emulations
fail to boot from ide/ata drive; the driver is not instantiated. The error
message is:
ata_piix 0000:00:0a.1: can't enable device: BAR 0 [io 0x01f0-0x01f7] not claimed
ata_piix: probe of 0000:00:0a.1 failed with error -22
Reverting this patch fixes the problem, and the message displayed by the driver
is:
ata_piix 0000:00:0a.1: enabling device (0000 -> 0001)
Bisect log is attached.
Guenter
---
# bad: [50b8b1d699ac313c0a07a3c185ffb23aecab8abb] Add linux-next specific files for 20210419
# good: [bf05bf16c76bb44ab5156223e1e58e26dfe30a88] Linux 5.12-rc8
git bisect start 'HEAD' 'v5.12-rc8'
# bad: [c4bb91fc07e59241cde97f913d7a2fbedc248f0d] Merge remote-tracking branch 'crypto/master'
git bisect bad c4bb91fc07e59241cde97f913d7a2fbedc248f0d
# bad: [499f739ad70f2a58aac985dceb25ca7666da88be] Merge remote-tracking branch 'jc_docs/docs-next'
git bisect bad 499f739ad70f2a58aac985dceb25ca7666da88be
# bad: [11b56408a328d1c5c4dfa7667c5dc46956b64aec] Merge remote-tracking branch 'parisc-hd/for-next'
git bisect bad 11b56408a328d1c5c4dfa7667c5dc46956b64aec
# good: [09ccc0ee1227f2cfe50d8dbbe241d115d9b3885f] Merge branch 'arm/defconfig' into for-next
git bisect good 09ccc0ee1227f2cfe50d8dbbe241d115d9b3885f
# good: [a5b76c2f17330e266a5c56dde21430e27b0d0dbb] Merge remote-tracking branch 'arm-soc/for-next'
git bisect good a5b76c2f17330e266a5c56dde21430e27b0d0dbb
# good: [1e4241f6813f1c1a0027d96df075ffd01808b3cf] Merge remote-tracking branch 'ti-k3/ti-k3-next'
git bisect good 1e4241f6813f1c1a0027d96df075ffd01808b3cf
# good: [7496a43be7a362391607d78e49a3f28de80029ce] Merge remote-tracking branch 'h8300/h8300-next'
git bisect good 7496a43be7a362391607d78e49a3f28de80029ce
# good: [66633abd0642f1e89d26e15f36fb13d3a1c535ff] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again
git bisect good 66633abd0642f1e89d26e15f36fb13d3a1c535ff
# good: [2c92ef8ff8d327797c1920ae7f938bcc6f3f7421] MIPS: Fix strnlen_user access check
git bisect good 2c92ef8ff8d327797c1920ae7f938bcc6f3f7421
# good: [3a070801c61f4e3987d59b1068368ba71d727208] Merge remote-tracking branch 'microblaze/next'
git bisect good 3a070801c61f4e3987d59b1068368ba71d727208
# good: [317f553bb677e324c9c865ff7f14597bc5ceeb9c] MIPS: pci-legacy: remove redundant info messages
git bisect good 317f553bb677e324c9c865ff7f14597bc5ceeb9c
# bad: [6ce48897ce476bed86fde28752c27596e8753277] MIPS: Loongson64: Add kexec/kdump support
git bisect bad 6ce48897ce476bed86fde28752c27596e8753277
# bad: [99bca615d89510917864fac6b26fd343eff2aba2] MIPS: pci-legacy: use generic pci_enable_resources
git bisect bad 99bca615d89510917864fac6b26fd343eff2aba2
# good: [0af83d2e447af3e5098583cb6320bb1b1fb0976b] MIPS: pci-legacy: remove busn_resource field
git bisect good 0af83d2e447af3e5098583cb6320bb1b1fb0976b
# first bad commit: [99bca615d89510917864fac6b26fd343eff2aba2] MIPS: pci-legacy: use generic pci_enable_resources
This mostly reverts commit 99bca615d895 ("MIPS: pci-legacy: use generic
pci_enable_resources"). Fixes regressions such as:
ata_piix 0000:00:0a.1: can't enable device: BAR 0 [io 0x01f0-0x01f7] not
claimed
ata_piix: probe of 0000:00:0a.1 failed with error -22
The only changes from the strict revert are to fix checkpatch errors:
ERROR: spaces required around that '=' (ctx:VxV)
#33: FILE: arch/mips/pci/pci-legacy.c:252:
+ for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
^
ERROR: do not use assignment in if condition
#67: FILE: arch/mips/pci/pci-legacy.c:284:
+ if ((err = pcibios_enable_resources(dev, mask)) < 0)
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Ilya Lipnitskiy <[email protected]>
---
Guenter, sorry about this - let's just revert this for now to minimize
turmoil to the legacy PCI code. Obviously, this needs more testing
before applying. Thanks for your report!
arch/mips/pci/pci-legacy.c | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index c24226ea0a6e..468722c8a5c6 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -241,9 +241,45 @@ static int __init pcibios_init(void)
subsys_initcall(pcibios_init);
+static int pcibios_enable_resources(struct pci_dev *dev, int mask)
+{
+ u16 cmd, old_cmd;
+ int idx;
+ struct resource *r;
+
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ old_cmd = cmd;
+ for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
+ /* Only set up the requested stuff */
+ if (!(mask & (1<<idx)))
+ continue;
+
+ r = &dev->resource[idx];
+ if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
+ continue;
+ if ((idx == PCI_ROM_RESOURCE) &&
+ (!(r->flags & IORESOURCE_ROM_ENABLE)))
+ continue;
+ if (!r->start && r->end) {
+ pci_err(dev,
+ "can't enable device: resource collisions\n");
+ return -EINVAL;
+ }
+ if (r->flags & IORESOURCE_IO)
+ cmd |= PCI_COMMAND_IO;
+ if (r->flags & IORESOURCE_MEM)
+ cmd |= PCI_COMMAND_MEMORY;
+ }
+ if (cmd != old_cmd) {
+ pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
+ pci_write_config_word(dev, PCI_COMMAND, cmd);
+ }
+ return 0;
+}
+
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
- int err = pci_enable_resources(dev, mask);
+ int err = pcibios_enable_resources(dev, mask);
if (err < 0)
return err;
--
2.31.1
On Mon, Apr 19, 2021 at 11:39:43PM -0700, Ilya Lipnitskiy wrote:
> This mostly reverts commit 99bca615d895 ("MIPS: pci-legacy: use generic
> pci_enable_resources"). Fixes regressions such as:
> ata_piix 0000:00:0a.1: can't enable device: BAR 0 [io 0x01f0-0x01f7] not
> claimed
> ata_piix: probe of 0000:00:0a.1 failed with error -22
>
> The only changes from the strict revert are to fix checkpatch errors:
> ERROR: spaces required around that '=' (ctx:VxV)
> #33: FILE: arch/mips/pci/pci-legacy.c:252:
> + for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
> ^
>
> ERROR: do not use assignment in if condition
> #67: FILE: arch/mips/pci/pci-legacy.c:284:
> + if ((err = pcibios_enable_resources(dev, mask)) < 0)
>
> Reported-by: Guenter Roeck <[email protected]>
> Signed-off-by: Ilya Lipnitskiy <[email protected]>
Tested-by: Guenter Roeck <[email protected]>
Guenter
On Mon, Apr 19, 2021 at 11:39:43PM -0700, Ilya Lipnitskiy wrote:
> This mostly reverts commit 99bca615d895 ("MIPS: pci-legacy: use generic
> pci_enable_resources"). Fixes regressions such as:
> ata_piix 0000:00:0a.1: can't enable device: BAR 0 [io 0x01f0-0x01f7] not
> claimed
> ata_piix: probe of 0000:00:0a.1 failed with error -22
>
> The only changes from the strict revert are to fix checkpatch errors:
> ERROR: spaces required around that '=' (ctx:VxV)
> #33: FILE: arch/mips/pci/pci-legacy.c:252:
> + for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
> ^
>
> ERROR: do not use assignment in if condition
> #67: FILE: arch/mips/pci/pci-legacy.c:284:
> + if ((err = pcibios_enable_resources(dev, mask)) < 0)
>
> Reported-by: Guenter Roeck <[email protected]>
> Signed-off-by: Ilya Lipnitskiy <[email protected]>
> ---
> Guenter, sorry about this - let's just revert this for now to minimize
> turmoil to the legacy PCI code. Obviously, this needs more testing
> before applying. Thanks for your report!
>
> arch/mips/pci/pci-legacy.c | 38 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]