Re: [PATCH] thunderbolt: debugfs: Don't stop reading SB registers if just one fails

[Mika Westerberg]

Hi,

On Thu, Apr 09, 2026 at 01:22:01PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> 
> The GEN4 TxFFE register is not part of the USB4 v1.0 specification, so
> understandably some pre-USB4v2 retimers (like the Parade PS8830) don't
> seem to implement it.
>
> The immediate idea to counter this would be to introduce a version
> check for that specific register, but on a second thought, the current
> flow only returns a quiet -EIO if there's _any_ failures, without
> hinting at what the actual problem is.

Please don't use _any_ emphasis in the commit messages here or in the
future.

> To take care of both of these issues, simply print an error line for
> each SB register read that fails and go on with attempting to read the
> others.
> 
> Note that this is not quite in-spec behavior ("The SB Register Space
> registers shall have the structure and fields described in Table 4-17.
> Registers not listed in Table 4-20 are undefined and shall not be
> used."), but it's the easiest fix that shouldn't (TM) have real-world
> bad side effects.

Also drop the "(TM)" thing.

I assume you have tested this on a hardware that supports this too, right?

> Fixes: 6d241fa00159 ("thunderbolt: Add sideband register access to debugfs")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> ---
>  drivers/thunderbolt/debugfs.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/debugfs.c b/drivers/thunderbolt/debugfs.c
> index 042f6a0d0f7f..8237e1ea6d09 100644
> --- a/drivers/thunderbolt/debugfs.c
> +++ b/drivers/thunderbolt/debugfs.c
> @@ -2361,8 +2361,10 @@ static int sb_regs_show(struct tb_port *port, const struct sb_reg *sb_regs,
>  		memset(data, 0, sizeof(data));
>  		ret = usb4_port_sb_read(port, target, index, regs->reg, data,
>  					regs->size);
> -		if (ret)
> -			return ret;
> +		if (ret) {
> +			seq_printf(s, "0x%02x Error reading register: %d\n", regs->reg, ret);

Why not tb_port_dgb/warn()() here instead so it goes into dmesg, not to the
output.

> +			continue;
> +		}
>  
>  		seq_printf(s, "0x%02x", regs->reg);
>  		for (j = 0; j < regs->size; j++)
> 
> ---
> base-commit: db7efce4ae23ad5e42f5f55428f529ff62b86fab
> change-id: 20260409-topic-tbt_sb_debugfs-2e500fee9706
> 
> Best regards,
> -- 
> Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>