From patchwork Tue Oct 15 13:19:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 3045081 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 176FD9F243 for ; Tue, 15 Oct 2013 13:20:40 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B40FB202C8 for ; Tue, 15 Oct 2013 13:20:38 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B43B82017B for ; Tue, 15 Oct 2013 13:20:33 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VW4Y2-00018O-Dd; Tue, 15 Oct 2013 13:20:30 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VW4Xz-0007oD-RS; Tue, 15 Oct 2013 13:20:27 +0000 Received: from devils.ext.ti.com ([198.47.26.153]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VW4Xw-0007mh-79 for linux-arm-kernel@lists.infradead.org; Tue, 15 Oct 2013 13:20:25 +0000 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id r9FDJabA024820; Tue, 15 Oct 2013 08:19:36 -0500 Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id r9FDJaVb016989; Tue, 15 Oct 2013 08:19:36 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.2.342.3; Tue, 15 Oct 2013 08:19:35 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id r9FDJZsL030634; Tue, 15 Oct 2013 08:19:35 -0500 Date: Tue, 15 Oct 2013 08:19:34 -0500 From: Felipe Balbi To: Roger Quadros Subject: Re: [PATCH 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework Message-ID: <20131015131934.GG11380@radagast> References: <1378136591-7463-1-git-send-email-kishon@ti.com> <1378136591-7463-3-git-send-email-kishon@ti.com> <525814A9.7060106@ti.com> <525BB8C0.1070802@ti.com> <525BC5A9.3060904@ti.com> <20131015115741.GD11380@radagast> <525D30C2.8060208@ti.com> MIME-Version: 1.0 In-Reply-To: <525D30C2.8060208@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131015_092024_438492_1BF564A7 X-CRM114-Status: GOOD ( 33.67 ) X-Spam-Score: -4.4 (----) Cc: mark.rutland@arm.com, linux-doc@vger.kernel.org, tony@atomide.com, Vivek Gautam , s.nawrocki@samsung.com, ian.campbell@citrix.com, linux@arm.linux.org.uk, Kishon Vijay Abraham I , grant.likely@linaro.org, devicetree@vger.kernel.org, george.cherian@ti.com, rob@landley.net, pawel.moll@arm.com, swarren@wwwdotorg.org, rob.herring@calxeda.com, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, balbi@ti.com, bcousson@baylibre.com, galak@codeaurora.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: balbi@ti.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KHOP_BIG_TO_CC, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, On Tue, Oct 15, 2013 at 03:10:42PM +0300, Roger Quadros wrote: > >>>>> @@ -665,6 +669,9 @@ struct dwc3 { > >>>>> struct usb_phy *usb2_phy; > >>>>> struct usb_phy *usb3_phy; > >>>>> > >>>>> + struct phy *usb2_generic_phy; > >>>>> + struct phy *usb3_generic_phy; > >>>>> + > >>>>> void __iomem *regs; > >>>>> size_t regs_size; > >>>>> > >>>>> > >>> > >>> Do you have any suggestions on how to get only individual PHYs? like only > >>> usb2phy or usb3phy? > >> > >> My earlier understanding was that both PHYs are needed only if .speed is "super-speed" > >> and only usb2phy is needed for "high-speed". But as per Vivek's email it seems > >> Samsung's exynos5 SoC doesn't need usb2phy for "super-speed". > >> > >> So to keeps things flexible, I can propose the following approach > >> - if speed == 'high-speed' usb2phy must be present. usb3phy will be ignored if supplied. > >> - if speed == 'super-speed' usb3phy must be present and usb2phy is optional but must be > >> initialized if supplied. > >> - if speed is not specified, we default to 'super-speed'. > >> > >> Felipe, does this address the issue you were facing with OMAP5? > > > > on OMAP5 we cannot skip USB3 PHY initialization. But then it becomes a > > question of supporting a test feature (in OMAP5 case it would be cool to > > force controller to lower speeds for testing) or coping with a broken > > DTS. > > > > I don't think we can protect ourselves from all possible broken > configurations of DTS. > I would vote for simplicity and maximum flexibility. > > So IMO we should just depend on DTS to provide the phys that are > needed by the platform. > In the driver we initialize whatever PHY is provided and don't > complain if any or even all PHYs are missing. considering that DTS is an ABI, I really think eventually we *will* have broken DTBs burned into ROM and we will have to find ways to work with those too. Same thing already happens today with ACPI tables. > If this is not good enough then could you please suggest an > alternative? Thanks. The alternative would be to mandate nop xceiv for the "missing" PHY, but that doesn't solve anything, really, as DTS authors might still forget about the NOP xceiv and you can argue that forcing NOP xceiv would be a SW configuration. So, perhaps we go with the approach that all PHYs are optional, and here's my original patch which makes USB3 PHY optional: commit 979b84f96e4b7559b596b2933ae198aba267f260 Author: Felipe Balbi Date: Sun Jun 30 18:39:23 2013 +0300 usb: dwc3: core: make USB3 PHY optional If we want a port to work at any speed lower than Superspeed, it makes no sense to even initialize/power up the USB3 transceiver, provided it won't be used. We can use the oportunity to save some power and leave the superspeed transceiver powered off. There is at least one such case which is Texas Instruments' AM437x which has one of its USB3 ports without a matching USB3 PHY (that port is hardwired to work on USB2 only). Signed-off-by: Felipe Balbi what you guys are saying though, is that every PHY should be optional. Do we have any device which doesn't provide USB2 PHY, only USB3 ? Dude, that's so non-standard! USB *must* be backwards compatible so I'd expect USB2 PHY to always be available. diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 74f9cf0..7a5ab93 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -387,16 +387,34 @@ static int dwc3_probe(struct platform_device *pdev) if (node) { dwc->maximum_speed = of_usb_get_maximum_speed(node); - dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0); - dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1); + switch (dwc->maximum_speed) { + case USB_SPEED_SUPER: + dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0); + dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1); + break; + case USB_SPEED_HIGH: + case USB_SPEED_FULL: + case USB_SPEED_LOW: + dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0); + break; + } dwc->needs_fifo_resize = of_property_read_bool(node, "tx-fifo-resize"); dwc->dr_mode = of_usb_get_dr_mode(node); } else if (pdata) { dwc->maximum_speed = pdata->maximum_speed; - dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); - dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3); + switch (dwc->maximum_speed) { + case USB_SPEED_SUPER: + dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3); + break; + case USB_SPEED_HIGH: + case USB_SPEED_FULL: + case USB_SPEED_LOW: + dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + break; + } dwc->needs_fifo_resize = pdata->tx_fifo_resize; dwc->dr_mode = pdata->dr_mode; @@ -424,19 +442,21 @@ static int dwc3_probe(struct platform_device *pdev) return -EPROBE_DEFER; } - if (IS_ERR(dwc->usb3_phy)) { - ret = PTR_ERR(dwc->usb3_phy); + if (dwc->maximum_speed == USB_SPEED_SUPER) { + if (IS_ERR(dwc->usb3_phy)) { + ret = PTR_ERR(dwc->usb3_phy); - /* - * if -ENXIO is returned, it means PHY layer wasn't - * enabled, so it makes no sense to return -EPROBE_DEFER - * in that case, since no PHY driver will ever probe. - */ - if (ret == -ENXIO) - return ret; + /* + * if -ENXIO is returned, it means PHY layer wasn't + * enabled, so it makes no sense to return -EPROBE_DEFER + * in that case, since no PHY driver will ever probe. + */ + if (ret == -ENXIO) + return ret; - dev_err(dev, "no usb3 phy configured\n"); - return -EPROBE_DEFER; + dev_err(dev, "no usb3 phy configured\n"); + return -EPROBE_DEFER; + } } dwc->xhci_resources[0].start = res->start;