On Mon, Jan 13, 2025 at 11:50:23AM +0800, Wenliang Yan wrote:
> Add support for SQ52206 to the Ina238 driver. Add registers,
> add calculation formulas, increase compatibility, add
> compatibility programs for multiple chips.
>
> Signed-off-by: Wenliang Yan <wenliang202407@xxxxxxx>
> ---
...
> @@ -452,6 +554,9 @@ static umode_t ina238_is_visible(const void *drvdata,
> enum hwmon_sensor_types type,
> u32 attr, int channel)
> {
> + const struct ina238_data *data = drvdata;
> + bool has_power_highest = data->config->has_power_highest;
> +
> switch (type) {
> case hwmon_in:
> switch (attr) {
> @@ -479,6 +584,10 @@ static umode_t ina238_is_visible(const void *drvdata,
> return 0444;
> case hwmon_power_max:
> return 0644;
> + case hwmon_power_input_highest:
> + if (has_power_highest)
> + return 0444;
> + break;
This doesn't work as intended. The break; results in the code
entering the hwmon_temp: case. It has to be "return 0;" or
the entire function needs to be rewritten to use "break;"
to exit the switch statements, and "return 0;" at the end
of the function.
> static int ina238_probe(struct i2c_client *client)
> {
...
> - if (data->gain == 1)
> + config = data->config->config_default;
> + if (chip == sq52206)
> + if (data->gain == 1)
> + config |= SQ52206_CONFIG_ADCRANGE_HIGH; /* ADCRANGE = 10/11 is /1 */
> + else if (data->gain == 2)
> + config |= SQ52206_CONFIG_ADCRANGE_LOW; /* ADCRANGE = 01 is /2 */
> + else if (data->gain == 1)
The "else" here is the else from "if (data->gain == 2)" which is wrong.
The entire if/else block from "if (chip == sq52206)" needs to be in {}
to avoid that.
> config |= INA238_CONFIG_ADCRANGE; /* ADCRANGE = 1 is /1 */
> ret = regmap_write(data->regmap, INA238_CONFIG, config);
> if (ret < 0) {
> @@ -605,7 +735,8 @@ static int ina238_probe(struct i2c_client *client)
>
> hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
> &ina238_chip_info,
> - NULL);
> + data->config->has_energy ?
> + NULL : ina238_groups);
This seems to be wrong check. I would assume that ina238_groups is passed
if data->config->has_energy is true, not if it is false.
Guenter
