From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752153AbdJSKrl (ORCPT ); Thu, 19 Oct 2017 06:47:41 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:41575 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751922AbdJSKri (ORCPT ); Thu, 19 Oct 2017 06:47:38 -0400 X-AuditID: b6c32a48-c0fff70000001005-df-59e882c7adaa Message-id: <59E882C9.9080506@samsung.com> Date: Thu, 19 Oct 2017 19:47:37 +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 MIME-version: 1.0 To: Linus Walleij , Rob Herring Cc: MyungJoo Ham , "linux-kernel@vger.kernel.org" , "linux-gpio@vger.kernel.org" , John Stultz , Guenter Roeck , "devicetree@vger.kernel.org" Subject: Re: [PATCH 1/8] extcon: gpio: Add DT bindings In-reply-to: <59C9B52D.8050909@samsung.com> Content-type: text/plain; charset="utf-8" Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA01Se0hTYRTn293urtbqNnt8GJTdElLbcnPOa2gYRV3QP8QItJde3Yea7sHu FI0IzR5mEZYVQxZbkZrDKOeD+SB1RosYBZlWKEkpapJET7OMtd1r4H+/c77f75zfOd8hMHkF Hk4UGizIbGCLKTxU3DkYpVV4z8xkxS54Ymj74+cS2vdHQV9fbBLRbfZFQA9123B68rZPRI9W 3sNpf69bmkIwLudFnBkb6cWZrvkKKXOl3QmYb65N6ZLDKKkAsTpkjkCGPKOu0JCfTKUezN6b Ha+NVSlUiXQCFWFg9SiZ2peWrthfWBywQkWUssUlgVQ6y3HUzt1JZmOJBUUUGDlLMnVEpVIr VbEJSrVardTEHduljg9QclBBx91Okal5R5n10hRWAdxUDQghIKmBD+te4DUglJCTbgCHGqvx 4IOcnAdw3r65BhA8qfZ3jMDpAfDyU4dUCN4BeNF6RhwUyMho2Ln4FASxmIyE/a8aJUGMB/J9 M2/4oqvJLXD41wTPWUdmwi77T6mgXQN/1b3j66wlU+Ho2AIWbICRLhG8/97Gk8JILax74OQL hZAx8JPzCS/AyCg48/3aEt4M21rmeDEkR3HorWuRCnPug11nfyzhMDjrbV/CG+GUsxUIggsA ttmCNoJBNYDjzZUSgRUHn9VUioQWq2D14F+psBgZrD4vFygM7L55FgjpPbBhOkNY41cRfFF1 ohZsql82aP0y3/XLfDsA5gTrkYnT5yNObdIoOVbPlRjylXlGvQvw1xfNuEH/8zQPIAlArZQ5 bk5nySVsKVeu9wBIYNRamTF9Jksu07HlJ5HZmG0uKUacB8QHPuUqFr4uzxi4ZYMlW6VJjNVo teoELR24sA2y050jmXIyn7WgIoRMyPxfJyJCwivAlVz7wKkM8WTSqa2XxwccDv/LW0e3GaN3 R/Z97CmauLF/p43pGE85ec55qXWFJPe8W8zc+TB8Y/i4psfqlzc9OYB889SjqrLPcy/ts/Dt 0KEJfXdmSsNEaWRoFqYc8deapDlf9DbXa6vdZPc51NSIx6vcrrdYQ2Laddfa/GWUmCtgVdGY mWP/AVQrwViTAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrELMWRmVeSWpSXmKPExsVy+t9jAd3jTS8iDdYuEbGYf+Qcq8WZ37oW U/4sZ7LYPP8Po8XlXXPYLJ4sPMNkcbtxBZvF/z072B04PDat6mTzuHNtD5vHzu8N7B59W1Yx enzeJBfAGsVlk5Kak1mWWqRvl8CVsXXJNqaClToVM7qfMTcw7lDqYuTgkBAwkZjwS7uLkYtD SGAno8TW7fdZIJwHjBLnj71j72Lk5OAV0JLY9ucEI4jNIqAqceDKMlYQmw0ovv/FDTYQm19A UeLqj8eMIENFBSIkuk9UQrQKSvyYfI8FxBYR8Ja4fecnM8h8ZoFNTBJn/veDzREWMJOYvH4V G8TiT0wSWzfPAlvMKaAt8WbVMRaQocwC6hJTpuSChJkF5CU2r3nLPIFRYBaSHbMQqmYhqVrA yLyKUTK1oDg3PbfYqMAoL7Vcrzgxt7g0L10vOT93EyMw5Lcd1urfwfh4SfwhRgEORiUeXo8p zyOFWBPLiitzDzFKcDArifDmB7yIFOJNSaysSi3Kjy8qzUktPsQozcGiJM7Ln38sUkggPbEk NTs1tSC1CCbLxMEp1cAo/OK7zqxlM6JTLPnzQsT2hEsdjzjqO7dtWqhEU+qDg694POISDr26 ayjEFZqxzXNe3LQl+xz2Jjv6+f4LTFbLlw6NE7Wz27hQvDv71LGbsU7K3S7zDks9ylyidIWn 9s+HhQtbnomGikdN13tyVPLOo495in7TzA/w+Pg3K7yUY1/0a0uL7yklluKMREMt5qLiRABw jC5ndQIAAA== X-CMS-MailID: 20171019104735epcas2p47b916a46d6984b61e15ffc6805726885 X-Msg-Generator: CA X-Sender-IP: 182.195.42.143 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: 102P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20170926003941epcas4p2a1987500f6e213b6a2ffe973ab1db922 X-RootMTR: 20170926003941epcas4p2a1987500f6e213b6a2ffe973ab1db922 References: <20170924145622.4031-1-linus.walleij@linaro.org> <20170924145622.4031-2-linus.walleij@linaro.org> <59C9B52D.8050909@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rob, [snip] >> >>>> +External Connector Using GPIO >>> >>> What kind of connector? All connectors external to something... And >>> GPIO is not a kind of connector on its own, but an implementation. >> >> Yeah that is a problem with the whole subsystem I guess. Should >> I add a paragraph describing the usecases? >> >> The whole thing is a bit >> of Androidism and is named like this because Android named it like >> this in their kernel tree. >> >>>> +Required properties: >>>> +- compatible: should be "extcon-gpio" >>>> +- extcon-gpios: the GPIO lines used for the external connectors >>> >>> This doesn't tell me what the GPIOs functions are and should. For >>> example we have hpd-gpios for HDMI hotplug detect in HDMI connector >>> binding. >> >> The idea is that this array corresponds to the extcon-connector-types >> right below, I'll clarify that if you think the overall idea is OK. >> >>>> + See gpio/gpio.txt >>>> +- extcon-connector-types: set to an unsigned integer value arrat representing the types >>>> + of this connector, matched to the corresponding GPIO lines in the previous array. >>> >>> This should be determined by the compatible string. >> >> So a GPIO connector is very versatile. It is "general purpose" by definition, >> so it needs to be subclassed. >> >> One possibility is to allow just one GPIO and have one comptible-string per >> use case. >> compatible = "gpio-usb-connector"; >> compatible = "gpio-charger-connector"; >> compatible = "gpio-headphone-connector"; >> etc etc >> >> These bindings on the other hand, assume that the driver will be able >> to handle an array of gpios with an associated array of connector types, >> which reflects how many of the existing extcon-type driver hardware works: >> a single PMIC providing some 5 misc extcons for example. >> >> For this reason there is a generic compatible string and then the device >> is subclassed per-gpio with the per-gpio connector type. The "drivers/input/keyboard/gpio_keys.c" driver used the 'linux,code' property to get the key type as following in the device-tree file: gpio-keys { compatible = "gpio-keys"; key-1 { gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; linux,code = ; label = "SW2-1"; wakeup-source; }; key-2 { gpios = <&gpio5 1 GPIO_ACTIVE_LOW>; linux,code = ; label = "SW2-2"; wakeup-source; }; }; IMO, extcon-gpio.c uses the 'gpio-connectors' compatible instead of 'extcon-gpio' and then define the connector type in the include/dt-bindings/extcon/connectors.h. How about this? For example, In the include/dt-bindings/extcon/connectors.h, #define CONNECTOR_USB 1 /* EXTCON_USB */ #define CONNECTOR_USB_HOST 2 /* EXTCON_USB_HOST */ #define CONNECTOR_CHG_USB_SDP 5 /* EXTCON_CHG_USB_SDP */ #define CONNECTOR_CHG_USB_DCP 6 /* EXTCON_CHG_USB_DCP */ #define CONNECTOR_CHG_USB_CDP 7 /* EXTCON_CHG_USB_CDP */ #define CONNECTOR_CHG_USB_ACA 8 /* EXTCON_CHG_USB_ACA */ ... #define CONNECTOR_HDMI 40 /* EXTCON_DISP_HDMI */ ... In the devicetree example for 'gpio-connectors', usb-connector { compatible = "gpio-connectors"; gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; linux,connector = ; wakeup-source; }; hdmi-connector { gpios = <&gpio5 1 GPIO_ACTIVE_LOW>; linux,connector = ; wakeup-source; }; >> >>>> +/* USB external connector */ >>>> +#define EXTCON_USB 1 >>>> +#define EXTCON_USB_HOST 2 >>>> +#define EXTCON_CHG_USB_SDP 5 /* Standard Downstream Port */ >>>> +#define EXTCON_CHG_USB_DCP 6 /* Dedicated Charging Port */ >>>> +#define EXTCON_CHG_USB_CDP 7 /* Charging Downstream Port */ >>>> +#define EXTCON_CHG_USB_ACA 8 /* Accessory Charger Adapter */ >>>> +#define EXTCON_CHG_USB_FAST 9 >>>> +#define EXTCON_CHG_USB_SLOW 10 >>>> +#define EXTCON_CHG_WPT 11 /* Wireless Power Transfer */ >>>> +#define EXTCON_CHG_USB_PD 12 /* USB Power Delivery */ >>> >>> These don't all look to be mutually exclusive. >>> >>> For USB PD, isn't that discoverable? > > Currently, EXTCON_CHG_USB_PD is not used on any extcon provider drivers. > I think that EXTCON_CHG_USB_PD is discoverable according to the H/W design > such as using ADC. > > Also, The charger connectors of extcon are related to power_supply subsystem > because power_supply is in charge of behavior when the connector is attached/detached. > So, the extcon defines the EXTCON_CHG_USB_PD in order to detect this type. > >> >> Someone else would have to answer, this is based on the current >> connector types supported by the Linux kernel. >> >>>> +/* Display external connector */ >>>> +#define EXTCON_DISP_HDMI 40 /* High-Definition Multimedia Interface */ >>>> +#define EXTCON_DISP_MHL 41 /* Mobile High-Definition Link */ >>>> +#define EXTCON_DISP_DVI 42 /* Digital Visual Interface */ >>>> +#define EXTCON_DISP_VGA 43 /* Video Graphics Array */ >>>> +#define EXTCON_DISP_DP 44 /* Display Port */ >>>> +#define EXTCON_DISP_HMD 45 /* Head-Mounted Display */ >>> >>> We already have connector bindings for most of these. Use those as a >>> model for whatever you want to do. >> >> I guess they are not in Documentation/devicetree/bindings/extcon/* >> >> Please point me in the right direction and I'll check it out. >> >> Yours, >> Linus Walleij >> >> >> > -- Best Regards, Chanwoo Choi Samsung Electronics