This is a note to let you know that I've just added the patch titled
usb: dwc3: Fix NULL pointer exception in dwc3_pci_remove()
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.
If you have any questions about this process, please let me know.
>From 7b412b04a0c7000293008231ce8413056abb1982 Mon Sep 17 00:00:00 2001
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com>
Date: Wed, 17 Oct 2018 11:40:26 -0700
Subject: usb: dwc3: Fix NULL pointer exception in dwc3_pci_remove()
In dwc3_pci_quirks() function, gpiod lookup table is only registered for
baytrail SOC. But in dwc3_pci_remove(), we try to unregistered it
without any checks. This leads to NULL pointer de-reference exception in
gpiod_remove_lookup_table() when unloading the module for non baytrail
SOCs. This patch fixes this issue.
Fixes: 5741022cbdf3 ("usb: dwc3: pci: Add GPIO lookup table on platforms
without ACPI GPIO resources")
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus(a)linux.intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi(a)linux.intel.com>
---
drivers/usb/dwc3/dwc3-pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 1286076a8890..842795856bf4 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -283,8 +283,10 @@ static int dwc3_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
static void dwc3_pci_remove(struct pci_dev *pci)
{
struct dwc3_pci *dwc = pci_get_drvdata(pci);
+ struct pci_dev *pdev = dwc->pci;
- gpiod_remove_lookup_table(&platform_bytcr_gpios);
+ if (pdev->device == PCI_DEVICE_ID_INTEL_BYT)
+ gpiod_remove_lookup_table(&platform_bytcr_gpios);
#ifdef CONFIG_PM
cancel_work_sync(&dwc->wakeup_work);
#endif
--
2.19.1
This is a note to let you know that I've just added the patch titled
usb: dwc3: gadget: Properly check last unaligned/zero chain TRB
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.
If you have any questions about this process, please let me know.
>From ba3a51ac32ebcf8d0a54b37f1af268ad8a31c52f Mon Sep 17 00:00:00 2001
From: Thinh Nguyen <Thinh.Nguyen(a)synopsys.com>
Date: Thu, 2 Aug 2018 20:17:16 -0700
Subject: usb: dwc3: gadget: Properly check last unaligned/zero chain TRB
Current check for the last extra TRB for zero and unaligned transfers
does not account for isoc OUT. The last TRB of the Buffer Descriptor for
isoc OUT transfers will be retired with HWO=0. As a result, we won't
return early. The req->remaining will be updated to include the BUFSIZ
count of the extra TRB, and the actual number of transferred bytes
calculation will be wrong.
To fix this, check whether it's a short or zero packet and the last TRB
chain bit to return early.
Fixes: c6267a51639b ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Thinh Nguyen <thinhn(a)synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi(a)linux.intel.com>
---
drivers/usb/dwc3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 679c12e14522..06e22afdf3d1 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2259,7 +2259,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
* with one TRB pending in the ring. We need to manually clear HWO bit
* from that TRB.
*/
- if ((req->zero || req->unaligned) && (trb->ctrl & DWC3_TRB_CTRL_HWO)) {
+ if ((req->zero || req->unaligned) && !(trb->ctrl & DWC3_TRB_CTRL_CHN)) {
trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
return 1;
}
--
2.19.1
This is a note to let you know that I've just added the patch titled
usb: dwc3: core: Clean up ULPI device
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.
If you have any questions about this process, please let me know.
>From 08fd9a82fda86529bb2f2af3c2f7cb657b4d3066 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
Date: Mon, 27 Aug 2018 18:30:16 +0300
Subject: usb: dwc3: core: Clean up ULPI device
If dwc3_core_init_mode() fails with deferred probe,
next probe fails on sysfs with
sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:11.0/dwc3.0.auto/dwc3.0.auto.ulpi'
To avoid this failure, clean up ULPI device.
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi(a)linux.intel.com>
---
drivers/usb/dwc3/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index becfbb87f791..2f2048aa5fde 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1499,6 +1499,7 @@ static int dwc3_probe(struct platform_device *pdev)
err5:
dwc3_event_buffers_cleanup(dwc);
+ dwc3_ulpi_exit(dwc);
err4:
dwc3_free_scratch_buffers(dwc);
--
2.19.1
This reverts commit 8c934095fa2f ("PCI: dwc: Clear MSI interrupt status
after it is handled, not before").
This is a very real race that we observed quickly after switching from
4.13 to 4.16. Using a custom PCI-e endpoint and driver, I was able to
track it to the precise race and verify the fixed behavior, as will be
described below.
This bug was originally fixed in 2013, in commit ca1658921b63 ("PCI:
designware: Fix missing MSI IRQs") The discussion of that commit,
archived in patchwork [1], is informative and worth reading.
The bug was re-added in the '8c934 commit this is reverting, which
appeared in the 4.14 kernel.
Unfortunately, Synopsys appears to consider the operation of this PCI-e
controller secret. They provide no publicly available docs for it nor
allow the references manuals of SoCs using the controller to publish any
documentation of it.
So I can not say certain this code is correctly using the controller's
features. However, extensive testing gives me high confidence in the
accuracy of what is described below.
If an MSI is received by the PCI-e controller while the status register
bit for that MSI is set, the PCI-e controller will NOT generate another
interrupt. In addition, it will NOT queue or otherwise mark the
interrupt as "pending", which means it will NOT generate an interrupt
when the status bit is unmasked.
This gives the following race scenario:
1. An MSI is received by, and the status bit for the MSI is set in, the
DWC PCI-e controller.
2. dw_handle_msi_irq() calls a driver's registered interrupt handler
for the MSI received.
3. At some point, the interrupt handler must decide, correctly, that
there is no more work to do and return.
4. The hardware generates a new MSI. As the MSI's status bit is still
set, this new MSI is ignored.
6. dw_handle_msi_irq() unsets the MSI status bit.
The MSI received at point 4 will never be acted upon. It occurred after
the driver had finished checking the hardware status for interrupt
conditions to act on. Since the MSI status was masked, it does not
generated a new IRQ, neither when it was received nor when the MSI is
unmasked.
It seems clear there is an unsolvable race here.
After this patch, the sequence becomes as follows:
1. An MSI is received and the status bit for the MSI is set in the
DWC PCI-e controller.
2. dw_handle_msi_irq() clears this MSI status bit.
3. dw_handle_msi_irq() calls a driver's registered interrupt handler
for the MSI received.
3. At some point, the interrupt handler must decide, correctly, that
there is no more work to do and return.
4. The hardware generates a new MSI. This sets the MSI status bit and
triggers another interrupt in the interrupt controller(s) above the DWC
PCI-e controller. As the the dwc-pcie handler is not re-entrant, it is
not run again at this time.
6. dw_handle_msi_irq() finishes. The MSI status bit remains set.
7. The IRQ is re-raised and dw_handle_msi_irq() runs again.
8. dw_handle_msi_irq() invokes the MSI's registered interrupt handler
again as the status bit was still set.
The observant will notice that point 4 present the opportunity for the
SoC's interrupt controller to lose the interrupt in the same manner as
the bug in this driver. The driver for that interrupt controller will
be written to properly deal with this. In some cases the hardware
supports an EOI concept, where the 2nd IRQ is masked and internally
queued in the hardware, to be re-raised at EOI in step 7. In other
cases the IRQ will be unmasked and re-raised at step 4, but the kernel
will see the handler is INPROGRESS and not re-invoke it, and instead set
a PENDING flag, which causes the handler to re-run at step 7.
[1] https://patchwork.kernel.org/patch/3333681/
Fixes: 8c934095fa2f ("PCI: dwc: Clear MSI interrupt status after it is handled, not before")
Cc: stable(a)vger.kernel.org
Cc: Vignesh R <vigneshr(a)ti.com>
Cc: Faiz Abbas <faiz_abbas(a)ti.com>
Cc: Jingoo Han <jingoohan1(a)gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel(a)synopsys.com>
Cc: Joao Pinto <jpinto(a)synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi(a)arm.com>
Cc: Bjorn Helgaas <bhelgaas(a)google.com>
Signed-off-by: Trent Piepho <tpiepho(a)impinj.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 29a05759a294..9a3960c95ad3 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -98,10 +98,10 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
irq = irq_find_mapping(pp->irq_domain,
(i * MAX_MSI_IRQS_PER_CTRL) +
pos);
- generic_handle_irq(irq);
dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS +
(i * MSI_REG_CTRL_BLOCK_SIZE),
4, 1 << pos);
+ generic_handle_irq(irq);
pos++;
}
}
--
2.14.4
The bootloader may pass physical address of the boot parameters structure
to the MMUv3 kernel in the register a2. Code in the _SetupMMU block in
the arch/xtensa/kernel/head.S is supposed to map that physical address to
the virtual address in the configured virtual memory layout.
This code haven't been updated when additional 256+256 and 512+512
memory layouts were introduced and it may produce wrong addresses when
used with these layouts.
Cc: stable(a)vger.kernel.org
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
arch/xtensa/kernel/head.S | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/xtensa/kernel/head.S b/arch/xtensa/kernel/head.S
index 2f76118ecf62..9053a5622d2c 100644
--- a/arch/xtensa/kernel/head.S
+++ b/arch/xtensa/kernel/head.S
@@ -88,9 +88,12 @@ _SetupMMU:
initialize_mmu
#if defined(CONFIG_MMU) && XCHAL_HAVE_PTP_MMU && XCHAL_HAVE_SPANNING_WAY
rsr a2, excsave1
- movi a3, 0x08000000
+ movi a3, XCHAL_KSEG_PADDR
+ bltu a2, a3, 1f
+ sub a2, a2, a3
+ movi a3, XCHAL_KSEG_SIZE
bgeu a2, a3, 1f
- movi a3, 0xd0000000
+ movi a3, XCHAL_KSEG_CACHED_VADDR
add a2, a2, a3
wsr a2, excsave1
1:
--
2.11.0
From: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
I have a Thinkpad X220 Tablet in my hands that is losing vblank
interrupts whenever LP3 watermarks are used.
If I nudge the latency value written to the WM3 register just
by one in either direction the problem disappears. That to me
suggests that the punit will not enter the corrsponding
powersave mode (MPLL shutdown IIRC) unless the latency value
in the register matches exactly what we read from SSKPD. Ie.
it's not really a latency value but rather just a cookie
by which the punit can identify the desired power saving state.
On HSW/BDW this was changed such that we actually just write
the WM level number into those bits, which makes much more
sense given the observed behaviour.
We could try to handle this by disallowing LP3 watermarks
only when vblank interrupts are enabled but we'd first have
to prove that only vblank interrupts are affected, which
seems unlikely. Also we can't grab the wm mutex from the
vblank enable/disable hooks because those are called with
various spinlocks held. Thus we'd have to redesigne the
watermark locking. So to play it safe and keep the code
simple we simply disable LP3 watermarks on all SNB machines.
To do that we simply zero out the latency values for
watermark level 3, and we adjust the watermark computation
to check for that. The behaviour now matches that of the
g4x/vlv/skl wm code in the presence of a zeroed latency
value.
Cc: stable(a)vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101269
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103713
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 41 ++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 27498ded4949..ef1ae2ede379 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2493,6 +2493,9 @@ static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate,
uint32_t method1, method2;
int cpp;
+ if (mem_value == 0)
+ return USHRT_MAX;
+
if (!intel_wm_plane_visible(cstate, pstate))
return 0;
@@ -2522,6 +2525,9 @@ static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate,
uint32_t method1, method2;
int cpp;
+ if (mem_value == 0)
+ return USHRT_MAX;
+
if (!intel_wm_plane_visible(cstate, pstate))
return 0;
@@ -2545,6 +2551,9 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
{
int cpp;
+ if (mem_value == 0)
+ return USHRT_MAX;
+
if (!intel_wm_plane_visible(cstate, pstate))
return 0;
@@ -3008,6 +3017,34 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv)
intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
}
+static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
+{
+ /*
+ * On some SNB machines (Thinkpad X220 Tablet at least)
+ * LP3 usage can cause vblank interrupts to be lost.
+ * The DEIIR bit will go high but it looks like the CPU
+ * never gets interrupted.
+ *
+ * It's not clear whether other interrupt source could
+ * be affected or if this is somehow limited to vblank
+ * interrupts only. To play it safe we disable LP3
+ * watermarks entirely.
+ */
+ if (dev_priv->wm.pri_latency[3] == 0 &&
+ dev_priv->wm.spr_latency[3] == 0 &&
+ dev_priv->wm.cur_latency[3] == 0)
+ return;
+
+ dev_priv->wm.pri_latency[3] = 0;
+ dev_priv->wm.spr_latency[3] = 0;
+ dev_priv->wm.cur_latency[3] = 0;
+
+ DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n");
+ intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
+ intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
+ intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
+}
+
static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
{
intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
@@ -3024,8 +3061,10 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
- if (IS_GEN6(dev_priv))
+ if (IS_GEN6(dev_priv)) {
snb_wm_latency_quirk(dev_priv);
+ snb_wm_lp3_irq_quirk(dev_priv);
+ }
}
static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)
--
2.18.1