Re: [PATCH 2/2] power: supply: Add Silicon Mitus SM5714 fuel gauge driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] power: supply: sm5714: missing MODULE_IMPORT_NS() for
  IIO_CONSUMER
- [Medium] power: supply: sm5714: NULL deref on fg->psy in
  sm5714_fg_get_status()
--

commit f1f865d4819006729764f420d2f1ef002104e726
Author: Asdlerto Korry <korryasdlerto@gmail.com>

power: supply: Add Silicon Mitus SM5714 fuel gauge driver

This commit adds a power supply driver for the Silicon Mitus SM5714 fuel
gauge, reading state of charge, voltage, current, and temperature over I2C
and an optional thermistor IIO channel.

> diff --git a/drivers/power/supply/sm5714_fuel_gauge.c b/drivers/power/supply/sm5714_fuel_gauge.c
> new file mode 100644
> index 0000000000000..531583fb211bb
> --- /dev/null
> +++ b/drivers/power/supply/sm5714_fuel_gauge.c

[ ... ]

> +static int sm5714_fg_get_status(struct sm5714_fg *fg, int *status)
> +{
> +	unsigned int aux, soc;
> +	int ma, ret;
> +
> +	ret = regmap_read(fg->regmap, SM5714_FG_REG_AUX_STAT, &aux);
> +	if (ret)
> +		return ret;
> +	ret = sm5714_fg_read_sram(fg, SM5714_FG_SRAM_SOC, &soc);
> +	if (ret)
> +		return ret;
> +	ret = sm5714_fg_get_current_ma(fg, SM5714_FG_SRAM_CURRENT_AVG, &ma);
> +	if (ret)
> +		return ret;
> +
> +	/* the charger stops after top-off while the adapter stays connected */
> +	if (!(aux & SM5714_FG_AUX_STAT_CHG_ON) && power_supply_am_i_supplied(fg->psy) <= 0)
> +		*status = POWER_SUPPLY_STATUS_DISCHARGING;
> +	else if (sm5714_fg_soc_permille(soc) >= 990)
> +		*status = POWER_SUPPLY_STATUS_FULL;
> +	else if (ma > 0)
> +		*status = POWER_SUPPLY_STATUS_CHARGING;
> +	else
> +		*status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +
> +	return 0;
> +}
> +
> +static int sm5714_fg_get_property(struct power_supply *psy,
> +				  enum power_supply_property psp,
> +				  union power_supply_propval *val)
> +{
> +	struct sm5714_fg *fg = power_supply_get_drvdata(psy);
> +	unsigned int raw;
> +	int ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		return sm5714_fg_get_status(fg, &val->intval);

[Severity: Medium]
Can sm5714_fg_get_status() pass a NULL fg->psy to
power_supply_am_i_supplied()?

__power_supply_register() exposes the sysfs attributes and increments
psy->use_cnt before devm_power_supply_register() returns and assigns
fg->psy in sm5714_fg_probe():

CPU0                              CPU1
sm5714_fg_probe()
  devm_power_supply_register()
    __power_supply_register()
      device_add()
      atomic_inc(&psy->use_cnt)
                                  power_supply_show_property()
                                    sm5714_fg_get_property(psy, ...)
                                      sm5714_fg_get_status(fg, ...)
                                        power_supply_am_i_supplied(fg->psy)
                                          ...
                                          __power_supply_is_supplied_by()
                                            supply->supplied_from
                                            <-- NULL pointer dereference
  fg->psy = ...

Passing psy from sm5714_fg_get_property() into sm5714_fg_get_status()
would avoid reading fg->psy before it is initialized.

[ ... ]

> +MODULE_AUTHOR("Asdlerto Korry <korryasdlerto@gmail.com>");
> +MODULE_DESCRIPTION("Silicon Mitus SM5714 fuel gauge driver");
> +MODULE_LICENSE("GPL");

[Severity: High]
Does building this driver with CONFIG_BATTERY_SM5714=m fail in modpost due
to a missing MODULE_IMPORT_NS("IIO_CONSUMER")?  devm_iio_channel_get() and
iio_read_channel_processed() are exported in the IIO_CONSUMER symbol
namespace.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-sm5714-fuel-gauge-v1-0-b9d163fff6ff@gmail.com?part=2