From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751223AbdJDEgp (ORCPT ); Wed, 4 Oct 2017 00:36:45 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:52889 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750871AbdJDEgn (ORCPT ); Wed, 4 Oct 2017 00:36:43 -0400 X-ME-Sender: X-Sasl-enc: aIbKCQCgUPj31KALPC/qLCQrGyewvJwTURSexbZR85fY 1507091801 Message-ID: <1507091793.5452.10.camel@aj.id.au> Subject: Re: [PATCH 3/8] ARM: dts: aspeed: Add I2C buses From: Andrew Jeffery To: Brendan Higgins , Joel Stanley Cc: Rob Herring , Mark Rutland , Russell King , devicetree , linux-arm-kernel@lists.infradead.org, Linux Kernel Mailing List , Rick Altherr , =?ISO-8859-1?Q?C=E9dric?= Le Goater , linux-aspeed@lists.ozlabs.org Date: Wed, 04 Oct 2017 15:06:33 +1030 In-Reply-To: References: <20170928075149.8154-1-joel@jms.id.au> <20170928075149.8154-4-joel@jms.id.au> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-wN7roEnls3pshcsqkUzh" X-Mailer: Evolution 3.22.6-1ubuntu1 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-wN7roEnls3pshcsqkUzh Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2017-09-28 at 11:35 -0700, Brendan Higgins wrote: > > On Thu, Sep 28, 2017 at 12:51 AM, Joel Stanley wrote: > > Now with an upstream i2c bus driver, we can add=C2=A0=C2=A0the 14 i2c b= uses that > > exist in ASPEED G4 and G5 generation SoCs. > >=C2=A0 > > It also adds aliases for the 14 built-in I2C busses to ensure userspace > > sees the numbering staring from zero and counting up. > >=C2=A0 > > Signed-off-by: Joel Stanley > > --- >=C2=A0 > > Reviewed-by: Brendan Higgins >=C2=A0 > nit: can we make the i2c labels and the pinctrl labels match? >=C2=A0 > For example: >=C2=A0 > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0i2c13: i2c-bus@480 { > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0#address-cells =3D <1>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0#size-cells =3D <0>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0#interrupt-cells =3D <1>; > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0reg =3D <0x480 0x40>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0compatible =3D "aspeed,ast2500-i2c-bus"; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0clocks =3D <&clk_apb>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0bus-frequency =3D <100000>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0interrupts =3D <13>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0interrupt-parent =3D <&i2c_ic>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0pinctrl-names =3D "default"; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0pinctrl-0 =3D <&pinctrl_i2c14_default>; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0status =3D "disabled"; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}; >=C2=A0 > "i2c13" has a pinctrl-0 of "pinctrl_i2c14_default" >=C2=A0 > I know that pinctrl_i2c14_default is consistent with the function and > groups it uses, but I would like to see them all be consistent at some > point in the future. I doubt we'll be making them consistent: How the i2c devices are aliased by Linux are independent of how they're labelled in hardware or the datasheet. For sanity's sake pinmux's functions and groups follow the datasheet (thoug= h if you want to maintain the Aspeed pinctrl drivers, be my guest :D) We could rename the pinmux nodes to offset them from the function and group= , but it feels less self-contained if the nodes are named at the whim of the device requesting the function if there's no further configuration on top o= f the function and group*. It also makes it harder to auto-explode the functi= on name into the node definitions as we'd then need some further mangling to m= ake things line up. * The pinctrl devicetree bindings allow for definition of nested nodes to specify collections and combinations of pin function and group, and pin configuration such as drive-strength. Thus the nodes can be a lot more comp= lex than what we've defined up until now, but what we've got enables a lot of pretty straight-forward uses-cases. We could alias the i2c devices in line with the hardware, but that breaks t= he convention of starting device numbering at 0. Ultimately Joel and I chose to make the break at the point where the i2c de= vice requests the mux function. It's up to the device to know what hardware mux configuration it needs and that's what this approach documents. The inconsistency is irritating but I don't feel like it's the end of the world= , and I think the alternatives are (slightly) worse. Cheers, Andrew --=-wN7roEnls3pshcsqkUzh Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQIcBAABCgAGBQJZ1GVRAAoJEJ0dnzgO5LT58D8P/373Y2qg+iX5cpGawE7Bs3RS XmYYAoMnz1s8nmJJZP9QW6HvIxRFGSwmhweGKtnFCpJQxr6BDaUarD5qoOc7+EAL QN50K1+gyVo8CO4BEWFinw6c5xMKtudKZoWfBLFLJE7fGTkCpNGz/E86u3Zt7BU1 w+cJyxIVeHO6Mf/duU00eIFJO7w5xU2VjM01jNMz96R7l6NmOsX7tSZuJl9+Avt6 Xv7WzgnUc49UHupP6vquf/0fBwgpfcUTj0r6JMD8aGsDuqCplTTpbXX0iltklz2b rOa30/ItKt1AgsX6+WSCRTnaD6v9M/Fr2Zm3dicHFwP5SAewMLSl23bCFkGVIwhX yvbZMHwIpoxBtFeG/nYXVgdF29z+/HK8H5Du2+AzDNePH1uexVB4ZR8wOm2YdQyF nUWVfa7Di0kJcBg+qFn+PH9rZwHcpBfrPxrxaCssYH68okV6qnRwTciJfP3+hana NOvxmuDbBrB1ebQcONAS+Orv0wBebL+iGtU4qLl8LYHqbwozzHk56CLQyyrsvq0Y scJxslcHWcj6Z/nrJpMzMZUafJ4+2DoinJyH9pbhDhjs2qcFBzG/+23UCagSqefC BBre9s8hGWIkzWR8N6aHXAYFxvcptVyboMz0cCFTrmfhwMkNjJHHBbLX5Hy1fZTd TNT9+5m8orAgxIBxpmIl =SE2F -----END PGP SIGNATURE----- --=-wN7roEnls3pshcsqkUzh--