On Fri, Sep 28, 2018 at 04:33:49PM +0100, 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>
> ---
> Changes are based on usb-testing (ae8a2ca8a221)
>
> drivers/usb/typec/tcpm/tcpm.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 4f1f421..33b3981 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2433,6 +2433,10 @@ static int tcpm_pd_build_pps_request(struct tcpm_port *port, u32 *rdo)
>
> flags = RDO_USB_COMM | RDO_NO_SUSPEND;
>
> + /* Round down output voltage and current to align with PPS valid steps */
> + out_mv = out_mv - (out_mv % RDO_PROG_VOLT_MV_STEP);
> + op_ma = op_ma - (op_ma % RDO_PROG_CURR_MA_STEP);
> +
The original intent was to have out_mv (ie out_volt) set to valid values.
Some remnants of that are still in the code.
out_mv += RDO_PROG_VOLT_MV_STEP - (out_mv % RDO_PROG_VOLT_MV_STEP);
port->pps_data.out_volt =
min(pdo_pps_apdo_max_voltage(pdo), port->pps_data.out_volt);
Somehow that got lost over time. Personally I think it would be better to set
port->pps_data.out_vol and port->pps_data.op_curr to valid step sizes instead
of post-adjusting the values when used.
Thanks,
Guenter
> op_mw = (op_ma * out_mv) / 1000;
> if (op_mw < port->operating_snk_mw) {
> /*
> --
> 1.9.1
>
