usb: pci-quirks: do not access OHCI_FMINTERVAL register on ULI hw

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Since preserving OHCI_FMINTERVAL was originally done only for NVIDIA hardware and only later (in c6187597) was turned unconditional, and

c6187597 commit message again mentions only NVIDIA, I think it should be safe to disable preserving OHCI_FMINTERVAL if device vendor is ULI.

Signed-off-by Nikita Yushchenko <[email protected]>

Quoted 3 lines

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Which access, the read or the write?

It looks like you haven't read ohci_shutdown() and ohci_run() in drivers/usb/host/ohci-hcd.c. They both access the fminterval register, and this patch doesn't change them.

So I don't see why this patch should make any difference to the system's operation.

27.05.2014 19:08, Alan Stern пишет:

Quoted 6 lines · Nikita Yushchenko

Quoted 3 lines

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Which access, the read or the write?

things are a bit more complex.

If inserting printk's as below

drivers/usb/host/pci-quirks.c +3 -0

+               printk("A5\n");
                udelay(1);
        }
+       printk("A6\n");
        writel(fminterval, base + OHCI_FMINTERVAL);
+       printk("A7\n");
 
        /* Now the controller is safely in SUSPEND and nothing can wake it up */
        iounmap(base);

attempt to boot (using arch/powerpc/configs/mpc85xx_smp_defconfig and deb file built by "make p2020ds.dtb" from the same kernel tree) results in

...
A1
random: nonblocking pool is initialized
irq 26: nobody cared (try booting with the "irqpoll" option)

CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc6-dirty #5
Call Trace:
[effe5ed0] [c000726c] show_stack+0x48/0x15c (unreliable)
[effe5f10] [c05eece4] dump_stack+0x78/0xa0
[effe5f20] [c0087acc] __report_bad_irq+0x38/0x100
[effe5f40] [c0088060] note_interrupt+0x224/0x280
[effe5f70] [c008563c] handle_irq_event_percpu+0xc8/0x178
[effe5fa0] [c0085730] handle_irq_event+0x44/0x74
[effe5fc0] [c0088f88] handle_fasteoi_irq+0xd4/0x1d0
[effe5fd0] [c0084d20] generic_handle_irq+0x30/0x50
[effe5fe0] [c00048d0] __do_irq+0x2c/0x70
[effe5ff0] [c000d9a0] call_do_irq+0x24/0x3c
[ef047d10] [c00049a0] do_IRQ+0x8c/0xf8
[ef047d30] [c000f598] ret_from_except+0x0/0x18
--- Exception: 501 at quirk_usb_early_handoff+0x794/0x85c
    LR = quirk_usb_early_handoff+0x788/0x85c

[ef047e30] [c02bd66c] pci_fixup_device+0xe0/0x19c [ef047e60] [c07d4e1c] pci_apply_final_quirks+0x60/0x13c [ef047e90] [c0002494] do_one_initcall+0x158/0x19c [ef047f00] [c07ba928] kernel_init_freeable+0x130/0x1e4 [ef047f30] [c0002a78] kernel_init+0x14/0x10c [ef047f40] [c000f014] ret_from_kernel_thread+0x5c/0x64 handlers:
[<c001cc78>] fsl_pci_pme_handle
Disabling IRQ #26

A2
A3
A4
A5
A4
A5
A4
A5
A4
A5
A4

and hang.

with my patch applied, board just boots ok and works.

Quoted 4 lines

It looks like you haven't read ohci_shutdown() and ohci_run() in drivers/usb/host/ohci-hcd.c. They both access the fminterval register, and this patch doesn't change them.

Quoted 2 lines

So I don't see why this patch should make any difference to the system's operation.

I think problem is caused by access to OHCI regs from PCI quirks - before driver was initialized. ULI1553 southbridge chip could be in strange state at this point.

It would help to print the value of fminterval. And here to print the value obtained by the readl().

Quoted 17 lines

irq 26: nobody cared (try booting with the "irqpoll" option)

CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc6-dirty #5
Call Trace:
[effe5ed0] [c000726c] show_stack+0x48/0x15c (unreliable)
[effe5f10] [c05eece4] dump_stack+0x78/0xa0
[effe5f20] [c0087acc] __report_bad_irq+0x38/0x100
[effe5f40] [c0088060] note_interrupt+0x224/0x280
[effe5f70] [c008563c] handle_irq_event_percpu+0xc8/0x178
[effe5fa0] [c0085730] handle_irq_event+0x44/0x74
[effe5fc0] [c0088f88] handle_fasteoi_irq+0xd4/0x1d0
[effe5fd0] [c0084d20] generic_handle_irq+0x30/0x50
[effe5fe0] [c00048d0] __do_irq+0x2c/0x70
[effe5ff0] [c000d9a0] call_do_irq+0x24/0x3c
[ef047d10] [c00049a0] do_IRQ+0x8c/0xf8
[ef047d30] [c000f598] ret_from_except+0x0/0x18
--- Exception: 501 at quirk_usb_early_handoff+0x794/0x85c
    LR = quirk_usb_early_handoff+0x788/0x85c

That exception looks very suspicious. Do you know what it means?

It looks like this IRQ 26 stuff has no connection with OHCI, right?

Quoted 12 lines

A2
A3
A4
A5
A4
A5
A4
A5
A4
A5
A4

Quoted 1 line

and hang.

Meaning that it stops completely, not that it goes into an infinite loop?

Quoted 3 lines

I think problem is caused by access to OHCI regs from PCI quirks - before driver was initialized. ULI1553 southbridge chip could be in strange state at this point.

If that is the cause, we ought to be able to see it from the values printed out by the debugging statements. And if that is so, it's a serious problem. The southbridge chip really should be working at this point, because the quirk_usb_handoff_* routines need to be able to communicate with the host controllers.

Quoted 4 lines

Quoted 3 lines

I think problem is caused by access to OHCI regs from PCI quirks - before driver was initialized. ULI1553 southbridge chip could be in strange state at this point.

Quoted 5 lines

If that is the cause, we ought to be able to see it from the values printed out by the debugging statements. And if that is so, it's a serious problem. The southbridge chip really should be working at this point, because the quirk_usb_handoff_* routines need to be able to communicate with the host controllers.

In this case, communication looks possible. However, read of OHCI_FMINTERVAL register somehow breaks it.

I don't know how linux usb subsystem should behave against such "half-existing" hardware. Perhaps hanging is not the best idea... but maybe it should be fixed elsewhere, e.g. by masking non-wired devices in platform PCI setup. Perhaps controlled by some device-tree key.

Quoted 5 lines

I don't know how linux usb subsystem should behave against such "half-existing" hardware. Perhaps hanging is not the best idea... but maybe it should be fixed elsewhere, e.g. by masking non-wired devices in platform PCI setup. Perhaps controlled by some device-tree key.

Does it have a unique svid/sdid set for the platform - if so you could just blacklist that combination of vid/did/svid/sdid.

29.05.2014 18:42, One Thousand Gnomes пишет:

Quoted 6 lines

Quoted 5 lines

I don't know how linux usb subsystem should behave against such "half-existing" hardware. Perhaps hanging is not the best idea... but maybe it should be fixed elsewhere, e.g. by masking non-wired devices in platform PCI setup. Perhaps controlled by some device-tree key.

Quoted 2 lines

Does it have a unique svid/sdid set for the platform - if so you could just blacklist that combination of vid/did/svid/sdid.

Unfortunately vid/did/svid/sdid come from ULI 1553 southbridge chip, that is used by other hardware as well. AFAIK it is still being manufactured, and could appear in PCs or laptops.

29.05.2014 18:42, One Thousand Gnomes пишет:

Quoted 8 lines

Quoted 5 lines

I don't know how linux usb subsystem should behave against such "half-existing" hardware. Perhaps hanging is not the best idea... but maybe it should be fixed elsewhere, e.g. by masking non-wired devices in platform PCI setup. Perhaps controlled by some device-tree key.

Does it have a unique svid/sdid set for the platform - if so you could just blacklist that combination of vid/did/svid/sdid.

