Re: [PATCH v2 2/6] usb: typec: tcpm: Address incorrect values of tcpm psy for pps supply
On 4/7/21 1:07 PM, Badhri Jagan Sridharan wrote:
> tcpm_pd_select_pps_apdo overwrites port->pps_data.min_volt,
> port->pps_data.max_volt, port->pps_data.max_curr even before
> port partner accepts the requests. This leaves incorrect values
> in current_limit and supply_voltage that get exported by
> "tcpm-source-psy-". Solving this problem by caching the request
> values in req_min_volt, req_max_volt, req_max_curr, req_out_volt,
> req_op_curr. min_volt, max_volt, max_curr gets updated once the
> partner accepts the request. current_limit, supply_voltage gets updated
> once local port's tcpm enters SNK_TRANSITION_SINK when the accepted
> current_limit and supply_voltage is enforced.
>
> Fixes: f2a8aa053c176 ("typec: tcpm: Represent source supply through
> power_supply")
> Signed-off-by: Badhri Jagan Sridharan
> Reviewed-by: Adam Thomson
Reviewed-by: Guenter Roeck
> ---
> Changes since V1:
> * Moved to kerneldoc header as suggested by Greg KH.
> * Added reviewed by tags.
> ---
> drivers/usb/typec/tcpm/tcpm.c | 88 +--
> 1 file changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 4ea4b30ae885..b4a40099d7e9 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -268,12 +268,27 @@ struct pd_mode_data {
> struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
> };
>
> +/*
> + * @min_volt: Actual min voltage at the local port
> + * @req_min_volt: Requested min voltage to the port partner
> + * @max_volt: Actual max voltage at the local port
> + * @req_max_volt: Requested max voltage to the port partner
> + * @max_curr: Actual max current at the local port
> + * @req_max_curr: Requested max current of the port partner
> + * @req_out_volt: Requested output voltage to the port partner
> + * @req_op_curr: Requested operating current to the port partner
> + * @supported: Parter has atleast one APDO hence supports PPS
> + * @active: PPS mode is active
> + */
> struct pd_pps_data {
> u32 min_volt;
> + u32 req_min_volt;
> u32 max_volt;
> + u32 req_max_volt;
> u32 max_curr;
> - u32 out_volt;
> - u32 op_curr;
> + u32 req_max_curr;
> + u32 req_out_volt;
> + u32 req_op_curr;
> bool supported;
> bool active;
> };
> @@ -2498,8 +2513,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> break;
> case SNK_NEGOTIATE_PPS_CAPABILITIES:
> /* Revert data back from any requested PPS updates */
> - port->pps_data.out_volt = port->supply_voltage;
> - port->pps_data.op_curr = port->current_limit;
> + port->pps_data.req_out_volt = port->supply_voltage;
> + port->pps_data.req_op_curr = port->current_limit;
> port->pps_status = (type == PD_CTRL_WAIT ?
> -EAGAIN : -EOPNOTSUPP);
>
> @@ -2548,8 +2563,11 @@ static void tcpm_pd_ctrl_request(struct tcpm_port
> *port,
> break;
> case SNK_NEGOTIATE_PPS_CAPABILITIES:
> port->pps_data.active = true;
> - port->req_supply_voltage = port->pps_data.out_volt;
> - port->req_current_limit = port->pps_data.op_curr;
> + port->pps_data.min_volt = port->pps_data.req_min_volt;
> + port->pps_data.max_volt = port->pps_data.req_max_volt;
> + port->pps_data.max_curr = port->pps_data.req_max_curr;
> + port->req_supply_voltage = port->pps_data.req_out_volt;
> + port->req_current_limit = port->pps_data.req_op_curr;
> tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> break;
> case SOFT_RESET_SEND:
> @@ -3108,16 +3126,16 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
> tcpm_port *port)
> src = port->source_caps[src_pdo];
> snk = port->snk_pdo[snk_pdo];
>
> - port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
> - pdo_pps_apdo_min_voltage(snk));
> - port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
> - pdo_pps_apdo_max_voltage(snk));
> - port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> - port->pps_data.out_volt = min(port->pps_data.max_volt,
> - max(port->pps_data.min_volt,
> - port->pps_data.out_volt));
> - port->pps_data.op_curr = min(port->pps_data.max_curr,
> - port->pps_data.op_curr);
> + port->pps_data.req_min_volt = max(pdo_pps_apdo_min_voltage(src),
> +
Re: [PATCH v2 2/6] usb: typec: tcpm: Address incorrect values of tcpm psy for pps supply
On Wed, Apr 07, 2021 at 01:07:19PM -0700, Badhri Jagan Sridharan wrote:
> tcpm_pd_select_pps_apdo overwrites port->pps_data.min_volt,
> port->pps_data.max_volt, port->pps_data.max_curr even before
> port partner accepts the requests. This leaves incorrect values
> in current_limit and supply_voltage that get exported by
> "tcpm-source-psy-". Solving this problem by caching the request
> values in req_min_volt, req_max_volt, req_max_curr, req_out_volt,
> req_op_curr. min_volt, max_volt, max_curr gets updated once the
> partner accepts the request. current_limit, supply_voltage gets updated
> once local port's tcpm enters SNK_TRANSITION_SINK when the accepted
> current_limit and supply_voltage is enforced.
>
> Fixes: f2a8aa053c176 ("typec: tcpm: Represent source supply through
> power_supply")
> Signed-off-by: Badhri Jagan Sridharan
> Reviewed-by: Adam Thomson
Reviewed-by: Heikki Krogerus
> ---
> Changes since V1:
> * Moved to kerneldoc header as suggested by Greg KH.
> * Added reviewed by tags.
> ---
> drivers/usb/typec/tcpm/tcpm.c | 88 +--
> 1 file changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 4ea4b30ae885..b4a40099d7e9 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -268,12 +268,27 @@ struct pd_mode_data {
> struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
> };
>
> +/*
> + * @min_volt: Actual min voltage at the local port
> + * @req_min_volt: Requested min voltage to the port partner
> + * @max_volt: Actual max voltage at the local port
> + * @req_max_volt: Requested max voltage to the port partner
> + * @max_curr: Actual max current at the local port
> + * @req_max_curr: Requested max current of the port partner
> + * @req_out_volt: Requested output voltage to the port partner
> + * @req_op_curr: Requested operating current to the port partner
> + * @supported: Parter has atleast one APDO hence supports PPS
> + * @active: PPS mode is active
> + */
> struct pd_pps_data {
> u32 min_volt;
> + u32 req_min_volt;
> u32 max_volt;
> + u32 req_max_volt;
> u32 max_curr;
> - u32 out_volt;
> - u32 op_curr;
> + u32 req_max_curr;
> + u32 req_out_volt;
> + u32 req_op_curr;
> bool supported;
> bool active;
> };
> @@ -2498,8 +2513,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> break;
> case SNK_NEGOTIATE_PPS_CAPABILITIES:
> /* Revert data back from any requested PPS updates */
> - port->pps_data.out_volt = port->supply_voltage;
> - port->pps_data.op_curr = port->current_limit;
> + port->pps_data.req_out_volt = port->supply_voltage;
> + port->pps_data.req_op_curr = port->current_limit;
> port->pps_status = (type == PD_CTRL_WAIT ?
> -EAGAIN : -EOPNOTSUPP);
>
> @@ -2548,8 +2563,11 @@ static void tcpm_pd_ctrl_request(struct tcpm_port
> *port,
> break;
> case SNK_NEGOTIATE_PPS_CAPABILITIES:
> port->pps_data.active = true;
> - port->req_supply_voltage = port->pps_data.out_volt;
> - port->req_current_limit = port->pps_data.op_curr;
> + port->pps_data.min_volt = port->pps_data.req_min_volt;
> + port->pps_data.max_volt = port->pps_data.req_max_volt;
> + port->pps_data.max_curr = port->pps_data.req_max_curr;
> + port->req_supply_voltage = port->pps_data.req_out_volt;
> + port->req_current_limit = port->pps_data.req_op_curr;
> tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> break;
> case SOFT_RESET_SEND:
> @@ -3108,16 +3126,16 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
> tcpm_port *port)
> src = port->source_caps[src_pdo];
> snk = port->snk_pdo[snk_pdo];
>
> - port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
> - pdo_pps_apdo_min_voltage(snk));
> - port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
> - pdo_pps_apdo_max_voltage(snk));
> - port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> - port->pps_data.out_volt = min(port->pps_data.max_volt,
> - max(port->pps_data.min_volt,
> - port->pps_data.out_volt));
> - port->pps_data.op_curr = min(port->pps_data.max_curr,
> - port->pps_data.op_curr);
> + port->pps_data.req_min_volt = max(pdo_pps_apdo_min_voltage(src),
> +
[PATCH v2 2/6] usb: typec: tcpm: Address incorrect values of tcpm psy for pps supply
tcpm_pd_select_pps_apdo overwrites port->pps_data.min_volt,
port->pps_data.max_volt, port->pps_data.max_curr even before
port partner accepts the requests. This leaves incorrect values
in current_limit and supply_voltage that get exported by
"tcpm-source-psy-". Solving this problem by caching the request
values in req_min_volt, req_max_volt, req_max_curr, req_out_volt,
req_op_curr. min_volt, max_volt, max_curr gets updated once the
partner accepts the request. current_limit, supply_voltage gets updated
once local port's tcpm enters SNK_TRANSITION_SINK when the accepted
current_limit and supply_voltage is enforced.
Fixes: f2a8aa053c176 ("typec: tcpm: Represent source supply through
power_supply")
Signed-off-by: Badhri Jagan Sridharan
Reviewed-by: Adam Thomson
---
Changes since V1:
* Moved to kerneldoc header as suggested by Greg KH.
* Added reviewed by tags.
---
drivers/usb/typec/tcpm/tcpm.c | 88 +--
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 4ea4b30ae885..b4a40099d7e9 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -268,12 +268,27 @@ struct pd_mode_data {
struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
};
+/*
+ * @min_volt: Actual min voltage at the local port
+ * @req_min_volt: Requested min voltage to the port partner
+ * @max_volt: Actual max voltage at the local port
+ * @req_max_volt: Requested max voltage to the port partner
+ * @max_curr: Actual max current at the local port
+ * @req_max_curr: Requested max current of the port partner
+ * @req_out_volt: Requested output voltage to the port partner
+ * @req_op_curr: Requested operating current to the port partner
+ * @supported: Parter has atleast one APDO hence supports PPS
+ * @active: PPS mode is active
+ */
struct pd_pps_data {
u32 min_volt;
+ u32 req_min_volt;
u32 max_volt;
+ u32 req_max_volt;
u32 max_curr;
- u32 out_volt;
- u32 op_curr;
+ u32 req_max_curr;
+ u32 req_out_volt;
+ u32 req_op_curr;
bool supported;
bool active;
};
@@ -2498,8 +2513,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
break;
case SNK_NEGOTIATE_PPS_CAPABILITIES:
/* Revert data back from any requested PPS updates */
- port->pps_data.out_volt = port->supply_voltage;
- port->pps_data.op_curr = port->current_limit;
+ port->pps_data.req_out_volt = port->supply_voltage;
+ port->pps_data.req_op_curr = port->current_limit;
port->pps_status = (type == PD_CTRL_WAIT ?
-EAGAIN : -EOPNOTSUPP);
@@ -2548,8 +2563,11 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
break;
case SNK_NEGOTIATE_PPS_CAPABILITIES:
port->pps_data.active = true;
- port->req_supply_voltage = port->pps_data.out_volt;
- port->req_current_limit = port->pps_data.op_curr;
+ port->pps_data.min_volt = port->pps_data.req_min_volt;
+ port->pps_data.max_volt = port->pps_data.req_max_volt;
+ port->pps_data.max_curr = port->pps_data.req_max_curr;
+ port->req_supply_voltage = port->pps_data.req_out_volt;
+ port->req_current_limit = port->pps_data.req_op_curr;
tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
break;
case SOFT_RESET_SEND:
@@ -3108,16 +3126,16 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
tcpm_port *port)
src = port->source_caps[src_pdo];
snk = port->snk_pdo[snk_pdo];
- port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
- pdo_pps_apdo_min_voltage(snk));
- port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
- pdo_pps_apdo_max_voltage(snk));
- port->pps_data.max_curr = min_pps_apdo_current(src, snk);
- port->pps_data.out_volt = min(port->pps_data.max_volt,
- max(port->pps_data.min_volt,
- port->pps_data.out_volt));
- port->pps_data.op_curr = min(port->pps_data.max_curr,
-port->pps_data.op_curr);
+ port->pps_data.req_min_volt = max(pdo_pps_apdo_min_voltage(src),
+
pdo_pps_apdo_min_voltage(snk));
+ port->pps_data.req_max_volt = min(pdo_pps_apdo_max_voltage(src),
+
