From mboxrd@z Thu Jan 1 00:00:00 1970 From: "H. Nikolaus Schaller" Subject: Re: [PATCH] drivers: gpio: pca953x: add compatibility for pcal6524 and pcal9555a Date: Wed, 14 Mar 2018 13:39:30 +0100 Message-ID: <0BF8A237-1EB9-465A-9A7C-75163F093F18@goldelico.com> References: <49f6922ba342cf69bfd2a787d0c2a93b4df2c429.1520664883.git.hns@goldelico.com> Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Andy Shevchenko , Yong Li Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Linus Walleij , Alexandre Courbot , devicetree , "open list:GPIO SUBSYSTEM" , Linux Kernel Mailing List , letux-kernel@openphoenux.org, kernel@pyra-handheld.com List-Id: linux-gpio@vger.kernel.org Hi Andy, > Am 13.03.2018 um 17:56 schrieb Andy Shevchenko = : >=20 > On Sat, Mar 10, 2018 at 1:00 PM, H. Nikolaus Schaller = wrote: >> The Pyra-Handheld originally used the tca6424 but recently we have >> replaced it by the pin and package compatible pcal6524. So let's >> add this to the bindings and the driver. >>=20 >> And while we are at it, the pcal9555a does not have a compatible = entry >> either but is already supported by the device id table. >=20 >=20 >> + { "pcal6524", 24 | PCA953X_TYPE | PCA_INT | PCA_PCAL, }, >> { "pcal9555a", 16 | PCA953X_TYPE | PCA_INT | PCA_PCAL, }, >=20 > So, from your description I can get that PCA_PCAL is redundant for > 6524. Is it correct? We might not be using all features of the 6524 and therefore our test coverage is not 100%. So I would not draw the conclusion that PCA_PCAL is *not* needed. The data sheet should tell. > What does L means in the model code? Good question. The data sheets don't tell. But 6424 and 6524 are not = identical from register set and overall functions, although quite similar. Only for pin and package. As far as I understand the 6524 (and all PCAL) have additional interrupt latch mechanisms and registers so that it is possible to disable each I/O pin individually as an interrupt while for the 6424 they are always enabled. Maybe this is the "L" designator. But we aren't using interrupts yet. > Perhaps we need to rename PCA_PCAL to be more specific? My assumption is that it should be there for all PCAL variants, but I think the original author who introduced this constant should = know. Hm. git blame tells that you have introduced it :) But this seems to be a false alarm because you have just changed formatting. It was introduced by commit 44896beae605b93f2232301befccb7ef42953198 Author: Yong Li Date: Thu Apr 7 12:56:32 2016 +0800 gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2 =20 Galileo Gen2 board uses the PCAL9535 as the GPIO expansion, it is different from PCA9535 and includes interrupt mask/status = registers, The current driver does not support the interrupt registers = configuration, it causes some gpio pins cannot trigger interrupt events, this patch fix this issue. =20 The original patch was submitted by Josef Ahmad = http://git.yoctoproject.org/cgit/cgit.cgi/meta-intel-quark/tree/recipes-ke= rnel/linux/files/0015-Quark-GPIO-1-2-quark.patch =20 Signed-off-by: Yong Li Reviewed-by: Andy Shevchenko Signed-off-by: Linus Walleij >=20 >=20 >> + { .compatible =3D "nxp,pcal6524", .data =3D OF_953X(24, = PCA_INT), }, >> + { .compatible =3D "nxp,pcal9555a", .data =3D OF_953X(16, = PCA_INT), }, >=20 > Other way around, you missed PCA_PCAL in the second case. Ah, ok. It wasn't clear how these flag relate to the i2c table because = they are hidden by a macro here. I'd assume that PCA_PCAL is missing for = both. So it might be that we run the pcal6524 in non-PCAL mode because we use = DT. I can do a test as soon as I have access to the hardware.=20 BR and thanks, Nikolaus