From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol(a)tdk.com>
Use IRQ ONESHOT flag to ensure the timestamp is not updated in the
hard handler during the thread handler. And use a fixed value of 1
sample that correspond to this first timestamp.
This way we can ensure the timestamp is always corresponding to the
value used by the timestamping mechanism. Otherwise, it is possible
that between FIFO count read and FIFO processing the timestamp is
overwritten in the hard handler.
Fixes: 111e1abd0045 ("iio: imu: inv_mpu6050: use the common inv_sensors timestamp module")
Cc: stable(a)vger.kernel.org
Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol(a)tdk.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 4 ++--
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 0dc0f22a5582..3d3b27f28c9d 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -100,8 +100,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
goto end_session;
/* Each FIFO data contains all sensors, so same number for FIFO and sensor data */
fifo_period = NSEC_PER_SEC / INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
- inv_sensors_timestamp_interrupt(&st->timestamp, nb, pf->timestamp);
- inv_sensors_timestamp_apply_odr(&st->timestamp, fifo_period, nb, 0);
+ inv_sensors_timestamp_interrupt(&st->timestamp, 1, pf->timestamp);
+ inv_sensors_timestamp_apply_odr(&st->timestamp, fifo_period, 1, 0);
/* clear internal data buffer for avoiding kernel data leak */
memset(data, 0, sizeof(data));
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index 1b603567ccc8..84273660ca2e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -300,6 +300,7 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
if (!st->trig)
return -ENOMEM;
+ irq_type |= IRQF_ONESHOT;
ret = devm_request_threaded_irq(&indio_dev->dev, st->irq,
&inv_mpu6050_interrupt_timestamp,
&inv_mpu6050_interrupt_handle,
--
2.34.1
From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol(a)tdk.com>
Use IRQ_ONESHOT flag to ensure the timestamp is not updated in the
hard handler during the thread handler. And compute and use the
effective watermark value that correspond to this first timestamp.
This way we can ensure the timestamp is always corresponding to the
value used by the timestamping mechanism. Otherwise, it is possible
that between FIFO count read and FIFO processing the timestamp is
overwritten in the hard handler.
Fixes: ec74ae9fd37c ("iio: imu: inv_icm42600: add accurate timestamping")
Cc: stable(a)vger.kernel.org
Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol(a)tdk.com>
---
.../imu/inv_icm42600/inv_icm42600_buffer.c | 19 +++++++++++++++++--
.../imu/inv_icm42600/inv_icm42600_buffer.h | 2 ++
.../iio/imu/inv_icm42600/inv_icm42600_core.c | 1 +
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
index 63b85ec88c13..a8cf74c84c3c 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
@@ -222,10 +222,15 @@ int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st)
latency_accel = period_accel * wm_accel;
/* 0 value for watermark means that the sensor is turned off */
+ if (wm_gyro == 0 && wm_accel == 0)
+ return 0;
+
if (latency_gyro == 0) {
watermark = wm_accel;
+ st->fifo.watermark.eff_accel = wm_accel;
} else if (latency_accel == 0) {
watermark = wm_gyro;
+ st->fifo.watermark.eff_gyro = wm_gyro;
} else {
/* compute the smallest latency that is a multiple of both */
if (latency_gyro <= latency_accel)
@@ -241,6 +246,13 @@ int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st)
watermark = latency / period;
if (watermark < 1)
watermark = 1;
+ /* update effective watermark */
+ st->fifo.watermark.eff_gyro = latency / period_gyro;
+ if (st->fifo.watermark.eff_gyro < 1)
+ st->fifo.watermark.eff_gyro = 1;
+ st->fifo.watermark.eff_accel = latency / period_accel;
+ if (st->fifo.watermark.eff_accel < 1)
+ st->fifo.watermark.eff_accel = 1;
}
/* compute watermark value in bytes */
@@ -514,7 +526,7 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
/* handle gyroscope timestamp and FIFO data parsing */
if (st->fifo.nb.gyro > 0) {
ts = &gyro_st->ts;
- inv_sensors_timestamp_interrupt(ts, st->fifo.nb.gyro,
+ inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_gyro,
st->timestamp.gyro);
ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
if (ret)
@@ -524,7 +536,7 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
/* handle accelerometer timestamp and FIFO data parsing */
if (st->fifo.nb.accel > 0) {
ts = &accel_st->ts;
- inv_sensors_timestamp_interrupt(ts, st->fifo.nb.accel,
+ inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_accel,
st->timestamp.accel);
ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
if (ret)
@@ -577,6 +589,9 @@ int inv_icm42600_buffer_init(struct inv_icm42600_state *st)
unsigned int val;
int ret;
+ st->fifo.watermark.eff_gyro = 1;
+ st->fifo.watermark.eff_accel = 1;
+
/*
* Default FIFO configuration (bits 7 to 5)
* - use invalid value
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
index 8b85ee333bf8..f6c85daf42b0 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
@@ -32,6 +32,8 @@ struct inv_icm42600_fifo {
struct {
unsigned int gyro;
unsigned int accel;
+ unsigned int eff_gyro;
+ unsigned int eff_accel;
} watermark;
size_t count;
struct {
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index 96116a68ab29..62fdae530334 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -537,6 +537,7 @@ static int inv_icm42600_irq_init(struct inv_icm42600_state *st, int irq,
if (ret)
return ret;
+ irq_type |= IRQF_ONESHOT;
return devm_request_threaded_irq(dev, irq, inv_icm42600_irq_timestamp,
inv_icm42600_irq_handler, irq_type,
"inv_icm42600", st);
--
2.34.1
Hi all,
I have noticed strange messages in kernel version 6.9, obviously from CPU topology
detection, which were not present in 6.8.y and earlier kernels.
This is coming from an older server machine: 2-socket Ivy Bridge Xeon E5-2697 v2 (24C/48T)
in an Asus Z9PE-D16/2L motherboard (Intel C-602A chipset); BIOS patched to the latest
available from Asus. All memory slots occupied, so 256 GB RAM in total.
From a "good boot", e.g. kernel 6.8.11, dmesg output looks like this:
[ 1.823797] smpboot: x86: Booting SMP configuration:
[ 1.823799] .... node #0, CPUs: #1 #2 #3 #4 #5 #6 #7 #8 #9 #10 #11
[ 1.827514] .... node #1, CPUs: #12 #13 #14 #15 #16 #17 #18 #19 #20 #21 #22 #23
[ 0.011462] smpboot: CPU 12 Converting physical 0 to logical die 1
[ 1.875532] .... node #0, CPUs: #24 #25 #26 #27 #28 #29 #30 #31 #32 #33 #34 #35
[ 1.882453] .... node #1, CPUs: #36 #37 #38 #39 #40 #41 #42 #43 #44 #45 #46 #47
[ 1.887532] MDS CPU bug present and SMT on, data leak possible. See
https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/mds.html for more details.
[ 1.933640] smp: Brought up 2 nodes, 48 CPUs
[ 1.933640] smpboot: Max logical packages: 2
[ 1.933640] smpboot: Total of 48 processors activated (259199.61 BogoMIPS)
From a "bad" boot, e.g. kernel 6.9.2, dmesg output has these messages in it:
[ 1.785937] smpboot: x86: Booting SMP configuration:
[ 1.785939] .... node #0, CPUs: #4
[ 1.786215] .... node #1, CPUs: #12 #16
[ 1.793547] MDS CPU bug present and SMT on, data leak possible. See
https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/mds.html for more details.
[ 1.797547] .... node #0, CPUs: #1 #2 #3 #5 #6 #7 #8 #9 #10 #11
[ 1.801858] .... node #1, CPUs: #13 #14 #15 #17 #18 #19 #20 #21 #22 #23
[ 1.804687] .... node #0, CPUs: #24 #25 #26 #27 #28 #29 #30 #31 #32 #33 #34 #35
[ 1.810728] .... node #1, CPUs: #36 #37 #38 #39 #40 #41 #42 #43 #44 #45 #46 #47
[ 1.901547] smp: Brought up 2 nodes, 48 CPUs
[ 1.901547] smpboot: Total of 48 processors activated (259207.87 BogoMIPS)
[ 1.903803] BUG: arch topology borken
[ 1.903879] the SMT domain not a subset of the CLS domain
[ 1.903970] BUG: arch topology borken
[ 1.904040] the SMT domain not a subset of the CLS domain
[ 1.904128] BUG: arch topology borken
[ 1.904198] the SMT domain not a subset of the CLS domain
... and this "BUG" and the following line repeat 48 times which is the number of logical
CPUs this machine has. Also, there is a funny typo in the message, but that might be
intended, I guess?! Moreover I noticed, from node #1, CPU #12 detection message is
missing, so the counting maybe wrong?!
However the machine boots, and except from these strange messages, I cannot detect any
other abnormal behaviour. It is running ~15 QEMU/KVM virtual machines just fine. Because
these messages look unusual and a bit scary though, I have bisected the issue, to be able
to report it here. The first bad commit I found is this one:
22d63660c35eb751c63a709bf901a64c1726592a is the first bad commit
commit 22d63660c35eb751c63a709bf901a64c1726592a
Author: Thomas Gleixner <tglx(a)linutronix.de>
Date: Tue Feb 13 22:04:08 2024 +0100
x86/cpu: Use common topology code for Intel
Intel CPUs use either topology leaf 0xb/0x1f evaluation or the legacy
SMP/HT evaluation based on CPUID leaf 0x1/0x4.
Move it over to the consolidated topology code and remove the random
topology hacks which are sprinkled into the Intel and the common code.
No functional change intended.
Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de>
Tested-by: Juergen Gross <jgross(a)suse.com>
Tested-by: Sohil Mehta <sohil.mehta(a)intel.com>
Tested-by: Michael Kelley <mhklinux(a)outlook.com>
Tested-by: Zhang Rui <rui.zhang(a)intel.com>
Tested-by: Wang Wendy <wendy.wang(a)intel.com>
Tested-by: K Prateek Nayak <kprateek.nayak(a)amd.com>
Link: https://lore.kernel.org/r/20240212153624.893644349@linutronix.de
arch/x86/kernel/cpu/common.c | 65 -----------------------------------
arch/x86/kernel/cpu/cpu.h | 4 ---
arch/x86/kernel/cpu/intel.c | 25 --------------
arch/x86/kernel/cpu/topology.c | 22 ------------
arch/x86/kernel/cpu/topology_common.c | 5 ++-
5 files changed, 4 insertions(+), 117 deletions(-)
root@linus:/usr/src/linux#
I attach my bisect log, and full dmesg output from a good and from a bad kernel version.
Moreover, the last 3 bad kernels from my bisect session did not boot at all, including the
one with commit SHA1 from the first bad commit above. These kernels also had the series of
"BUG" messages scrolling through on the console, and then additionally a kernel panic,
seemingly coming from a divide exception from function init_intel_microcode:
<5>[ 5.968685] Key type dns_resolver registered
<4>[ 5.974402] ENERGY_PERF_BIAS: Set to 'normal', was 'performance'
<4>[ 5.977017] divide error: 0000 [#1] PREEMPT SMP PTI
<4>[ 5.977116] CPU: 9 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc4+ #1
<4>[ 5.977213] Hardware name: ASUSTeK COMPUTER INC. Z9PE-D16 Series/Z9PE-D16 Series,
BIOS 5601 06/11/2015
<4>[ 5.977337] RIP: 0010:init_intel_microcode+0x3c/0x80
<4>[ 5.977436] Code: ff 75 44 40 80 fe 05 76 3e 48 8b 05 b6 45 f7 ff a9 00 00 00 40 75
30 8b 05 85 46 f7 ff 0f b7 0d aa 46 f7 ff 31 d2 48 c1 e0 0a <48> f7 f1 89 05 9b f9 46 ff
48 c7 c0 c0 98 e4 a8 31 d2 31 c9 31 f6
<4>[ 5.977602] RSP: 0000:ffffb79b8008fd80 EFLAGS: 00010206
<4>[ 5.977697] RAX: 0000000001e00000 RBX: 0000000000000000 RCX: 0000000000000000
<4>[ 5.977795] RDX: 0000000000000000 RSI: 0000000000000006 RDI: 0000000000000000
<4>[ 5.977894] RBP: ffffb79b8008fdf8 R08: 0000000000000000 R09: 0000000000000000
<4>[ 5.977992] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
<4>[ 5.978090] R13: 000000000000019a R14: ffffb79b8008fe08 R15: ffff96ad4026cf00
<4>[ 5.978187] FS: 0000000000000000(0000) GS:ffff96cc3fa40000(0000) knlGS:0000000000000000
<4>[ 5.978308] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 5.978402] CR2: 0000000000000000 CR3: 0000000e6d236001 CR4: 00000000001706f0
<4>[ 5.978500] Call Trace:
<4>[ 5.978588] <TASK>
<4>[ 5.978675] ? show_regs+0x6d/0x80
<4>[ 5.978767] ? die+0x37/0xa0
<4>[ 5.978857] ? do_trap+0xd4/0xf0
<4>[ 5.978948] ? do_error_trap+0x71/0xb0
<4>[ 5.979040] ? init_intel_microcode+0x3c/0x80
<4>[ 5.979131] ? exc_divide_error+0x3a/0x70
<4>[ 5.979226] ? init_intel_microcode+0x3c/0x80
<4>[ 5.979317] ? asm_exc_divide_error+0x1b/0x20
<4>[ 5.979427] ? init_intel_microcode+0x3c/0x80
<4>[ 5.979520] ? microcode_init+0x196/0x260
<4>[ 5.979612] ? __pfx_microcode_init+0x10/0x10
<4>[ 5.979718] do_one_initcall+0x5e/0x340
<4>[ 5.979813] kernel_init_freeable+0x322/0x490
<4>[ 5.979906] ? __pfx_kernel_init+0x10/0x10
<4>[ 5.979998] kernel_init+0x1b/0x200
<4>[ 5.980089] ret_from_fork+0x47/0x70
<4>[ 5.980180] ? __pfx_kernel_init+0x10/0x10
<4>[ 5.980272] ret_from_fork_asm+0x1b/0x30
<4>[ 5.980364] </TASK>
<4>[ 5.980450] Modules linked in:
<4>[ 5.980544] ---[ end trace 0000000000000000 ]---
<4>[ 6.959943] RIP: 0010:init_intel_microcode+0x3c/0x80
<4>[ 6.960041] Code: ff 75 44 40 80 fe 05 76 3e 48 8b 05 b6 45 f7 ff a9 00 00 00 40 75
30 8b 05 85 46 f7 ff 0f b7 0d aa 46 f7 ff 31 d2 48 c1 e0 0a <48> f7 f1 89 05 9b f9 46 ff
48 c7 c0 c0 98 e4 a8 31 d2 31 c9 31 f6
<4>[ 6.960207] RSP: 0000:ffffb79b8008fd80 EFLAGS: 00010206
<4>[ 6.960316] RAX: 0000000001e00000 RBX: 0000000000000000 RCX: 0000000000000000
<4>[ 6.960414] RDX: 0000000000000000 RSI: 0000000000000006 RDI: 0000000000000000
<4>[ 6.960512] RBP: ffffb79b8008fdf8 R08: 0000000000000000 R09: 0000000000000000
<4>[ 6.960610] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
<4>[ 6.960708] R13: 000000000000019a R14: ffffb79b8008fe08 R15: ffff96ad4026cf00
<4>[ 6.960806] FS: 0000000000000000(0000) GS:ffff96cc3fa40000(0000) knlGS:0000000000000000
<4>[ 6.960927] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 6.961021] CR2: 0000000000000000 CR3: 0000000e6d236001 CR4: 00000000001706f0
<0>[ 6.961120] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
<0>[ 6.961312] Kernel Offset: 0x25c00000 from 0xffffffff81000000 (relocation range:
0xffffffff80000000-0xffffffffbfffffff)
I also attached full dmesg log file "dmesg-erst-7373208397568540677" of this panic which I
could find in /var/lib/systemd/pstore.
Beste Grüße,
Peter Schneider
--
Climb the mountain not to plant your flag, but to embrace the challenge,
enjoy the air and behold the view. Climb it so you can see the world,
not so the world can see you. -- David McCullough Jr.
OpenPGP: 0xA3828BD796CCE11A8CADE8866E3A92C92C3FF244
Download: https://www.peters-netzplatz.de/download/pschneider1968_pub.aschttps://keys.mailvelope.com/pks/lookup?op=get&search=pschneider1968@googlem…https://keys.mailvelope.com/pks/lookup?op=get&search=pschneider1968@gmail.c…
LPM consists of HIPM (host initiated power management) and DIPM
(device initiated power management).
ata_eh_set_lpm() will only enable HIPM if both the HBA and the device
supports it.
However, DIPM will be enabled as long as the device supports it.
The HBA will later reject the device's request to enter a power state
that it does not support (Slumber/Partial/DevSleep) (DevSleep is never
initiated by the device).
For a HBA that doesn't support any LPM states, simply don't set a LPM
policy such that all the HIPM/DIPM probing/enabling will be skipped.
Not enabling HIPM or DIPM in the first place is safer than relying on
the device following the AHCI specification and respecting the NAK.
(There are comments in the code that some devices misbehave when
receiving a NAK.)
Performing this check in ahci_update_initial_lpm_policy() also has the
advantage that a HBA that doesn't support any LPM states will take the
exact same code paths as a port that is external/hot plug capable.
Fixes: 7627a0edef54 ("ata: ahci: Drop low power policy board type")
Cc: stable(a)vger.kernel.org
Signed-off-by: Niklas Cassel <cassel(a)kernel.org>
---
We have not received any bug reports with this.
The devices that were quirked recently all supported both Partial and
Slumber.
This is more a defensive action, as it seems unnecessary to enable DIPM
in the first place, if the HBA doesn't support any LPM states.
drivers/ata/ahci.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 07d66d2c5f0d..214de08de642 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1735,6 +1735,12 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap)
if (ap->pflags & ATA_PFLAG_EXTERNAL)
return;
+ /* If no LPM states are supported by the HBA, do not bother with LPM */
+ if ((ap->host->flags & ATA_HOST_NO_PART) &&
+ (ap->host->flags & ATA_HOST_NO_SSC) &&
+ (ap->host->flags & ATA_HOST_NO_DEVSLP))
+ return;
+
/* user modified policy via module param */
if (mobile_lpm_policy != -1) {
policy = mobile_lpm_policy;
--
2.45.1
On SC7180, in host mode, it is observed that stressing out controller
in host mode results in HC died error and only restarting the host
mode fixes it. Disable SS instances in park mode for these targets to
avoid host controller being dead.
Reported-by: Doug Anderson <dianders(a)google.com>
Cc: <stable(a)vger.kernel.org>
Fixes: 0b766e7fe5a2 ("arm64: dts: qcom: sc7180: Add USB related nodes")
Signed-off-by: Krishna Kurapati <quic_kriskura(a)quicinc.com>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 2b481e20ae38..cc93b5675d5d 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -3063,6 +3063,7 @@ usb_1_dwc3: usb@a600000 {
iommus = <&apps_smmu 0x540 0>;
snps,dis_u2_susphy_quirk;
snps,dis_enblslpm_quirk;
+ snps,parkmode-disable-ss-quirk;
phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
phy-names = "usb2-phy", "usb3-phy";
maximum-speed = "super-speed";
--
2.34.1
MSGF_LEG_MASK is laid out with INTA in bit 0, INTB in bit 1, INTC in bit
2, and INTD in bit 3. Hardware IRQ numbers start at 0, and we register
PCI_NUM_INTX irqs. So to enable INTA (aka hwirq 0) we should set bit 0.
Remove the subtraction of one.
This bug would cause legacy interrupts not to be delivered, as enabling
INTB would actually enable INTA, and enabling INTA wouldn't enable
anything at all. It is likely that this got overlooked for so long since
most PCIe hardware uses MSIs. This fixes the following UBSAN error:
UBSAN: shift-out-of-bounds in ../drivers/pci/controller/pcie-xilinx-nwl.c:389:11
shift exponent 18446744073709551615 is too large for 32-bit type 'int'
CPU: 1 PID: 61 Comm: kworker/u10:1 Not tainted 6.6.20+ #268
Hardware name: xlnx,zynqmp (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace (arch/arm64/kernel/stacktrace.c:235)
show_stack (arch/arm64/kernel/stacktrace.c:242)
dump_stack_lvl (lib/dump_stack.c:107)
dump_stack (lib/dump_stack.c:114)
__ubsan_handle_shift_out_of_bounds (lib/ubsan.c:218 lib/ubsan.c:387)
nwl_unmask_leg_irq (drivers/pci/controller/pcie-xilinx-nwl.c:389 (discriminator 1))
irq_enable (kernel/irq/internals.h:234 kernel/irq/chip.c:170 kernel/irq/chip.c:439 kernel/irq/chip.c:432 kernel/irq/chip.c:345)
__irq_startup (kernel/irq/internals.h:239 kernel/irq/chip.c:180 kernel/irq/chip.c:250)
irq_startup (kernel/irq/chip.c:270)
__setup_irq (kernel/irq/manage.c:1800)
request_threaded_irq (kernel/irq/manage.c:2206)
pcie_pme_probe (include/linux/interrupt.h:168 drivers/pci/pcie/pme.c:348)
<snip>
Fixes: 9a181e1093af ("PCI: xilinx-nwl: Modify IRQ chip for legacy interrupts")
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Sean Anderson <sean.anderson(a)linux.dev>
---
Changes in v4:
- Explain likely effects of the off-by-one error
- Trim down UBSAN backtrace
Changes in v3:
- Expand commit message
drivers/pci/controller/pcie-xilinx-nwl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 0408f4d612b5..437927e3bcca 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -371,7 +371,7 @@ static void nwl_mask_intx_irq(struct irq_data *data)
u32 mask;
u32 val;
- mask = 1 << (data->hwirq - 1);
+ mask = 1 << data->hwirq;
raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
nwl_bridge_writel(pcie, (val & (~mask)), MSGF_LEG_MASK);
@@ -385,7 +385,7 @@ static void nwl_unmask_intx_irq(struct irq_data *data)
u32 mask;
u32 val;
- mask = 1 << (data->hwirq - 1);
+ mask = 1 << data->hwirq;
raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
nwl_bridge_writel(pcie, (val | mask), MSGF_LEG_MASK);
--
2.35.1.1320.gc452695387.dirty
Recently, suspend testing on sc7180-trogdor based devices has started
to sometimes fail with messages like this:
port a88000.serial:0.0: PM: calling pm_runtime_force_suspend+0x0/0xf8 @ 28934, parent: a88000.serial:0
port a88000.serial:0.0: PM: dpm_run_callback(): pm_runtime_force_suspend+0x0/0xf8 returns -16
port a88000.serial:0.0: PM: pm_runtime_force_suspend+0x0/0xf8 returned -16 after 33 usecs
port a88000.serial:0.0: PM: failed to suspend: error -16
I could reproduce these problems by logging in via an agetty on the
debug serial port (which was _not_ used for kernel console) and
running:
cat /var/log/messages
...and then (via an SSH session) forcing a few suspend/resume cycles.
Tracing through the code and doing some printf()-based debugging shows
that the -16 (-EBUSY) comes from the recently added
serial_port_runtime_suspend().
The idea of the serial_port_runtime_suspend() function is to prevent
the port from being _runtime_ suspended if it still has bytes left to
transmit. Having bytes left to transmit isn't a reason to block
_system_ suspend, though. If a serdev device in the kernel needs to
block system suspend it should block its own suspend and it can use
serdev_device_wait_until_sent() to ensure bytes are sent.
The DEFINE_RUNTIME_DEV_PM_OPS() used by the serial_port code means
that the system suspend function will be pm_runtime_force_suspend().
In pm_runtime_force_suspend() we can see that before calling the
runtime suspend function we'll call pm_runtime_disable(). This should
be a reliable way to detect that we're called from system suspend and
that we shouldn't look for busyness.
Fixes: 43066e32227e ("serial: port: Don't suspend if the port is still busy")
Cc: stable(a)vger.kernel.org
Reviewed-by: Tony Lindgren <tony.lindgren(a)linux.intel.com>
Signed-off-by: Douglas Anderson <dianders(a)chromium.org>
---
In v1 [1] this was part of a 2-patch series. I'm now just sending this
patch on its own since the Qualcomm GENI serial driver has ended up
having a whole pile of problems that are taking a while to unravel.
It makes sense to disconnect the two efforts. The core problem fixed
by this patch and the geni problems never had any dependencies anyway.
[1] https://lore.kernel.org/r/20240523162207.1.I2395e66cf70c6e67d774c56943825c2…
Changes in v3:
- Adjust comment as per Tony Lindgren.
- Add Cc: stable.
Changes in v2:
- Fix "regulator" => "regular" in comment.
- Fix "PM Runtime" => "runtime PM" in comment.
- Commit messages says how serdev devices should ensure bytes xfered.
drivers/tty/serial/serial_port.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c
index 91a338d3cb34..d35f1d24156c 100644
--- a/drivers/tty/serial/serial_port.c
+++ b/drivers/tty/serial/serial_port.c
@@ -64,6 +64,13 @@ static int serial_port_runtime_suspend(struct device *dev)
if (port->flags & UPF_DEAD)
return 0;
+ /*
+ * Nothing to do on pm_runtime_force_suspend(), see
+ * DEFINE_RUNTIME_DEV_PM_OPS.
+ */
+ if (!pm_runtime_enabled(dev))
+ return 0;
+
uart_port_lock_irqsave(port, &flags);
if (!port_dev->tx_enabled) {
uart_port_unlock_irqrestore(port, flags);
--
2.45.1.288.g0e0cd299f1-goog