From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-1668029-1516798371-2-16863096086894519690 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, HEADER_FROM_DIFFERENT_DOMAINS 0.25, ME_NOAUTH 0.01, RCVD_IN_DNSWL_HI -5, T_RP_MATCHES_RCVD -0.01, LANGUAGES en, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='209.132.180.67', Host='vger.kernel.org', Country='US', FromHeader='com', MailFrom='org' X-Spam-charsets: plain='US-ASCII' X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: linux-usb-owner@vger.kernel.org ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1516798370; b=gjSI7y0AWSvJgapyNlounabJYVSvcBOYHdmcDuFAr68s4Mr ZwgvIf7RpTwdzWzOojB8mq4ZQgHMAAs+J3dC2lLyncgMP1H5KyU5CMoMZ/fLRsu2 sufT6KAvP0V/BP1gk3CQOv4JqCIKNfo6HySwb3kBfaLo5S+xtCrIGH/Afnm5ctLM uoU1aP/jdFiN8tW8l7YApKEoJJsi6gpXJj0AZUS04LBNCURIjXAFBCuWdsvNpLaF M4QXFYxUOkckSIbXdA3KGqCO8usRLqcNpsL7nNv9lqCmu4nukxqDF8A34oMy0s/f z0Ff2SLIMjHaVYLbfTL044AeCw5LF/f3VchdbgQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=date:from:to:subject:cc:in-reply-to :references:message-id:mime-version:content-type :content-transfer-encoding:sender:list-id; s=arctest; t= 1516798370; bh=McgAamg9ZEsd6MSTS69MWooI1l8Cl//uekxHTQ1W3cA=; b=L UGxT0WLroRDvrQ+PA95QUS3Yjz/AJy7/kWM983P+uC3J7PnkLfqDP6gV7tkr62EQ TeP2MRnecPV0fQFkI0/ZIpNaC+We6CPVVc+/SsRpWenxdte7Xhz2ZyKfbqBL1Yvk X+DJn6v9WqPVqLJdNPGHEKLwqhxxFKSpi8/HbvQJgdtWmhkWwUQqLxb0slQZ0saN aD+ky/l38Xk0ikBk8Hnn+noF4kj3r6LBC7q5279LfTvB1F2vRiOf8VsDUZt/g1+c cERwozm14DLo8fzsyBpfRuNXfdOu6iYkHFBUx40Qj8qWCUX8LZe+aDfjZSOVxq5e LtydO2wt76Tes30LkQuDQ== ARC-Authentication-Results: i=1; mx1.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,has-list-id=yes,d=none) header.from=socionext.com; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=linux-usb-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=socionext.com header.result=pass header_is_org_domain=yes Authentication-Results: mx1.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,has-list-id=yes,d=none) header.from=socionext.com; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=linux-usb-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=socionext.com header.result=pass header_is_org_domain=yes Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933605AbeAXMwc convert rfc822-to-8bit (ORCPT ); Wed, 24 Jan 2018 07:52:32 -0500 Received: from mx.socionext.com ([202.248.49.38]:17911 "EHLO mx.socionext.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933488AbeAXMwb (ORCPT ); Wed, 24 Jan 2018 07:52:31 -0500 Date: Wed, 24 Jan 2018 21:52:28 +0900 From: Kunihiko Hayashi To: Felipe Balbi Subject: Re: [PATCH 2/4] usb: dwc3: add dwc3 glue layer for UniPhier SoCs Cc: , Greg Kroah-Hartman , Masahiro Yamada , Rob Herring , "Mark Rutland" , , , , Jassi Brar , Masami Hiramatsu , Kishon Vijay Abraham I In-Reply-To: <87o9lklnwb.fsf@linux.intel.com> References: <1516712454-2915-3-git-send-email-hayashi.kunihiko@socionext.com> <87o9lklnwb.fsf@linux.intel.com> Message-Id: <20180124215228.ED43.4A936039@socionext.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 8BIT X-Mailer: Becky! ver. 2.70 [ja] Sender: linux-usb-owner@vger.kernel.org X-Mailing-List: linux-usb@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: Hello Felipe, Thank you for your comments. On Tue, 23 Jan 2018 15:12:36 +0200 wrote: > > Hi, > > Kunihiko Hayashi writes: > > Add a specific glue layer for UniPhier SoC platform to support > > USB host mode. It manages hardware operating sequences to enable multiple > > clock gates and assert resets, and to prepare to use dwc3 controller > > on the SoC. > > > > This patch also handles the physical layer that has same register space > > as the glue layer, because it needs to integrate initialziation sequence > > between glue and phy. > > > > In case of some SoCs, since some initialization values for PHY are > > included in nvmem, this patch includes the way to get the values from nvmem. > > > > It supports PXs2 and LD20 SoCs. > > > > Signed-off-by: Kunihiko Hayashi > > Signed-off-by: Motoya Tanigawa > > Signed-off-by: Masami Hiramatsu > > --- > > drivers/usb/dwc3/Kconfig | 9 + > > drivers/usb/dwc3/Makefile | 1 + > > drivers/usb/dwc3/dwc3-uniphier.c | 554 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 564 insertions(+) > > create mode 100644 drivers/usb/dwc3/dwc3-uniphier.c ...snip... > > + > > +static void dwc3u_ssphy_testio_write(struct dwc3u_priv *priv, int port, > > + u32 data) > > anything with sshphy or hsphy in the name should probably be part of a > PHY driver using drivers/phy/ framework. I can try to separate phy control from this driver. However, phy registers belongs to "dwc3-glue" IO map area (65b00000), and this area also contains a reset bit for "dwc3-core" hardware. Although the phy driver is called from dwc3-core driver, we need to deassert the reset bit before probing dwc3-core driver. As shown later, I think that it's difficut to determine the order of initializing the registers in this area. > > +static void dwc3u_vbus_disable(struct dwc3u_priv *priv) > > +{ > > + int i; > > + > > + for (i = 0; i < priv->nvbus; i++) { > > + dwc3u_maskwrite(priv, VBUS_CONTROL(i), > > + DRVVBUS_REG_EN | DRVVBUS_REG, > > + DRVVBUS_REG_EN | 0); > > + } > > +} > > drivers/regulator maybe? VBUS_CONTROL register is used for determing whether "dwc3-glue" hardware enables vbus connected with "regulator" hardware. The regulator driver should manage "regulator" hardware, and I don't think that the driver should manage this register. > > +static void dwc3u_reset_init(struct dwc3u_priv *priv) > > +{ > > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, 0); > > + usleep_range(1000, 2000); > > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, LINK_RESET); > > +} > > + > > +static void dwc3u_reset_clear(struct dwc3u_priv *priv) > > +{ > > + dwc3u_maskwrite(priv, RESET_CTL, LINK_RESET, 0); > > +} > > drivers/reset ? The reset driver manages "sysctrl" IO map area in our SoC. RESET_CTL register belongs to "dwc3-glue" IO map area, and the kernel can't access this area until enabling usb clocks and deasserting usb resets in "sysctrl". I think that "dwc3-glue" register control should be separated from "sysctrl". > > +static int dwc3u_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct device_node *node; > > + struct dwc3u_priv *priv; > > + struct resource *res; > > + struct clk *clk; > > + int i, nr_clks; > > + int ret = 0; > > + > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->data = of_device_get_match_data(dev); > > + if (WARN_ON(!priv->data)) > > + return -EINVAL; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->base = devm_ioremap_resource(dev, res); > > + if (IS_ERR(priv->base)) > > + return PTR_ERR(priv->base); > > + > > + priv->dev = dev; > > + > > + node = dev->of_node; > > + nr_clks = of_clk_get_parent_count(node); > > + if (!nr_clks) { > > + dev_err(dev, "failed to get clock property\n"); > > + return -ENODEV; > > + } > > + > > + priv->clks = devm_kcalloc(priv->dev, nr_clks, sizeof(struct clk *), > > + GFP_KERNEL); > > + if (!priv->clks) > > + return -ENOMEM; > > + > > + for (i = 0; i < nr_clks; i++) { > > + clk = of_clk_get(node, i); > > + if (IS_ERR(clk)) { > > + ret = PTR_ERR(clk); > > + goto out_clk_disable; > > + } > > + ret = clk_prepare_enable(clk); > > + if (ret < 0) { > > + clk_put(clk); > > + goto out_clk_disable; > > + } > > + priv->clks[i] = clk; > > + priv->nclks = i; > > + } > > + > > + priv->rst = devm_reset_control_array_get_optional_shared(priv->dev); > > + if (IS_ERR(priv->rst)) { > > + ret = PTR_ERR(priv->rst); > > + goto out_clk_disable; > > + } > > + ret = reset_control_deassert(priv->rst); > > + if (ret) > > + goto out_clk_disable; > > + > > + ret = dwc3u_init(priv); > > + if (ret) > > + goto out_rst_assert; > > + > > + platform_set_drvdata(pdev, priv); > > + > > + ret = of_platform_populate(node, NULL, NULL, priv->dev); > > + if (ret) > > + goto out_exit; > > with the stuff that should be using generic frameworks removed, this > looks like dwc3-of-simple.c, which you should be using. Before probing dwc3-core driver and accessing "dwc3-core" register, we need to do the following items in order: - enable clock and deassert resets in "sysctrl" with calling the clock/reset driver, so that the kernel can access "dwc3-glue" IO map area. - enable vbus and setup phy with the "dwc3-glue" register. - deassert "dwc3-core" reset with the "dwc3-glue" register, so that the kernel can access "dwc3-core" IO map area. The dwc3-of-simple driver only enables the clock driver before probing dwc3-core driver, and it seems that the driver have no ways to deassert the reset driver and manage "dwc3-glue" IO map area. Are there any ways to manage them before probing dwc3-core driver? Thank you, --- Best Regards, Kunihiko Hayashi