Re: [PATCH v2 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] hwmon: ltc4283: missing default radc fallback in ltc4283_setup()
--

commit c323a5fb63d971c9df83e57980902ef0c39f7d96
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: ltc4283: Add support for the LTC4284 swap controller

This commit adds support for the LTC4284 hot swap controller to the ltc4283
hwmon driver, handling its dual current sense channels and dedicated ADC
power sense resistor.

> diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c
> index 9ec409bc26482..a8587beed853a 100644
> --- a/drivers/hwmon/ltc4283.c
> +++ b/drivers/hwmon/ltc4283.c
[ ... ]
> @@ -1240,6 +1335,27 @@ static const u32 ltc4283_fet_bad_delay[] = {
>  	256, 512, 1002, 2005
>  };
>  
> +static int ltc4283_read_resistor(struct device *dev, const char *prop,
> +				 u32 *val, bool optional)
> +{
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, prop, val);
> +	if (ret) {
> +		if (optional)
> +			return 0;
> +		return dev_err_probe(dev, ret, "Failed to read %s\n", prop);
> +	}
> +
> +	if (*val < LTC4283_MIN_RSENSE || *val > LTC4283_MAX_RSENSE)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "%s(%u) too small or too large [%u %u]\n",
> +				     prop, *val, LTC4283_MIN_RSENSE,
> +				     LTC4283_MAX_RSENSE);
> +	*val /= CENTI;
> +	return 0;
> +}
> +
>  static int ltc4283_setup(struct ltc4283_hwmon *st, struct device *dev)
>  {
>  	u32 val;
> @@ -1252,24 +1368,38 @@ static int ltc4283_setup(struct ltc4283_hwmon *st, struct device *dev)
>  
>  	/*
>  	 * Default to LTC4283_MIN_RSENSE so we can probe without FW properties.
> -	 */
> -	st->rsense = LTC4283_MIN_RSENSE;
> -	ret = device_property_read_u32(dev, "adi,rsense-nano-ohms",
> -				       &st->rsense);
> -	if (!ret) {
> -		if (st->rsense < LTC4283_MIN_RSENSE || st->rsense > LTC4283_MAX_RSENSE)
> -			return dev_err_probe(dev, -EINVAL,
> -					     "adi,rsense-nano-ohms(%u) too small or too large [%u %u]\n",
> -					     st->rsense, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE);
> -	}
> -
> -	/*
>  	 * The resolution for rsense is tenths of micro (eg: 62.5 uOhm) which
>  	 * means we need nano in the bindings. However, to make things easier to
>  	 * handle (with respect to overflows) we divide it by 100 as we don't
>  	 * really need the last two digits.
>  	 */
> -	st->rsense /= CENTI;
> +	st->rsense = LTC4283_MIN_RSENSE / CENTI;
> +	ret = ltc4283_read_resistor(dev, "adi,rsense-nano-ohms", &st->rsense, true);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * For LTC4284, rsense is for SENSE1, rsense2 for SENSE2, and radc is
> +	 * the effective sense resistance for ADC+- pin.
> +	 */
> +	if (st->info->id == ID_LTC4284) {
> +		ret = ltc4283_read_resistor(dev, "adi,rsense2-nano-ohms",
> +					    &st->rsense2, true);
> +		if (ret)
> +			return ret;
> +
> +		ret = ltc4283_read_resistor(dev, "adi,radc-nano-ohms",
> +					    &st->radc, false);
> +		if (ret)
> +			return ret;

[Severity: Low]
Should st->radc also default to LTC4283_MIN_RSENSE / CENTI and treat
"adi,radc-nano-ohms" as optional?

Unlike st->rsense, passing optional = false here causes an LTC4284 device
instantiated via ltc4283_i2c_id without firmware properties to fail probe
in ltc4283_setup().  Defaulting st->radc to LTC4283_MIN_RSENSE / CENTI
would allow LTC4284 to probe without firmware properties just like LTC4283
and would also make the optional parameter to ltc4283_read_resistor()
unnecessary.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-staging-ltc4284-v2-0-14a270a38f7b@analog.com?part=2