From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751904AbbJGH7J (ORCPT ); Wed, 7 Oct 2015 03:59:09 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:57249 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101AbbJGH7H (ORCPT ); Wed, 7 Oct 2015 03:59:07 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee691-f79d66d000001509-2d-5614d0c8718e Content-transfer-encoding: 8BIT Message-id: <5614D0C8.4090300@samsung.com> Date: Wed, 07 Oct 2015 16:59:04 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Roger Quadros , linux-kernel@vger.kernel.org Cc: k.kozlowski@samsung.com, ckeepax@opensource.wolfsonmicro.com, gregkh@linuxfoundation.org, ramakrishna.pallala@intel.com, patches@opensource.wolfsonmicro.com, myungjoo.ham@samsung.com Subject: Re: [PATCH v3] extcon: Modify the id and name of external connector References: <1444178883-5499-1-git-send-email-cw00.choi@samsung.com> <5614CAC7.3080002@ti.com> <5614CCE2.8070705@samsung.com> <5614CED6.2040602@ti.com> In-reply-to: <5614CED6.2040602@ti.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrKIsWRmVeSWpSXmKPExsWyRsSkWPfEBZEwg8/7LC3+TbnBbtG8eD2b xesXhhaXd81hs7jduILNYvnb/2wWC9/cZLLoeaTlwOGxeM9LJo/9c9ewe7yc+JvNo2/LKkaP 4ze2M3l83iQXwBbFZZOSmpNZllqkb5fAlXFjr0vBKc+Kq/N+sDUwXrHuYuTkkBAwkZj4ZTUT hC0mceHeerYuRi4OIYEVjBLzbmxlhyn68/Y2E0RiFqPEk7ezmEESvAKCEj8m32PpYuTgYBaQ lzhyKRvCVJeYMiUXovwBo8TC3/vASngFtCQe/fYHMVkEVCWar+mDDGEDiu5/cYMNJCwqECHR faISJCwiYC/RO3sWK4jNLLCPUWL9jHIQW1jAR+L/sWssENNnM0r8WLqJDSTBKaAmsWfHfkaQ hITAPXaJtku7wRIsAgIS3yYfAjtBQkBWYtMBZoivJCUOrrjBMoFRbBaSX2Yh/DIL4ZcFjMyr GEVTC5ILipPSi0z1ihNzi0vz0vWS83M3MQJj7/S/ZxN3MN4/YH2IUYCDUYmH96exSJgQa2JZ cWXuIUZToBsmMkuJJucDIzyvJN7Q2MzIwtTE1NjI3NJMSZxXR/pnsJBAemJJanZqakFqUXxR aU5q8SFGJg5OqQbGExw7TXbJcn6/cULbMMZxisCz3c1HXkxr/d7VFyflfWLjnB01F345RT+5 M1P0UQSj6Sr36cmnm8x6t/OffXbwhZHF9WMp/gcO35iqnxn/6OAzk3+hNV9rGvpVtBvv7KlZ 8C15ueztsz7a8k7b2CWnr0kpyuNqnd21fLk7H6/cl9CqgyeFHsu4K7EUZyQaajEXFScCABpl E8q4AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrEIsWRmVeSWpSXmKPExsVy+t9jQd0TF0TCDOZ8MrL4N+UGu0Xz4vVs Fq9fGFpc3jWHzeJ24wo2i+Vv/7NZLHxzk8mi55GWA4fH4j0vmTz2z13D7vFy4m82j74tqxg9 jt/YzuTxeZNcAFtUA6NNRmpiSmqRQmpecn5KZl66rZJ3cLxzvKmZgaGuoaWFuZJCXmJuqq2S i0+ArltmDtA9SgpliTmlQKGAxOJiJX07TBNCQ9x0LWAaI3R9Q4LgeowM0EDCGsaMG3tdCk55 Vlyd94OtgfGKdRcjJ4eEgInEn7e3mSBsMYkL99azdTFycQgJzGKUePJ2FjNIgldAUOLH5Hss XYwcHMwC8hJHLmVDmOoSU6bkQpQ/YJRY+HsfWAmvgJbEo9/+ICaLgKpE8zV9kCFsQNH9L26w gYRFBSIkuk9UgoRFBOwlemfPYgWxmQX2MUqsn1EOYgsL+Ej8P3aNBWL6bEaJH0s3sYEkOAXU JPbs2M84gRHoRITbZiHcNgvhtgWMzKsYJVILkguKk9JzjfJSy/WKE3OLS/PS9ZLzczcxguP7 mfQOxsO73A8xCnAwKvHw/jAWCRNiTSwrrsw9xCjBwawkwvviBFCINyWxsiq1KD++qDQntfgQ oynQdxOZpUST84GpJ68k3tDYxMzI0sjc0MLI2FxJnPfGIYYwIYH0xJLU7NTUgtQimD4mDk6p Bsat7Cy3Nlyaz1Gkc96gX39nQq7o+gPndTf+8XSeIP/i922FK3+y75os++tR0Pif9YpFn7Ts T4N51as5j58/E9L0efXfqs2XTLOf+hw+F7RZVuugSXTon6yPcwNbo0XYV+bdO5Zh7uZWsPbF jdwc+d+L/9k9blVeLlnQf3rm3oYdSpN3LzhmWCWtxFKckWioxVxUnAgA8itbnQUDAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015년 10월 07일 16:50, Roger Quadros wrote: > On 07/10/15 10:42, Chanwoo Choi wrote: >> On 2015년 10월 07일 16:33, Roger Quadros wrote: >>> On 07/10/15 03:48, Chanwoo Choi wrote: >>>> This patch modifies the id and name of external connector with the >>>> additional prefix to clarify both attribute and meaning of external >>>> connector as following: >>>> - EXTCON_CHG_* mean the charger connector. >>>> - EXTCON_JACK_* mean the jack connector. >>>> - EXTCON_DISP_* mean the display port connector. >>>> >>>> Following table show the new name of external connector with old name: >>>> -------------------------------------------------- >>>> Old extcon name | New extcon name | >>>> -------------------------------------------------- >>>> EXTCON_TA | EXTCON_CHG_USB_DCP | >>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP | >>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST| >>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW| >>> >>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW. >>> If I see the charger IC drivers that use FAST & SLOW, they either call it >>> APPLE_500MA/1A or SPECIAL_500MA/1A. >> >> Right. >> >>> >>> They are probably not compliant with USB DCP specification and hence >>> the special name. >>> >>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead? >> >> I don't prefer to use "SPECIAL" adjective for connector name. >> I prefer following name at the first patch. >> - EXTCON_CHG_USB_FAST >> - EXTCON_CHG_USB_SLOW > > OK, that is fine with me. >> >>> >>>> -------------------------------------------------- >>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE | >>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE | >>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN | >>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT | >>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN | >>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT | >>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN | >>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT | >>>> -------------------------------------------------- >>>> EXTCON_HMDI | EXTCON_DISP_HDMI | >>>> EXTCON_MHL | EXTCON_DISP_MHL | >>>> EXTCON_DVI | EXTCON_DISP_DVI | >>>> EXTCON_VGA | EXTCON_DISP_VGA | >>>> -------------------------------------------------- >>>> >>>> And, when altering the name of USB charger connector, EXTCON refers to the >>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard >>>> name of USB charging port as following. Following name of USB charging port >>>> are already used in power_supply subsystem. We chan check it on patch[2]. >>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */ >>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */ >>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */ >>>> >>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip >>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers") >>>> >>>> Signed-off-by: Chanwoo Choi >>>> [ckeepax: For the Arizona changes] >>>> Acked-by: Charles Keepax >>>> --- >>>> Changes from v2: >>>> (https://lkml.org/lkml/2015/10/6/239) >>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to >>>> Roger's comment and drop patch2 about EXTCON_CHG_USB. >>>> - Fix the warning issue provided by scripts/checkpatch.pl >>>> >>>> Changes from v1: >>>> (https://lkml.org/lkml/2015/10/3/304) >>>> - Add acked tag by Charles Keepax for arizona changes >>>> - Modify the name of USB charger connector as following: >>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST >>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW >>>> - Add the missing EXTCON_CHG_USB_ACA charger connector >>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected >>>> or not >>>> >>>> drivers/extcon/extcon-arizona.c | 18 ++++++------ >>>> drivers/extcon/extcon-axp288.c | 12 ++++---- >>>> drivers/extcon/extcon-max14577.c | 17 +++++------ >>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++---------- >>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++-------- >>>> drivers/extcon/extcon-max8997.c | 21 +++++++------- >>>> drivers/extcon/extcon-rt8973a.c | 4 +-- >>>> drivers/extcon/extcon-sm5502.c | 4 +-- >>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++------------------- >>>> include/linux/extcon.h | 61 +++++++++++++++++++++++----------------- >>>> 10 files changed, 137 insertions(+), 119 deletions(-) >>>> >>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c >>>> index a1ab0a56b798..e4890dd4fefd 100644 >>>> --- a/drivers/extcon/extcon-arizona.c >>>> +++ b/drivers/extcon/extcon-arizona.c >>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = { >>>> >>>> static const unsigned int arizona_cable[] = { >>>> EXTCON_MECHANICAL, >>>> - EXTCON_MICROPHONE, >>>> - EXTCON_HEADPHONE, >>>> - EXTCON_LINE_OUT, >>>> + EXTCON_JACK_MICROPHONE, >>>> + EXTCON_JACK_HEADPHONE, >>>> + EXTCON_JACK_LINE_OUT, >>>> EXTCON_NONE, >>>> }; >>>> >>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data) >>>> struct arizona_extcon_info *info = data; >>>> struct arizona *arizona = info->arizona; >>>> int id_gpio = arizona->pdata.hpdet_id_gpio; >>>> - unsigned int report = EXTCON_HEADPHONE; >>>> + unsigned int report = EXTCON_JACK_HEADPHONE; >>>> int ret, reading; >>>> bool mic = false; >>>> >>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data) >>>> >>>> /* Report high impedence cables as line outputs */ >>>> if (reading >= 5000) >>>> - report = EXTCON_LINE_OUT; >>>> + report = EXTCON_JACK_LINE_OUT; >>>> else >>>> - report = EXTCON_HEADPHONE; >>>> + report = EXTCON_JACK_HEADPHONE; >>>> >>>> ret = extcon_set_cable_state_(info->edev, report, true); >>>> if (ret != 0) >>>> @@ -732,7 +732,7 @@ err: >>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC); >>>> >>>> /* Just report headphone */ >>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true); >>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true); >>>> if (ret != 0) >>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret); >>>> >>>> @@ -789,7 +789,7 @@ err: >>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC); >>>> >>>> /* Just report headphone */ >>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true); >>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true); >>>> if (ret != 0) >>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret); >>>> >>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work) >>>> arizona_identify_headphone(info); >>>> >>>> ret = extcon_set_cable_state_(info->edev, >>>> - EXTCON_MICROPHONE, true); >>>> + EXTCON_JACK_MICROPHONE, true); >>>> if (ret != 0) >>>> dev_err(arizona->dev, "Headset report failed: %d\n", >>>> ret); >>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c >>>> index 9668d6a94e38..14e748051288 100644 >>>> --- a/drivers/extcon/extcon-axp288.c >>>> +++ b/drivers/extcon/extcon-axp288.c >>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq { >>>> }; >>>> >>>> static const unsigned int axp288_extcon_cables[] = { >>>> - EXTCON_SLOW_CHARGER, >>>> - EXTCON_CHARGE_DOWNSTREAM, >>>> - EXTCON_FAST_CHARGER, >>>> + EXTCON_CHG_USB_DCP_SLOW, >>>> + EXTCON_CHG_USB_CDP, >>>> + EXTCON_CHG_USB_DCP_FAST, >>>> EXTCON_NONE, >>>> }; >>>> >>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info) >>>> dev_dbg(info->dev, "sdp cable is connecetd\n"); >>> >>> as per comment this seems to be SDP. >> >> This patch modifies the only connector name. >> If some comment should be fixed, we need to implement on separate patch. > > Agreed. >> >>> >>>> notify_otg = true; >>>> notify_charger = true; >>>> - cable = EXTCON_SLOW_CHARGER; >>>> + cable = EXTCON_CHG_USB_DCP_SLOW; >>> >>> So shouldn't this be cable = EXTCON_USB? >> >> No, EXTCON_CHG_USB_*_SLOW is right. >> >>> >>>> break; >>>> case DET_STAT_CDP: >>>> dev_dbg(info->dev, "cdp cable is connecetd\n"); >>>> notify_otg = true; >>>> notify_charger = true; >>>> - cable = EXTCON_CHARGE_DOWNSTREAM; >>>> + cable = EXTCON_CHG_USB_CDP; >>>> break; >>>> case DET_STAT_DCP: >>>> dev_dbg(info->dev, "dcp cable is connecetd\n"); >>> >>> This looks like standard DCP. >> >> ditto. Need the separate patch to fix it. > > Agreed. > >> >>> >>>> notify_charger = true; >>>> - cable = EXTCON_FAST_CHARGER; >>>> + cable = EXTCON_CHG_USB_DCP_FAST; >>> >>> Shouldn't this be cable = EXTCON_CHG_USB_DCP? >> >> No, EXTCON_CHG_USB_*_FAST is right. > > Why? The case is DET_STAT_DCP. You're right. I'll fix it as following: case DET_STAT_SDP -> EXTCON_USB case DET_STAT_CDP -> EXTCON_CHG_USB_CDP case DET_STAT_DCP -> EXTCON_CHG_USB_DCP Thanks, Chanwoo Choi