Hi,
On Thu, Dec 19, 2024 at 03:19:12PM +0530, Bhavin Sharma wrote:
> Adds initial support for the STC3117 fuel gauge.
>
> The driver provides functionality to monitor key parameters including:
> - Voltage
> - Current
> - State of Charge (SOC)
> - Temperature
> - Status
>
> Co-developed-by: Hardevsinh Palaniya <hardevsinh.palaniya@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Bhavin Sharma <bhavin.sharma@xxxxxxxxxxxxxxxxx>
> ---
I missed one thing in my review. Apart from that things look good to
me now.
> [...]
> +static union stc3117_internal_ram {
> + u8 ram_bytes[STC3117_RAM_SIZE];
> + struct {
> + u16 testword; /* 0-1 Bytes */
> + u16 hrsoc; /* 2-3 Bytes */
> + u16 cc_cnf; /* 4-5 Bytes */
> + u16 vm_cnf; /* 6-7 Bytes */
> + u8 soc; /* 8 Byte */
> + u8 state; /* 9 Byte */
> + u8 unused[5]; /* 10-14 Bytes */
> + u8 crc; /* 15 Byte */
> + } reg;
> +} ram_data;
> +
> +struct stc3117_data {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct delayed_work update_work;
> + struct power_supply *battery;
> +
> + u8 soc_tab[16];
> + int cc_cnf;
> + int vm_cnf;
> + int cc_adj;
> + int vm_adj;
> + int avg_current;
> + int avg_voltage;
> + int batt_current;
> + int voltage;
> + int temp;
> + int soc;
> + int ocv;
> + int hrsoc;
> + int presence;
> +};
> +
> +struct stc3117_battery_info {
> + int voltage_min_mv;
> + int voltage_max_mv;
> + int battery_capacity_mah;
> + int sense_resistor;
> +} battery_info;
You may not use a static variable for holding the battery
information and RAM data. A system could have two stc3117
fuel gauges with different data. Instead you can add them
into struct stc3117_data, which is allocated per instance.
Greetings,
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
