Hi Sebastian, On Wed, 2025-01-15 at 22:30 +0100, Sebastian Reichel wrote: > Hi, > > On Thu, Jan 02, 2025 at 12:15:03PM +0100, Thomas Antoine via B4 Relay wrote: > > From: Thomas Antoine <t.antoine@xxxxxxxxxxxx> > > > > The interface of the Maxim max77759 fuel gauge has a lot of common with the > > Maxim max1720x. The major difference is the lack of non-volatile memory > > slave address. No slave is available at address 0xb of the i2c bus, which > > is coherent with the following driver from google: line 5836 disables > > non-volatile memory for m5 gauge. > > > > Link: https://android.googlesource.com/kernel/google-modules/bms/+/1a68c36bef474573cc8629cc1d121eb6a81ab68c/max1720x_battery.c > > > > Other differences include the lack of V_BATT register to read the battery > > level and a difference in the way to identify the chip (the same register > > is used but not the same mask). > > > > Add support for the max77759 by allowing to use the non-volatile > > memory or not based on the chip. Also add the V_CELL regsister as a > > fallback to read voltage value in the case where read of V_BATT fails. > > > > The cast is necessary to avoid an overflow when the value of the register > > is above 54975 (equivalent to a voltage around 4.29 V). > > > > The regmap of the max77759 will lead the read to fail for V_BATT and to > > correctly use V_CELL instead. This regmap was proposed by André Draszik in > > > > Link: https://lore.kernel.org/all/d1bade77b5281c1de6b2ddcb4dbbd033e455a116.camel@xxxxxxxxxx/ > > > > Signed-off-by: Thomas Antoine <t.antoine@xxxxxxxxxxxx> > > --- > > Please add output from to the cover letter to allow easily verifying > that all values are correctly scaled. > > ./tools/testing/selftests/power_supply/test_power_supply_properties.sh > > > +static const struct regmap_access_table max77759_write_table = { > > + .yes_ranges = max77759_registers, > > + .n_yes_ranges = ARRAY_SIZE(max77759_registers), > > + .no_ranges = max77759_ro_registers, > > + .n_no_ranges = ARRAY_SIZE(max77759_ro_registers), > > +}; > > Drop the yes_range from the write table. It is wrong and confusing. Can you please clarify why having yes_ranges is wrong? Without yes_ranges, all registers not in no_ranges are allowed to be written to. Here, max77759_registers already specifies all the registers that exist (and is also used in the max77759_read_table), and for write-access this is further limited by the read-only registers in no_ranges. As an example, register 0x50 doesn't exist, and without yes_ranges this would allow write access to it. If yes_ranges was dropped, all the information about non-existing registers would have to be duplicated into no_ranges by inversing max77759_registers. We already know the non-existing registers, and inversing that list just to add to no_ranges seems non-ideal, error-prone, and just duplicated information. Cheers, Andre'
