Re: [PATCH 14/21] soundwire: amd: Preserve lane control bits in amd_sdw_port_enable()

[Mukunda,Vijendar]

On 10/30/26 16:16, Lee Jones wrote:
> [You don't often get email from lee@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> lane_ctrl_ch_en_reg holds both the channel enable mask
> (AMD_DPN_CH_EN_CHMASK) and the lane control field
> (AMD_DPN_CH_EN_LCTRL) programmed by amd_sdw_transport_params().  When
> enable_ch->enable is false, amd_sdw_port_enable() writes 0 to the entire
> register instead of clearing only AMD_DPN_CH_EN_CHMASK, wiping out the
> configured lane control bits.
>
> Clear only the AMD_DPN_CH_EN_CHMASK field when disabling port channels.

Multi-lane is not supported on any of the platforms handled by this
function (ACP6.3, ACP7.0, ACP7.1, ACP7.2). AMD_DPN_CH_EN_LCTRL
(bits [2:0]) is never programmed for these platforms, so it always
reads back as 0. Writing 0 to the entire register is therefore
functionally identical to clearing only AMD_DPN_CH_EN_CHMASK.

Upcoming AMD platforms use an entirely different implementation and
will not use this code path either.

The described bug has no practical impact on any hardware this code
runs on, so the Fixes: tag and Cc: stable@xxxxxxxxxxxxxxx are not
warranted. Please drop this patch.

> Fixes: d8f48fbdfd9a ("soundwire: amd: Add support for AMD Manager driver")
> Closes: https://sashiko.dev/#/bug/linux-9a1be49a-7c40-47d3-9260-b933e590c90b
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> ---
>   drivers/soundwire/amd_manager.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index a8220235a2ae..9dfcdde3f2f0 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -706,11 +706,11 @@ static int amd_sdw_port_enable(struct sdw_bus *bus,
>           * parameters.
>           */
>          dpn_ch_enable = readl(amd_manager->mmio + lane_ctrl_ch_en_reg);
> -       u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
>          if (enable_ch->enable)
> -               writel(dpn_ch_enable, amd_manager->mmio + lane_ctrl_ch_en_reg);
> +               u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
>          else
> -               writel(0, amd_manager->mmio + lane_ctrl_ch_en_reg);
> +               u32p_replace_bits(&dpn_ch_enable, 0, AMD_DPN_CH_EN_CHMASK);
> +       writel(dpn_ch_enable, amd_manager->mmio + lane_ctrl_ch_en_reg);
>          return 0;
>   }
>
> --
> 2.56.0.317.g454f8302e1-goog