Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968422AbdDSRXo (ORCPT ); Wed, 19 Apr 2017 13:23:44 -0400 Received: from mail-lf0-f50.google.com ([209.85.215.50]:34620 "EHLO mail-lf0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966617AbdDSRXm (ORCPT ); Wed, 19 Apr 2017 13:23:42 -0400 Subject: Re: [RFC] usb-phy-generic: Add support to SMSC USB3315 To: Peter Senna Tschudin References: <20170419061413.20961-1-peter.senna@collabora.com> <20170419102414.GA2734@collabora.com> Cc: Peter Chen , Stephen Boyd , Fabien Lahoudere , Felipe Balbi , Greg Kroah-Hartman , "open list:USB PHY LAYER" , open list From: Sergei Shtylyov Organization: Cogent Embedded Message-ID: Date: Wed, 19 Apr 2017 20:23:38 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170419102414.GA2734@collabora.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2137 Lines: 65 On 04/19/2017 01:24 PM, 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. >>> >>> 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 >> [...] >>> @@ -216,18 +221,29 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) >>> otg->host = host; >>> return 0; >>> } >> >> Need empty line here. >> >>> +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) >> >> You hadn't run the patch thru scripts/checkpatch.pl before posting -- >> need space between *if* and (. >> >> [...] >>> @@ -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")) { >> >> Same here. >> >> [...] >> >> MBR, Sergei > > Thank you for the review Sergei! Should I send V2 of this RFC fixing > these issues or wait for comments on the validity of this approach? Wait, of course, if this is RFC... WBR, Sergei