Unfortunately vid/did/svid/sdid come from ULI 1553 southbridge chip, that is used by other hardware as well. AFAIK it is still being manufactured, and could appear in PCs or laptops.

Quoted 2 lines

It would help to print the value of fminterval. And here to print the value obtained by the readl().

I've checked these... all values read as 0xffffffff - which does not look correct

Quoted 18 lines

29.05.2014 18:42, One Thousand Gnomes пишет:

Quoted 8 lines

Quoted 5 lines

I don't know how linux usb subsystem should behave against such "half-existing" hardware. Perhaps hanging is not the best idea... but maybe it should be fixed elsewhere, e.g. by masking non-wired devices in platform PCI setup. Perhaps controlled by some device-tree key.

Does it have a unique svid/sdid set for the platform - if so you could just blacklist that combination of vid/did/svid/sdid.

Unfortunately vid/did/svid/sdid come from ULI 1553 southbridge chip, that is used by other hardware as well. AFAIK it is still being manufactured, and could appear in PCs or laptops.

Quoted 2 lines

It would help to print the value of fminterval. And here to print the value obtained by the readl().

Quoted 2 lines

I've checked these... all values read as 0xffffffff - which does not look correct

You could have the platform setup code read one of those hardware registers, such as FMINTERVAL. If it obtains 0xffffffff, don't register the OHCI controller as a platform device.

Quoted 6 lines

Quoted 2 lines

I've checked these... all values read as 0xffffffff - which does not look correct

You could have the platform setup code read one of those hardware registers, such as FMINTERVAL. If it obtains 0xffffffff, don't register the OHCI controller as a platform device.

It is not a platform_device, it is PCI device that is found via bus scan.

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Since preserving OHCI_FMINTERVAL was originally done only for NVIDIA hardware and only later (in c6187597) was turned unconditional, and c6187597 commit message again mentions only NVIDIA, I think it should be

safe to disable preserving OHCI_FMINTERVAL if device vendor is ULI.

Signed-off-by Nikita Yushchenko <[email protected]>

---
  drivers/usb/host/pci-quirks.c |    8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

Hello.

On 05/27/2014 08:56 AM, Nikita Yushchenko wrote:

Quoted 6 lines

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Since preserving OHCI_FMINTERVAL was originally done only for NVIDIA hardware and only later (in c6187597) was turned unconditional, and c6187597 commit message again mentions only NVIDIA, I think it should be

Please also specify that commit's summary line in parens.

Quoted 8 lines

Quoted 6 lines

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Since preserving OHCI_FMINTERVAL was originally done only for NVIDIA hardware and only later (in c6187597) was turned unconditional, and c6187597 commit message again mentions only NVIDIA, I think it should be

Please also specify that commit's summary line in parens.

Sorry but can't get what you mean. Could you please be a bit more verbose?

Nikita

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Since preserving OHCI_FMINTERVAL was originally done only for NVIDIA hardware and only later (in c6187597) was turned unconditional, and c6187597 commit message again mentions only NVIDIA, I think it should be safe to disable preserving OHCI_FMINTERVAL if device vendor is ULI.

Signed-off-by Nikita Yushchenko <[email protected]>

---
 drivers/usb/host/pci-quirks.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

drivers/usb/host/pci-quirks.c +0 -0

index 00661d3..5acbd5b 100644

Quoted 7 lines

This access causes hang on Freescale P2020DS board (that has OHCI provided by ULI 1533 chip).

Since preserving OHCI_FMINTERVAL was originally done only for NVIDIA hardware and only later (in c6187597) was turned unconditional, and c6187597 commit message again mentions only NVIDIA, I think it should be safe to disable preserving OHCI_FMINTERVAL if device vendor is ULI.

Have you tested this?

drivers/usb/host/pci-quirks.c static void quirk_usb_handoff_ohci(struct pci_dev *pdev) · line 574

Quoted 2 lines

drivers/usb/host/pci-quirks.c +1 -1

-	u32 fminterval;
+	u32 uninitialized_var(fminterval);

Why is this change needed? Sounds like a compiler bug to me :)