On Thu, 10 Oct 2024 at 21:34, Kalesh Anakkur Purayil <kalesh-anakkur.purayil@xxxxxxxxxxxx> wrote: > > On Thu, Oct 10, 2024 at 12:50 AM Sanman Pradhan > <sanman.p211993@xxxxxxxxx> wrote: > > > > From: Sanman Pradhan <sanmanpradhan@xxxxxxxx> > > > > This patch adds support for hardware monitoring to the fbnic driver, > > allowing for temperature and voltage sensor data to be exposed to > > userspace via the HWMON interface. The driver registers a HWMON device > > and provides callbacks for reading sensor data, enabling system > > admins to monitor the health and operating conditions of fbnic. > > > > Signed-off-by: Sanman Pradhan <sanmanpradhan@xxxxxxxx> > > > > --- > > v5: > > - Drop hwmon_unregister label from fbnic_pci.c > > > > v4: https://patchwork.kernel.org/project/netdevbpf/patch/20241008143212.2354554-1-sanman.p211993@xxxxxxxxx/ > > > > v3: https://patchwork.kernel.org/project/netdevbpf/patch/20241004204953.2223536-1-sanman.p211993@xxxxxxxxx/ > > > > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241003173618.2479520-1-sanman.p211993@xxxxxxxxx/ > > > > v1: https://lore.kernel.org/netdev/153c5be4-158e-421a-83a5-5632a9263e87@xxxxxxxxxxxx/T/ > > > > --- > > drivers/net/ethernet/meta/fbnic/Makefile | 1 + > > drivers/net/ethernet/meta/fbnic/fbnic.h | 4 + > > drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c | 81 +++++++++++++++++++ > > drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 7 ++ > > drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 3 + > > 5 files changed, 96 insertions(+) > > create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c > > > > diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile > > index ed4533a73c57..41494022792a 100644 > > --- a/drivers/net/ethernet/meta/fbnic/Makefile > > +++ b/drivers/net/ethernet/meta/fbnic/Makefile > > @@ -11,6 +11,7 @@ fbnic-y := fbnic_devlink.o \ > > fbnic_ethtool.o \ > > fbnic_fw.o \ > > fbnic_hw_stats.o \ > > + fbnic_hwmon.o \ > > fbnic_irq.o \ > > fbnic_mac.o \ > > fbnic_netdev.o \ > > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h > > index 0f9e8d79461c..2d3aa20bc876 100644 > > --- a/drivers/net/ethernet/meta/fbnic/fbnic.h > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h > > @@ -18,6 +18,7 @@ > > struct fbnic_dev { > > struct device *dev; > > struct net_device *netdev; > > + struct device *hwmon; > > > > u32 __iomem *uc_addr0; > > u32 __iomem *uc_addr4; > > @@ -127,6 +128,9 @@ void fbnic_devlink_unregister(struct fbnic_dev *fbd); > > int fbnic_fw_enable_mbx(struct fbnic_dev *fbd); > > void fbnic_fw_disable_mbx(struct fbnic_dev *fbd); > > > > +void fbnic_hwmon_register(struct fbnic_dev *fbd); > > +void fbnic_hwmon_unregister(struct fbnic_dev *fbd); > > + > > int fbnic_pcs_irq_enable(struct fbnic_dev *fbd); > > void fbnic_pcs_irq_disable(struct fbnic_dev *fbd); > > > > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c > > new file mode 100644 > > index 000000000000..bcd1086e3768 > > --- /dev/null > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c > > @@ -0,0 +1,81 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright (c) Meta Platforms, Inc. and affiliates. */ > > + > > +#include <linux/hwmon.h> > > + > > +#include "fbnic.h" > > +#include "fbnic_mac.h" > > + > > +static int fbnic_hwmon_sensor_id(enum hwmon_sensor_types type) > > +{ > > + if (type == hwmon_temp) > > + return FBNIC_SENSOR_TEMP; > > + if (type == hwmon_in) > > + return FBNIC_SENSOR_VOLTAGE; > > + > > + return -EOPNOTSUPP; > > +} > > + > > +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; > > +} > > + > > +static int fbnic_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > > + u32 attr, int channel, long *val) > > +{ > > + struct fbnic_dev *fbd = dev_get_drvdata(dev); > > + const struct fbnic_mac *mac = fbd->mac; > > + int id; > > + > > + id = fbnic_hwmon_sensor_id(type); > > + return id < 0 ? id : mac->get_sensor(fbd, id, val); > > +} > > + > > +static const struct hwmon_ops fbnic_hwmon_ops = { > > + .is_visible = fbnic_hwmon_is_visible, > > + .read = fbnic_hwmon_read, > > +}; > > + > > +static const struct hwmon_channel_info *fbnic_hwmon_info[] = { > > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT), > > + HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), > > + NULL > > +}; > > + > > +static const struct hwmon_chip_info fbnic_chip_info = { > > + .ops = &fbnic_hwmon_ops, > > + .info = fbnic_hwmon_info, > > +}; > > + > > +void fbnic_hwmon_register(struct fbnic_dev *fbd) > > +{ > > + if (!IS_REACHABLE(CONFIG_HWMON)) > > + return; > > + > > + fbd->hwmon = hwmon_device_register_with_info(fbd->dev, "fbnic", > > + fbd, &fbnic_chip_info, > > + NULL); > > + if (IS_ERR(fbd->hwmon)) { > > + dev_notice(fbd->dev, > > + "Failed to register hwmon device %pe\n", > > + fbd->hwmon); > > + fbd->hwmon = NULL; > > + } > > +} > > + > > +void fbnic_hwmon_unregister(struct fbnic_dev *fbd) > > +{ > > + if (!IS_REACHABLE(CONFIG_HWMON) || !fbd->hwmon) > > + return; > > + > > + hwmon_device_unregister(fbd->hwmon); > > + fbd->hwmon = NULL; > > +} > > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h > > index 476239a9d381..05a591653e09 100644 > > --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h > > @@ -47,6 +47,11 @@ enum { > > #define FBNIC_LINK_MODE_PAM4 (FBNIC_LINK_50R1) > > #define FBNIC_LINK_MODE_MASK (FBNIC_LINK_AUTO - 1) > > > > +enum fbnic_sensor_id { > > + FBNIC_SENSOR_TEMP, /* Temp in millidegrees Centigrade */ > > + FBNIC_SENSOR_VOLTAGE, /* Voltage in millivolts */ > > +}; > > + > > /* This structure defines the interface hooks for the MAC. The MAC hooks > > * will be configured as a const struct provided with a set of function > > * pointers. > > @@ -83,6 +88,8 @@ struct fbnic_mac { > > > > void (*link_down)(struct fbnic_dev *fbd); > > void (*link_up)(struct fbnic_dev *fbd, bool tx_pause, bool rx_pause); > > + > > + int (*get_sensor)(struct fbnic_dev *fbd, int id, long *val); > Thank you for addressing the comments. One last question. > [Kalesh] I do not see the corresponding implementation of this > function. Am I missing soemthing here? > > }; > > > > int fbnic_mac_init(struct fbnic_dev *fbd); > > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c > > index a4809fe0fc24..ef9dc8c67927 100644 > > --- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c > > @@ -289,6 +289,8 @@ static int fbnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > > > fbnic_devlink_register(fbd); > > > > + fbnic_hwmon_register(fbd); > > + > > if (!fbd->dsn) { > > dev_warn(&pdev->dev, "Reading serial number failed\n"); > > goto init_failure_mode; > > @@ -345,6 +347,7 @@ static void fbnic_remove(struct pci_dev *pdev) > > fbnic_netdev_free(fbd); > > } > > > > + fbnic_hwmon_unregister(fbd); > > fbnic_devlink_unregister(fbd); > > fbnic_fw_disable_mbx(fbd); > > fbnic_free_irqs(fbd); > > -- > > 2.43.5 > > > > > -- > Regards, > Kalesh A P Thank you Kalesh, for pointing out the corresponding implementation and reviewing the patch. I've submitted v6 of my patch for review. Thank you. Regards, Sanman Pradhan
