From patchwork Mon Nov 21 21:10:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Lamparter X-Patchwork-Id: 697407 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tN1Zy3VQYz9sQw for ; Tue, 22 Nov 2016 08:15:06 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=googlemail.com header.i=@googlemail.com header.b="mDkdOqaQ"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3tN1Zy1zB8zDvyr for ; Tue, 22 Nov 2016 08:15:06 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=googlemail.com header.i=@googlemail.com header.b="mDkdOqaQ"; dkim-atps=neutral X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from mail-wj0-x244.google.com (mail-wj0-x244.google.com [IPv6:2a00:1450:400c:c01::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3tN1V225wfzDvvq for ; Tue, 22 Nov 2016 08:10:50 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=googlemail.com header.i=@googlemail.com header.b="mDkdOqaQ"; dkim-atps=neutral Received: by mail-wj0-x244.google.com with SMTP id kp2so4552333wjc.0 for ; Mon, 21 Nov 2016 13:10:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=gp6oOyo7Q5pFVWLy7TOSLiXWwjkKc2PkAbdW6O6E52M=; b=mDkdOqaQh2Vc5GTejmB3JoLCPuTpgkyShETWjMivzSq471UCsYUn7UsuEh7AJvoG/E Mj2k9PeF8XVWg2t6GkL60LPjmnlfmvBLPaZ3pADNhjxifykqpLdMfqSpS4TzT6+aIC7R TFSlkoK9cBMa/cv5OhpmOun17ZYKVBZTwBegvqywwTQ0iT4NsmHU87sGDV08vro8V67h h55biHosCpjhZpOonTHC8Ic/uF+qurytasnouOmmpbnFmVpTs8dazN1XIiWZ12VmAmxO NyFlbDCyaqFcbrm0AIJubVxcOW8tiWOVHPrIJyvyLRJNMtK8vMHVdVVkVAeMSZO07lhH 4SkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=gp6oOyo7Q5pFVWLy7TOSLiXWwjkKc2PkAbdW6O6E52M=; b=ZWaL0y2MGiOemJPbexcc6+c4yVH4zKSM73DGRs2dqPGcpLIlz/azI7cv+8ho6ghu1N ubHOjOS49Q2d/A09J3BOPoal/jIUu1Sc3dUWkFVRI6H0GC7MgUt6FwZtmmxZP/Q102R4 GFrI1PJ1tFD7qqUXEBmJQ8cnl1PmEFv38VoY/J5a9bbfXIS4VOY/x6I6DdQzi9KOjflB lUtRUzhoogpM00vNKRUlFMAo5taomSWZrcQSYYp8MPmcrDL8vNedbGAzMir3Q5zfc4CS zb0AfvbQVhS8wLjO+KqNw+S2ZOI7EBIG7/MSiBRLFwBfaITC9qhmi/IB5pyd4+b3Iatj hzCQ== X-Gm-Message-State: AKaTC00WQWGhWDIwYTFiGJmF5dMeEP/Q9cBv7+5erC4dHTjFHEuNp/pS58K93ft9EkE5Vg== X-Received: by 10.194.75.227 with SMTP id f3mr10504699wjw.19.1479762645628; Mon, 21 Nov 2016 13:10:45 -0800 (PST) Received: from debian64.daheim (pD9F89726.dip0.t-ipconnect.de. [217.248.151.38]) by smtp.gmail.com with ESMTPSA id r7sm27031210wjp.43.2016.11.21.13.10.44 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 21 Nov 2016 13:10:44 -0800 (PST) From: Christian Lamparter X-Google-Original-From: Christian Lamparter Received: from localhost.daheim ([127.0.0.1] helo=debian64.localnet) by debian64.daheim with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.88) (envelope-from ) id 1c8vrQ-0000U5-3G; Mon, 21 Nov 2016 22:10:44 +0100 To: John Youn Subject: Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst Date: Mon, 21 Nov 2016 22:10:43 +0100 Message-ID: <2016796.9qgSSGQn9z@debian64> User-Agent: KMail/5.2.3 (Linux/4.9.0-rc4-wt+; KDE/5.27.0; x86_64; ; ) In-Reply-To: References: <6396482.jQaH1ArAfZ@debian64> MIME-Version: 1.0 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefan Wahren , Rob Herring , "devicetree@vger.kernel.org" , "linux-usb@vger.kernel.org" , Paul Mackerras , Christian Lamparter , Mark Rutland , linuxppc-dev@lists.ozlabs.org, Felipe Balbi Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Hello John, On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote: > On 11/18/2016 12:18 PM, Christian Lamparter wrote: > > On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote: > >> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote: > >>> Hi John, > >>> > >>> Am 17.11.2016 um 00:47 schrieb John Youn: > >>>> Add the "snps,ahb-burst" binding and read it in. > >>>> > >>>> This property controls which burst type to perform on the AHB bus as a > >>>> master in internal DMA mode. This overrides the legacy param value, > >>>> which we need to keep around for now since several platforms use it. > >>>> > >>>> Some platforms may see better or worse performance based on this > >>>> value. The HAPS platform is one example where all INCRx have worse > >>>> performance than INCR. > >>>> > >>>> Other platforms (such as the Canyonlands board) report that the default > >>>> value causes system hangs. > >>>> > >>>> Signed-off-by: John Youn > >>>> Cc: Christian Lamparter > >>>> --- > >>>> Documentation/devicetree/bindings/usb/dwc2.txt | 2 + > >>>> drivers/usb/dwc2/core.h | 9 +++++ > >>>> drivers/usb/dwc2/params.c | 56 ++++++++++++++++++++++++++ > >>>> 3 files changed, 67 insertions(+) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt > >>>> index 6c7c2bce..9e7b4b4 100644 > >>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt > >>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt > >>> > >>> according to Documentation/devicetree/bindings/submitting-patches.txt > >>> this change should be a separate patch. > >>> > >>>> @@ -26,6 +26,8 @@ Optional properties: > >>>> Refer to phy/phy-bindings.txt for generic phy consumer properties > >>>> - dr_mode: shall be one of "host", "peripheral" and "otg" > >>>> Refer to usb/generic.txt > >>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are: > >>>> + "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4". > >>> > >>> This doesn't apply in case of the bcm2835. I would prefer this option is > >>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on > >>> this platform"). > >> > >> Also, perhaps you should allow that the compatible string can define the > >> default. > >> > > I hoped you would say that :). > > > > I've attached a patch (on top of John Youn changes) that does > > just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR > > value into the .data, if that's a problem, I can certainly > > respin the patch and put it in a dedicated struct. > > > > Regards > > > > Christian > > --- > > From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001 > > From: Christian Lamparter > > Date: Fri, 18 Nov 2016 21:03:19 +0100 > > Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg > > > > This patch adds a of_device_id table which can be used by > > existing devices to supply a ahb-burst value for the platform > > without having to add a "snps,ahb-burst" entry to the dts. > > > > Note: Adding new devices to this table is discouraged. > > please consider adding the "snps,ahb-burst" property > > with the correct configuration to your device tree > > file instead. > > > > Signed-off-by: Christian Lamparter > > --- > > drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > > index e0fc9aa..51be266 100644 > > --- a/drivers/usb/dwc2/params.c > > +++ b/drivers/usb/dwc2/params.c > > @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = { > > [GAHBCFG_HBSTLEN_INCR16] = "INCR16", > > }; > > > > +/* > > + * This table provides AHB burst configuration for existing > > + * device tree bindings that work poorly with the default setting. > > + * > > + * Note: Adding new devices to this table is discouraged. > > + * please consider adding the "snps,ahb-burst" property > > + * with the correct configuration to your device tree > > + * file instead. > > + */ > > +static const struct of_device_id dwc2_compat_ahb_bursts[] = { > > + { > > + .compatible = "amcc,dwc-otg", > > + .data = (void *) GAHBCFG_HBSTLEN_INCR16, > > + }, > > +}; > > + > > static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg) > > { > > struct device_node *node = hsotg->dev->of_node; > > @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg) > > ret = device_property_read_string(hsotg->dev, > > "snps,ahb-burst", &str); > > if (ret < 0) { > > + const struct of_device_id *match; > > + > > + match = of_match_node(dwc2_compat_ahb_bursts, node); > > + if (match) > > + ret = (int)match->data; > > + > > return ret; > > } else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) { > > dev_warn(hsotg->dev, > > > I'd prefer if you use the binding which requires no extra code in > dwc2. I'm fine with either option. However it think that this would require that either Mark or Rob would allow an exception to the "keep existing dts the way they are) and ack the following change to the canyonlands.dts. In that case I wouldn't need the overwrite in dwc2_get_property_ahb_burst. Regards, Christian --- From e78604cb0b8ea8db277ef9bf321a613f8e0c7129 Mon Sep 17 00:00:00 2001 From: Christian Lamparter Date: Mon, 21 Nov 2016 21:46:19 +0100 Subject: [PATCH] powerpc/dts: set snps,ahb-burst to INCR16 The dwc2 driver defaults to INCR4 which can cause a system hang when the USB and SATA is used concurrently. Note: This patch requires: "usb: dwc2: add amcc,dwc-otg support" (which already landed in the usb subsystem queue) and "usb: dwc2: Add AHB burst configuration" Signed-off-by: Christian Lamparter --- arch/powerpc/boot/dts/canyonlands.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts index 0d6ac92..90db712 100644 --- a/arch/powerpc/boot/dts/canyonlands.dts +++ b/arch/powerpc/boot/dts/canyonlands.dts @@ -179,6 +179,7 @@ USBOTG0: usbotg@bff80000 { compatible = "amcc,dwc-otg"; + snps,ahb-burst = "INCR16"; reg = <0x4 0xbff80000 0x10000>; interrupt-parent = <&USBOTG0>; #interrupt-cells = <1>;