linux-kernel - Re: [PATCH v2 2/2] hwmon: Add driver for TI INA232 Current and Power Monitor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <dc0bba60-72e1-46a5-a341-a2aa30095042@wanadoo.fr>
Date: Fri, 10 Jan 2025 22:19:02 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Leo Yang <leo.yang.sy0@...il.com>, jdelvare@...e.com, linux@...ck-us.net,
 robh@...nel.org, davem@...emloft.net, krzk+dt@...nel.org,
 conor+dt@...nel.org, Leo-Yang@...ntatw.com, corbet@....net,
 Delphine_CC_Chiu@...ynn.com, linux-hwmon@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 2/2] hwmon: Add driver for TI INA232 Current and Power
 Monitor

 > Re: [PATCH v2 2/2] hwmon: Add driver for TI INA232 Current and Power 
Monitor

Should the subject be INA233?


Le 10/01/2025 à 09:15, Leo Yang a écrit :
> Support ina233 driver for Meta Yosemite V4.
> 
> Driver for Texas Instruments INA233 Current and Power Monitor
> With I2C-, SMBus-, and PMBus-Compatible Interface
> 
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202501092213.X9mbPW5Q-lkp@intel.com/
> Closes: https://lore.kernel.org/oe-kbuild-all/202501061734.nPNdRKqO-lkp@intel.com/
> Signed-off-by: Leo Yang <Leo-Yang@...ntatw.com>

...

> +static void calculate_coef(int *m, int *R, int power_coef)
> +{
> +	s64 scaled_m;
> +	int scale_factor = 0;
> +	int scale_coef = 1;
> +	bool is_integer = false;

Is is_integer really needed?

See below.

> +
> +	/*
> +	 * 1000000 from Current_LSB A->uA .
> +	 * scale_coef is for scaling up to minimize rounding errors,
> +	 * If there is no decimal information, No need to scale.
> +	 */
> +	if (1000000 % *m) {
> +		/* Scaling to keep integer precision */
> +		scale_factor = -3;
> +		scale_coef = 1000;
> +	} else {
> +		is_integer = true;
> +	}
> +
> +	/*
> +	 * Unit Conversion (Current_LSB A->uA) and use scaling(scale_factor)
> +	 * to keep integer precision.
> +	 * Formulae referenced from spec.
> +	 */
> +	scaled_m = div_s64(1000000 * scale_coef, *m * power_coef);
> +
> +	/* Maximize while keeping it bounded.*/
> +	while (scaled_m > MAX_M_VAL || scaled_m < MIN_M_VAL) {
> +		scaled_m = div_s64(scaled_m, 10);
> +		scale_factor++;
> +	}
> +	/* Scale up only if fractional part exists. */
> +	while (scaled_m * 10 < MAX_M_VAL && scaled_m * 10 > MIN_M_VAL && !is_integer) {

s/!is_integer/scale_coef != 1/
?

> +		scaled_m *= 10;
> +		scale_factor--;
> +	}
> +
> +	*m = scaled_m;
> +	*R = scale_factor;
> +}

...

> +static const struct i2c_device_id ina233_id[] = {
> +	{"ina233", 0},

Extra spaces to be consistant with ina233_of_match below?

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ina233_id);
> +
> +static const struct of_device_id __maybe_unused ina233_of_match[] = {
> +	{ .compatible = "ti,ina233" },
> +	{ },

No need for an extra comma after a terminator.

> +};
> +MODULE_DEVICE_TABLE(of, ina233_of_match);

...

CJ