Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752605AbaD1EGQ (ORCPT ); Mon, 28 Apr 2014 00:06:16 -0400 Received: from mail-qc0-f182.google.com ([209.85.216.182]:37082 "EHLO mail-qc0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751872AbaD1EGN (ORCPT ); Mon, 28 Apr 2014 00:06:13 -0400 MIME-Version: 1.0 In-Reply-To: References: <1398424683-20168-2-git-send-email-gautam.vivek@samsung.com> Date: Mon, 28 Apr 2014 09:36:12 +0530 X-Google-Sender-Auth: e7fw8-6rl1oTskNwEkDq-4VWK2Q Message-ID: Subject: Re: [PATCH v2 1/2] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework From: Vivek Gautam To: Alan Stern Cc: Linux USB Mailing List , "linux-samsung-soc@vger.kernel.org" , "devicetree@vger.kernel.org" , linux-doc@vger.kernel.org, "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Greg KH , Felipe Balbi , Kukjin Kim , Kamil Debski , Jingoo Han Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Fri, Apr 25, 2014 at 8:06 PM, Alan Stern wrote: > On Fri, 25 Apr 2014, Vivek Gautam wrote: > >> Add support to consume phy provided by Generic phy framework. >> Keeping the support for older usb-phy intact right now, in order >> to prevent any functionality break in absence of relevant >> device tree side change for ohci-exynos. >> Once we move to new phy in the device nodes for ohci, we can >> remove the support for older phys. >> >> Signed-off-by: Vivek Gautam >> Cc: Jingoo Han >> Cc: Alan Stern > > >> +static int exynos_ohci_phyg_on(struct phy *phy[]) >> +{ >> + int i; >> + int ret = 0; >> + >> + for (i = 0; ret == 0 && i < PHY_NUMBER; i++) >> + if (phy[i]) >> + ret = phy_power_on(phy[i]); >> + if (ret) >> + for (i--; i >= 0; i--) >> + if (phy[i]) >> + phy_power_off(phy[i]); >> + >> + return ret; >> +} >> + >> +static int exynos_ohci_phyg_off(struct phy *phy[]) >> +{ >> + int i; >> + int ret = 0; >> + >> + for (i = 0; ret == 0 && i < PHY_NUMBER; i++) >> + if (phy[i]) >> + ret = phy_power_off(phy[i]); >> + >> + return ret; >> +} > > You probably shouldn't break out of this loop if ret is nonzero; you > should continue to power off the remaining phys. ok, will remove the 'ret' check in for loop. > > I'd be inclined to put these two routines directly into > exynos_ohci_phy_enable() and exynos_ohci_phy_disable(), since they > aren't used anywhere else. Sure, will make these routines as a part of exynos_ohci_phy_enable() and exynos_ohci_phy_disable(). > >> @@ -151,6 +253,7 @@ skip_phy: >> >> fail_add_hcd: >> exynos_ohci_phy_disable(pdev); >> + exynos_ohci_phyg_off(exynos_ohci->phy_g); > > Why did you add this line? It doesn't do anything useful, because > exynos_ohci_phy_disable() already calls exynos_ohci_phyg_off(). Ah ! my bad, will remove this. -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/