Thread (13 messages) flat view 13 messages, 3 authors, 13d ago

Re: [net-next,v2,2/6] eth: fbnic: expose all hwmon attributes unconditionally as read-only

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-03 03:24:09
Also in: linux-hwmon, lkml

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] fbnic_hwmon_is_visible() is reduced to an unconditional `return
  0444;` that ignores all four of its arguments (drvdata, type, attr,…

--

eth: fbnic: expose all hwmon attributes unconditionally as read-only

This changes fbnic_hwmon_is_visible() so that it returns 0444 for every
sensor type and attribute instead of matching on hwmon_temp_input and
hwmon_in_input.  The stated motivation is to prepare for threshold and
alarm attributes added later in the series.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
index 6c8c66ab86c13..38bb26cb8e6cb 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
@@ -20,12 +20,7 @@ static umode_t fbnic_hwmon_is_visible(const void *drvdata,
 				      enum hwmon_sensor_types type,
 				      u32 attr, int channel)
 {
-	if (type == hwmon_temp && attr == hwmon_temp_input)
-		return 0444;
-	if (type == hwmon_in && attr == hwmon_in_input)
-		return 0444;
-
-	return 0;
+	return 0444;
 }
[Severity: Low]
This isn't a bug, but now that fbnic_hwmon_is_visible() ignores all four
of its parameters (drvdata, type, attr and channel) and returns a constant,
could the static visibility field in struct hwmon_ops be used instead?

include/linux/hwmon.h documents it that way:

  /**
   * struct hwmon_ops - hwmon device operations
   * @visible:	Static visibility. If non-zero, 'is_visible' is ignored.
   * @is_visible: Callback to return attribute visibility. Mandatory unless
   *		'visible' is non-zero.

and hwmon_device_register_with_info() accepts either form, so
fbnic_hwmon_ops could become:

  static const struct hwmon_ops fbnic_hwmon_ops = {
	.visible = 0444,
	.read = fbnic_hwmon_read,
  };

with fbnic_hwmon_is_visible() deleted entirely.  Other drivers such as
drivers/net/phy/realtek/realtek_hwmon.c already use the .visible form.

This shape persists through the end of the series (fbnic_hwmon_is_visible()
is still a plain "return 0444;" installed via .is_visible at the last
patch), so a later patch does not convert it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831212234.1212971-1-limzhineng2%40gmail.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help