From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752420AbbEQH61 (ORCPT ); Sun, 17 May 2015 03:58:27 -0400 Received: from mail-ie0-f194.google.com ([209.85.223.194]:33132 "EHLO mail-ie0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751843AbbEQH6T (ORCPT ); Sun, 17 May 2015 03:58:19 -0400 MIME-Version: 1.0 Reply-To: cw00.choi@samsung.com In-Reply-To: References: <1431700278-30465-1-git-send-email-cw00.choi@samsung.com> <1431700278-30465-3-git-send-email-cw00.choi@samsung.com> Date: Sun, 17 May 2015 16:58:19 +0900 Message-ID: Subject: Re: [PATCH 2/2] extcon: Update the prototype of extcon_register_notifier() with enum extcon From: Chanwoo Choi To: Krzysztof Kozlowski Cc: linux-kernel , "myungjoo.ham@samsung.com" , Charles Keepax , gg@slimlogic.co.uk, Kishon Vijay Abraham I , jaewon02.kim@samsung.com, Roger Quadros , "Pallala, Ramakrishna" , george.cherian@ti.com, Felipe Balbi , Aaro Koskinen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, May 17, 2015 at 4:41 PM, Krzysztof Kozlowski wrote: > 2015-05-15 23:31 GMT+09:00 Chanwoo Choi : >> Previously, extcon consumer driver used the extcon_register_interest() >> to register the notifier chain and then to receive the notifier event >> when external connector's state is changed. When registering the notifier chain >> for specific external connector with extcon_register_interest(), it used the >> the string name of external connector directly. There are potential problem >> because of unclear, non-standard and inconsequent cable name. Namely, >> it is not appropriate method to identify each external connector. >> >> So, this patch modify the prototype of extcon_register_notifier() by using >> the 'enum extcon' which are the unique id for each external connector >> instead of unclear string method. >> >> - Previously, the extcon consumer driver used the extcon_register_interest() >> with 'cable_name' to point out the specific external connector. Also. it used >> the un-needed structure (struct extcon_specific_cable_nb). >> : int extcon_register_interest(struct extcon_specific_cable_nb *obj, >> const char *extcon_name, const char *cable_name, >> struct notifier_block *nb) >> >> - Newly, the updated extcon_register_notifier() would definitely support >> the same feature to detech the changed state of external connector without >> any specific structure (struct extcon_specific_cable_nb). >> : int extcon_register_notifier(struct extcon_dev *edev, enum extcon id, >> struct notifier_block *nb) >> >> This patch support the both extcon_register_interest() and new extcon_register_ >> notifier(). But the extcon_{register|unregister}_interest() will be deprecated >> because extcon core would support the notifier event for extcon consumer driver >> with only updated extcon_register_notifier() and 'extcon_specific_cable_nb' >> will be removed if there are no extcon consumer driver with legacy >> extcon_{register|unregister}_interest(). >> >> Signed-off-by: Chanwoo Choi >> Cc: MyungJoo Ham >> Cc: George Cherian >> Cc: Felipe Balbi >> Cc: Aaro Koskinen >> --- >> drivers/extcon/extcon.c | 91 ++++++++++++++++++++++++++----------------------- >> include/linux/extcon.h | 17 +++++---- >> 2 files changed, 56 insertions(+), 52 deletions(-) >> >> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c >> index 4aeb585..14c8c95 100644 >> --- a/drivers/extcon/extcon.c >> +++ b/drivers/extcon/extcon.c >> @@ -111,6 +111,16 @@ static int check_mutually_exclusive(struct extcon_dev *edev, u32 new_state) >> return 0; >> } >> >> +static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached) >> +{ >> + if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) { > > How about switching "state" to unsigned long and using bit operations > everywhere, like test_bit()? As I replied on other patch, I'm having the plan to extend the number of supported external connectors of each extcon device. I'll consider to use the test_bit() on next time for updating extcon core. > > The patch itself looks good and change above is not actually related > to the code here, so: > Reviewed-by: Krzysztof Kozlowski Thanks, Chanwoo Choi