Re: [PATCH 3/3] usb: typec: ps883x: fix configuration error handling

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Tue, Feb 18, 2025 at 04:29:33PM +0100, Johan Hovold wrote:
> Propagate errors to the consumers when configuring the retimer so that
> they can act on any failures as intended, for example:
> 
> 	ps883x_retimer 2-0008: failed to write conn_status_0: -5
> 	pmic_glink_altmode.pmic_glink_altmode pmic_glink.altmode.0: failed to setup retimer to DP: -5
> 
> Fixes: 257a087c8b52 ("usb: typec: Add support for Parade PS8830 Type-C Retimer")
> Cc: Abel Vesa <abel.vesa@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/mux/ps883x.c | 36 ++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux/ps883x.c b/drivers/usb/typec/mux/ps883x.c
> index f8b47187f4cf..ad59babf7cce 100644
> --- a/drivers/usb/typec/mux/ps883x.c
> +++ b/drivers/usb/typec/mux/ps883x.c
> @@ -58,12 +58,31 @@ struct ps883x_retimer {
>  	unsigned int svid;
>  };
>  
> -static void ps883x_configure(struct ps883x_retimer *retimer, int cfg0,
> -			     int cfg1, int cfg2)
> +static int ps883x_configure(struct ps883x_retimer *retimer, int cfg0,
> +			    int cfg1, int cfg2)
>  {
> -	regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_0, cfg0);
> -	regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_1, cfg1);
> -	regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_2, cfg2);
> +	struct device *dev = &retimer->client->dev;
> +	int ret;
> +
> +	ret = regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_0, cfg0);
> +	if (ret) {
> +		dev_err(dev, "failed to write conn_status_0: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_1, cfg1);
> +	if (ret) {
> +		dev_err(dev, "failed to write conn_status_1: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_2, cfg2);
> +	if (ret) {
> +		dev_err(dev, "failed to write conn_status_2: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static int ps883x_set(struct ps883x_retimer *retimer)
> @@ -74,8 +93,7 @@ static int ps883x_set(struct ps883x_retimer *retimer)
>  
>  	if (retimer->orientation == TYPEC_ORIENTATION_NONE ||
>  	    retimer->mode == TYPEC_STATE_SAFE) {
> -		ps883x_configure(retimer, cfg0, cfg1, cfg2);
> -		return 0;
> +		return ps883x_configure(retimer, cfg0, cfg1, cfg2);
>  	}
>  
>  	if (retimer->mode != TYPEC_STATE_USB && retimer->svid != USB_TYPEC_DP_SID)
> @@ -113,9 +131,7 @@ static int ps883x_set(struct ps883x_retimer *retimer)
>  		return -EOPNOTSUPP;
>  	}
>  
> -	ps883x_configure(retimer, cfg0, cfg1, cfg2);
> -
> -	return 0;
> +	return ps883x_configure(retimer, cfg0, cfg1, cfg2);
>  }
>  
>  static int ps883x_sw_set(struct typec_switch_dev *sw,
> -- 
> 2.45.3

-- 
heikki