* [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver
@ 2016-11-23 3:46 John Stultz
2016-11-23 3:46 ` [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver John Stultz
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: John Stultz @ 2016-11-23 3:46 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring,
John Youn, Douglas Anderson, Chen Yu, Kishon Vijay Abraham I,
Felipe Balbi, Greg Kroah-Hartman, linux-usb
After earlier attempts[1] at submitting somewhat hackish fixes
to the dwc2 driver, I realized the core issue seemed to be the
overly simplistic phy driver.
I've connected the phy-hi6220-usb.c driver to extcon so it now
gets connection and disconnection signals on the usb gadget
cable. And I modified the driver so it registers a usb-phy and
calls usb_gadget_vbus_connect/disconnect() appropriately.
Unfortunately this doesn't quite work with the dwc2 driver,
so I've hacked that driver to allow it to function.
With these changes, while likely not correct, things function
well, and I was able to drop two of the hackish fixups from the
earlier set. I still needed one patch to keep the usb bus from
suspending while in gadget mode, so I've included that in this
series.
[1]: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1272880.html
Feedback and guidance would be greatly appreciated!
thanks
-john
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
John Stultz (3):
phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver
HACK: dwc2: force dual use of uphy and phy
usb: dwc2: Avoid suspending if we're in gadget mode
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++
drivers/phy/Kconfig | 2 +
drivers/phy/phy-hi6220-usb.c | 139 ++++++++++++++++++++++++++++++
drivers/usb/dwc2/hcd.c | 3 +
drivers/usb/dwc2/platform.c | 4 +-
5 files changed, 157 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver
2016-11-23 3:46 [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
@ 2016-11-23 3:46 ` John Stultz
2016-12-01 8:23 ` Kishon Vijay Abraham I
2016-11-23 3:46 ` [RFC][PATCH 2/3] HACK: dwc2: force dual use of uphy and phy John Stultz
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: John Stultz @ 2016-11-23 3:46 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring,
John Youn, Douglas Anderson, Chen Yu, Kishon Vijay Abraham I,
Felipe Balbi, Greg Kroah-Hartman, linux-usb
This wires extconn support to hikey's phy driver, and
connects it to the usb UDC layer via a usb_phy structure.
Not sure if this is the right way to connect phy -> UDC,
but I'm lacking a clear example.
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++
drivers/phy/Kconfig | 2 +
drivers/phy/phy-hi6220-usb.c | 139 ++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 17839db..171fbb2 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -732,10 +732,21 @@
regulator-always-on;
};
+ usb_vbus: usb-vbus {
+ compatible = "linux,extcon-usb-gpio";
+ id-gpio = <&gpio2 6 1>;
+ };
+
+ usb_id: usb-id {
+ compatible = "linux,extcon-usb-gpio";
+ id-gpio = <&gpio2 5 1>;
+ };
+
usb_phy: usbphy {
compatible = "hisilicon,hi6220-usb-phy";
#phy-cells = <0>;
phy-supply = <&fixed_5v_hub>;
+ extcon = <&usb_vbus>, <&usb_id>;
hisilicon,peripheral-syscon = <&sys_ctrl>;
};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index fe00f91..76f4f17 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -254,8 +254,10 @@ config PHY_MT65XX_USB3
config PHY_HI6220_USB
tristate "hi6220 USB PHY support"
depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+ depends on EXTCON
select GENERIC_PHY
select MFD_SYSCON
+ select USB_PHY
help
Enable this to support the HISILICON HI6220 USB PHY.
diff --git a/drivers/phy/phy-hi6220-usb.c b/drivers/phy/phy-hi6220-usb.c
index b2141cb..89d8475 100644
--- a/drivers/phy/phy-hi6220-usb.c
+++ b/drivers/phy/phy-hi6220-usb.c
@@ -12,7 +12,12 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
+#include <linux/usb/phy_companion.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/phy.h>
#include <linux/regmap.h>
+#include <linux/extcon.h>
#define SC_PERIPH_CTRL4 0x00c
@@ -44,9 +49,21 @@
#define EYE_PATTERN_PARA 0x7053348c
+
+struct hi6220_usb_cable {
+ struct notifier_block nb;
+ struct extcon_dev *extcon;
+ int state;
+};
+
struct hi6220_priv {
struct regmap *reg;
struct device *dev;
+ struct usb_phy phy;
+
+ struct delayed_work work;
+ struct hi6220_usb_cable vbus;
+ struct hi6220_usb_cable id;
};
static void hi6220_phy_init(struct hi6220_priv *priv)
@@ -112,23 +129,85 @@ static int hi6220_phy_exit(struct phy *phy)
return hi6220_phy_setup(priv, false);
}
+
static struct phy_ops hi6220_phy_ops = {
.init = hi6220_phy_start,
.exit = hi6220_phy_exit,
.owner = THIS_MODULE,
};
+static void hi6220_detect_work(struct work_struct *work)
+{
+ struct hi6220_priv *priv =
+ container_of(to_delayed_work(work), struct hi6220_priv, work);
+ struct usb_otg *otg = priv->phy.otg;
+
+ if (!IS_ERR(priv->vbus.extcon))
+ priv->vbus.state = extcon_get_cable_state_(priv->vbus.extcon,
+ EXTCON_USB);
+ if (!IS_ERR(priv->id.extcon))
+ priv->id.state = extcon_get_cable_state_(priv->id.extcon,
+ EXTCON_USB_HOST);
+ if (otg->gadget) {
+ if (priv->id.state)
+ usb_gadget_vbus_connect(otg->gadget);
+ else
+ usb_gadget_vbus_disconnect(otg->gadget);
+ }
+}
+
+static int hi6220_otg_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct hi6220_usb_cable *vbus = container_of(nb,
+ struct hi6220_usb_cable, nb);
+ struct hi6220_priv *priv = container_of(vbus,
+ struct hi6220_priv, vbus);
+
+ schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
+ return NOTIFY_DONE;
+}
+
+static int hi6220_otg_id_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct hi6220_usb_cable *id = container_of(nb,
+ struct hi6220_usb_cable, nb);
+ struct hi6220_priv *priv = container_of(id, struct hi6220_priv, id);
+
+ schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
+ return NOTIFY_DONE;
+}
+
+static int hi6220_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+ otg->host = host;
+ return 0;
+}
+
+static int hi6220_otg_set_peripheral(struct usb_otg *otg,
+ struct usb_gadget *gadget)
+{
+ otg->gadget = gadget;
+ return 0;
+}
+
static int hi6220_phy_probe(struct platform_device *pdev)
{
struct phy_provider *phy_provider;
struct device *dev = &pdev->dev;
struct phy *phy;
+ struct usb_otg *otg;
struct hi6220_priv *priv;
+ struct extcon_dev *ext_id, *ext_vbus;
+ int ret;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
+ INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
+
priv->dev = dev;
priv->reg = syscon_regmap_lookup_by_phandle(dev->of_node,
"hisilicon,peripheral-syscon");
@@ -137,13 +216,73 @@ static int hi6220_phy_probe(struct platform_device *pdev)
return PTR_ERR(priv->reg);
}
+
+ ext_id = ERR_PTR(-ENODEV);
+ ext_vbus = ERR_PTR(-ENODEV);
+ if (of_property_read_bool(dev->of_node, "extcon")) {
+ /* Each one of them is not mandatory */
+ ext_vbus = extcon_get_edev_by_phandle(&pdev->dev, 0);
+ if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
+ return PTR_ERR(ext_vbus);
+
+ ext_id = extcon_get_edev_by_phandle(&pdev->dev, 1);
+ if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
+ return PTR_ERR(ext_id);
+ }
+
+ priv->vbus.extcon = ext_vbus;
+ if (!IS_ERR(ext_vbus)) {
+ priv->vbus.nb.notifier_call = hi6220_otg_vbus_notifier;
+ ret = extcon_register_notifier(ext_vbus, EXTCON_USB,
+ &priv->vbus.nb);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "register VBUS notifier failed\n");
+ return ret;
+ }
+
+ priv->vbus.state = extcon_get_cable_state_(ext_vbus,
+ EXTCON_USB);
+ }
+
+ priv->id.extcon = ext_id;
+ if (!IS_ERR(ext_id)) {
+ priv->id.nb.notifier_call = hi6220_otg_id_notifier;
+ ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
+ &priv->id.nb);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "register ID notifier failed\n");
+ return ret;
+ }
+
+ priv->id.state = extcon_get_cable_state_(ext_id,
+ EXTCON_USB_HOST);
+ }
+
hi6220_phy_init(priv);
phy = devm_phy_create(dev, NULL, &hi6220_phy_ops);
if (IS_ERR(phy))
return PTR_ERR(phy);
+ otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
+ if (!otg)
+ return -ENOMEM;
+
+ priv->dev = &pdev->dev;
+ priv->phy.dev = priv->dev;
+ priv->phy.label = "hi6220_usb_phy";
+ priv->phy.otg = otg;
+ priv->phy.type = USB_PHY_TYPE_USB2;
+ otg->set_host = hi6220_otg_set_host;
+ otg->set_peripheral = hi6220_otg_set_peripheral;
+ otg->usb_phy = &priv->phy;
+
+ platform_set_drvdata(pdev, priv);
+
phy_set_drvdata(phy, priv);
+
+ usb_add_phy_dev(&priv->phy);
+
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC][PATCH 2/3] HACK: dwc2: force dual use of uphy and phy
2016-11-23 3:46 [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
2016-11-23 3:46 ` [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver John Stultz
@ 2016-11-23 3:46 ` John Stultz
2016-11-23 3:46 ` [RFC][PATCH 3/3] usb: dwc2: Avoid suspending if we're in gadget mode John Stultz
2016-12-01 1:35 ` [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
3 siblings, 0 replies; 8+ messages in thread
From: John Stultz @ 2016-11-23 3:46 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring,
John Youn, Douglas Anderson, Chen Yu, Kishon Vijay Abraham I,
Felipe Balbi, Greg Kroah-Hartman, linux-usb
I can't seem to figure out how to connect a generic phy device
to the usb UDC logic, as the dwc2 code seems to exclusively work
with either generic phys or usb-phys.
So to try to make this work with the phy-hi6220-usb driver, tweak
the dwc2 logic to support call hooks to both phy and uphy devices.
I realize this is likely wrong, but without a good pointer as to
how this should be done, I'm a bit wandering around in the dark.
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/usb/dwc2/platform.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 8e1728b..8fb2f4d 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -297,7 +297,7 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
if (hsotg->uphy)
ret = usb_phy_init(hsotg->uphy);
- else if (hsotg->plat && hsotg->plat->phy_init)
+ if (hsotg->plat && hsotg->plat->phy_init)
ret = hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
else {
ret = phy_power_on(hsotg->phy);
@@ -411,7 +411,7 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
}
}
- if (!hsotg->phy) {
+ if (1 || !hsotg->phy) {
hsotg->uphy = devm_usb_get_phy(hsotg->dev, USB_PHY_TYPE_USB2);
if (IS_ERR(hsotg->uphy)) {
ret = PTR_ERR(hsotg->uphy);
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC][PATCH 3/3] usb: dwc2: Avoid suspending if we're in gadget mode
2016-11-23 3:46 [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
2016-11-23 3:46 ` [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver John Stultz
2016-11-23 3:46 ` [RFC][PATCH 2/3] HACK: dwc2: force dual use of uphy and phy John Stultz
@ 2016-11-23 3:46 ` John Stultz
2016-12-01 1:35 ` [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
3 siblings, 0 replies; 8+ messages in thread
From: John Stultz @ 2016-11-23 3:46 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring,
John Youn, Douglas Anderson, Chen Yu, Kishon Vijay Abraham I,
Felipe Balbi, Greg Kroah-Hartman, linux-usb
I've found when booting HiKey with the usb gadget cable attached
if I then try to connect via adb, I get an infinite spew of:
dwc2 f72c0000.usb: dwc2_hsotg_ep_sethalt(ep ffffffc0790ecb18 ep1out, 0)
dwc2 f72c0000.usb: dwc2_hsotg_ep_sethalt(ep ffffffc0790eca18 ep1in, 0)
It seems that the usb autosuspend is suspending the bus shortly
after bootup when the gadget cable is attached. So when adbd
then tries to use the device, it doesn't work and it then tries
to restart it over and over via the ep_sethalt calls (via
FUNCTIONFS_CLEAR_HALT ioctl).
Chen Yu suggested this patch to avoid suspending if we're
in device mode, and it avoids the problem.
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Suggested-by: Chen Yu <chenyu56@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/usb/dwc2/hcd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index df5a065..619ccfe 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4365,6 +4365,9 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd)
if (!HCD_HW_ACCESSIBLE(hcd))
goto unlock;
+ if (hsotg->op_state == OTG_STATE_B_PERIPHERAL)
+ goto unlock;
+
if (!hsotg->core_params->hibernation)
goto skip_power_saving;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver
2016-11-23 3:46 [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
` (2 preceding siblings ...)
2016-11-23 3:46 ` [RFC][PATCH 3/3] usb: dwc2: Avoid suspending if we're in gadget mode John Stultz
@ 2016-12-01 1:35 ` John Stultz
3 siblings, 0 replies; 8+ messages in thread
From: John Stultz @ 2016-12-01 1:35 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring,
John Youn, Douglas Anderson, Chen Yu, Kishon Vijay Abraham I,
Felipe Balbi, Greg Kroah-Hartman, Linux USB List
On Tue, Nov 22, 2016 at 7:46 PM, John Stultz <john.stultz@linaro.org> wrote:
> After earlier attempts[1] at submitting somewhat hackish fixes
> to the dwc2 driver, I realized the core issue seemed to be the
> overly simplistic phy driver.
>
> I've connected the phy-hi6220-usb.c driver to extcon so it now
> gets connection and disconnection signals on the usb gadget
> cable. And I modified the driver so it registers a usb-phy and
> calls usb_gadget_vbus_connect/disconnect() appropriately.
>
> Unfortunately this doesn't quite work with the dwc2 driver,
> so I've hacked that driver to allow it to function.
>
> With these changes, while likely not correct, things function
> well, and I was able to drop two of the hackish fixups from the
> earlier set. I still needed one patch to keep the usb bus from
> suspending while in gadget mode, so I've included that in this
> series.
>
> [1]: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1272880.html
>
> Feedback and guidance would be greatly appreciated!
>
>
> John Stultz (3):
> phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver
> HACK: dwc2: force dual use of uphy and phy
> usb: dwc2: Avoid suspending if we're in gadget mode
Curious if there was any feedback on this patchset or the general approach?
thanks
-john
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver
2016-11-23 3:46 ` [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver John Stultz
@ 2016-12-01 8:23 ` Kishon Vijay Abraham I
2016-12-01 20:12 ` John Stultz
0 siblings, 1 reply; 8+ messages in thread
From: Kishon Vijay Abraham I @ 2016-12-01 8:23 UTC (permalink / raw)
To: John Stultz, lkml
Cc: Wei Xu, Guodong Xu, Amit Pundir, Rob Herring, John Youn,
Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman,
linux-usb
Hi,
On Wednesday 23 November 2016 09:16 AM, John Stultz wrote:
> This wires extconn support to hikey's phy driver, and
> connects it to the usb UDC layer via a usb_phy structure.
>
> Not sure if this is the right way to connect phy -> UDC,
> but I'm lacking a clear example.
>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Chen Yu <chenyu56@huawei.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++
> drivers/phy/Kconfig | 2 +
> drivers/phy/phy-hi6220-usb.c | 139 ++++++++++++++++++++++++++++++
> 3 files changed, 152 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 17839db..171fbb2 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -732,10 +732,21 @@
> regulator-always-on;
> };
>
> + usb_vbus: usb-vbus {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 6 1>;
> + };
> +
> + usb_id: usb-id {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio2 5 1>;
> + };
> +
> usb_phy: usbphy {
> compatible = "hisilicon,hi6220-usb-phy";
> #phy-cells = <0>;
> phy-supply = <&fixed_5v_hub>;
> + extcon = <&usb_vbus>, <&usb_id>;
> hisilicon,peripheral-syscon = <&sys_ctrl>;
> };
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index fe00f91..76f4f17 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -254,8 +254,10 @@ config PHY_MT65XX_USB3
> config PHY_HI6220_USB
> tristate "hi6220 USB PHY support"
> depends on (ARCH_HISI && ARM64) || COMPILE_TEST
> + depends on EXTCON
> select GENERIC_PHY
> select MFD_SYSCON
> + select USB_PHY
> help
> Enable this to support the HISILICON HI6220 USB PHY.
>
> diff --git a/drivers/phy/phy-hi6220-usb.c b/drivers/phy/phy-hi6220-usb.c
> index b2141cb..89d8475 100644
> --- a/drivers/phy/phy-hi6220-usb.c
> +++ b/drivers/phy/phy-hi6220-usb.c
> @@ -12,7 +12,12 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
> +#include <linux/usb/phy_companion.h>
> +#include <linux/usb/otg.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/phy.h>
> #include <linux/regmap.h>
> +#include <linux/extcon.h>
>
> #define SC_PERIPH_CTRL4 0x00c
>
> @@ -44,9 +49,21 @@
>
> #define EYE_PATTERN_PARA 0x7053348c
>
> +
> +struct hi6220_usb_cable {
> + struct notifier_block nb;
> + struct extcon_dev *extcon;
> + int state;
> +};
> +
> struct hi6220_priv {
> struct regmap *reg;
> struct device *dev;
> + struct usb_phy phy;
> +
> + struct delayed_work work;
> + struct hi6220_usb_cable vbus;
> + struct hi6220_usb_cable id;
> };
>
> static void hi6220_phy_init(struct hi6220_priv *priv)
> @@ -112,23 +129,85 @@ static int hi6220_phy_exit(struct phy *phy)
> return hi6220_phy_setup(priv, false);
> }
>
> +
> static struct phy_ops hi6220_phy_ops = {
> .init = hi6220_phy_start,
> .exit = hi6220_phy_exit,
> .owner = THIS_MODULE,
> };
>
> +static void hi6220_detect_work(struct work_struct *work)
> +{
> + struct hi6220_priv *priv =
> + container_of(to_delayed_work(work), struct hi6220_priv, work);
> + struct usb_otg *otg = priv->phy.otg;
> +
> + if (!IS_ERR(priv->vbus.extcon))
> + priv->vbus.state = extcon_get_cable_state_(priv->vbus.extcon,
> + EXTCON_USB);
> + if (!IS_ERR(priv->id.extcon))
> + priv->id.state = extcon_get_cable_state_(priv->id.extcon,
> + EXTCON_USB_HOST);
> + if (otg->gadget) {
> + if (priv->id.state)
> + usb_gadget_vbus_connect(otg->gadget);
> + else
> + usb_gadget_vbus_disconnect(otg->gadget);
> + }
> +}
> +
> +static int hi6220_otg_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct hi6220_usb_cable *vbus = container_of(nb,
> + struct hi6220_usb_cable, nb);
> + struct hi6220_priv *priv = container_of(vbus,
> + struct hi6220_priv, vbus);
> +
> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> + return NOTIFY_DONE;
> +}
> +
> +static int hi6220_otg_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct hi6220_usb_cable *id = container_of(nb,
> + struct hi6220_usb_cable, nb);
> + struct hi6220_priv *priv = container_of(id, struct hi6220_priv, id);
> +
> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> + return NOTIFY_DONE;
> +}
> +
> +static int hi6220_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> + otg->host = host;
> + return 0;
> +}
> +
> +static int hi6220_otg_set_peripheral(struct usb_otg *otg,
> + struct usb_gadget *gadget)
> +{
> + otg->gadget = gadget;
> + return 0;
> +}
> +
> static int hi6220_phy_probe(struct platform_device *pdev)
> {
> struct phy_provider *phy_provider;
> struct device *dev = &pdev->dev;
> struct phy *phy;
> + struct usb_otg *otg;
> struct hi6220_priv *priv;
> + struct extcon_dev *ext_id, *ext_vbus;
> + int ret;
>
> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> return -ENOMEM;
>
> + INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
> +
> priv->dev = dev;
> priv->reg = syscon_regmap_lookup_by_phandle(dev->of_node,
> "hisilicon,peripheral-syscon");
> @@ -137,13 +216,73 @@ static int hi6220_phy_probe(struct platform_device *pdev)
> return PTR_ERR(priv->reg);
> }
>
> +
> + ext_id = ERR_PTR(-ENODEV);
> + ext_vbus = ERR_PTR(-ENODEV);
> + if (of_property_read_bool(dev->of_node, "extcon")) {
> + /* Each one of them is not mandatory */
> + ext_vbus = extcon_get_edev_by_phandle(&pdev->dev, 0);
> + if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
> + return PTR_ERR(ext_vbus);
> +
> + ext_id = extcon_get_edev_by_phandle(&pdev->dev, 1);
> + if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
> + return PTR_ERR(ext_id);
> + }
> +
> + priv->vbus.extcon = ext_vbus;
> + if (!IS_ERR(ext_vbus)) {
> + priv->vbus.nb.notifier_call = hi6220_otg_vbus_notifier;
> + ret = extcon_register_notifier(ext_vbus, EXTCON_USB,
> + &priv->vbus.nb);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "register VBUS notifier failed\n");
> + return ret;
> + }
> +
> + priv->vbus.state = extcon_get_cable_state_(ext_vbus,
> + EXTCON_USB);
> + }
> +
> + priv->id.extcon = ext_id;
> + if (!IS_ERR(ext_id)) {
> + priv->id.nb.notifier_call = hi6220_otg_id_notifier;
> + ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
> + &priv->id.nb);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "register ID notifier failed\n");
> + return ret;
> + }
> +
> + priv->id.state = extcon_get_cable_state_(ext_id,
> + EXTCON_USB_HOST);
> + }
> +
> hi6220_phy_init(priv);
>
> phy = devm_phy_create(dev, NULL, &hi6220_phy_ops);
> if (IS_ERR(phy))
> return PTR_ERR(phy);
>
> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> + if (!otg)
> + return -ENOMEM;
> +
> + priv->dev = &pdev->dev;
> + priv->phy.dev = priv->dev;
> + priv->phy.label = "hi6220_usb_phy";
> + priv->phy.otg = otg;
> + priv->phy.type = USB_PHY_TYPE_USB2;
> + otg->set_host = hi6220_otg_set_host;
> + otg->set_peripheral = hi6220_otg_set_peripheral;
> + otg->usb_phy = &priv->phy;
> +
> + platform_set_drvdata(pdev, priv);
> +
> phy_set_drvdata(phy, priv);
> +
> + usb_add_phy_dev(&priv->phy);
This would be like using two independent phy infrastructure :-( Should we just
handle the extcon events in USB driver?
Thanks
Kishon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver
2016-12-01 8:23 ` Kishon Vijay Abraham I
@ 2016-12-01 20:12 ` John Stultz
2016-12-03 1:07 ` John Youn
0 siblings, 1 reply; 8+ messages in thread
From: John Stultz @ 2016-12-01 20:12 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: lkml, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring, John Youn,
Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman,
Linux USB List
On Thu, Dec 1, 2016 at 12:23 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi,
>
> On Wednesday 23 November 2016 09:16 AM, John Stultz wrote:
>> This wires extconn support to hikey's phy driver, and
>> connects it to the usb UDC layer via a usb_phy structure.
>>
>> Not sure if this is the right way to connect phy -> UDC,
>> but I'm lacking a clear example.
>>
>> Cc: Wei Xu <xuwei5@hisilicon.com>
>> Cc: Guodong Xu <guodong.xu@linaro.org>
>> Cc: Amit Pundir <amit.pundir@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: John Youn <johnyoun@synopsys.com>
>> Cc: Douglas Anderson <dianders@chromium.org>
>> Cc: Chen Yu <chenyu56@huawei.com>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-usb@vger.kernel.org
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++
>> drivers/phy/Kconfig | 2 +
>> drivers/phy/phy-hi6220-usb.c | 139 ++++++++++++++++++++++++++++++
>> 3 files changed, 152 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> index 17839db..171fbb2 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> @@ -732,10 +732,21 @@
>> regulator-always-on;
>> };
>>
>> + usb_vbus: usb-vbus {
>> + compatible = "linux,extcon-usb-gpio";
>> + id-gpio = <&gpio2 6 1>;
>> + };
>> +
>> + usb_id: usb-id {
>> + compatible = "linux,extcon-usb-gpio";
>> + id-gpio = <&gpio2 5 1>;
>> + };
>> +
>> usb_phy: usbphy {
>> compatible = "hisilicon,hi6220-usb-phy";
>> #phy-cells = <0>;
>> phy-supply = <&fixed_5v_hub>;
>> + extcon = <&usb_vbus>, <&usb_id>;
>> hisilicon,peripheral-syscon = <&sys_ctrl>;
>> };
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index fe00f91..76f4f17 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -254,8 +254,10 @@ config PHY_MT65XX_USB3
>> config PHY_HI6220_USB
>> tristate "hi6220 USB PHY support"
>> depends on (ARCH_HISI && ARM64) || COMPILE_TEST
>> + depends on EXTCON
>> select GENERIC_PHY
>> select MFD_SYSCON
>> + select USB_PHY
>> help
>> Enable this to support the HISILICON HI6220 USB PHY.
>>
>> diff --git a/drivers/phy/phy-hi6220-usb.c b/drivers/phy/phy-hi6220-usb.c
>> index b2141cb..89d8475 100644
>> --- a/drivers/phy/phy-hi6220-usb.c
>> +++ b/drivers/phy/phy-hi6220-usb.c
>> @@ -12,7 +12,12 @@
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> #include <linux/phy/phy.h>
>> +#include <linux/usb/phy_companion.h>
>> +#include <linux/usb/otg.h>
>> +#include <linux/usb/gadget.h>
>> +#include <linux/usb/phy.h>
>> #include <linux/regmap.h>
>> +#include <linux/extcon.h>
>>
>> #define SC_PERIPH_CTRL4 0x00c
>>
>> @@ -44,9 +49,21 @@
>>
>> #define EYE_PATTERN_PARA 0x7053348c
>>
>> +
>> +struct hi6220_usb_cable {
>> + struct notifier_block nb;
>> + struct extcon_dev *extcon;
>> + int state;
>> +};
>> +
>> struct hi6220_priv {
>> struct regmap *reg;
>> struct device *dev;
>> + struct usb_phy phy;
>> +
>> + struct delayed_work work;
>> + struct hi6220_usb_cable vbus;
>> + struct hi6220_usb_cable id;
>> };
>>
>> static void hi6220_phy_init(struct hi6220_priv *priv)
>> @@ -112,23 +129,85 @@ static int hi6220_phy_exit(struct phy *phy)
>> return hi6220_phy_setup(priv, false);
>> }
>>
>> +
>> static struct phy_ops hi6220_phy_ops = {
>> .init = hi6220_phy_start,
>> .exit = hi6220_phy_exit,
>> .owner = THIS_MODULE,
>> };
>>
>> +static void hi6220_detect_work(struct work_struct *work)
>> +{
>> + struct hi6220_priv *priv =
>> + container_of(to_delayed_work(work), struct hi6220_priv, work);
>> + struct usb_otg *otg = priv->phy.otg;
>> +
>> + if (!IS_ERR(priv->vbus.extcon))
>> + priv->vbus.state = extcon_get_cable_state_(priv->vbus.extcon,
>> + EXTCON_USB);
>> + if (!IS_ERR(priv->id.extcon))
>> + priv->id.state = extcon_get_cable_state_(priv->id.extcon,
>> + EXTCON_USB_HOST);
>> + if (otg->gadget) {
>> + if (priv->id.state)
>> + usb_gadget_vbus_connect(otg->gadget);
>> + else
>> + usb_gadget_vbus_disconnect(otg->gadget);
>> + }
>> +}
>> +
>> +static int hi6220_otg_vbus_notifier(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> + struct hi6220_usb_cable *vbus = container_of(nb,
>> + struct hi6220_usb_cable, nb);
>> + struct hi6220_priv *priv = container_of(vbus,
>> + struct hi6220_priv, vbus);
>> +
>> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int hi6220_otg_id_notifier(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> + struct hi6220_usb_cable *id = container_of(nb,
>> + struct hi6220_usb_cable, nb);
>> + struct hi6220_priv *priv = container_of(id, struct hi6220_priv, id);
>> +
>> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int hi6220_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
>> +{
>> + otg->host = host;
>> + return 0;
>> +}
>> +
>> +static int hi6220_otg_set_peripheral(struct usb_otg *otg,
>> + struct usb_gadget *gadget)
>> +{
>> + otg->gadget = gadget;
>> + return 0;
>> +}
>> +
>> static int hi6220_phy_probe(struct platform_device *pdev)
>> {
>> struct phy_provider *phy_provider;
>> struct device *dev = &pdev->dev;
>> struct phy *phy;
>> + struct usb_otg *otg;
>> struct hi6220_priv *priv;
>> + struct extcon_dev *ext_id, *ext_vbus;
>> + int ret;
>>
>> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> if (!priv)
>> return -ENOMEM;
>>
>> + INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
>> +
>> priv->dev = dev;
>> priv->reg = syscon_regmap_lookup_by_phandle(dev->of_node,
>> "hisilicon,peripheral-syscon");
>> @@ -137,13 +216,73 @@ static int hi6220_phy_probe(struct platform_device *pdev)
>> return PTR_ERR(priv->reg);
>> }
>>
>> +
>> + ext_id = ERR_PTR(-ENODEV);
>> + ext_vbus = ERR_PTR(-ENODEV);
>> + if (of_property_read_bool(dev->of_node, "extcon")) {
>> + /* Each one of them is not mandatory */
>> + ext_vbus = extcon_get_edev_by_phandle(&pdev->dev, 0);
>> + if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
>> + return PTR_ERR(ext_vbus);
>> +
>> + ext_id = extcon_get_edev_by_phandle(&pdev->dev, 1);
>> + if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
>> + return PTR_ERR(ext_id);
>> + }
>> +
>> + priv->vbus.extcon = ext_vbus;
>> + if (!IS_ERR(ext_vbus)) {
>> + priv->vbus.nb.notifier_call = hi6220_otg_vbus_notifier;
>> + ret = extcon_register_notifier(ext_vbus, EXTCON_USB,
>> + &priv->vbus.nb);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "register VBUS notifier failed\n");
>> + return ret;
>> + }
>> +
>> + priv->vbus.state = extcon_get_cable_state_(ext_vbus,
>> + EXTCON_USB);
>> + }
>> +
>> + priv->id.extcon = ext_id;
>> + if (!IS_ERR(ext_id)) {
>> + priv->id.nb.notifier_call = hi6220_otg_id_notifier;
>> + ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
>> + &priv->id.nb);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "register ID notifier failed\n");
>> + return ret;
>> + }
>> +
>> + priv->id.state = extcon_get_cable_state_(ext_id,
>> + EXTCON_USB_HOST);
>> + }
>> +
>> hi6220_phy_init(priv);
>>
>> phy = devm_phy_create(dev, NULL, &hi6220_phy_ops);
>> if (IS_ERR(phy))
>> return PTR_ERR(phy);
>>
>> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
>> + if (!otg)
>> + return -ENOMEM;
>> +
>> + priv->dev = &pdev->dev;
>> + priv->phy.dev = priv->dev;
>> + priv->phy.label = "hi6220_usb_phy";
>> + priv->phy.otg = otg;
>> + priv->phy.type = USB_PHY_TYPE_USB2;
>> + otg->set_host = hi6220_otg_set_host;
>> + otg->set_peripheral = hi6220_otg_set_peripheral;
>> + otg->usb_phy = &priv->phy;
>> +
>> + platform_set_drvdata(pdev, priv);
>> +
>> phy_set_drvdata(phy, priv);
>> +
>> + usb_add_phy_dev(&priv->phy);
>
> This would be like using two independent phy infrastructure :-( Should we just
> handle the extcon events in USB driver?
Yes. I was told that the older hikey usb-phy driver got nacked since
new drivers should use the generic phy infrastructure, so I agree that
registering a usb-phy device in a generic phy driver felt improper. My
trouble was that its not obvious what is the way it should be done.
So as for adding extcon events to the usb driver, do you have a
pointer to a driver that does it in a way that folks like?
thanks
-john
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver
2016-12-01 20:12 ` John Stultz
@ 2016-12-03 1:07 ` John Youn
0 siblings, 0 replies; 8+ messages in thread
From: John Youn @ 2016-12-03 1:07 UTC (permalink / raw)
To: John Stultz, Kishon Vijay Abraham I
Cc: lkml, Wei Xu, Guodong Xu, Amit Pundir, Rob Herring, John Youn,
Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman,
Linux USB List
On 12/1/2016 12:12 PM, John Stultz wrote:
> On Thu, Dec 1, 2016 at 12:23 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi,
>>
>> On Wednesday 23 November 2016 09:16 AM, John Stultz wrote:
>>> This wires extconn support to hikey's phy driver, and
>>> connects it to the usb UDC layer via a usb_phy structure.
>>>
>>> Not sure if this is the right way to connect phy -> UDC,
>>> but I'm lacking a clear example.
>>>
>>> Cc: Wei Xu <xuwei5@hisilicon.com>
>>> Cc: Guodong Xu <guodong.xu@linaro.org>
>>> Cc: Amit Pundir <amit.pundir@linaro.org>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: John Youn <johnyoun@synopsys.com>
>>> Cc: Douglas Anderson <dianders@chromium.org>
>>> Cc: Chen Yu <chenyu56@huawei.com>
>>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: linux-usb@vger.kernel.org
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>> ---
>>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++
>>> drivers/phy/Kconfig | 2 +
>>> drivers/phy/phy-hi6220-usb.c | 139 ++++++++++++++++++++++++++++++
>>> 3 files changed, 152 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> index 17839db..171fbb2 100644
>>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>>> @@ -732,10 +732,21 @@
>>> regulator-always-on;
>>> };
>>>
>>> + usb_vbus: usb-vbus {
>>> + compatible = "linux,extcon-usb-gpio";
>>> + id-gpio = <&gpio2 6 1>;
>>> + };
>>> +
>>> + usb_id: usb-id {
>>> + compatible = "linux,extcon-usb-gpio";
>>> + id-gpio = <&gpio2 5 1>;
>>> + };
>>> +
>>> usb_phy: usbphy {
>>> compatible = "hisilicon,hi6220-usb-phy";
>>> #phy-cells = <0>;
>>> phy-supply = <&fixed_5v_hub>;
>>> + extcon = <&usb_vbus>, <&usb_id>;
>>> hisilicon,peripheral-syscon = <&sys_ctrl>;
>>> };
>>>
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index fe00f91..76f4f17 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -254,8 +254,10 @@ config PHY_MT65XX_USB3
>>> config PHY_HI6220_USB
>>> tristate "hi6220 USB PHY support"
>>> depends on (ARCH_HISI && ARM64) || COMPILE_TEST
>>> + depends on EXTCON
>>> select GENERIC_PHY
>>> select MFD_SYSCON
>>> + select USB_PHY
>>> help
>>> Enable this to support the HISILICON HI6220 USB PHY.
>>>
>>> diff --git a/drivers/phy/phy-hi6220-usb.c b/drivers/phy/phy-hi6220-usb.c
>>> index b2141cb..89d8475 100644
>>> --- a/drivers/phy/phy-hi6220-usb.c
>>> +++ b/drivers/phy/phy-hi6220-usb.c
>>> @@ -12,7 +12,12 @@
>>> #include <linux/module.h>
>>> #include <linux/platform_device.h>
>>> #include <linux/phy/phy.h>
>>> +#include <linux/usb/phy_companion.h>
>>> +#include <linux/usb/otg.h>
>>> +#include <linux/usb/gadget.h>
>>> +#include <linux/usb/phy.h>
>>> #include <linux/regmap.h>
>>> +#include <linux/extcon.h>
>>>
>>> #define SC_PERIPH_CTRL4 0x00c
>>>
>>> @@ -44,9 +49,21 @@
>>>
>>> #define EYE_PATTERN_PARA 0x7053348c
>>>
>>> +
>>> +struct hi6220_usb_cable {
>>> + struct notifier_block nb;
>>> + struct extcon_dev *extcon;
>>> + int state;
>>> +};
>>> +
>>> struct hi6220_priv {
>>> struct regmap *reg;
>>> struct device *dev;
>>> + struct usb_phy phy;
>>> +
>>> + struct delayed_work work;
>>> + struct hi6220_usb_cable vbus;
>>> + struct hi6220_usb_cable id;
>>> };
>>>
>>> static void hi6220_phy_init(struct hi6220_priv *priv)
>>> @@ -112,23 +129,85 @@ static int hi6220_phy_exit(struct phy *phy)
>>> return hi6220_phy_setup(priv, false);
>>> }
>>>
>>> +
>>> static struct phy_ops hi6220_phy_ops = {
>>> .init = hi6220_phy_start,
>>> .exit = hi6220_phy_exit,
>>> .owner = THIS_MODULE,
>>> };
>>>
>>> +static void hi6220_detect_work(struct work_struct *work)
>>> +{
>>> + struct hi6220_priv *priv =
>>> + container_of(to_delayed_work(work), struct hi6220_priv, work);
>>> + struct usb_otg *otg = priv->phy.otg;
>>> +
>>> + if (!IS_ERR(priv->vbus.extcon))
>>> + priv->vbus.state = extcon_get_cable_state_(priv->vbus.extcon,
>>> + EXTCON_USB);
>>> + if (!IS_ERR(priv->id.extcon))
>>> + priv->id.state = extcon_get_cable_state_(priv->id.extcon,
>>> + EXTCON_USB_HOST);
>>> + if (otg->gadget) {
>>> + if (priv->id.state)
>>> + usb_gadget_vbus_connect(otg->gadget);
>>> + else
>>> + usb_gadget_vbus_disconnect(otg->gadget);
>>> + }
>>> +}
>>> +
>>> +static int hi6220_otg_vbus_notifier(struct notifier_block *nb,
>>> + unsigned long event, void *ptr)
>>> +{
>>> + struct hi6220_usb_cable *vbus = container_of(nb,
>>> + struct hi6220_usb_cable, nb);
>>> + struct hi6220_priv *priv = container_of(vbus,
>>> + struct hi6220_priv, vbus);
>>> +
>>> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
>>> + return NOTIFY_DONE;
>>> +}
>>> +
>>> +static int hi6220_otg_id_notifier(struct notifier_block *nb,
>>> + unsigned long event, void *ptr)
>>> +{
>>> + struct hi6220_usb_cable *id = container_of(nb,
>>> + struct hi6220_usb_cable, nb);
>>> + struct hi6220_priv *priv = container_of(id, struct hi6220_priv, id);
>>> +
>>> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
>>> + return NOTIFY_DONE;
>>> +}
>>> +
>>> +static int hi6220_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
>>> +{
>>> + otg->host = host;
>>> + return 0;
>>> +}
>>> +
>>> +static int hi6220_otg_set_peripheral(struct usb_otg *otg,
>>> + struct usb_gadget *gadget)
>>> +{
>>> + otg->gadget = gadget;
>>> + return 0;
>>> +}
>>> +
>>> static int hi6220_phy_probe(struct platform_device *pdev)
>>> {
>>> struct phy_provider *phy_provider;
>>> struct device *dev = &pdev->dev;
>>> struct phy *phy;
>>> + struct usb_otg *otg;
>>> struct hi6220_priv *priv;
>>> + struct extcon_dev *ext_id, *ext_vbus;
>>> + int ret;
>>>
>>> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> if (!priv)
>>> return -ENOMEM;
>>>
>>> + INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
>>> +
>>> priv->dev = dev;
>>> priv->reg = syscon_regmap_lookup_by_phandle(dev->of_node,
>>> "hisilicon,peripheral-syscon");
>>> @@ -137,13 +216,73 @@ static int hi6220_phy_probe(struct platform_device *pdev)
>>> return PTR_ERR(priv->reg);
>>> }
>>>
>>> +
>>> + ext_id = ERR_PTR(-ENODEV);
>>> + ext_vbus = ERR_PTR(-ENODEV);
>>> + if (of_property_read_bool(dev->of_node, "extcon")) {
>>> + /* Each one of them is not mandatory */
>>> + ext_vbus = extcon_get_edev_by_phandle(&pdev->dev, 0);
>>> + if (IS_ERR(ext_vbus) && PTR_ERR(ext_vbus) != -ENODEV)
>>> + return PTR_ERR(ext_vbus);
>>> +
>>> + ext_id = extcon_get_edev_by_phandle(&pdev->dev, 1);
>>> + if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
>>> + return PTR_ERR(ext_id);
>>> + }
>>> +
>>> + priv->vbus.extcon = ext_vbus;
>>> + if (!IS_ERR(ext_vbus)) {
>>> + priv->vbus.nb.notifier_call = hi6220_otg_vbus_notifier;
>>> + ret = extcon_register_notifier(ext_vbus, EXTCON_USB,
>>> + &priv->vbus.nb);
>>> + if (ret < 0) {
>>> + dev_err(&pdev->dev, "register VBUS notifier failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + priv->vbus.state = extcon_get_cable_state_(ext_vbus,
>>> + EXTCON_USB);
>>> + }
>>> +
>>> + priv->id.extcon = ext_id;
>>> + if (!IS_ERR(ext_id)) {
>>> + priv->id.nb.notifier_call = hi6220_otg_id_notifier;
>>> + ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
>>> + &priv->id.nb);
>>> + if (ret < 0) {
>>> + dev_err(&pdev->dev, "register ID notifier failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + priv->id.state = extcon_get_cable_state_(ext_id,
>>> + EXTCON_USB_HOST);
>>> + }
>>> +
>>> hi6220_phy_init(priv);
>>>
>>> phy = devm_phy_create(dev, NULL, &hi6220_phy_ops);
>>> if (IS_ERR(phy))
>>> return PTR_ERR(phy);
>>>
>>> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
>>> + if (!otg)
>>> + return -ENOMEM;
>>> +
>>> + priv->dev = &pdev->dev;
>>> + priv->phy.dev = priv->dev;
>>> + priv->phy.label = "hi6220_usb_phy";
>>> + priv->phy.otg = otg;
>>> + priv->phy.type = USB_PHY_TYPE_USB2;
>>> + otg->set_host = hi6220_otg_set_host;
>>> + otg->set_peripheral = hi6220_otg_set_peripheral;
>>> + otg->usb_phy = &priv->phy;
>>> +
>>> + platform_set_drvdata(pdev, priv);
>>> +
>>> phy_set_drvdata(phy, priv);
>>> +
>>> + usb_add_phy_dev(&priv->phy);
>>
>> This would be like using two independent phy infrastructure :-( Should we just
>> handle the extcon events in USB driver?
>
> Yes. I was told that the older hikey usb-phy driver got nacked since
> new drivers should use the generic phy infrastructure, so I agree that
> registering a usb-phy device in a generic phy driver felt improper. My
> trouble was that its not obvious what is the way it should be done.
I'm not really sure how this should be solved either. But it seems
wrong to use both PHY frameworks like that.
>
> So as for adding extcon events to the usb driver, do you have a
> pointer to a driver that does it in a way that folks like?
I would be ok with this. Not sure if other folks might have
objections.
Take a look in the probe function in dwc2/platform.c where we handle
things similar to this.
Regards,
John
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-12-03 1:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23 3:46 [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
2016-11-23 3:46 ` [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver John Stultz
2016-12-01 8:23 ` Kishon Vijay Abraham I
2016-12-01 20:12 ` John Stultz
2016-12-03 1:07 ` John Youn
2016-11-23 3:46 ` [RFC][PATCH 2/3] HACK: dwc2: force dual use of uphy and phy John Stultz
2016-11-23 3:46 ` [RFC][PATCH 3/3] usb: dwc2: Avoid suspending if we're in gadget mode John Stultz
2016-12-01 1:35 ` [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver John Stultz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).