On Wed, Dec 04, 2024 at 05:24:54PM +0100, Johan Hovold wrote:
> On Tue, Nov 12, 2024 at 07:01:11PM +0200, Abel Vesa wrote:
> > The Parade PS8830 is a USB4, DisplayPort and Thunderbolt 4 retimer,
> > controlled over I2C. It usually sits between a USB/DisplayPort PHY
> > and the Type-C connector, and provides orientation and altmode handling.
> >
> > The boards that use this retimer are the ones featuring the Qualcomm
> > Snapdragon X Elite SoCs.
>
> > +static int ps883x_sw_set(struct typec_switch_dev *sw,
> > + enum typec_orientation orientation)
> > +{
> > + struct ps883x_retimer *retimer = typec_switch_get_drvdata(sw);
> > + int ret = 0;
> > +
> > + ret = typec_switch_set(retimer->typec_switch, orientation);
> > + if (ret)
> > + return ret;
> > +
> > + mutex_lock(&retimer->lock);
> > +
> > + if (retimer->orientation != orientation) {
> > + retimer->orientation = orientation;
> > +
> > + ret = ps883x_set(retimer);
> > + }
> > +
> > + mutex_unlock(&retimer->lock);
> > +
> > + return ret;
> > +}
>
> This seems to indicate a bigger problem, but I see this function called
> during early resume while the i2c controller is suspended:
>
> [ 54.213900] ------------[ cut here ]------------
> [ 54.213942] i2c i2c-2: Transfer while suspended
> [ 54.214125] WARNING: CPU: 0 PID: 126 at drivers/i2c/i2c-core.h:56 __i2c_transfer+0x874/0x968 [i2c_core]
> ...
> [ 54.214833] CPU: 0 UID: 0 PID: 126 Comm: kworker/0:2 Not tainted 6.13.0-rc1 #11
> [ 54.214844] Hardware name: Qualcomm CRD, BIOS 6.0.231221.BOOT.MXF.2.4-00348.1-HAMOA-1 12/21/2023
> [ 54.214852] Workqueue: events pmic_glink_altmode_worker [pmic_glink_altmode]
> ...
> [ 54.215090] Call trace:
> [ 54.215097] __i2c_transfer+0x874/0x968 [i2c_core] (P)
> [ 54.215112] __i2c_transfer+0x874/0x968 [i2c_core] (L)
> [ 54.215126] i2c_transfer+0x94/0xf0 [i2c_core]
> [ 54.215140] i2c_transfer_buffer_flags+0x5c/0x90 [i2c_core]
> [ 54.215153] regmap_i2c_write+0x20/0x58 [regmap_i2c]
> [ 54.215166] _regmap_raw_write_impl+0x740/0x894
> [ 54.215184] _regmap_bus_raw_write+0x60/0x7c
> [ 54.215192] _regmap_write+0x60/0x1b4
> [ 54.215200] regmap_write+0x4c/0x78
> [ 54.215207] ps883x_set+0xb0/0x10c [ps883x]
> [ 54.215219] ps883x_sw_set+0x74/0x98 [ps883x]
> [ 54.215227] typec_switch_set+0x58/0x90 [typec]
> [ 54.215248] pmic_glink_altmode_worker+0x3c/0x23c [pmic_glink_altmode]
> [ 54.215257] process_one_work+0x20c/0x610
> [ 54.215274] worker_thread+0x23c/0x378
> [ 54.215283] kthread+0x124/0x128
> [ 54.215291] ret_from_fork+0x10/0x20
> [ 54.215303] irq event stamp: 28140
> [ 54.215309] hardirqs last enabled at (28139): [<ffffd15e3bc2a434>] __up_console_sem+0x6c/0x80
> [ 54.215325] hardirqs last disabled at (28140): [<ffffd15e3c596aa4>] el1_dbg+0x24/0x8c
> [ 54.215341] softirqs last enabled at (28120): [<ffffd15e3bb9b82c>] handle_softirqs+0x4c4/0x4dc
> [ 54.215355] softirqs last disabled at (27961): [<ffffd15e3bb501ec>] __do_softirq+0x14/0x20
> [ 54.215363] ---[ end trace 0000000000000000 ]---
> [ 54.216889] Enabling non-boot CPUs ...
>
> This can be reproduced on the CRD (or T14s) by disconnecting, for
> example, a mass storage device while the laptop is suspended.
>
I wonder if this is because drivers/rpmsg/qcom_glink_smem.c line 309
registers the GLINK interrupt as IRQF_NO_SUSPEND as a remnant from being
used for rpm communication...
This is no longer needed (glink/rpm code path is now different), but
iirc the cleanup got stuck in the question of dealing with wakeup
capabilities (and priorities).
Regards,
Bjorn
