Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Tue, 15 Sep 2020 06:31:19 -0700
From:   Angus Ainslie <angus@akkea.ca>
To:     Chanwoo Choi <cw00.choi@samsung.com>
Cc:     kernel@puri.sm, MyungJoo Ham <myungjoo.ham@samsung.com>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        bryan.odonoghue@linaro.org
Subject: Re: [PATCH 1/4] extcon: Add USB VBUS properties
In-Reply-To: <bfa20bf2-e13b-1cd4-52d9-c8fa922d5aac@samsung.com>
References: <20200914164639.1487650-1-angus@akkea.ca>
 <CGME20200914165800epcas1p12e04260289513eac00f442388d5ba374@epcas1p1.samsung.com>
 <20200914164639.1487650-2-angus@akkea.ca>
 <bfa20bf2-e13b-1cd4-52d9-c8fa922d5aac@samsung.com>
Message-ID: <23ce56387f39fa097b36380dbe9661a0@akkea.ca>
User-Agent: Roundcube Webmail/1.3.6
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@akkea.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. */
>>