This message generated a parse failure. Raw output follows here. Please use 'back' to navigate. From devnull@lkml.org Thu Apr 25 14:20:43 2024 >From mailfetcher Thu Nov 10 00:51:15 2016 Envelope-to: lkml@grols.ch Delivery-date: Thu, 10 Nov 2016 00:51:14 +0100 Received: from srv.grols.ch [5.172.41.101] by 7a0960bb2a7f with IMAP (fetchmail-6.3.26) for (single-drop); Thu, 10 Nov 2016 00:51:15 +0100 (CET) Received: from vger.kernel.org ([209.132.180.67]) by home.grols.ch with esmtp (Exim 4.87) (envelope-from ) id 1c4ce9-0000h5-6O for lkml@grols.ch; Thu, 10 Nov 2016 00:51:14 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753532AbcKIXub (ORCPT ); Wed, 9 Nov 2016 18:50:31 -0500 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:43818 "EHLO out4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750927AbcKIXua (ORCPT ); Wed, 9 Nov Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 5DA4A20A16; Wed, 9 Nov 2016 18:50:28 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute7.internal (MEProxy); Wed, 09 Nov 2016 18:50:28 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=aj.id.au; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=YksRF29R43H1JucgMJvmOLokzlg=; b=VO4+ DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=smtpout; bh=YksRF29R43H1Ju cg X-ME-Sender: X-Sasl-enc: giKITkWy5IXvqKzYyULigp2b1znIqN8zc50qYGpIGrN4 1478735427 Received: from [10.104.0.15] (unknown [203.0.153.9]) by mail.messagingengine.com (Postfix) with ESMTPA id ACFDEF29CD; Wed, 9 Nov 2016 18:50:24 -0500 (EST) Message-Id: <1478735416.6206.10.camel@aj.id.au> Subject: Re: [PATCH v2 4/6] pinctrl: aspeed: Read and write bits in LPCHC and GFX controllers From: Andrew Jeffery To: Rob Herring Cc: Lee Jones , Linus Walleij , Joel Stanley , Mark Rutland , linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux Date: Thu, 10 Nov 2016 10:20:16 +1030 In-Reply-To: <20161109182632.etsvezgfu7nhtl55@rob-hp-laptop> References: <1478097481-14895-1-git-send-email-andrew@aj.id.au> <1478097481-14895-5-git-send-email-andrew@aj.id.au> <20161109182632.etsvezgfu7nhtl55@rob-hp-laptop> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-RkBzIy0S9LKstNLrgLeV" X-Mailer: Evolution 3.22.1-0ubuntu2 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-Id: X-Mailing-List: linux-kernel@vger.kernel.org Received-SPF: none client-ip=209.132.180.67; envelope-from=linux-kernel-owner@vger.kernel.org; helo=vger.kernel.org X-Spam-Score: 5.2 X-Spam-Score-Bar: +++++ X-Spam-Action: greylist X-Spam-Report: Action: greylist Symbol: SUSPICIOUS_RECIPS(1.50) Symbol: R_SPF_NA(0.00) Symbol: FORGED_SENDER(0.30) Symbol: PRECEDENCE_BULK(0.00) Symbol: MIME_GOOD(-0.20) Symbol: FORGED_RECIPIENTS(2.00) Symbol: BAYES_SPAM(1.81) Symbol: R_DKIM_ALLOW(-0.20) Message-ID: 14 --=-RkBzIy0S9LKstNLrgLeV Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2016-11-09 at 12:26 -0600, Rob Herring wrote: > On Thu, Nov 03, 2016 at 01:07:59AM +1030, Andrew Jeffery wrote: > > The System Control Unit IP block in the Aspeed SoCs is typically where > > the pinmux configuration is found, but not always. A number of pins > > depend on state in one of LPC Host Control (LPCHC) or SoC Display > > Controller (GFX) IP blocks, so the Aspeed pinmux drivers should have th= e > > means to adjust these as necessary. > >=20 > > We use syscon to cast a regmap over the GFX and LPCHCR blocks, which is > > used as an arbitration layer between the relevant driver and the pinctr= l > > subsystem. The regmaps are then exposed to the SoC-specific pinctrl > > drivers by phandles in the devicetree, and are selected during a mux > > request by querying a new 'ip' member in struct aspeed_sig_desc. > >=20 > > > > Signed-off-by: Andrew Jeffery > > --- > > Since v1: > >=20 > > The change is now proactive: instead of reporting that we need to flip = bits in > > controllers we can't access, the patch provides access via regmaps for = the > > relevant controllers. The implementation also splits out the IP block I= D into > > its own variable rather than packing the value into the upper bits of t= he reg > > member of struct aspeed_sig_desc. This drives some churn in the diff, b= ut I've > > tried to minimise it. > >=20 > > =C2=A0.../devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 50 +++++++++= ++++--- > > =C2=A0drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0| 18 +++--- > > =C2=A0drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0| 39 ++++++++++--- > > =C2=A0drivers/pinctrl/aspeed/pinctrl-aspeed.c=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0| 66 +++++++++++++--------- > > =C2=A0drivers/pinctrl/aspeed/pinctrl-aspeed.h=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0| 32 ++++++++--- > > =C2=A05 files changed, 144 insertions(+), 61 deletions(-) > >=20 > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.t= xt b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt > > index 2ad18c4ea55c..115b0cce6c1c 100644 > > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt > > @@ -4,12 +4,19 @@ Aspeed Pin Controllers > > =C2=A0The Aspeed SoCs vary in functionality inside a generation but hav= e a common mux > > =C2=A0device register layout. > > =C2=A0 > > -Required properties: > > -- compatible : Should be any one of the following: > > - "aspeed,ast2400-pinctrl" > > - "aspeed,g4-pinctrl" > > - "aspeed,ast2500-pinctrl" > > - "aspeed,g5-pinctrl" > > +Required properties for g4: > > +- compatible :=C2=A0 Should be any one of the following: > > + "aspeed,ast2400-pinctrl" > > + "aspeed,g4-pinctrl" > > + > > +Required properties for g5: > > +- compatible :=C2=A0 Should be any one of the following: > > + "aspeed,ast2500-pinctrl" > > + "aspeed,g5-pinctrl" > > + > > +- aspeed,external-nodes: A cell of phandles to external controller nod= es: > > + 0: compatible with "aspeed,ast2500-gfx", "syscon" > > + 1: compatible with "aspeed,ast2500-lpchc", "syscon" > > =C2=A0 > > =C2=A0The pin controller node should be a child of a syscon node with t= he required > > =C2=A0property: > > @@ -47,7 +54,7 @@ RGMII1 RGMII2 RMII1 RMII2 SD1 SPI1 SPI1DEBUG SPI1PASS= THRU TIMER4 TIMER5 TIMER6 > > =C2=A0TIMER7 TIMER8 VGABIOSROM > > =C2=A0 > > =C2=A0 > > -Examples: > > +g4 Example: > > =C2=A0 > > =C2=A0syscon: scu@1e6e2000 { > > =C2=A0 compatible =3D "syscon", "simple-mfd"; > > @@ -63,5 +70,34 @@ syscon: scu@1e6e2000 { > > =C2=A0 }; > > =C2=A0}; > > =C2=A0 > > +g5 Example: > > + > > +apb { > > + gfx: display@1e6e6000 { > > + compatible =3D "aspeed,ast2500-gfx", "syscon"; > > + reg =3D <0x1e6e6000 0x1000>; > > + }; > > + > > + lpchc: lpchc@1e7890a0 { > > + compatible =3D "aspeed,ast2500-lpchc", "syscon"; > > + reg =3D <0x1e7890a0 0xc4>; > > + }; > > + > > + syscon: scu@1e6e2000 { > > + compatible =3D "syscon", "simple-mfd"; > > + reg =3D <0x1e6e2000 0x1a8>; > > + > > + pinctrl: pinctrl { >=20 > Why the single child node here? Doesn't look like any reason for it in=C2= =A0 > the example.=C2=A0 The SCU contains other miscellaneous functionality besides pinctrl registers, but that's not relevant for the pinctrl bindings. This is an example for the g5 SoCs demonstrating use of the aspeed,external-nodes property, which isn't required for the g4 and is why I split the examples. Maybe I should split out the bindings for each SoC generation into separate files? >=20 > > + compatible =3D "aspeed,g5-pinctrl"; > > + aspeed,external-nodes =3D <&gfx, &lpchc>; You didn't comment on my approach here, but I'm interested in feedback. I've gone the route of fixed ordering of the phandles, but there are two other approaches: 1. Relax the fixed ordering requirement by adding an "aspeed,external- node-names" property and requiring correlated indices between them 2. Using separate properties for each required external node Approach 1 seems pretty idiomatic and only crossed my mind after I'd sent the patch. Approach 2 seems a bit ugly as the number of properties scales with the number of controllers participating in the pinmux configuration. Something that also wasn't clear to me was whether I need the "aspeed" prefix on the property name. What's the convention here? Do I need it in this case? Cheers, Andrew > > + > > > > + pinctrl_i2c3_default: i2c3_default { > > > > + function =3D "I2C3"; > > > > + groups =3D "I2C3"; > > > > + }; > > > > + }; > > > > + }; > > +}; > > + > > =C2=A0Please refer to pinctrl-bindings.txt in this directory for detail= s of the > > =C2=A0common pinctrl bindings used by client devices. --=-RkBzIy0S9LKstNLrgLeV Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQIcBAABCgAGBQJYI7Y4AAoJEJ0dnzgO5LT5Z3UP/15q4VHuVMo7RqBR2owlVspN Cg2qn5w1k4QOFyCWfrUQXuFhUZ47PmROEPIBFrfGqdTZY+/sH7WNO8XA+D8GjcKg p25zaJrDixNQ/jj+Wb3yUvD/Qc1guUDMVfzgvPVCOWClHrYewyp0U1NsnBVAW+Gb 64LfxWlpMirg9cD9jAddgOxzSmdBsufsZZx3qQL15PsOoDolP7SberTallX9fsRM FckoNTxCkPMYjBo9uADzslbV5wDWu+93QfF0AC/PXO8hJ65LX/CBQjhBaD+u5H2p ZCmz7zEmydp7kgHuM1Sq4o5mgRx7LfP2Q84i3BWGyYX8xt52Tj0sYhHN+4030MO7 MJj9e8xQK7QkP3lrd7RppSGEEz0FUeoKQpkvM8Kg/sR+tQfLbcp9V9KIH+sSQxS5 8zPOB1GD39R78ZY+W44ZuroDpcjSLDDlZ+wfcsX8dilTL3CAgghu5DPM8icMsndJ vRI0hRoRfzSjlpHE2ByKdAT0Wb05soA3yOpUnAlG9WmkJt6H5QJwckDpVEkQLByu 0VkDPtNwc8bPGxc6a0diT0Wf0T+NjDrOrxpLz49WLZ9tnBliLuRAtV65AwOQaJNm Zz38DAbEvgpba6gairQFwpWYR+GM4pgqWuPEiXzQlcqIqJMDvGxrS8xY2K7L088T UPjW7mp5IzXVFWTm3Ip5 =A6TW -----END PGP SIGNATURE----- --=-RkBzIy0S9LKstNLrgLeV--