From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751280AbeAPV65 (ORCPT + 1 other); Tue, 16 Jan 2018 16:58:57 -0500 Received: from mout.gmx.net ([212.227.15.19]:56730 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796AbeAPV6y (ORCPT ); Tue, 16 Jan 2018 16:58:54 -0500 Date: Tue, 16 Jan 2018 22:58:47 +0100 From: Jonathan =?utf-8?Q?Neusch=C3=A4fer?= To: Linus Walleij Cc: Jonathan =?utf-8?Q?Neusch=C3=A4fer?= , "linux-kernel@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org list" , linux-gpio@vger.kernel.org, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Albert Herranz , Segher Boessenkool Subject: Re: [PATCH 3/6] gpio: Add GPIO driver for Nintendo Wii Message-ID: <20180116215847.qlygkbnecni7bzog@latitude> References: <20180115031401.19577-1-j.neuschaefer@gmx.net> <20180115031401.19577-4-j.neuschaefer@gmx.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="exhxaxcsmwxhn2ch" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-Provags-ID: V03:K0:0nCNeRB6xs26K7UOe12nZDcW2sV4YmVpg3LGXgv//G/0TA0xJjD p/p7kQXoPxsmRK8uaB0HahA2ki3pJdemDvaqcXCgUiIwdeKX/KVgG1rsMv7Wjp0nq05C3No i/5djV/WB4508tQT1rVFDX6M+BmPKfQJxSYvKv0BJJIdCSvd+JHCKPHmgMfAERcJ5xmex5W w0eQUBlVrN2Bhi5GdTbpQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:46G4/BRd8Xc=:RvPPFB9RoKe+Vs7xO8mgca vTb+nNq71HkQ+xcdZlX06fZ49pgRe//2bYStp5jnQWkgxMN7WppbsccAT7mw5dBTVzLm1xj6C p1uQ9pdZXFOZIbFl6B9nBD3sCLRjfXmrqh4G6/1vZDHp+Oj/tWIxoQ2NlaiN/xVrSOs6YenCn fBi6GqK5G0iUoAxzK16H6ElFO2b1LvAqYfrQEBBqN3aUChFvboSU5UyTueMTSJZXhU1F87FUl LDi9eWXD7Ucg9519tD6b6KPgQQIeIueoo/k+1mQf9C4RRlsYop8fXlFCKkLmAG+WH3XOJ2czn /JEMYW+CZWFvwBluhgTilhyVZ9u/o0WeLNZLZZlxClQhcNyjBil92+e9mQ2H9GRNTSHC9JYBh AU1P/j0108CV2IBNHvjwCfGk6Txg34fl6fgOTS6nCnbLzkjOFH9i1igk3y0hbOY7p/S6h6UFp UFo/TKRjVRSKbPsz2+a71lSzacp9HnFrszUZkkjUbGI1Vbw6XBMLpRqb1t7wK9dnqzGtrP61W Ay8lkjvj3T0ZyTv6TlO2vhm74n09wdS7tcOQWiwsA7e2UImlYlth/k6H6ijKXAjvDr8kHFHTJ dbtLCsO/XWEvlJGM55fDpc09waDIHwoBrVI7ZqIHRTaF/n+PabB7ncUzPAhJ/3oEJ0D6gf1ah t0NmnsFw7tqRUJrGx+IjQa/2dfR7xm0vzjyiXNMzaIsjieUNiKQCkVHT+hoNUw725b9XHApdO zo36JBxN/YzYXP7Pu0FfsAU/wEcxlTrBO5v1wXit0IoBZ46sdES2gWusKA7/wy1f639Snertl hsl+slMrK7yPVzacIiM3/3trI9wYQ== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: --exhxaxcsmwxhn2ch Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 16, 2018 at 10:42:54AM +0100, Linus Walleij wrote: > On Mon, Jan 15, 2018 at 4:13 AM, Jonathan Neusch=C3=A4fer > wrote: >=20 > > This patch is based on code developed by Albert Herranz and the GameCube > > Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c, > > available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but > > has grown quite dissimilar. >=20 > I'm impressed by this effort. As with all reverse engineering. >=20 > > This driver currently uses __raw_readl and __raw_writel to access the > > GPIO controller's MMIO registers. I wonder if readl/writel plus explicit > > byte-swapping would be more correct, because it could be independent of > > the CPU's endianness. That said, this hardware only exists in two > > big-endian machines (Wii and Wii U). >=20 > I don't know about PPC but I think you're supposed to use > ioread32be() and iowrite32be() to do explicit BE access. Ah, that's the name! I didn't find ioread32*/iowrite32* in the documentation or source code. > But when I look at it, I think you can just use the gpio-mmio library > for this driver and cut down code cosiderably. I'll look into it. So far it looks good (drivers/gpio/gpio-iop.c has just 60 lines). > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >=20 > Can't you just save a pointer to struct device *dev in the > state container and use dev_info(state->dev, ...) etc instead > of this? Makes sense. I'll try this out. > > +#include >=20 > This include should not be needed. Okay. > > +/* > > + * Update the bit with the given bit offset in the given register to a= given > > + * value > > + */ > > +static void hlwd_gpio_update_bit(struct gpio_chip *gc, unsigned int re= g, > > + int offset, int value) > > +{ > > + struct hlwd_gpio *hlwd =3D gpiochip_get_data(gc); > > + unsigned long flags; > > + u32 bit =3D 1UL << offset; >=20 > #include >=20 > u32 bit =3D BIT(offset); >=20 > > + u32 tmp; > > + > > + spin_lock_irqsave(&hlwd->lock, flags); > > + tmp =3D __raw_readl(hlwd->regs + reg); > > + if (value) > > + __raw_writel(tmp | bit, hlwd->regs + reg); > > + else > > + __raw_writel(tmp & ~bit, hlwd->regs + reg); > > + spin_unlock_irqrestore(&hlwd->lock, flags); > > +} >=20 > This looks very much like it is reimplementing the stuff we already > have in drivers/gpio/gpio-mmio.h. >=20 > There is even a big endian access flag for the library. > And you get so much for free with gpio-mmio. >=20 > select GPIO_GENERIC > in Kconfig >=20 > the helpers come in from >=20 > Look at other drivers for inspiration: > git grep bgpio_init >=20 > If you need IRQ support you should probably have your own file > for this driver, but it will be just a few lines of wrapper using > bgpio_init() and BGPIOF_BIG_ENDIAN and/or possibly > BGPIOF_BIG_ENDIAN_BYTE_ORDER. Yes, I plan to add IRQ support in a later patch. >=20 > See the other drivers. Yep, gpio-mmio looks like a good option, thanks for the pointer! Thanks, Jonathan Neusch=C3=A4fer --exhxaxcsmwxhn2ch Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAABAgAGBQJaXnWOAAoJEAgwRJqO81/bHKUP/RbV3X1r22TTRalj4CrWEnoU 58cADjxMeqhMM1JswwD7sHIa0ViZCshGaoxKcx9p0wen1X6FW2ky4m6g+vsI7Zwl MN9hu1aewkk7ilZNcFrQYxFDIV5+vRhkTwTe4s/ZmFYri7bUjMH0yEW92gRXeTNH fiz9KiF8wtrtahE4hse4ET9yog4+Tj3DEwVMtZ8L0yQ8SmrjJ31z7R1J1tJokCC+ lW8fveywkSoDWACRvzj81G/xfFnFiw+wkrncyXWA81rdPlk6P2ZyhrA9EypV0gPx 8ngP0SpSwnmArz6IWXA2Gj+ef3dhnQvfiPqee2PBGLUGIvQT/7PWyUSbtex6XOAQ CNejveRGfQcZOZ4tRbr26xnUUkopX8VgotgLinC37k1s6kuMQY3P+JCF56d+C8X1 ouCgs+MnyRa2j2QZ8PrjqJNQbqbGAinC7MwYWxWXNlU4oejmyxG01igqtezvw2Fa NqIuRumi7XYTuJdvj/K1P30Fks43eKToqoj+TrMei2eyTJQUPsX4foLD29IAms4Y q2QtyXvxRDg4STCdJDOR0jNa4mGaqWNBAuRtB+sOHjsN5loj6JD14lkqooiV0U98 kZrxHcqmCYomrc8kZiOznWdNp1D6K6WwSaLE9klI9zOaifIxzcorQgVEHPKyuohB fH806q4e1JYi1PxeyXQu =UpvS -----END PGP SIGNATURE----- --exhxaxcsmwxhn2ch--