On Tue, Oct 26, 2021 at 05:33:51PM +0300, Heikki Krogerus wrote:

-static int ucsi_get_src_pdos(struct ucsi_connector *con)
+int ucsi_read_pdos(struct ucsi_connector *con, int partner, int source, u32 *pdos)
 {
+	u32 pdo[PDO_MAX_OBJECTS];
+	int num_pdos;
 	int ret;
 
 	/* UCSI max payload means only getting at most 4 PDOs at a time */
-	ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
+	ret = ucsi_get_pdos(con, partner, source, pdo, 0, UCSI_MAX_PDOS);
 	if (ret < 0)
 		return ret;
 
-	con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
-	if (con->num_pdos < UCSI_MAX_PDOS)
-		return 0;
+	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
+	if (num_pdos < UCSI_MAX_PDOS)
+		goto done;
 
 	/* get the remaining PDOs, if any */
-	ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
+	ret = ucsi_get_pdos(con, partner, source, pdo, UCSI_MAX_PDOS,
 			    PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
 	if (ret < 0)
 		return ret;
 
-	con->num_pdos += ret / sizeof(u32);
+	num_pdos += ret / sizeof(u32);
+done:
+	memcpy(pdos, pdo, num_pdos * sizeof(pdo));
+
+	return num_pdos;
+}
+
+static int ucsi_get_src_pdos(struct ucsi_connector *con)
+{
+	int ret;
+
+	ret = ucsi_read_pdos(con, 0, 1, con->src_pdos);

Second parameter should be 1 right?  Original intent of get_src_pdos()
is to retrieve the partner's source capabilities in order to populate
the power_supply.  Passing 0 as the partner param here changes the
behavior to retrieve the source PDOs of the port.

(BTW I'm going to send a quick patch for this to since this assumes that
port is sink and partner is source; when it's the other way around we
end up calling GET_PDOS on the sink partner when it might not even be
source capable).

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project