Re: [PATCH net-next v3 3/6] eth: fbnic: cache hwmon sensor readings

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Hardware monitoring functionality is implemented directly within the network driver instead of as an auxiliary device in drivers/hwmon/.
--

commit 4fa7201fcb9ec899fe4c2586948bbac1ad1bc36d
Author: Zinc Lim <limzhineng2@gmail.com>

eth: fbnic: cache hwmon sensor readings

This commit adds a caching mechanism for hardware monitoring sensor readings
to reduce the number of firmware mailbox messages. It seeds the cache with
a sentinel value and caches both temperature and voltage readings returned by
a single firmware response.

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
> index 38bb26cb8e6cb..c8e7ff1d36fc1 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
[ ... ]
> @@ -107,6 +118,10 @@ void fbnic_hwmon_register(struct fbnic_dev *fbd)
>  	if (!IS_REACHABLE(CONFIG_HWMON))
>  		return;
>  
> +	/* Seed cache with sentinel so the first read always refreshes. */
> +	fbd->hwmon_cache.temp_mdeg = FBNIC_SENSOR_NO_DATA;
> +	fbd->hwmon_cache.volt_mv = FBNIC_SENSOR_NO_DATA;
> +
>  	fbd->hwmon = hwmon_device_register_with_info(fbd->dev, "fbnic",
>  						     fbd, &fbnic_chip_info,
>  						     NULL);

[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing structural
issue that was explicitly flagged by the subsystem guidelines.

Should this hardware monitoring functionality be implemented as an auxiliary
device driver residing in drivers/hwmon/ instead of directly within the
network driver?

The hardware monitoring subsystem guidelines state that for devices where the
main functionality is not hardware monitoring (such as a network interface
controller), its hwmon functionality must be implemented as an auxiliary
device driver in drivers/hwmon/. Registering the hwmon device unconditionally
here in fbnic_hwmon_register() using hwmon_device_register_with_info()
bypasses hwmon maintainer review and breaks subsystem layering.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908225737.3034403-1-limzhineng2@gmail.com?part=3