linux-kernel - Re: [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211027010035.GA13345@jackp-linux.qualcomm.com>
Date:   Tue, 26 Oct 2021 18:00:35 -0700
From:   Jack Pham <jackp@...eaurora.org>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Prashant Malani <pmalani@...omium.org>,
        Benson Leung <bleung@...gle.com>,
        Adam Thomson <Adam.Thomson.Opensource@...semi.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        "Gopal, Saranya" <saranya.gopal@...el.com>,
        "Regupathy, Rajaram" <rajaram.regupathy@...el.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev

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