From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chanwoo Choi Subject: Re: [PATCH v3] extcon: gpio: Add the support for Device tree bindings Date: Mon, 26 Oct 2015 16:55:21 +0900 Message-ID: <562DDC69.7000604@samsung.com> References: <1445402226-3912-1-git-send-email-cw00.choi@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-reply-to: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Rob Herring , Sudeep Holla Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: devicetree@vger.kernel.org Hi Rob, =46irst of all, thanks for your review. On 2015=EB=85=84 10=EC=9B=94 22=EC=9D=BC 07:35, Rob Herring wrote: > On Tue, Oct 20, 2015 at 11:37 PM, 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' a= nd >> 'extcon-gpio' property. >=20 > I think this is too tied to the Linux driver. Instead, think about > what the connector contains. I think you should define a usb connecto= r > node and compatible (e.g. usb-connector or usb-ab-connector). This The Extcon framework support the various external connector. You mean that extcon-gpio.c should have the separate compatible for each external connector? =46or example, static const struct of_device_id gpio_extcon_of_match[] =3D { { .compatible =3D "extcon-usb", /* USB connector */ .data =3D EXTCON_USB_DATA, }, { .compatible =3D "extcon-chg-sdp", /* SDP charger connector */ .data =3D EXTCON_CHG_SDP_DATA, }, { .compatible =3D "extcon-chg-dcp", /* DCP charger connector */ .data =3D EXTCON_CHG_DCP_DATA, }, { .compatible =3D "extcon-jack-microphone", /* Microphone jack connecto= r */ .data =3D EXTCON_JACK_MICROPHONE_DATA, }, { .compatible =3D "extcon-disp-hdmi", /* HDMI connector*/ .data =3D EXTCON_DISP_HDMI_DATA, }, ...... }; static struct platform_driver extcon_gpio_driver =3D { .probe =3D gpio_extcon_probe, .remove =3D gpio_extcon_remove, .driver =3D { .name =3D "extcon-gpio", .of_match_table =3D gpio_extcon_of_match, }, }; > probably needs to distinguish the connector type as well especially > with TypeC coming. You're right. Currently Extcon framework don't support the setting of c= onnector type (e.g., USB Type A, Type B, Mini-A, Type-C ...) I'm planing to add the extcon core API to support the connector type. >=20 >> >> For exmaple: >> usb_cable: extcon-gpio-0 { >> compatible =3D "extcon-gpio"; >> extcon-id =3D ; >> extcon-gpio =3D <&gpio6 1 GPIO_ACTIVE_HIGH>; >=20 > This tied to a Vbus detect circuit? >=20 > So "vbus-detect-gpios" in the connector node. If extcon-gpio.c driver support the multiple comaptible for each extern= al connector, this driver will support the separate property name for gpio to get it = from devicetree. >=20 > For host side (or OTG host mode), you may also need a vbus-supply > regulator property. OTG will also need an id-gpios for ID pin. There is already drivers/extcon/extcon-usb-gpio.c driver for USB with G= PIO. [1] https://lkml.org/lkml/2015/2/2/187 >=20 >> } >> >> ta_cable: extcon-gpio-1 { >=20 > This is all the same connector as above? >=20 >> compatible =3D "extcon-gpio"; >> extcon-id =3D ; >> extcon-gpio =3D <&gpio3 2 GPIO_ACTIVE_LOW>; >=20 > This is just detecting D+ and D- are both pulled high? The extcon-gpio.c might be used to detect the external connector by usi= ng only GPIO which has not correlation to both D+ and D-. This GPIO pin is just use= d to detect the state whether external connector is attached or detached. >=20 > So "dcp-detect-gpios" in the connector node. ditto. >=20 >> debounce-ms =3D <50>; /* 50 millisecond */ >> wakeup-source; >=20 > wakeup-source implies an interrupt as I read Sudeep's series. Either > gpios need to be allowed or these need to be defined as interrupts. You are right. I'm going to consider it and modify this driver with mor= e proper method. >=20 >> } >> >> &dwc3_usb { >> extcon =3D <&usb_cable>; >> }; >> >> &charger { >> extcon =3D <&ta_cable>; >=20 > Not sure what to do with this. Both can point to a single connector > node I think. There are both extcon provider driver and client driver. The extcon provider driver provide client driver to get whether cable s= tate is attached or detached. And then client driver is waiting the notification from ex= tcon provider driver. Before waiting the notification from extcon providier driver, the clien= t driver should get the instance of specific extcon provider driver to register the notifie= r chain. When getting the instance of specific extcon provider in device tree, the client driver use the extcon_get_edev_by_phandle() function which parses the phandle name of 'extcon'. The latest Linux kernel includes the example[1][2] in arch/arm/boot/dts= /omap5-cm-t54.dts [1] http://www.spinics.net/lists/linux-omap/msg105074.html - ARM: dts: sbc-t54: add support for sbc-t54 with cm-t54 [2] ARM: dts: cm-t54: setup omap_dwc3 - http://www.spinics.net/lists/linux-omap/msg111174.html =46or example=20 &i2c1 { ...... palmas: palmas@48 { extcon_usb3: palmas_usb { compatible =3D "ti,plamas-usb-vid"; ti,enable-vbus-detection; ti,enable-id-detection; ti,wakeup; }; ...... }; ...... }; &usb3 { extcon =3D <&extcon_usb3>; vbus-supply =3D <...>; }; Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" i= n the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753339AbbJZHz0 (ORCPT ); Mon, 26 Oct 2015 03:55:26 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:53578 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752964AbbJZHzX convert rfc822-to-8bit (ORCPT ); Mon, 26 Oct 2015 03:55:23 -0400 X-AuditID: cbfee68d-f79ae6d00000149a-e8-562ddc69f578 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 8BIT Message-id: <562DDC69.7000604@samsung.com> Date: Mon, 26 Oct 2015 16:55:21 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Rob Herring , Sudeep Holla Cc: "linux-kernel@vger.kernel.org" , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , myungjoo.ham@samsung.com, "devicetree@vger.kernel.org" Subject: Re: [PATCH v3] extcon: gpio: Add the support for Device tree bindings References: <1445402226-3912-1-git-send-email-cw00.choi@samsung.com> In-reply-to: X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrOIsWRmVeSWpSXmKPExsWyRsSkSDfzjm6Ywc25Qhbzj5xjteh/s5DV 4tyrlYwWl3fNYbNYev0ik8XtxhVsFhOmr2WxaN17hN1i+akdLA6cHmvmrWH0uNzXy+SxcvkX No9NqzrZPPq2rGL0+LxJLoAtissmJTUnsyy1SN8ugSvjw57pLAW7NCpaJuQ1ME6T72Lk5JAQ MJGYs/UAI4QtJnHh3no2EFtIYAWjxO92Z5iaI7+PA8W5gOJLGSV+Tj0K1sArICjxY/I9li5G Dg5mAXWJKVNyQcLMAiISv1c8ZYawtSWWLXzNDNH7gFHizL9PLBC9WhL/r7WxgtgsAqoSa07/ AFvMBhTf/+IGG8hMUYEIie4TlSBhEQFPibbbn9lB5jALbGWSWNh9CWyOsICfxIPWl1ALuhkl Ote/BzuOUyBY4mLnLRaID16yS5x9HQ+xTEDi2+RDYEdLCMhKbDrADFEiKXFwxQ2WCYzis5C8 NgvhtVlIXpuF5LUFjCyrGEVTC5ILipPSiwz1ihNzi0vz0vWS83M3MQLj9/S/Z707GG8fsD7E KMDBqMTD+4JHN0yINbGsuDL3EKMp0EETmaVEk/OBSSKvJN7Q2MzIwtTE1NjI3NJMSZxXUepn sJBAemJJanZqakFqUXxRaU5q8SFGJg5OqQbGRQ+vvpst9OvG/ilpt5yEOcPLTdscbXadXK48 4eUjoePzTn1+cm7fA8ML63sP3poYmPFq6xuDgvb59crnvp1RYEywNVv9umfR3kXbXXcJ3l9l 8qdQzI+ZU2g9y65H04VbvHOfcdYUpv+5Z70jOydott3GRxEND1U4wo+I5Tq27c740fJrgnbB fSWW4oxEQy3mouJEAMR9bffaAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrLIsWRmVeSWpSXmKPExsVy+t9jAd3MO7phBhf+clvMP3KO1aL/zUJW i3OvVjJaXN41h81i6fWLTBa3G1ewWUyYvpbFonXvEXaL5ad2sDhweqyZt4bR43JfL5PHyuVf 2Dw2repk8+jbsorR4/MmuQC2qAZGm4zUxJTUIoXUvOT8lMy8dFsl7+B453hTMwNDXUNLC3Ml hbzE3FRbJRefAF23zBygs5QUyhJzSoFCAYnFxUr6dpgmhIa46VrANEbo+oYEwfUYGaCBhDWM GV2Pn7EVPFOvuDv9K1sD4yG5LkZODgkBE4kjv4+zQdhiEhfurQeyuTiEBJYySvycepQRJMEr ICjxY/I9li5GDg5mAXmJI5eyIUx1iSlTciHKHzBKnPn3iQWiXEvi/7U2VhCbRUBVYs3pH2Dz 2YDi+1/cYAPpFRWIkOg+UQkSFhHwlGi7/ZkdZA6zwFYmiYXdl8DmCAv4STxofckMsaCbUaJz /XuwezgFgiUudt5imcAoMAvJebMQzpuFcN4CRuZVjBKpBckFxUnpuYZ5qeV6xYm5xaV56XrJ +bmbGMFR/0xqB+PBXe6HGAU4GJV4eF/w6IYJsSaWFVfmHmKU4GBWEuHtnQEU4k1JrKxKLcqP LyrNSS0+xGgK9N9EZinR5HxgQsoriTc0NjEzsjQyN7QwMjZXEue9kKERJiSQnliSmp2aWpBa BNPHxMEp1cDYwJ9sO/f/6bNTdq0rO33yiJGC89sDjU2hGUd4DordPtBXYH6Q++8BiYjlbwzm sPTqc/s5xVV8nPFJJ3TvjC8i4sUJ7m5718s9vrl07+KE6cJJW8Vqt/I4b6w12JWS5Hr0ktOu qTHLefeI2lcJ7BZ9sFtey/Om9nbtGV9DfohGfmW+4Cn0/MhTJZbijERDLeai4kQApDS+ZBAD AAA= 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 Rob, First of all, thanks for your review. On 2015년 10월 22일 07:35, Rob Herring wrote: > On Tue, Oct 20, 2015 at 11:37 PM, 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. > > I think this is too tied to the Linux driver. Instead, think about > what the connector contains. I think you should define a usb connector > node and compatible (e.g. usb-connector or usb-ab-connector). This The Extcon framework support the various external connector. You mean that extcon-gpio.c should have the separate compatible for each external connector? For example, static const struct of_device_id gpio_extcon_of_match[] = { { .compatible = "extcon-usb", /* USB connector */ .data = EXTCON_USB_DATA, }, { .compatible = "extcon-chg-sdp", /* SDP charger connector */ .data = EXTCON_CHG_SDP_DATA, }, { .compatible = "extcon-chg-dcp", /* DCP charger connector */ .data = EXTCON_CHG_DCP_DATA, }, { .compatible = "extcon-jack-microphone", /* Microphone jack connector */ .data = EXTCON_JACK_MICROPHONE_DATA, }, { .compatible = "extcon-disp-hdmi", /* HDMI connector*/ .data = EXTCON_DISP_HDMI_DATA, }, ...... }; static struct platform_driver extcon_gpio_driver = { .probe = gpio_extcon_probe, .remove = gpio_extcon_remove, .driver = { .name = "extcon-gpio", .of_match_table = gpio_extcon_of_match, }, }; > probably needs to distinguish the connector type as well especially > with TypeC coming. You're right. Currently Extcon framework don't support the setting of connector type (e.g., USB Type A, Type B, Mini-A, Type-C ...) I'm planing to add the extcon core API to support the connector type. > >> >> For exmaple: >> usb_cable: extcon-gpio-0 { >> compatible = "extcon-gpio"; >> extcon-id = ; >> extcon-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; > > This tied to a Vbus detect circuit? > > So "vbus-detect-gpios" in the connector node. If extcon-gpio.c driver support the multiple comaptible for each external connector, this driver will support the separate property name for gpio to get it from devicetree. > > For host side (or OTG host mode), you may also need a vbus-supply > regulator property. OTG will also need an id-gpios for ID pin. There is already drivers/extcon/extcon-usb-gpio.c driver for USB with GPIO. [1] https://lkml.org/lkml/2015/2/2/187 > >> } >> >> ta_cable: extcon-gpio-1 { > > This is all the same connector as above? > >> compatible = "extcon-gpio"; >> extcon-id = ; >> extcon-gpio = <&gpio3 2 GPIO_ACTIVE_LOW>; > > This is just detecting D+ and D- are both pulled high? The extcon-gpio.c might be used to detect the external connector by using only GPIO which has not correlation to both D+ and D-. This GPIO pin is just used to detect the state whether external connector is attached or detached. > > So "dcp-detect-gpios" in the connector node. ditto. > >> debounce-ms = <50>; /* 50 millisecond */ >> wakeup-source; > > wakeup-source implies an interrupt as I read Sudeep's series. Either > gpios need to be allowed or these need to be defined as interrupts. You are right. I'm going to consider it and modify this driver with more proper method. > >> } >> >> &dwc3_usb { >> extcon = <&usb_cable>; >> }; >> >> &charger { >> extcon = <&ta_cable>; > > Not sure what to do with this. Both can point to a single connector > node I think. There are both extcon provider driver and client driver. The extcon provider driver provide client driver to get whether cable state is attached or detached. And then client driver is waiting the notification from extcon provider driver. Before waiting the notification from extcon providier driver, the client driver should get the instance of specific extcon provider driver to register the notifier chain. When getting the instance of specific extcon provider in device tree, the client driver use the extcon_get_edev_by_phandle() function which parses the phandle name of 'extcon'. The latest Linux kernel includes the example[1][2] in arch/arm/boot/dts/omap5-cm-t54.dts [1] http://www.spinics.net/lists/linux-omap/msg105074.html - ARM: dts: sbc-t54: add support for sbc-t54 with cm-t54 [2] ARM: dts: cm-t54: setup omap_dwc3 - http://www.spinics.net/lists/linux-omap/msg111174.html For example &i2c1 { ...... palmas: palmas@48 { extcon_usb3: palmas_usb { compatible = "ti,plamas-usb-vid"; ti,enable-vbus-detection; ti,enable-id-detection; ti,wakeup; }; ...... }; ...... }; &usb3 { extcon = <&extcon_usb3>; vbus-supply = <...>; }; Regards, Chanwoo Choi