From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Konrad Dybcio <konradybcio@kernel.org>,
Andreas Noever <andreas.noever@gmail.com>,
Mika Westerberg <westeri@kernel.org>,
Yehezkel Bernat <YehezkelShB@gmail.com>,
usb4-upstream@oss.qualcomm.com, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] thunderbolt: debugfs: Don't stop reading SB registers if just one fails
Date: Fri, 10 Apr 2026 16:29:43 +0200 [thread overview]
Message-ID: <7d65539f-ece6-4e7c-a13e-6b12920346fa@oss.qualcomm.com> (raw)
In-Reply-To: <20260409143203.GI3552@black.igk.intel.com>
On 4/9/26 4:32 PM, Mika Westerberg wrote:
> On Thu, Apr 09, 2026 at 02:59:22PM +0200, Konrad Dybcio wrote:
>> On 4/9/26 2:04 PM, Mika Westerberg wrote:
[...]
>>> I assume you have tested this on a hardware that supports this too, right?
>>
>> Hardware that exposes that register this does not exercise the altered
>> code path.
>
> Well it may happen now that previously we got -EIO from some other register
> and we stopped there, now this changes and we actually continue reading so
> this definitely should be tested.
The only register before USB4_SB_GEN4_TXFFE that isn't in-spec for
both retimers in v1.0 and v2.0 is USB4_SB_LRD_TUNING (0x07). The PS8830
interestingly reports all zeroes (not a bounce).
The registers following USB4_SB_GEN4_TXFFE in the array are
USB4_SB_VERSION and USB4_SB_DATA. The former is not accessed anywhere
else in the code, at first glance. The latter is, during NVM r/w and
in margining ops, which have definitely been in use for a long time.
Plus both of them are the v1.0 spec. The USB4_SB_GEN4_TXFFE specifically
isn't (the retimer supplement pdf lists it as Rsvd, the main spec pdf
omits it in the SB register table), as it wasn't previously useful (since
Gen4 came about in v2.0).
I don't think there's an easy way to limit the reading of this register
since the bit indicating Gen4 capability is in USB4_SB_LINK_CONF (0x0c),
which is Rsvd on retimers regardless of the spec revision. A connected
port could easily have higher/lower capabilities, too.
So all in all, my understanding is that any bugs caused by this patch
(which would have to be in the form of "reading a register causes a
wrongful change in behavior") would really surface spec non-compliance
from a retimer, which should be quirked out explicitly if that's the
case.
I only have hardware with various Parade retimers, none of which claim
Gen4 support.
Konrad
next prev parent reply other threads:[~2026-04-10 14:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-09 11:22 [PATCH] thunderbolt: debugfs: Don't stop reading SB registers if just one fails Konrad Dybcio
2026-04-09 12:04 ` Mika Westerberg
2026-04-09 12:59 ` Konrad Dybcio
2026-04-09 14:32 ` Mika Westerberg
2026-04-10 14:29 ` Konrad Dybcio [this message]
2026-04-10 14:43 ` Konrad Dybcio
2026-04-10 15:10 ` Mika Westerberg
2026-04-10 17:27 ` Konrad Dybcio
2026-04-10 14:44 ` Mika Westerberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7d65539f-ece6-4e7c-a13e-6b12920346fa@oss.qualcomm.com \
--to=konrad.dybcio@oss.qualcomm.com \
--cc=YehezkelShB@gmail.com \
--cc=andreas.noever@gmail.com \
--cc=konradybcio@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=usb4-upstream@oss.qualcomm.com \
--cc=westeri@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox