On 4/5/24 08:49, Heikki Krogerus wrote: > On Wed, Apr 03, 2024 at 10:55:29AM +0200, Javier Carrasco wrote: >>>> - ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1); >>>> - ret |= tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2); >>>> + ret = tps6598x_block_read(tps, TPS_REG_INT_EVENT1, event1, 11); >>> >>> This is not going to work with the older TI PD controllers. >>> >>> The lenght of these registers is 8 bytes on the older TI PD >>> controllers (TPS65981, TPS65982, etc.). I think we need to split this >>> function. >>> >> >> That is a good point. I had a look at the older TI PD controllers and I >> agree with you that we should split the function to cover both register >> lengths separately. >> >> I was thinking about adding a new compatible for the newer PD >> controllers (tps65987 and tps65988), keeping the current tps6598x for >> the older ones as well as backwards compatibility. But backwards >> compatibility would also mean that flags beyond the first 8 bytes would >> be ignored. >> >> On the other hand, the upper flags are only relevant for firmware >> updates, so we could check those (i.e. read 11 bytes) if a firmware was >> provided via "firmware-name", and ignore them (i.e. read 8 bytes) otherwise. >> >> Other ideas or improvements to mine are more than welcome. > > I don't have any good ideas. On ACPI platforms the same device ID may > be used with all of these, so we should actually try to figure out the > version from registers like VID, DID and Version (if they are > available). > > thanks, > VID and DID can be modified by the application firmware, but there is a byte in the Version register we can use for this. According to TI[1], it is guaranteed that the older TI PD controllers (TPS65981/2/6) will always deliver AB = 0x00 when reading from the Version register, which is 4 bytes long formatted like this: ABXX.YY.ZZ. The newer PD controllers (TPS65987/8) will return either AB = 0xF7 (DH parts) or AB = 0xF9 (DK parts). We can add some simple logic to read 8 bytes if AB is 0x00, which could be the default as well, and 11 bytes otherwise. Link: https://e2e.ti.com/support/power-management-group/power-management/f/power-management-forum/1346521/tps65987d-register-command-to-distinguish-between-tps6591-2-6-and-tps65987-8 [1] Thanks for your feedback and best regards, Javier Carrasco
