From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932319AbbJMDDT (ORCPT ); Mon, 12 Oct 2015 23:03:19 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:60090 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932181AbbJMDDR convert rfc822-to-8bit (ORCPT ); Mon, 12 Oct 2015 23:03:17 -0400 X-AuditID: cbfee68f-f796f6d0000014a4-1d-561c746a02e2 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 8BIT Message-id: <561C746A.1040004@samsung.com> Date: Tue, 13 Oct 2015 12:03:06 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Mark Rutland Cc: linux-kernel@vger.kernel.org, robh+dt@kernel.org, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, myungjoo.ham@samsung.com, devicetree@vger.kernel.org Subject: Re: [PATCH v2] extcon: gpio: Add the support for Device tree bindings References: <1444028299-11755-1-git-send-email-cw00.choi@samsung.com> <20151005111359.GF19064@leverpostej> In-reply-to: <20151005111359.GF19064@leverpostej> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrEIsWRmVeSWpSXmKPExsWyRsSkQDe7RCbMYPUjJov5R86xWvS/Wchq ce7VSkaLy7vmsFksvX6RyeJ24wo2iwnT17JYtO49wu7A4bFm3hpGj8t9vUweK5d/YfPYtKqT zaNvyypGj8+b5ALYorhsUlJzMstSi/TtErgypvbGF5w1rbi8/ylTA+MK7S5GTg4JAROJI3Mm MUPYYhIX7q1n62Lk4hASWMEo8fP3R3aYoitPXjJDJGYxSrx4fIsVJMErICjxY/I9li5GDg5m AXWJKVNyQcLMAiISS5umMkLY2hLLFr6G6n3AKDF97UN2iF4tiU832sHmsAioSmzp/A5mswHF 97+4wQYyU1QgQqL7RCVIWARofM+uLywgc5gFtjNKPDz4mQ0kISzgJzHz0WuwmUICORILTi4D i3MKGEpMnvefDeKBe+wSJ7cUQOwSkPg2+RDYzRICshKbDkA9LylxcMUNlgmM4rOQfDYL4bNZ SD6bheSzBYwsqxhFUwuSC4qT0ouM9YoTc4tL89L1kvNzNzECo/b0v2f9OxjvHrA+xCjAwajE w/siUiZMiDWxrLgy9xCjKdBBE5mlRJPzgakhryTe0NjMyMLUxNTYyNzSTEmcd6HUz2AhgfTE ktTs1NSC1KL4otKc1OJDjEwcnFINjIfK2NwPSJlELzOfulV5f49Ka8qvFYf2r2RhXHExLz6s Ytq1Y7Knbe4uzmL3e37xw4JaptPH/JmO3RdUcTnX+Pyv3I33m1okuDsmreUoE9vfdXoO91t3 5oKwOwvyPh80P39Vd9rTDWIrjdhyjtoq7ynvKWb+8PrRnjsar/U+xIinPX+wcZpwabYSS3FG oqEWc1FxIgAbH2P21QIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrOIsWRmVeSWpSXmKPExsVy+t9jQd2sEpkwg+MnLS3mHznHatH/ZiGr xblXKxktLu+aw2ax9PpFJovbjSvYLCZMX8ti0br3CLsDh8eaeWsYPS739TJ5rFz+hc1j06pO No++LasYPT5vkgtgi2pgtMlITUxJLVJIzUvOT8nMS7dV8g6Od443NTMw1DW0tDBXUshLzE21 VXLxCdB1y8wBukhJoSwxpxQoFJBYXKykb4dpQmiIm64FTGOErm9IEFyPkQEaSFjDmHH6xnbW gkbTihv7jzA1MN7R6mLk5JAQMJG48uQlM4QtJnHh3nq2LkYuDiGBWYwSLx7fYgVJ8AoISvyY fI+li5GDg1lAXuLIpWwIU11iypRciPIHjBLT1z5khyjXkvh0ox2slUVAVWJL53cwmw0ovv/F DTaQXlGBCInuE5UgYRGgMT27vrCAzGEW2M4o8fDgZzaQhLCAn8TMR6/BZgoJ5EgsOLkMLM4p YCgxed5/tgmMQEciXDcL4bpZCNctYGRexSiRWpBcUJyUnmuUl1quV5yYW1yal66XnJ+7iREc 6c+kdzAe3uV+iFGAg1GJh/dFpEyYEGtiWXFl7iFGCQ5mJRHec1lAId6UxMqq1KL8+KLSnNTi Q4ymQO9NZJYSTc4HJqG8knhDYxMzI0sjc0MLI2NzJXHeG4cYwoQE0hNLUrNTUwtSi2D6mDg4 pRoY+/Tm5Ox6VdAWnWz2+Hb7KkH/FpEtygcN569/+eFa4OWvS2wfPjj5aUJV+z4u6+++dQpu Owp7dstuWlugfPrERv0dNoUTT717bfrc79rbnWe95+9af1ZN+dNZPZ672qsnlCwSWKL76JjE 7/7m/4+NS2/+PcPLb+NYyHZWuuHLtSi2NZ1BNlU9rkosxRmJhlrMRcWJAM5sQYQKAwAA 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 Mark, On 2015년 10월 05일 20:13, Mark Rutland wrote: > On Mon, Oct 05, 2015 at 03:58:19PM +0900, Chanwoo Choi wrote: >> This patch adds the support for Device tree bindings of extcon-gpio driver. >> The extcon-gpio device tree node must include the both 'extcon-id' and >> 'extcon-gpio' property. >> >> For exmaple: >> usb_cable: extcon-gpio-0 { >> compatible = "extcon-gpio"; >> extcon-id = ; >> extcon-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >> } >> >> ta_cable: extcon-gpio-1 { >> compatible = "extcon-gpio"; >> extcon-id = ; >> extcon-gpio = <&gpio3 2 GPIO_ACTIVE_LOW>; >> debounce-ms = <50>; /* 50 millisecond */ >> wakeup-source; >> } >> >> &dwc3_usb { >> extcon = <&usb_cable>; >> }; >> >> &charger { >> extcon = <&ta_cable>; >> }; >> >> Signed-off-by: Chanwoo Choi >> --- >> Changes from v1: >> - Create the include/dt-bindings/extcon/extcon.h including the identification >> of external connector. These definitions are used in dts file. >> - Fix error if CONFIG_OF is disabled. >> >> .../devicetree/bindings/extcon/extcon-gpio.txt | 38 +++++++ >> drivers/extcon/extcon-gpio.c | 110 ++++++++++++++++----- >> include/dt-bindings/extcon/extcon.h | 44 +++++++++ >> include/linux/extcon/extcon-gpio.h | 6 +- >> 4 files changed, 173 insertions(+), 25 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt >> create mode 100644 include/dt-bindings/extcon/extcon.h >> >> diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt >> new file mode 100644 >> index 000000000000..70c36f729963 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt >> @@ -0,0 +1,38 @@ >> +GPIO Extcon device >> + >> +This is a virtual device used to generate specific external connector >> +from the GPIO pin connected to a GPIO pin. >> + >> +Required properties: >> +- compatible: Must be "extcon-gpio". >> +- extcon-id: unique id for specific external connector. >> + See include/dt-bindings/extcon/extcon.h. > > This property is either misnamed and badly described, or it is > pointless (the node is sufficient to form a unique ID which can be > referenced by phandle). > > What is this used for exactly? OK, I'll modify the description of 'extcon-id' property as following: - extcon-id: The extcon support the various type of external connector to check whether connector is attached or detached. The each external connector has the unique number to identify it. So this property includes the unique number which indicates the specific external connector. When external connector is attached or detached, GPIO pin detect the changed state. See include/ dt-bindings/extcon/extcon.h which defines the unique number for supported external connector from extcon framework. > >> +- extcon-gpio: GPIO pin to detect the external connector. See gpio binding. >> + >> +Optional properties: >> +- debounce-ms: the debounce dealy for GPIO pin in millisecond. >> +- wakeup-source: Boolean, extcon can wake-up the system. >> + >> +Example: Examples of extcon-gpio node as listed below: >> + >> + usb_cable: extcon-gpio-0 { >> + compatible = "extcon-gpio"; >> + extcon-id = ; >> + extcon-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; >> + } >> + >> + ta_cable: extcon-gpio-1 { >> + compatible = "extcon-gpio"; >> + extcon-id = ; >> + extcon-gpio = <&gpio3 2 GPIO_ACTIVE_LOW>; >> + debounce-ms = <50>; /* 50 millisecond */ >> + wakeup-source; >> + } >> + >> + &dwc3_usb { >> + extcon = <&usb_cable>; >> + }; >> + >> + &charger { >> + extcon = <&ta_cable>; >> + }; >> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c >> index 279ff8f6637d..7f3e24aae0c4 100644 >> --- a/drivers/extcon/extcon-gpio.c >> +++ b/drivers/extcon/extcon-gpio.c >> @@ -1,8 +1,8 @@ >> /* >> * extcon_gpio.c - Single-state GPIO extcon driver based on extcon class >> * >> - * Copyright (C) 2008 Google, Inc. >> - * Author: Mike Lockwood >> + * Copyright (C) 2015 Chanwoo Choi , Samsung Electronics >> + * Copyright (C) 2008 Mike Lockwood , Google, Inc. >> * >> * Modified by MyungJoo Ham to support extcon >> * (originally switch class is supported) >> @@ -26,12 +26,14 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> struct gpio_extcon_data { >> struct extcon_dev *edev; >> int irq; >> + bool irq_wakeup; >> struct delayed_work work; >> unsigned long debounce_jiffies; >> >> @@ -61,19 +63,50 @@ 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) >> +static int gpio_extcon_parse_of(struct device *dev, >> + struct gpio_extcon_data *data) >> { >> - struct gpio_extcon_pdata *pdata = data->pdata; >> + struct gpio_extcon_pdata *pdata; >> int ret; >> >> - ret = devm_gpio_request_one(dev, pdata->gpio, GPIOF_DIR_IN, >> - dev_name(dev)); >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) >> + return -ENOMEM; >> + >> + ret = device_property_read_u32(dev, "extcon-id", &pdata->extcon_id); >> + if (ret < 0) >> + return -EINVAL; > > No sanity checking of the value? When checking the validation of value, I should check both the unique number and name. But, the name of external connector is only accessed in the extcon core driver(drivers/extcon/extcon.c) So, I'll implement the sanity checking on extcon core when calling the extcon_dev_allocate(*supported_cable). > > Why device_property rather than of_property? There is no special reason. When using the device_property_*(), It is well-working without any problem. >> + >> + data->id_gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN); >> if (ret < 0) >> return ret; >> >> - data->id_gpiod = gpio_to_desc(pdata->gpio); >> - if (!data->id_gpiod) >> - return -EINVAL; >> + data->irq_wakeup = device_property_read_bool(dev, "wakeup-source"); >> + >> + device_property_read_u32(dev, "debounce-ms", &pdata->debounce); > > Likewise, surely there's an upper bound above? The unit of debounce-ms is millisecond. It is not determined limit value for just time. Thanks, Chanwoo Choi