RE: [PATCH v4 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

"Torreno, Alexis Czezar" <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-hwmon
Message-ID <PH0PR03MB6351E5F74D16634FA34A081CF1CB2@PH0PR03MB6351.namprd03.prod.outlook.com>
> >> +	switch (reg) {
> >> +	case PMBUS_READ_VOUT:
> >> +		ret = pmbus_read_word_data(client, page, phase, reg);
> >> +		if (ret < 0)
> >> +			return ret;
> >> +
> >> +		/* Apply voltage divider scaling if resistors are non-zero */
> >> +		if (data->vout_rfb1 && data->vout_rfb2) {
> >> +			temp = (u64)data->vout_rfb1 + (u64)data->vout_rfb2;
> >> +			temp = DIV_ROUND_CLOSEST_ULL((u64)ret * temp,
> data->vout_rfb2);
> >> +			ret = clamp_val(temp, 0, 0xFFFF);
> >> +		}
> >> +		return ret;
> >> +	default:
> >> +		return -ENODATA;
> >> +	}
> >> +}
> >
> > [Severity: High]
> > Does this code introduce a UAPI inconsistency by only scaling
> PMBUS_READ_VOUT?
> >
> > When in1_input is read, it returns the scaled output voltage.
> >
> > However, because this switch returns -ENODATA for limits (like
> > PMBUS_VOUT_OV_WARN_LIMIT or PMBUS_VOUT_OV_FAULT_LIMIT),
> reading limits
> > like in1_max will return the unscaled feedback voltage directly from the chip.
> >
> 
> I would suggest to add a comment explaining that this series does not support
> any limit registers. That should "address" the issues raised by Sashiko, and also
> help future readers understand why the limit registers are not scaled.
> 

Will try adding this, thanks.
- Alexis
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.