Hi, 2016-07-23 3:21 GMT+09:00 Guenter Roeck <groeck at google.com>: > Hi, > > On Fri, Jul 22, 2016 at 2:29 AM, Chanwoo Choi <cw00.choi at samsung.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