linux-kernel - Re: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

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]

Date:   Tue, 11 Sep 2018 08:33:22 -0700
From:   Guenter Roeck <groeck7@...il.com>
To:     "Angus Ainslie (Purism)" <angus@...ea.ca>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

I cant put my finger on it but this seems wrong. As i said both src and sink should never be true at the same time. I also din’t understand why turning off src should power off your board. Ultimately my concern is that we may be just painting over the real problem, and that would be really bad to do with dt properties.

Note that sending to groeck7@ makes it hard for me to see your emails and to reply. Using linux@...ck-us.net would be much better.

Guenter

Sent from my iPhone

> On Sep 11, 2018, at 7:59 AM, Angus Ainslie (Purism) <angus@...ea.ca> wrote:
> 
> If the board is being powered by USB disabling the source and sink
> can remove power from the board. Allow the source and sink to be
> initallized based on devicetree values.
> 
> Changed since V2:
> 
> Change the devicetree documentation.
> Change the devicetree property names.
> 
> Changed since V1:
> 
> use devicetree values instead of hardcoded initialization.
> 
> Signed-off-by: Angus Ainslie (Purism) <angus@...ea.ca>
> ---
> .../devicetree/bindings/usb/typec-tcpci.txt        |  6 ++++++
> drivers/usb/typec/tcpm.c                           | 14 +++++++++++---
> 2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> index 0dd1469e7318..b07418ae6482 100644
> --- a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> @@ -15,6 +15,12 @@ Required sub-node:
>   of connector node are specified in
>   Documentation/devicetree/bindings/connector/usb-connector.txt
> 
> +Optional properties for usb-c-connector sub-node:
> +- init-vbus-source: set the initalization value for vbus-source to true.
> +  If this property is not present the initial value will be false.
> +- init-vbus-sink: set the initalization value for vbus-sink to true.
> +  If this property is not present the initial value will be false.
> +
> Example:
> 
> ptn5110@50 {
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index ca7bedb46f7f..10c14ece3858 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -2462,9 +2462,7 @@ static int tcpm_init_vbus(struct tcpm_port *port)
> {
>    int ret;
> 
> -    ret = port->tcpc->set_vbus(port->tcpc, false, false);
> -    port->vbus_source = false;
> -    port->vbus_charge = false;
> +    ret = port->tcpc->set_vbus(port->tcpc, port->vbus_source, port->vbus_charge);
>    return ret;
> }
> 
> @@ -4266,6 +4264,16 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>        return -EINVAL;
>    port->port_type = port->typec_caps.type;
> 
> +    if (fwnode_property_present(fwnode, "init-vbus-source"))
> +        port->vbus_source = true;
> +    else
> +        port->vbus_source = false;
> +
> +    if (fwnode_property_present(fwnode, "init-vbus-sink"))
> +            port->vbus_charge = true;
> +    else
> +            port->vbus_charge = false;
> +
>    if (port->port_type == TYPEC_PORT_SNK)
>        goto sink;
> 
> -- 
> 2.17.1
>