Hi,
On 29 March 2017 at 06:56, NeilBrown <neilb@xxxxxxxx> 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 <neilb@xxxxxxxx>
>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
>> ---
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
