From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966156AbdIZCSH (ORCPT ); Mon, 25 Sep 2017 22:18:07 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:61760 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934681AbdIZCSE (ORCPT ); Mon, 25 Sep 2017 22:18:04 -0400 X-AuditID: b6c32a35-d60af9c000001157-08-59c9b8d81a45 MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="UTF-8" Message-id: <59C9B8D9.6050203@samsung.com> Date: Tue, 26 Sep 2017 11:18:01 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Linus Walleij , MyungJoo Ham Cc: linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, John Stultz , Guenter Roeck Subject: Re: [PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor In-reply-to: <20170924145622.4031-5-linus.walleij@linaro.org> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrPKsWRmVeSWpSXmKPExsWy7bCmge6NHScjDQ58VbU481vXYsqf5UwW m+f/YbS4vGsOm8WThWeYLG43rmBzYPO4c20Pm8fO7w3sHn1bVjF6fN4kF8ASlWqTkZqYklqk kJqXnJ+SmZduq+QdHO8cb2pmYKhraGlhrqSQl5ibaqvk4hOg65aZA7RbSaEsMacUKBSQWFys pG9nU5RfWpKqkJFfXGKrFG1oaKRnaGCuZ2RkpGdiHGtlZApUkpCa8eDoOuaC81oV2y/PYGxg fKHYxcjJISFgIvHo1R6WLkYuDiGBHYwSF+5dYoNwvjNKNK44BuRwgFXt+csJEd/AKLG0/Q0b SDevgKDEj8n3WEBqmAXkJY5cygYJMwtoSmzdvZ4dov4eo0T/u9vsEPVaEo0fPoHZLAKqEg9e vWQFsdmA4vtf3ACbyS+gKHH1x2NGEFtUIEJi5/xvYPUiAuES+44dYAYZyizQwSjxZgdIgoND WMBb4sEbTpAaTgFbicaNi8G+kRDYwybRcPsFM8QDLhKvltdAfCws8er4FnaIsLTEpaO2EOXt jBKb59yD6gWaf39lIytEg7HEqa5GJojP+CTefe1hhWjmlehoE4Io8ZA4vvclI4TtKPHy0WJW iOcPM0qcXP6MaQKj3Cyk8JqFCK9ZSOG1gJF5FaNYakFxbnpqsWGBoV5xYm5xaV66XnJ+7iZG cGLTMt3BOOWczyFGAQ5GJR7eBqaTkUKsiWXFlbmHGCU4mJVEeK9vBwrxpiRWVqUW5ccXleak Fh9iNAUG90RmKdHkfGDSzSuJNzSxNDAxMwKmLktDQyVxXtH11yKEBNITS1KzU1MLUotg+pg4 OKUaGO0exH0+Hc+zJV19a7H3ibPePmK1xQsfPg/0YL10+FHu0QXnVmZ7d3lKC85zCuL6+yJy 8sNHH1/a2wr/D587b9drhck1FwyE7s4QLpz8zHPyjDl2q5Wq7O9tFe7rqF2UvPr3Tx4fwbZT TxbfX/xi64SbZ/ZdnXNl48WbsbvTr3/dtHFSRGz58mW2SizFGYmGWsxFxYkA2Onm54IDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrALMWRmVeSWpSXmKPExsVy+t9jQd0bO05GGrzeLGFx5reuxZQ/y5ks Ns//w2hxedccNosnC88wWdxuXMHmwOZx59oeNo+d3xvYPfq2rGL0+LxJLoAlissmJTUnsyy1 SN8ugSvjwdF1zAXntSq2X57B2MD4QrGLkYNDQsBEYs9fzi5GLg4hgXWMEvuXt7J0MXJy8AoI SvyYfI8FpIZZQF7iyKVskDCzgLrEpHmLmEFsIYEHjBJ/9rtAlGtJNH74xA5iswioSjx49ZIV xGYDiu9/cYMNxOYXUJS4+uMxI8hIUYEIie4TlSBhEYFwidkL3jNDjO9glJi8WROkRFjAW+LB G6jLDjNKnFjxGqyGU8BWonHjYpYJjAKzkBw6C+HQWUgOXcDIvIpRMrWgODc9t9iowDAvtVyv ODG3uDQvXS85P3cTIzCctx3W6tvBeH9J/CFGAQ5GJR7eBqaTkUKsiWXFlbmHGCU4mJVEeK9v BwrxpiRWVqUW5ccXleakFh9ilOZgURLnvZ13LFJIID2xJDU7NbUgtQgmy8TBKdXAGGR8utD7 hpyB7CVuwcf7M5e/nJTQ6XTTYq/+ZMV19x6fyXqi8eSR7ZYlMTVswWU73Zj9Fz8O6fzIL60v xCPHzmF75cfzR30Xt32sWBJwaJ5wc0GESfLGllTV8t1iEqxNn8vZFjK2CynkBjV+M/X78Ovd YxEnk+Rdty5JLW1t8/VdEyOTZfFGiaU4I9FQi7moOBEAU6XjwWMCAAA= X-CMS-MailID: 20170926021800epcas1p48fcab9a034e7eea797476b2e1fdd4b6e X-Msg-Generator: CA X-Sender-IP: 182.195.42.142 X-Local-Sender: =?UTF-8?B?7LWc7LCs7JqwG1RpemVuIFBsYXRmb3JtIExhYihTL1fshLw=?= =?UTF-8?B?7YSwKRvsgrzshLHsoITsnpAbU2VuaW9yIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?Q2hhbndvbyBDaG9pG1RpemVuIFBsYXRmb3JtIExhYi4bU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgRW5naW5lZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG1RFTEUbQzEwVjgxMTE=?= CMS-TYPE: 101P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20170924150004epcas4p3217d627df09c3292114f93ec0a66e3b4 X-RootMTR: 20170924150004epcas4p3217d627df09c3292114f93ec0a66e3b4 References: <20170924145622.4031-1-linus.walleij@linaro.org> <20170924145622.4031-5-linus.walleij@linaro.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Linus, Looks good to me. But, there is one comment of gpiod_to_irq()'s return value. If you modify it, feel free to add my tag: Acked-by: Chanwoo Choi On 2017년 09월 24일 23:56, Linus Walleij wrote: > Since we are not getting the GPIO from any platform data and global > GPIO numberspace, we simply get the named "extcon" GPIO directly from > the device. Cut away "active low" since GPIO descriptors already know > if the line is active high or low. Simplify a bit with a > struct device *dev helper variable in probe() and cut the complex > init() function. > > Signed-off-by: Linus Walleij > --- > drivers/extcon/extcon-gpio.c | 66 ++++++++++---------------------------------- > 1 file changed, 15 insertions(+), 51 deletions(-) > > diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c > index 9c4094edd123..86f3ec6d6014 100644 > --- a/drivers/extcon/extcon-gpio.c > +++ b/drivers/extcon/extcon-gpio.c > @@ -18,7 +18,6 @@ > */ > > #include > -#include > #include > #include > #include > @@ -35,12 +34,8 @@ > * @work: Work fired by the interrupt. > * @debounce_jiffies: Number of jiffies to wait for the GPIO to stabilize, from the debounce > * value. > - * @id_gpiod: GPIO descriptor for this external connector. > + * @gpiod: GPIO descriptor for this external connector. > * @extcon_id: The unique id of specific external connector. > - * @gpio: Corresponding GPIO. > - * @gpio_active_low: Boolean describing whether gpio active state is 1 or 0 > - * If true, low state of gpio means active. > - * If false, high state of gpio means active. > * @debounce: Debounce time for GPIO IRQ in ms. > * @irq_flags: IRQ Flags (e.g., IRQF_TRIGGER_LOW). > * @check_on_resume: Boolean describing whether to check the state of gpio > @@ -51,10 +46,8 @@ struct gpio_extcon_data { > int irq; > struct delayed_work work; > unsigned long debounce_jiffies; > - struct gpio_desc *id_gpiod; > + struct gpio_desc *gpiod; > unsigned int extcon_id; > - unsigned gpio; > - bool gpio_active_low; > unsigned long debounce; > unsigned long irq_flags; > bool check_on_resume; > @@ -67,10 +60,7 @@ static void gpio_extcon_work(struct work_struct *work) > container_of(to_delayed_work(work), struct gpio_extcon_data, > work); > > - state = gpiod_get_value_cansleep(data->id_gpiod); > - if (data->gpio_active_low) > - state = !state; > - > + state = gpiod_get_value_cansleep(data->gpiod); > extcon_set_state_sync(data->edev, data->extcon_id, state); > } > > @@ -83,60 +73,34 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id) > return IRQ_HANDLED; > } > > -static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data) > -{ > - int ret; > - > - ret = devm_gpio_request_one(dev, data->gpio, GPIOF_DIR_IN, > - dev_name(dev)); > - if (ret < 0) > - return ret; > - > - data->id_gpiod = gpio_to_desc(data->gpio); > - if (!data->id_gpiod) > - return -EINVAL; > - > - if (data->debounce) { > - ret = gpiod_set_debounce(data->id_gpiod, > - data->debounce * 1000); > - if (ret < 0) > - data->debounce_jiffies = > - msecs_to_jiffies(data->debounce); > - } > - > - data->irq = gpiod_to_irq(data->id_gpiod); > - if (data->irq < 0) > - return data->irq; > - > - return 0; > -} > - > static int gpio_extcon_probe(struct platform_device *pdev) > { > struct gpio_extcon_data *data; > + struct device *dev = &pdev->dev; > int ret; > > - data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data), > - GFP_KERNEL); > + data = devm_kzalloc(dev, sizeof(struct gpio_extcon_data), GFP_KERNEL); > if (!data) > return -ENOMEM; > > if (!data->irq_flags || data->extcon_id > EXTCON_NONE) > return -EINVAL; > > - /* Initialize the gpio */ > - ret = gpio_extcon_init(&pdev->dev, data); > - if (ret < 0) > - return ret; > + data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN); > + if (IS_ERR(data->gpiod)) > + return PTR_ERR(data->gpiod); > + data->irq = gpiod_to_irq(data->gpiod); > + if (data->irq <= 0) "if (data->irq < 0)" is enough. If irq is zero, gpiod_to_irq() returns the -ENXIO. > + return data->irq; > > /* Allocate the memory of extcon devie and register extcon device */ > - data->edev = devm_extcon_dev_allocate(&pdev->dev, &data->extcon_id); > + data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id); > if (IS_ERR(data->edev)) { > - dev_err(&pdev->dev, "failed to allocate extcon device\n"); > + dev_err(dev, "failed to allocate extcon device\n"); > return -ENOMEM; > } > > - ret = devm_extcon_dev_register(&pdev->dev, data->edev); > + ret = devm_extcon_dev_register(dev, data->edev); > if (ret < 0) > return ret; > > @@ -146,7 +110,7 @@ static int gpio_extcon_probe(struct platform_device *pdev) > * Request the interrupt of gpio to detect whether external connector > * is attached or detached. > */ > - ret = devm_request_any_context_irq(&pdev->dev, data->irq, > + ret = devm_request_any_context_irq(dev, data->irq, > gpio_irq_handler, data->irq_flags, > pdev->name, data); > if (ret < 0) > -- Best Regards, Chanwoo Choi Samsung Electronics