Re: [PATCH v2] usb: typec: tcpm: Report back negotiated PPS voltage and current

[Guenter Roeck]

On 10/08/2018 05:53 AM, Adam Thomson wrote:
> Currently when requesting a specific voltage or current through
> the psy interface, for PPS, when reading back from that interface
> the values will always be the same as previously given, if the
> request was successful. However PPS only allows for 20mV voltage
> steps and 50mA current steps, and the psy class expects microvolt
> and micro amp requests, so inbetween values can be provided through
> this interface. Really when reading back the true values negotiated
> should be given, and not the ones originally asked for.
>
> To report the actual values negotiated with the Source, the values
> stored are now rounded down to the relevant step units prior to
> building the PPS request, so that those values are later correctly
> reported through the psy interface. In addition this improves the
> adjustments made to meet the operating power requirements of the
> platform, which previously could have been slightly out due to not
> using valid PPS units of voltage and current.
>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx>

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
> Changes are based on usb-testing (e7a2c3fa2857)
>
> Changes in v2:
>   - Rounding down of PPS voltage and current values moved to relevant API
>     functions so stored values are correctly updated prior to all subsequent
>     usage and decision making, as discuessed with Guenter.
>
>   drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index b06eac8..dbbd71f 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4113,6 +4113,9 @@ static int tcpm_pps_set_op_curr(struct tcpm_port *port, u16 op_curr)
>   		goto port_unlock;
>   	}
>   
> +	/* Round down operating current to align with PPS valid steps */
> +	op_curr = op_curr - (op_curr % RDO_PROG_CURR_MA_STEP);
> +
>   	reinit_completion(&port->pps_complete);
>   	port->pps_data.op_curr = op_curr;
>   	port->pps_status = 0;
> @@ -4166,6 +4169,9 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 out_volt)
>   		goto port_unlock;
>   	}
>   
> +	/* Round down output voltage to align with PPS valid steps */
> +	out_volt = out_volt - (out_volt % RDO_PROG_VOLT_MV_STEP);
> +
>   	reinit_completion(&port->pps_complete);
>   	port->pps_data.out_volt = out_volt;
>   	port->pps_status = 0;