Hi Andy, > Am 13.03.2018 um 17:56 schrieb Andy Shevchenko <andy.shevchenko@xxxxxxxxx>: > > On Sat, Mar 10, 2018 at 1:00 PM, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> 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. >> >> And while we are at it, the pcal9555a does not have a compatible entry >> either but is already supported by the device id table. > > >> + { "pcal6524", 24 | PCA953X_TYPE | PCA_INT | PCA_PCAL, }, >> { "pcal9555a", 16 | PCA953X_TYPE | PCA_INT | PCA_PCAL, }, > > 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 <yong.b.li@xxxxxxxxx> Date: Thu Apr 7 12:56:32 2016 +0800 gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2 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. The original patch was submitted by Josef Ahmad <josef.ahmad@xxxxxxxxxxxxxxx> http://git.yoctoproject.org/cgit/cgit.cgi/meta-intel-quark/tree/recipes-kernel/linux/files/0015-Quark-GPIO-1-2-quark.patch Signed-off-by: Yong Li <yong.b.li@xxxxxxxxx> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > > >> + { .compatible = "nxp,pcal6524", .data = OF_953X(24, PCA_INT), }, >> + { .compatible = "nxp,pcal9555a", .data = OF_953X(16, PCA_INT), }, > > 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. BR and thanks, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html