Re: [PATCH 2/2] hwmon: Add support for TI INA4230 power monitor

[Krzysztof Kozlowski]

On Wed, Feb 25, 2026 at 01:29:12PM +0400, Alexey Charkov wrote:
> Add a driver for the TI INA4230, a 4-channel power monitor with I2C
> interface.
> 
> The driver supports voltage, current, power and energy measurements, but
> skips the alert functionality in this initial implementation.
> 
> Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
> ---
>  MAINTAINERS             |   6 +
>  drivers/hwmon/Kconfig   |  11 +
>  drivers/hwmon/Makefile  |   1 +
>  drivers/hwmon/ina4230.c | 997 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 1015 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4d879f6a7b51..77f7a416e682 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12511,6 +12511,12 @@ S:	Maintained
>  F:	Documentation/hwmon/ina233.rst
>  F:	drivers/hwmon/pmbus/ina233.c
>  
> +INA4230 HWMON DRIVER
> +M:	Alexey Charkov <alchark@xxxxxxxxxxx>
> +L:	linux-hwmon@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	drivers/hwmon/ina4230.c

List here binding as well, please.


...

> +
> +static int ina4230_probe_child_from_dt(struct device *dev,
> +				       struct device_node *child,
> +				       struct ina4230_data *ina)
> +{
> +	struct ina4230_input *input;
> +	u32 val;
> +	int ret;
> +
> +	ret = of_property_read_u32(child, "reg", &val);
> +	if (ret) {
> +		dev_err(dev, "missing reg property of %pOFn\n", child);
> +		return ret;
> +	} else if (val > INA4230_CHANNEL4) {
> +		dev_err(dev, "invalid reg %d of %pOFn\n", val, child);

All these are probe, so return dev_err_probe

> +		return -EINVAL;
> +	}
> +

...

> +	ina->regmap = devm_regmap_init_i2c(client, &ina4230_regmap_config);
> +	if (IS_ERR(ina->regmap)) {
> +		dev_err(dev, "Unable to allocate register map\n");
> +		return PTR_ERR(ina->regmap);
> +	}
> +
> +	for (i = 0; i < F_MAX_FIELDS; i++) {
> +		ina->fields[i] = devm_regmap_field_alloc(dev,
> +							 ina->regmap,
> +							 ina4230_reg_fields[i]);
> +		if (IS_ERR(ina->fields[i])) {
> +			dev_err(dev, "Unable to allocate regmap fields\n");
> +			return PTR_ERR(ina->fields[i]);

Syntax is return dev_err_probe, but allocations should not have printks.
It is not possible to get there any other error code.

> +		}
> +	}
> +
> +	for (i = 0; i < INA4230_NUM_CHANNELS; i++) {
> +		ina->inputs[i].shunt_resistor = INA4230_RSHUNT_DEFAULT;
> +		/* Default for 1mA LSB current measurements */
> +		ina->inputs[i].max_expected_current = 32768000;
> +	}
> +
> +	ret = ina4230_probe_from_dt(dev, ina);
> +	if (ret) {
> +		dev_err(dev, "Unable to probe from device tree\n");
> +		return ret;

return dev_err_probe

> +	}
> +
> +	/* The driver will be reset, so use reset value */
> +	ina->reg_config1 = INA4230_CONFIG_DEFAULT;
> +	ina->reg_config2 = 0;
> +
> +	if (ina->single_shot)
> +		FIELD_MODIFY(INA4230_CONFIG1_MODE_MASK, &ina->reg_config1,
> +			     INA4230_MODE_BUS_SHUNT_SINGLE);
> +
> +	/* Disable channels if their inputs are disconnected */
> +	for (i = 0; i < INA4230_NUM_CHANNELS; i++) {
> +		if (ina->inputs[i].disconnected)
> +			ina->reg_config1 &= ~INA4230_CONFIG_CHx_EN(i);
> +	}
> +
> +	/* Set calibration values */
> +	for (i = 0; i < INA4230_NUM_CHANNELS; i++) {
> +		if (!ina->inputs[i].disconnected)
> +			ina4230_set_calibration(ina, i);
> +	}
> +
> +	ina->pm_dev = dev;
> +	dev_set_drvdata(dev, ina);
> +
> +	/* Enable PM runtime -- status is suspended by default */
> +	pm_runtime_enable(ina->pm_dev);
> +
> +	/* Initialize (resume) the device */
> +	for (i = 0; i < INA4230_NUM_CHANNELS; i++) {
> +		if (ina->inputs[i].disconnected)
> +			continue;
> +		/* Match the refcount with number of enabled channels */
> +		ret = pm_runtime_get_sync(ina->pm_dev);
> +		if (ret < 0)
> +			goto fail;
> +	}
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ina,
> +							 &ina4230_chip_info,
> +							 ina4230_groups);
> +	if (IS_ERR(hwmon_dev)) {
> +		dev_err(dev, "Unable to register hwmon device\n");
> +		ret = PTR_ERR(hwmon_dev);

just ret = dev_err_probe

> +		goto fail;
> +	}
> +
> +	return 0;
> +
> +fail:
> +	pm_runtime_disable(ina->pm_dev);
> +	pm_runtime_set_suspended(ina->pm_dev);
> +	/* pm_runtime_put_noidle() will decrease the PM refcount until 0 */
> +	for (i = 0; i < INA4230_NUM_CHANNELS; i++)
> +		pm_runtime_put_noidle(ina->pm_dev);
> +
> +	return ret;
> +}
> +
> +static void ina4230_remove(struct i2c_client *client)
> +{
> +	struct ina4230_data *ina = dev_get_drvdata(&client->dev);
> +	int i;
> +
> +	pm_runtime_disable(ina->pm_dev);
> +	pm_runtime_set_suspended(ina->pm_dev);
> +
> +	/* pm_runtime_put_noidle() will decrease the PM refcount until 0 */
> +	for (i = 0; i < INA4230_NUM_CHANNELS; i++)
> +		pm_runtime_put_noidle(ina->pm_dev);
> +}
> +
> +static int ina4230_suspend(struct device *dev)
> +{
> +	struct ina4230_data *ina = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/* Save config register value and enable cache-only */
> +	ret = regmap_read(ina->regmap, INA4230_CONFIG1, &ina->reg_config1);
> +	if (ret)
> +		return ret;
> +
> +	regcache_cache_only(ina->regmap, true);
> +	regcache_mark_dirty(ina->regmap);
> +
> +	return 0;
> +}
> +
> +static int ina4230_resume(struct device *dev)
> +{
> +	struct ina4230_data *ina = dev_get_drvdata(dev);
> +	int ret;
> +
> +	regcache_cache_only(ina->regmap, false);
> +
> +	/* Software reset the chip */
> +	ret = regmap_field_write(ina->fields[F_RST], true);
> +	if (ret) {
> +		dev_err(dev, "Unable to reset device\n");
> +		return ret;
> +	}
> +
> +	/* Restore cached register values to hardware */
> +	ret = regcache_sync(ina->regmap);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(ina4230_pm, ina4230_suspend, ina4230_resume,
> +				 NULL);
> +
> +static const struct of_device_id ina4230_of_match_table[] = {
> +	{ .compatible = "ti,ina4230", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ina4230_of_match_table);
> +
> +static const struct i2c_device_id ina4230_ids[] = {
> +	{ "ina4230" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ina4230_ids);
> +
> +static struct i2c_driver ina4230_i2c_driver = {
> +	.probe = ina4230_probe,
> +	.remove = ina4230_remove,
> +	.driver = {
> +		.name = INA4230_DRIVER_NAME,
> +		.of_match_table = ina4230_of_match_table,
> +		.pm = pm_ptr(&ina4230_pm),
> +	},
> +	.id_table = ina4230_ids,
> +};
> +module_i2c_driver(ina4230_i2c_driver);
> +
> +MODULE_AUTHOR("Alexey Charkov <alchark@xxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Texas Instruments INA4230 HWMon Driver");
> +MODULE_LICENSE("GPL");
> 
> -- 
> 2.52.0
>