From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S969203AbdIZNpu (ORCPT ); Tue, 26 Sep 2017 09:45:50 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:55842 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S967470AbdIZNpj (ORCPT ); Tue, 26 Sep 2017 09:45:39 -0400 Date: Tue, 26 Sep 2017 15:45:27 +0200 From: Maxime Ripard To: Quentin Schulz Cc: linus.walleij@linaro.org, robh+dt@kernel.org, mark.rutland@arm.com, wens@csie.org, linux@armlinux.org.uk, lee.jones@linaro.org, linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@googlegroups.com, thomas.petazzoni@free-electrons.com Subject: Re: [PATCH v2 03/10] pinctrl: axp209: use drv_data of pinctrl_pin_desc to store pin reg Message-ID: <20170926134527.kmhpxn4ysfmvdulf@flea> References: <7993a30fbc2e50a2d228fa0c8fad643c4034b101.1506428208.git-series.quentin.schulz@free-electrons.com> <20170926130108.uibnri32b73elfy6@flea> <0ae64e95-ee49-fb4c-e79b-e8c25c86580c@free-electrons.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zjqmg4hqb6yittcl" Content-Disposition: inline In-Reply-To: <0ae64e95-ee49-fb4c-e79b-e8c25c86580c@free-electrons.com> User-Agent: NeoMutt/20170914 (1.9.0) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --zjqmg4hqb6yittcl Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 26, 2017 at 01:17:05PM +0000, Quentin Schulz wrote: > Hi Maxime, >=20 > On 26/09/2017 15:01, Maxime Ripard wrote: > > On Tue, Sep 26, 2017 at 12:17:13PM +0000, Quentin Schulz wrote: > >> Instead of using a function to retrieve each pin's correct control > >> register, use drv_data within pinctrl_pin_desc to store the ctrl reg. > >> > >> Remove axp20x_gpio_get_reg and replace every occurrence by a get from > >> drv_data. > >=20 > > Why do you need to do that? This should be explained. > >=20 >=20 > Agreed that it misses an explanation. >=20 > Today, to get a register addr of one of the GPIOs in the PMIC, we > basically get the GPIO number and returns the register via this info. >=20 > There are 3 GPIOs in AXP209, 2 in AXP813. I didn't want to have a switch > case for the GPIO number and then an if/else inside one of the case to > check if the device is AXP209 or AXP813 in which case we return -EINVAL > instead of the GPIO2 reg. With support for new PMIC, we would have a > bunch of if conditions and complexify the process for something really > simple. I'm not sure how that relates to your code actually. The only thing that patch is doing is to move the register offset from a function to the structure associated to the pin. However, even in the AXP813 case, you're using exactly the same values, so that's not really needed. Now, you also mentionned the pin number. While this patch doesn't really address it, it's also no really needed. The number of pins is already known and registered in the GPIO framework. If the framework doesn't already do it (which would be surprising), you can just check that the pin number passed is not going to be higher than the one you registered. > IMHO, this also allows easier integration of future PMICs which might > have different regs for the GPIOs. Let's worry about future PMICs in the future. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --zjqmg4hqb6yittcl Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBAgAGBQJZyln3AAoJEBx+YmzsjxAgR0cQAKaqkt6y5c8mdQQgoGYBANNn MueAvgD9s3zXBR11QVzH8E4G9ohqnY9uuoSLAt57/GjW6bBYDY8aVhnlBWkXRhOZ YEtIDudV9e+NQ+tx9Hz7Y9lHIoURJPd7FXuBwdDauQSZBX2ZQKJwDSl3n/lWczQD u+Q0K5JTL5QgNXWX0otygTm4pTFna/yE38MX3SuE5tOZSe09kE4bg0BuOfzacRSS HGcRbguOqNyqz5SxYdd4+UTmwN9Y8RHBmbL9pksgDx4bn+qUeUOgTG6AtShnrXw4 dAXETG+GSpVeUlHP5GSKavPoY0suN4ivsExQYxeqD9h26UaRVxeHiUIL71Q4Cz9c +ypD50CVBSOtgFvU7rFTO2XeoPKgrXhtVyUVYaz3cO+poOc9KP61dz8s42L85x2W iPxY4sg6hA0JrQnTh2US/ikcAuTFChEG6uQxkNzgWfa/ihFnwNxa22iMlIcTDk8W VO5SwkWUIeOoQxeqWK7l4IJKGBN/krDk5XFOOQHS3I7UC2m578tGNjLQp3ZwInKy Z0Bp7lR9KAZ9BWj9DN+8z8X5Ep6nkMwSrcYNm3V1xumVzWckrz0bWcJ1Z27arPEF Zu9s2zzeYDOymH860kwI/aJQXFxUzVvRZECBYm4VXg0ValKLFak+dNx4aLkMNDlL izwyJt1SGNtHD/Th0K2t =M2Yq -----END PGP SIGNATURE----- --zjqmg4hqb6yittcl--