linux-kernel - RE: [RFC PATCH v2 6/7] typec: tcpm: Represent source supply through power

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:   Mon, 27 Nov 2017 13:43:36 +0000
From:   Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Adam Thomson <Adam.Thomson.Opensource@...semi.com>
CC:     Guenter Roeck <linux@...ck-us.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Sebastian Reichel <sre@...nel.org>,
        Yueyao Zhu <yueyao.zhu@...il.com>,
        Rui Miguel Silva <rmfrfs@...il.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Support Opensource" <Support.Opensource@...semi.com>
Subject: RE: [RFC PATCH v2 6/7] typec: tcpm: Represent source supply through
 power_supply class

On 25 November 2017 14:03, Hans de Goede wrote:

> Hi,
> 
> On 11/24/2017 01:19 PM, Heikki Krogerus wrote:
> > Hi,
> >
> > On Tue, Nov 14, 2017 at 11:44:47AM +0000, Adam Thomson wrote:
> >> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> >> index 78983e1..7c26c3d 100644
> >> --- a/drivers/usb/typec/tcpm.c
> >> +++ b/drivers/usb/typec/tcpm.c
> >> @@ -12,6 +12,7 @@
> >>   #include <linux/kernel.h>
> >>   #include <linux/module.h>
> >>   #include <linux/mutex.h>
> >> +#include <linux/power_supply.h>
> >>   #include <linux/proc_fs.h>
> >>   #include <linux/sched/clock.h>
> >>   #include <linux/seq_file.h>
> >> @@ -277,6 +278,10 @@ struct tcpm_port {
> >>   	u32 current_limit;
> >>   	u32 supply_voltage;
> >>
> >> +	/* Used to export TA voltage and current */
> >> +	struct power_supply *psy;
> >> +	struct power_supply_desc psy_desc;
> >> +
> >>   	u32 bist_request;
> >>
> >>   	/* PD state for Vendor Defined Messages */
> >> @@ -1756,6 +1761,7 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port)
> >>   	int ret = -EINVAL;
> >>
> >>   	port->pps_data.supported = false;
> >> +	port->psy_desc.type = POWER_SUPPLY_TYPE_USB_PD;
> >>
> >>   	/*
> >>   	 * Select the source PDO providing the most power while staying within
> >> @@ -1775,8 +1781,10 @@ static int tcpm_pd_select_pdo(struct tcpm_port
> *port)
> >>   			mv = pdo_min_voltage(pdo);
> >>   			break;
> >>   		case PDO_TYPE_APDO:
> >> -			if (pdo_apdo_type(pdo) == APDO_TYPE_PPS)
> >> +			if (pdo_apdo_type(pdo) == APDO_TYPE_PPS) {
> >>   				port->pps_data.supported = true;
> >> +				port->psy_desc.type =
> POWER_SUPPLY_TYPE_USB_PD_PPS;
> >> +			}
> >>   			continue;
> >>   		default:
> >>   			tcpm_log(port, "Invalid PDO type, ignoring");
> >> @@ -2248,6 +2256,9 @@ static void tcpm_reset_port(struct tcpm_port *port)
> >>   	port->try_snk_count = 0;
> >>   	port->supply_voltage = 0;
> >>   	port->current_limit = 0;
> >> +	port->psy_desc.type = POWER_SUPPLY_TYPE_USB_TYPE_C;
> >
> > Is it OK to everybody that the type of the psy is changed like that?
> > Hans?!
> >
> > We do have drivers that already change the type, for example
> > drivers/power/supply/isp1704_charger.c, but what does the user space
> > expect? The ABI for the power supply class was never documented I
> > guess.
> 
> The main userspace consumer of the power_supply sysfs class is upower,
> upower knows about 3 types: "Mains", "Battery" and "USB", anything
> else it gives a type of UP_DEVICE_KIND_UNKNOWN. So
> POWER_SUPPLY_TYPE_USB_TYPE_C
> vs POWER_SUPPLY_TYPE_USB_PD_PPS does matter to it.

Does or doesn't matter? I had a quick look at the code for upower and if I
understand correctly right now both USB_TYPE_C and USB_PD_PPS would be
represented as 'UP_DEVICE_KIND_UNKNOWN' based on g_ascii_strcasecmp result. Is
that correct?