Re: [PATCH 1/3] Documentation/hwmon: Add onsemi's FD5121 controllers' documentation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Sysfs attribute descriptions contain mismatched input/output semantics due to copy-paste errors.
- [Low] Incomplete sentence missing a trailing period.
--

commit 10fc2806e78d641ce1fc7f05d01f0cec8dcccbe0
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

Documentation/hwmon: Add onsemi's FD5121 controllers' documentation

This commit documents the hardware monitoring support for FD5121, FD5123,
and FD5125 devices. It describes the supported telemetry data exposed via
sysfs, including voltage, current, power, and temperature measurements.

> diff --git a/Documentation/hwmon/fd5121.rst b/Documentation/hwmon/fd5121.rst
> new file mode 100644
> index 0000000000000..c279db4641e4a
> --- /dev/null
> +++ b/Documentation/hwmon/fd5121.rst

[ ... ]

> +Usage Notes
> +-----------
> +
> +This driver does not probe for PMBus devices. You will have
> +to instantiate devices explicitly.
> +
> +Example: the following commands will load the driver for the
> +controller at address 0x50 on I2C bus #1::
> +
> +    # modprobe fd5121
> +    # echo fd5121 0x50 > /sys/bus/i2c/devices/i2c-1/new_device
> +
> +It can also be instantiated by declaring in device tree

[Severity: Low]
This isn't a bug, but this sentence ends abruptly without a trailing period.

> +Sysfs attributes
> +----------------
> +
> +The following attributes are supported:
> +
> +======================  ====================================
> +curr[1-2]_label		"iin[1-2]"
> +curr[3-4]_label		"iout[1-2]"
> +curr[1-2]_input		Measured input current.
> +curr[3-4]_input		Measured output current.
> +curr[1-4]_crit_alarm	Output current critical high alarm.
> +curr[1-4]_max_alarm	Output current high alarm.

[Severity: Low]
Are the alarm descriptions for curr[1-2] accurate? Since curr[1-2] are labeled
as input current, should their alarms be described as input current alarms
instead of output current alarms?

> +
> +in[1-2]_label		"vin[1-2]"
> +in[3-4]_label		"vout[1-2]"
> +in[1-4]_lcrit_alarm	Input voltage critical low alarm.
> +in[1-4]_crit_alarm	Input voltage critical high alarm.

[Severity: Low]
Similarly, are the alarm descriptions for in[3-4] correct? They are labeled as
output voltage, but their alarms are described as input voltage alarms.

> +in[1-2]_max_alarm	Input voltage high alarm.
> +in[1-2]_input           Measured input voltage.
> +in[3-4]_input           Measured output voltage.
> +
> +power[1-2]_label	"pin[1-2]"
> +power[3-4]_label	"pout[1-2]"
> +power[3-4]_crit_alarm	Output power critical high alarm.
> +power[1-2]_max_alarm	Output power high alarm.

[Severity: Low]
Should power[1-2]_max_alarm be described as an input power high alarm, given
that power[1-2] represents input power?

> +power[1-4]_max          Power limit.
> +power[1-4]_input        Measured input power.

[Severity: Low]
Does power[3-4]_input actually measure input power? Given that power[3-4] is
labeled as output power, shouldn't this be described as measured output power?

> +power[3-4]_crit         Critical maximum output power.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-support-fd5121-from-onsemi-v1-0-b31767689c65@onsemi.com?part=1