Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756230AbcLAIdF (ORCPT ); Thu, 1 Dec 2016 03:33:05 -0500 Received: from lelnx193.ext.ti.com ([198.47.27.77]:25525 "EHLO lelnx193.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750873AbcLAIdE (ORCPT ); Thu, 1 Dec 2016 03:33:04 -0500 Subject: Re: [RFC][PATCH 1/3] phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver To: John Stultz , lkml References: <1479872809-11958-1-git-send-email-john.stultz@linaro.org> <1479872809-11958-2-git-send-email-john.stultz@linaro.org> CC: Wei Xu , Guodong Xu , Amit Pundir , Rob Herring , John Youn , Douglas Anderson , Chen Yu , Felipe Balbi , Greg Kroah-Hartman , From: Kishon Vijay Abraham I Message-ID: <583FDDEA.1030201@ti.com> Date: Thu, 1 Dec 2016 13:53:06 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1479872809-11958-2-git-send-email-john.stultz@linaro.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7910 Lines: 269 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 > Cc: Guodong Xu > Cc: Amit Pundir > Cc: Rob Herring > Cc: John Youn > Cc: Douglas Anderson > Cc: Chen Yu > Cc: Kishon Vijay Abraham I > Cc: Felipe Balbi > Cc: Greg Kroah-Hartman > Cc: linux-usb@vger.kernel.org > Signed-off-by: John Stultz > --- > 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 > #include > #include > +#include > +#include > +#include > +#include > #include > +#include > > #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