Re: [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors

[Jani Nikula]

On Thu, 23 Jul 2026, Pengpeng Hou <pengpeng@xxxxxxxxxxx> wrote:
> ps8640_aux_transfer_msg() programs the AUX address registers, starts the
> AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
> register. Several of those regmap operations have return values, but the
> function only checks a stale ret after the status read.
>
> Propagate failures from the address write, transfer start, completion
> poll, and status read. This avoids returning a transfer length when the
> bridge register transaction or AUX completion wait failed.
>
> Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
> Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@xxxxxxxxxxx/
> - include the AUX address and length in the address-write diagnostic
> - describe poll failures without assuming that every error is a timeout
> - add the Fixes tag suggested by Doug
> - rebase onto the current drm-misc-next tree
>
>  drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 96332721cb69..56b48f8feab6 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -257,8 +257,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
>  	addr_len[PAGE0_SWAUX_LENGTH - base] = (len == 0) ? SWAUX_NO_PAYLOAD :
>  					      ((len - 1) & SWAUX_LENGTH_MASK);
>  
> -	regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> -			  ARRAY_SIZE(addr_len));
> +	ret = regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> +				ARRAY_SIZE(addr_len));
> +	if (ret) {
> +		DRM_DEV_ERROR(dev,

DRM_DEV_ERROR() is deprecated in favour of drm_err() or dev_err().

BR,
Jani.

> +			      "failed to write AUX address %#x, len %zu: %d\n",
> +			      msg->address, len, ret);
> +		return ret;
> +	}
>  
>  	if (len && (request == DP_AUX_NATIVE_WRITE ||
>  		    request == DP_AUX_I2C_WRITE)) {
> @@ -274,13 +280,22 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
>  		}
>  	}
>  
> -	regmap_write(map, PAGE0_SWAUX_CTRL, SWAUX_SEND);
> +	ret = regmap_write(map, PAGE0_SWAUX_CTRL, SWAUX_SEND);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to start AUX transfer: %d\n", ret);
> +		return ret;
> +	}
>  
>  	/* Zero delay loop because i2c transactions are slow already */
> -	regmap_read_poll_timeout(map, PAGE0_SWAUX_CTRL, data,
> -				 !(data & SWAUX_SEND), 0, 50 * 1000);
> +	ret = regmap_read_poll_timeout(map, PAGE0_SWAUX_CTRL, data,
> +				       !(data & SWAUX_SEND), 0, 50 * 1000);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to complete AUX transfer: %d\n",
> +			      ret);
> +		return ret;
> +	}
>  
> -	regmap_read(map, PAGE0_SWAUX_STATUS, &data);
> +	ret = regmap_read(map, PAGE0_SWAUX_STATUS, &data);
>  	if (ret) {
>  		DRM_DEV_ERROR(dev, "failed to read PAGE0_SWAUX_STATUS: %d\n",
>  			      ret);

-- 
Jani Nikula, Intel