Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754762AbdC3DAj (ORCPT ); Wed, 29 Mar 2017 23:00:39 -0400 Received: from mail-oi0-f54.google.com ([209.85.218.54]:34928 "EHLO mail-oi0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754280AbdC3DAh (ORCPT ); Wed, 29 Mar 2017 23:00:37 -0400 MIME-Version: 1.0 In-Reply-To: <8760it6me7.fsf@notabene.neil.brown.name> References: <0b16763df04e197fb5b3c21b19799280d3bce275.1490248054.git.baolin.wang@linaro.org> <8760it6me7.fsf@notabene.neil.brown.name> From: Baolin Wang Date: Thu, 30 Mar 2017 11:00:35 +0800 Message-ID: Subject: Re: [PATCH v2 1/2] usb: phy: Introduce one extcon device into usb phy To: NeilBrown Cc: Felipe Balbi , Greg KH , USB , LKML , Linaro Kernel Mailman List , Jun Li , Peter Chen , Mark Brown Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4052 Lines: 112 Hi, On 29 March 2017 at 06:56, NeilBrown wrote: > On Thu, Mar 23 2017, Baolin Wang wrote: > >> Usually usb phy need register one extcon device to get the connection >> notifications. It will remove some duplicate code if the extcon device >> is registered using common code instead of each phy driver having its >> own related extcon APIs. So we add one pointer of extcon device into >> usb phy structure, and some other helper functions to register extcon. >> >> Suggested-by: NeilBrown >> Signed-off-by: Baolin Wang >> --- >> Changes since v1: >> - Fix build errors with random config. >> --- >> drivers/usb/phy/Kconfig | 6 +++--- >> drivers/usb/phy/phy.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/usb/phy.h | 6 ++++++ >> 3 files changed, 53 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig >> index 61cef75..c9c61a8 100644 >> --- a/drivers/usb/phy/Kconfig >> +++ b/drivers/usb/phy/Kconfig >> @@ -4,6 +4,7 @@ >> menu "USB Physical Layer drivers" >> >> config USB_PHY >> + select EXTCON >> def_bool n >> >> # >> @@ -116,7 +117,7 @@ config OMAP_OTG >> >> config TAHVO_USB >> tristate "Tahvo USB transceiver driver" >> - depends on MFD_RETU && EXTCON >> + depends on MFD_RETU >> depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be 'y' >> select USB_PHY >> help >> @@ -148,7 +149,6 @@ config USB_MSM_OTG >> depends on (USB || USB_GADGET) && (ARCH_QCOM || COMPILE_TEST) >> depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be 'y' >> depends on RESET_CONTROLLER >> - depends on EXTCON >> select USB_PHY >> help >> Enable this to support the USB OTG transceiver on Qualcomm chips. It >> @@ -162,7 +162,7 @@ config USB_MSM_OTG >> config USB_QCOM_8X16_PHY >> tristate "Qualcomm APQ8016/MSM8916 on-chip USB PHY controller support" >> depends on ARCH_QCOM || COMPILE_TEST >> - depends on RESET_CONTROLLER && EXTCON >> + depends on RESET_CONTROLLER >> select USB_PHY >> select USB_ULPI_VIEWPORT >> help >> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c >> index 98f75d2..baa8b18 100644 >> --- a/drivers/usb/phy/phy.c >> +++ b/drivers/usb/phy/phy.c >> @@ -100,6 +100,41 @@ static int devm_usb_phy_match(struct device *dev, void *res, void *match_data) >> return *phy == match_data; >> } >> >> +static int usb_add_extcon(struct usb_phy *x) >> +{ >> + int ret; >> + >> + if (of_property_read_bool(x->dev->of_node, "extcon")) { >> + x->edev = extcon_get_edev_by_phandle(x->dev, 0); > > This is not what I meant to suggest. > This provides an extcon only for some phy devices. I meant that every > single usb_phy should always have an extcon. > So phy.c should probably call extcon_dev_allocate() and > extcon_dev_register() - or similar. That means every usb phy acts as one extcon device, but I think usb phy device is not one extcon device, ID/VBUS GPIOs are really extcon devices, which means usb phy extcon device is duplicate, right? > > The driver then calls extcon_set_state() or similar on this extcon > device in whatever way might be appropriate. > For a phy driver like phy-qcom-8x16-usb it might just pass on > events from some separate extcon device. > > For the (hypothetical?) device that Felipe suggests with separate > ID and VBUS extcons, it would pass on events from multiple different > extcons. i.e. it would act like a multiplexer. > > Other drivers would do things from interrupt handlers, based on values > read from registers. > > The important point is that each usb_phy doesn't get a pointer to some > separate extcon. Rather it has an extcon that it completely owns. The > name of the extcon is the same of the phy. The extcon should be seen as > a part of the phy, not a separate thing which provides service to the > phy. > > Thanks, > NeilBrown -- Baolin.wang Best Regards