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/[email protected]/msg1272880.html
Feedback and guidance would be greatly appreciated!
thanks
-john
Cc: Wei Xu <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: John Youn <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
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
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 <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: John Youn <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
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
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 <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: John Youn <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Suggested-by: Chen Yu <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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
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 <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: John Youn <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
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
On Tue, Nov 22, 2016 at 7:46 PM, John Stultz <[email protected]> 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/[email protected]/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
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 <[email protected]>
> Cc: Guodong Xu <[email protected]>
> Cc: Amit Pundir <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: John Youn <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Chen Yu <[email protected]>
> Cc: Kishon Vijay Abraham I <[email protected]>
> Cc: Felipe Balbi <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> 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
On Thu, Dec 1, 2016 at 12:23 AM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]>
>> Cc: Guodong Xu <[email protected]>
>> Cc: Amit Pundir <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: John Youn <[email protected]>
>> Cc: Douglas Anderson <[email protected]>
>> Cc: Chen Yu <[email protected]>
>> Cc: Kishon Vijay Abraham I <[email protected]>
>> Cc: Felipe Balbi <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: John Stultz <[email protected]>
>> ---
>> 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
On 12/1/2016 12:12 PM, John Stultz wrote:
> On Thu, Dec 1, 2016 at 12:23 AM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]>
>>> Cc: Guodong Xu <[email protected]>
>>> Cc: Amit Pundir <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: John Youn <[email protected]>
>>> Cc: Douglas Anderson <[email protected]>
>>> Cc: Chen Yu <[email protected]>
>>> Cc: Kishon Vijay Abraham I <[email protected]>
>>> Cc: Felipe Balbi <[email protected]>
>>> Cc: Greg Kroah-Hartman <[email protected]>
>>> Cc: [email protected]
>>> Signed-off-by: John Stultz <[email protected]>
>>> ---
>>> 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