On Tue, 2024-12-03 at 11:11 +0100, Thomas Antoine wrote: > On 12/3/24 10:31, André Draszik wrote: > > On Tue, 2024-12-03 at 10:08 +0100, Thomas Antoine wrote: > > > On 12/3/24 07:47, André Draszik wrote: > > > > > From: Thomas Antoine <t.antoine@xxxxxxxxxxxx> > > > > > [...] > > > > > /* > > > > > * Model Gauge M5 Algorithm output register > > > > > * Volatile data (must not be cached) > > > > > @@ -369,6 +387,8 @@ static int max1720x_battery_get_property(struct > > > > > power_supply *psy, > > > > > val->strval = max17201_model; > > > > > else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205) > > > > > val->strval = max17205_model; > > > > > + else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX77759) > > > > > + val->strval = max77759_model; > > > > > else > > > > > > > > This is a 16 bit register, and while yes, MAX172XX_DEV_NAME_TYPE_MASK only > > > > cares about the bottom 4 bits, the register is described as 'Firmware > > > > Version Information'. > > > > > > > > But maybe it's ok to do it like that, at least for now. > > > > > > I thought this method would be ok as long as there is no collision on > > > values. I hesitated to change the model evaluation method based on chip > > > model, where the max77759 would thus have an hard-coded value and the > > > max1720x would still evaluate the register value. I did not do it because > > > it led to a lot more changes for no difference. > > > > Downstream uses the upper bits for max77759: > > https://android.googlesource.com/kernel/google-modules/bms/+/refs/heads/android-gs-raviole-mainline/max_m5.h#135 > > > > I don't know what the original max17201/5 report in this register > > for those bits, though. Given for max77759 this register returns > > the firmware version, I assume the lower bits can change. > > Based on this datasheet of the max1720x, the upper bits are the revision > and the four lower bits are device. So it could change. > https://www.analog.com/media/en/technical-documentation/data-sheets/MAX17201-MAX17215.pdf#MAX17201%20DS.indd%3A.213504%3A15892 > > If the four lower bits are not always 0 for the max77759 then I guess it > is necessary to change this as it wouldn't work with all max77759. Maybe the best way forward is to go by the compatible (from DT), and if max77759 to then print a warning if the upper bits are != 0x62 and != 0x63. And maybe even refuse to load in that case. Cheers, Andre'
