From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932072AbbG0X0v (ORCPT ); Mon, 27 Jul 2015 19:26:51 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:55963 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754545AbbG0X0s (ORCPT ); Mon, 27 Jul 2015 19:26:48 -0400 X-AuditID: cbfee690-f796f6d000005054-f6-55b6be35b534 Message-id: <55B6BE35.7020409@samsung.com> Date: Tue, 28 Jul 2015 08:26:45 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-version: 1.0 To: Roger Quadros Cc: ivan.ivanov@linaro.org, linux-omap@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel , Greg Kroah-Hartman Subject: Re: [PATCH v2 1/2] extcon: fix hang and extcon_get/set_cable_state(). References: <1436194018-18696-2-git-send-email-rogerq@ti.com> <1436274375-10308-1-git-send-email-rogerq@ti.com> <55B60A73.8080905@ti.com> In-reply-to: <55B60A73.8080905@ti.com> Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprJIsWRmVeSWpSXmKPExsWyRsSkUNd037ZQg+an6hbNi9ezWVyeP5Pd 4vKuOWwWs5f0s1gsWtbKbNHzSMuBzePOtT1sHvvnrmH3OH5jO5PH501yASxRXDYpqTmZZalF +nYJXBkv+6+yFuzRqdi4fBZTA+M0hS5GTg4JAROJTdNbWCFsMYkL99azdTFycQgJrGCUWLag nwmm6PnjnUwQiaWMEtv3dENVPWCU+P7nCztIFa+AlsTMte+Bqjg4WARUJR4cdAAJswGF97+4 wQZiiwqESaycfoUFolxQ4sfke2C2iICixL2VEJuZBTYzSry+8RosISzgJ/Fn8iF2iGVXGCXW b1oAdhKngJpEz7OTYFOZBdQlJs1bxAxhy0tsXvOWGeLsfewSO3eng9gsAgIS3yYfYgE5TkJA VmLTAagSSYmDK26wTGAUm4XkpllIps5CMnUBI/MqRtHUguSC4qT0IhO94sTc4tK8dL3k/NxN jMAYO/3v2YQdjPcOWB9iFOBgVOLhnbBuW6gQa2JZcWXuIUZToCsmMkuJJucDIzmvJN7Q2MzI wtTE1NjI3NJMSZz3tdTPYCGB9MSS1OzU1ILUovii0pzU4kOMTBycUg2M6ZebtzlmPjVUYLnl /O2b8SX2nE/SCwNkQrebzq3+2ZK8w+aDimPw+kRfjt47YUw7Hzx6WLpmtuanEy82+79+2x0W Hlpq9u32+wU7uM+qyomKrWreGC/2uEfNYONX08yLlQrum26G6CrcOf3+Y7teEV/WjW0/Hxp8 PcX8fKNAnzrjDqdvXhe4lViKMxINtZiLihMBCYbmh6wCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrOIsWRmVeSWpSXmKPExsVy+t9jAV3TfdtCDTbPVrJoXryezeLy/Jns Fpd3zWGzmL2kn8Vi0bJWZoueR1oObB53ru1h89g/dw27x/Eb25k8Pm+SC2CJamC0yUhNTEkt UkjNS85PycxLt1XyDo53jjc1MzDUNbS0MFdSyEvMTbVVcvEJ0HXLzAFarqRQlphTChQKSCwu VtK3wzQhNMRN1wKmMULXNyQIrsfIAA0krGHMeNl/lbVgj07FxuWzmBoYpyl0MXJySAiYSDx/ vJMJwhaTuHBvPVsXIxeHkMBSRonte7qhnAeMEt//fGEHqeIV0JKYufY9UAcHB4uAqsSDgw4g YTag8P4XN9hAbFGBMImV06+wQJQLSvyYfA/MFhFQlLi3EmIBs8BmRonXN16DJYQF/CT+TD7E DrHsCqPE+k0LwE7iFFCT6Hl2Emwqs4C6xKR5i5ghbHmJzWveMk9gFJiFZMksJGWzkJQtYGRe xSiRWpBcUJyUnmuUl1quV5yYW1yal66XnJ+7iREcyc+kdzAe3uV+iFGAg1GJh/fFhm2hQqyJ ZcWVuYcYJTiYlUR4GSuAQrwpiZVVqUX58UWlOanFhxhNgaEwkVlKNDkfmGTySuINjU3MjCyN zA0tjIzNlcR59U02hQoJpCeWpGanphakFsH0MXFwSjUwbnxVt1d8Zg7fycqAvtTLyTPlHz7/ NL8vh331lBmrje5y6mQGKsRLu3iLirrvKDh461/kk/c8qUEvHocqP86y4/hmInHyjmDM0VXP OHl+uPZ+lfowoWNi4hl2ZmND0dXr55dbbzpwf26Eldr8Kw+CDGuuau422fG4SKVZf8qutGPz fTnOnuI7rsRSnJFoqMVcVJwIAInVgX/6AgAA 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 Hi Roger, On 07/27/2015 07:39 PM, Roger Quadros wrote: > Chanwoo, > > On 10/07/15 18:54, Chanwoo Choi wrote: >> Hi Roger, >> >> Thanks for your working. >> >> I'll review, test and apply it on next week because I'm on vacation now. > > Can you please take this for -rc cycle? Thanks. I'm sorry for delay review. I'll do it within this week. Thanks, Chanwoo Choi > > cheers, > -roger > >> >> Thanks, >> Chanwoo Choi >> >> On Tue, Jul 7, 2015 at 3:06 PM, Roger Quadros wrote: >>> Users of find_cable_index_by_name() will cause a kernel hang >>> as the while loop counter is never incremented and end condition >>> is never reached. >>> >>> extcon_get_cable_state() and extcon_set_cable_state() are broken >>> because they use cable index instead of cable id. This causes >>> the first cable state (cable.0) to be always invalid in sysfs >>> or extcon_get_cable_state() users. >>> >>> Introduce a new function find_cable_id_by_name() that fixes >>> both of the above issues. >>> >>> Fixes: commit 73b6ecdb93e8 ("extcon: Redefine the unique id of supported external connectors without 'enum extcon' type") >>> Cc: Greg Kroah-Hartman >>> Signed-off-by: Roger Quadros >>> --- >>> drivers/extcon/extcon.c | 38 +++++++++++++++++++++++++++++--------- >>> 1 file changed, 29 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c >>> index 76157ab..987dd3c 100644 >>> --- a/drivers/extcon/extcon.c >>> +++ b/drivers/extcon/extcon.c >>> @@ -124,22 +124,34 @@ static int find_cable_index_by_id(struct extcon_dev *edev, const unsigned int id >>> return -EINVAL; >>> } >>> >>> -static int find_cable_index_by_name(struct extcon_dev *edev, const char *name) >>> +static int find_cable_id_by_name(struct extcon_dev *edev, const char *name) >>> { >>> - unsigned int id = EXTCON_NONE; >>> + unsigned int id = -EINVAL; >>> int i = 0; >>> >>> - if (edev->max_supported == 0) >>> - return -EINVAL; >>> - >>> - /* Find the the number of extcon cable */ >>> + /* Find the id of extcon cable */ >>> while (extcon_name[i]) { >>> if (!strncmp(extcon_name[i], name, CABLE_NAME_MAX)) { >>> id = i; >>> break; >>> } >>> + >>> + i++; >>> } >>> >>> + return id; >>> +}; >>> + >>> +static int find_cable_index_by_name(struct extcon_dev *edev, const char *name) >>> +{ >>> + unsigned int id = EXTCON_NONE; >>> + >>> + if (edev->max_supported == 0) >>> + return -EINVAL; >>> + >>> + /* Find the the number of extcon cable */ >>> + id = find_cable_id_by_name(edev, name); >>> + >>> if (id == EXTCON_NONE) >>> return -EINVAL; >>> >>> @@ -228,9 +240,11 @@ static ssize_t cable_state_show(struct device *dev, >>> struct extcon_cable *cable = container_of(attr, struct extcon_cable, >>> attr_state); >>> >>> + int i = cable->cable_index; >>> + >>> return sprintf(buf, "%d\n", >>> extcon_get_cable_state_(cable->edev, >>> - cable->cable_index)); >>> + cable->edev->supported_cable[i])); >>> } >>> >>> /** >>> @@ -341,6 +355,9 @@ int extcon_get_cable_state_(struct extcon_dev *edev, const unsigned int id) >>> { >>> int index; >>> >>> + if (id == EXTCON_NONE) >>> + return -EINVAL; >>> + >>> index = find_cable_index_by_id(edev, id); >>> if (index < 0) >>> return index; >>> @@ -361,7 +378,7 @@ EXPORT_SYMBOL_GPL(extcon_get_cable_state_); >>> */ >>> int extcon_get_cable_state(struct extcon_dev *edev, const char *cable_name) >>> { >>> - return extcon_get_cable_state_(edev, find_cable_index_by_name >>> + return extcon_get_cable_state_(edev, find_cable_id_by_name >>> (edev, cable_name)); >>> } >>> EXPORT_SYMBOL_GPL(extcon_get_cable_state); >>> @@ -380,6 +397,9 @@ int extcon_set_cable_state_(struct extcon_dev *edev, unsigned int id, >>> u32 state; >>> int index; >>> >>> + if (id == EXTCON_NONE) >>> + return -EINVAL; >>> + >>> index = find_cable_index_by_id(edev, id); >>> if (index < 0) >>> return index; >>> @@ -404,7 +424,7 @@ EXPORT_SYMBOL_GPL(extcon_set_cable_state_); >>> int extcon_set_cable_state(struct extcon_dev *edev, >>> const char *cable_name, bool cable_state) >>> { >>> - return extcon_set_cable_state_(edev, find_cable_index_by_name >>> + return extcon_set_cable_state_(edev, find_cable_id_by_name >>> (edev, cable_name), cable_state); >>> } >>> EXPORT_SYMBOL_GPL(extcon_set_cable_state); >>> -- >>> 2.1.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >