From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030578AbdEWSQn (ORCPT ); Tue, 23 May 2017 14:16:43 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:56799 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S967184AbdEWSQm (ORCPT ); Tue, 23 May 2017 14:16:42 -0400 Message-ID: <1495563396.12055.93.camel@collabora.co.uk> Subject: Re: [RFC] usb-phy-generic: Add support to SMSC USB3315 From: Fabien Lahoudere To: Peter Chen , Peter Senna Tschudin Cc: Stephen Boyd , Felipe Balbi , Greg Kroah-Hartman , "open list:USB PHY LAYER" , open list Date: Tue, 23 May 2017 20:16:36 +0200 In-Reply-To: <20170420085046.GA11378@b29397-desktop> References: <20170419061413.20961-1-peter.senna@collabora.com> <20170420085046.GA11378@b29397-desktop> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.5-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, We investigate on the topic and now our device tree look like: in imx53.dtsi: usbh2: usb@53f80400 { compatible = "fsl,imx53-usb", "fsl,imx27-usb";         reg = <0x53f80400 0x0200>;         interrupts = <16>;         clocks = <&clks IMX5_CLK_USBOH3_GATE>;         fsl,usbmisc = <&usbmisc 2>;         dr_mode = "host";         status = "disabled"; }; usbmisc: usbmisc@53f80800 { #index-cells = <1>; compatible = "fsl,imx53-usbmisc"; reg = <0x53f80800 0x200>; clocks = <&clks IMX5_CLK_USBOH3_GATE>; }; and in our dts: &usbh2 {         pinctrl-names = "default"; pinctrl-0 = <&pinctrl_usbh2>; disable-int60ck;         dr_mode = "host";         //fsl,usbphy = <&usbphy2>;         vbus-supply = <®_usbh2_vbus>;         status = "okay"; ulpi {                 phy {                         compatible = "smsc,usb3315-ulpi";                         reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>; clock-names = "main_clk"; /*                          * Hardware uses CKO2 at 24MHz at several places. Set the parent  *  clock of CKO2 to OSC.                          */ clock-frequency = <24000000>; clocks = <&clks IMX5_CLK_CKO2>;                         assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>; assigned-clock-parents = <&clks IMX5_CLK_OSC>;                         status = "okay";                 };         }; }; And we create a basic driver to check what happened: static int smsc_usb3315_phy_probe(struct ulpi *ulpi) {         printk(KERN_ERR "Fabien: %s:%d-%s\n", __FILE__, __LINE__, __func__);         return 0; } static const struct of_device_id smsc_usb3315_phy_match[] = {         { .compatible = "smsc,usb3315-phy", },         { } }; MODULE_DEVICE_TABLE(of, smsc_usb3315_phy_match); static struct ulpi_driver smsc_usb3315_phy_driver = {         .probe = smsc_usb3315_phy_probe,         .driver = {                 .name = "smsc_usb3315_phy",                 .of_match_table = smsc_usb3315_phy_match,         }, }; module_ulpi_driver(smsc_usb3315_phy_driver); /*MODULE_ALIAS("platform:usb_phy_generic");*/ MODULE_AUTHOR("GE Healthcare"); MODULE_DESCRIPTION("SMSC USB 3315 ULPI Phy driver"); MODULE_LICENSE("GPL v2"); I checked that the driver is registered by drivers/usb/common/ulpi.c:__ulpi_register_driver successfully. But our probe function (smsc_usb3315_phy_probe) is never called. Our understanding is that we need to use the ulpi_bus instead of devm_usb_get_phy_by_phandle(&pdev- >dev, "fsl,usbphy", 0); in drivers/usb/chipidea/ci_hdrc_imx.c. Is our approach good? How can we use this bus from our controller probe function ? Thanks Fabien On Thu, 2017-04-20 at 16:50 +0800, Peter Chen wrote: > On Wed, Apr 19, 2017 at 06:14:13AM +0000, Peter Senna Tschudin wrote: > > We need the SMSC USB3315 clock and regulator to always be initialized. > > We also need the PHY driver to take the PHY out of reset. This patch > > extends the existing USB generic nop phy driver to include a new > > initialization path. > > > > A new compatible string "smsc,usb3315" is used to decide which > > initialization path to use. > > > > Hi Peter, > > This is an ULPI PHY, so it is not suitable using generic USB PHY. > Taking a look of drivers/phy/phy-qcom-usb-hs.c, you may have some > clues. > > Peter > > > CC: Peter Chen > > CC: Stephen Boyd > > CC: Fabien Lahoudere > > Signed-off-by: Peter Senna Tschudin > > --- > > > > This is a follow-up of previous discussion: > >   https://www.spinics.net/lists/linux-usb/msg146680.html > > > >  drivers/usb/phy/phy-generic.c | 33 +++++++++++++++++++++++++++++---- > >  drivers/usb/phy/phy-generic.h |  1 + > >  2 files changed, 30 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c > > index 89d6e7a..6ea9ce4 100644 > > --- a/drivers/usb/phy/phy-generic.c > > +++ b/drivers/usb/phy/phy-generic.c > > @@ -151,6 +151,9 @@ int usb_gen_phy_init(struct usb_phy *phy) > >   struct usb_phy_generic *nop = dev_get_drvdata(phy->dev); > >   int ret; > >   > > + if (nop->init_done) > > + return 0; > > + > >   if (!IS_ERR(nop->vcc)) { > >   if (regulator_enable(nop->vcc)) > >   dev_err(phy->dev, "Failed to enable power\n"); > > @@ -164,6 +167,8 @@ int usb_gen_phy_init(struct usb_phy *phy) > >   > >   nop_reset(nop); > >   > > + nop->init_done = true; > > + > >   return 0; > >  } > >  EXPORT_SYMBOL_GPL(usb_gen_phy_init); > > @@ -216,18 +221,29 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) > >   otg->host = host; > >   return 0; > >  } > > +int smsc_usb3315_init(struct usb_phy_generic *nop) > > +{ > > + /* > > +  * If the gpio for controlling reset state is not available, try again > > +  * later > > +  */ > > + if(!nop->gpiod_reset) > > + return -EPROBE_DEFER; > > + > > + return usb_gen_phy_init(&nop->phy); > > +} > >   > >  int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop, > >   struct usb_phy_generic_platform_data *pdata) > >  { > > + struct device_node *node = NULL; > >   enum usb_phy_type type = USB_PHY_TYPE_USB2; > >   int err = 0; > > - > >   u32 clk_rate = 0; > >   bool needs_vcc = false; > >   > >   if (dev->of_node) { > > - struct device_node *node = dev->of_node; > > + node = dev->of_node; > >   > >   if (of_property_read_u32(node, "clock-frequency", &clk_rate)) > >   clk_rate = 0; > > @@ -304,6 +320,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop, > >   nop->phy.otg->set_host = nop_set_host; > >   nop->phy.otg->set_peripheral = nop_set_peripheral; > >   > > + if(node && of_device_is_compatible(node, "smsc,usb3315")) { > > + err = smsc_usb3315_init(nop); > > + if (err) > > + return err; > > + } > > + > >   return 0; > >  } > >  EXPORT_SYMBOL_GPL(usb_phy_gen_create_phy); > > @@ -318,6 +340,10 @@ static int usb_phy_generic_probe(struct platform_device *pdev) > >   if (!nop) > >   return -ENOMEM; > >   > > + platform_set_drvdata(pdev, nop); > > + > > + nop->init_done = false; > > + > >   err = usb_phy_gen_create_phy(dev, nop, dev_get_platdata(&pdev->dev)); > >   if (err) > >   return err; > > @@ -346,8 +372,6 @@ static int usb_phy_generic_probe(struct platform_device *pdev) > >   return err; > >   } > >   > > - platform_set_drvdata(pdev, nop); > > - > >   return 0; > >  } > >   > > @@ -362,6 +386,7 @@ static int usb_phy_generic_remove(struct platform_device *pdev) > >   > >  static const struct of_device_id nop_xceiv_dt_ids[] = { > >   { .compatible = "usb-nop-xceiv" }, > > + { .compatible = "smsc,usb3315" }, > >   { } > >  }; > >   > > diff --git a/drivers/usb/phy/phy-generic.h b/drivers/usb/phy/phy-generic.h > > index 0d0eadd..db4ade6 100644 > > --- a/drivers/usb/phy/phy-generic.h > > +++ b/drivers/usb/phy/phy-generic.h > > @@ -14,6 +14,7 @@ struct usb_phy_generic { > >   struct gpio_desc *gpiod_vbus; > >   struct regulator *vbus_draw; > >   bool vbus_draw_enabled; > > + bool init_done; > >   unsigned long mA; > >   unsigned int vbus; > >  }; > > --  > > 2.9.3 > > > >