linux-kernel - Re: [PATCH 1/4] extcon: Add USB VBUS properties

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: <23ce56387f39fa097b36380dbe9661a0@akkea.ca>
Date:   Tue, 15 Sep 2020 06:31:19 -0700
From:   Angus Ainslie <angus@...ea.ca>
To:     Chanwoo Choi <cw00.choi@...sung.com>
Cc:     kernel@...i.sm, MyungJoo Ham <myungjoo.ham@...sung.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        bryan.odonoghue@...aro.org
Subject: Re: [PATCH 1/4] extcon: Add USB VBUS properties

Hi,

On 2020-09-14 18:40, Chanwoo Choi wrote:
> Hi,
> 
> On 9/15/20 1:46 AM, Angus Ainslie wrote:
>> USB type C, USB BC1.2 and USB power delivery allow different voltages
>> and currents for VBUS so we need these additional properties.
>> 
>> Also USB type C allows separate device and power roles so add a VBUS 
>> SRC
>> property.
>> 
>> Signed-off-by: Angus Ainslie <angus@...ea.ca>
>> ---
>>  include/linux/extcon.h | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
>> index fd183fb9c20f..c4d48f4f74c4 100644
>> --- a/include/linux/extcon.h
>> +++ b/include/linux/extcon.h
>> @@ -117,14 +117,29 @@
>>   * @type:       integer (intval)
>>   * @value:      0 (USB/USB2) or 1 (USB3)
>>   * @default:    0 (USB/USB2)
>> + * - EXTCON_PROP_USB_VBUS_SRC
> 
> Could you explain more correct meaning of both sink and source?
> 

Sure I can add some comments.

>> + * @type:	integer (intval)
>> + * @value:	0 (sink) or 1 (source)
>> + * @default:	0 (sink)
>> + * - EXTCON_PROP_USB_VBUS_VOLTAGE
>> + * @type:	integer (intval)
>> + * @value:	negotiated vbus voltage in mV
>> + * @default:	5000
> 
> Could you suggest the data why do you set default value as 5000?
> 

The lowest USB VBUS is 5V so I can add that to the comments.

>> + * - EXTCON_PROP_USB_VBUS_CURRENT
>> + * @type:	integer (intval)
>> + * @value:	negotiated vbus current in mA
>> + * @default:	100
> 
> ditto. Why default value is 100?
> 

USB spec says that until the current is negotiated the max that anything 
can draw is 100mA. I can add a comment to that effect.

Thanks
Angus

>>   *
>>   */
>>  #define EXTCON_PROP_USB_VBUS		0
>>  #define EXTCON_PROP_USB_TYPEC_POLARITY	1
>>  #define EXTCON_PROP_USB_SS		2
>> +#define EXTCON_PROP_USB_VBUS_SRC	3
>> +#define EXTCON_PROP_USB_VBUS_VOLTAGE	4
>> +#define EXTCON_PROP_USB_VBUS_CURRENT	5
>> 
>>  #define EXTCON_PROP_USB_MIN		0
>> -#define EXTCON_PROP_USB_MAX		2
>> +#define EXTCON_PROP_USB_MAX		5
>>  #define EXTCON_PROP_USB_CNT	(EXTCON_PROP_USB_MAX - 
>> EXTCON_PROP_USB_MIN + 1)
>> 
>>  /* Properties of EXTCON_TYPE_CHG. */
>>