linux-kernel - Re: [v6 PATCH 1/6] extcon: Add Type-C and DP support

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:	Sat, 23 Jul 2016 11:19:21 +0900
From:	Chanwoo Choi <cwchoi00@...il.com>
To:	Guenter Roeck <groeck@...gle.com>
Cc:	Chanwoo Choi <cw00.choi@...sung.com>,
	Chris Zhong <zyw@...k-chips.com>,
	Douglas Anderson <dianders@...omium.org>,
	Tomasz Figa <tfiga@...omium.org>,
	Heiko Stübner <heiko@...ech.de>,
	姚智情 <yzq@...k-chips.com>,
	Guenter Roeck <groeck@...omium.org>,
	"myungjoo.ham@...sung.com" <myungjoo.ham@...sung.com>,
	wulf <wulf@...k-chips.com>,
	Stéphane Marchesin <marcheu@...omium.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [v6 PATCH 1/6] extcon: Add Type-C and DP support

Hi,

2016-07-23 3:21 GMT+09:00 Guenter Roeck <groeck@...gle.com>:
> Hi,
>
> On Fri, Jul 22, 2016 at 2:29 AM, Chanwoo Choi <cw00.choi@...sung.com> wrote:
>> Hi Chris,
>>
>> I'm sorry for late reply. I finished the first draft to support the extcon property.
>> You can check the patches[1]. But, I need more time to test it. After tested it,
>> I'll send the patches.
>>
>> [1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>>
>> Chanwoo Choi (4):
>>   extcon: Add the extcon_type to group each connector into five category
>>   extcon: Add the support for extcon property according to type of connector
>>   extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
>>   extcon: Add the sync APIs to support the  notification for extcon property
>>
>> Chris Zhong (1):
>>   extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>>
>
> Couple of comments:
>
> - In extcon.c, line 560: 'state' is unused.
>
> - extcon_set_property(edev, EXTCON_USB, EXTCON_USB_VBUS, 0);
>   does not work. As written, it has to be something like:

you shoud use the 'union extcon_property_valu' as below.

>
>   union extcon_property_value prop;
>   ...
>   prop.intval = vbus_state;
>   extcon_set_property(edev, EXTCON_USB, EXTCON_USB_VBUS, prop);
>
> - For USB, the state of EXTCON_USB, EXTCON_USB_HOST, and
> EXTCON_DISP_DP tend to change at the same time, together with the
> associated properties.
>   It might be desirable to have an equivalent of extcon_sync_all(edev);

Each external connector has the separate notifier independently.
All of operation in extcon should be done by each external connector unit.

I think that extcon_sync_all() is not appropriate. If the some extcon device
support the many connectors, maybe extcon send the un-needed notifications
for all connectors.

Also, extcon_sync_all(edev) is same operation with follwoing three
function calls:
extcon_sync(edev, EXTCON_USB);
extcon_sync(edev, EXTCON_USB_HOST);
extcon_sync(edev, EXTCON_DISP_DP);

Also,
EXTCON_DISP_DP has the both EXTCON_TYPE_USB and EXTCON_TYPE_DISP.
So, the extcon client driver is able to set the vbus property with
EXTCON_DISP_DP as following:
- extcon_set_property(edev, EXTCON_DISP_DP, EXTCON_USB_VBUS, prop)

Thanks,
Chanwoo Choi