| Message ID | 20241011235847.1209435-1-sanman.p211993@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [net-next,v6] eth: fbnic: Add hardware monitoring support via HWMON interface | expand |
One minor nit in line. LGTM otherwise. Thanks for taking care of the comments. On Sat, Oct 12, 2024 at 5:29 AM Sanman Pradhan <sanman.p211993@gmail.com> wrote: > > From: Sanman Pradhan <sanmanpradhan@meta.com> > > 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@meta.com> > > --- > v6: > - Add get_sensor implementation > > v5: https://patchwork.kernel.org/project/netdevbpf/patch/20241009192018.2683416-1-sanman.p211993@gmail.com/ > > v4: https://patchwork.kernel.org/project/netdevbpf/patch/20241008143212.2354554-1-sanman.p211993@gmail.com/ > > v3: https://patchwork.kernel.org/project/netdevbpf/patch/20241004204953.2223536-1-sanman.p211993@gmail.com/ > > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241003173618.2479520-1-sanman.p211993@gmail.com/ > > v1: https://lore.kernel.org/netdev/153c5be4-158e-421a-83a5-5632a9263e87@roeck-us.net/T/ > > --- > drivers/net/ethernet/meta/fbnic/Makefile | 1 + > drivers/net/ethernet/meta/fbnic/fbnic.h | 5 ++ > drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 7 ++ > drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c | 81 +++++++++++++++++++ > drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 23 ++++++ > drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 7 ++ > drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 3 + > 7 files changed, 127 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..ff0ff012c8d6 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; > @@ -30,6 +31,7 @@ struct fbnic_dev { > > struct fbnic_fw_mbx mbx[FBNIC_IPC_MBX_INDICES]; > struct fbnic_fw_cap fw_cap; > + struct fbnic_fw_completion *cmpl_data; > /* Lock protecting Tx Mailbox queue to prevent possible races */ > spinlock_t fw_tx_lock; > > @@ -127,6 +129,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_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > index 221faf8c6756..7cd8841920e4 100644 > --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > @@ -44,6 +44,13 @@ struct fbnic_fw_cap { > u8 link_fec; > }; > > +struct fbnic_fw_completion { > + struct { > + s32 millivolts; > + s32 millidegrees; > + } tsene; > +}; > + > void fbnic_mbx_init(struct fbnic_dev *fbd); > void fbnic_mbx_clean(struct fbnic_dev *fbd); > void fbnic_mbx_poll(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.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > index 7b654d0a6dac..aabfb0b72f52 100644 > --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > @@ -686,6 +686,28 @@ fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset, > MAC_STAT_TX_BROADCAST); > } > > +static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id, long *val) > +{ > + struct fbnic_fw_completion fw_cmpl; > + int err = 0; [Kalesh] No need of local variable "rc" > + s32 *sensor; > + > + switch (id) { > + case FBNIC_SENSOR_TEMP: > + sensor = &fw_cmpl.tsene.millidegrees; > + break; > + case FBNIC_SENSOR_VOLTAGE: > + sensor = &fw_cmpl.tsene.millivolts; > + break; > + default: > + return -EINVAL; > + } > + > + *val = *sensor; > + > + return err; > +} > + > static const struct fbnic_mac fbnic_mac_asic = { > .init_regs = fbnic_mac_init_regs, > .pcs_enable = fbnic_pcs_enable_asic, > @@ -695,6 +717,7 @@ static const struct fbnic_mac fbnic_mac_asic = { > .get_eth_mac_stats = fbnic_mac_get_eth_mac_stats, > .link_down = fbnic_mac_link_down_asic, > .link_up = fbnic_mac_link_up_asic, > + .get_sensor = fbnic_mac_get_sensor_asic, > }; > > /** > 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); > }; > > 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 >
On Mon, Oct 14, 2024 at 8:49 AM Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> wrote: > > One minor nit in line. LGTM otherwise. > > Thanks for taking care of the comments. > > On Sat, Oct 12, 2024 at 5:29 AM Sanman Pradhan <sanman.p211993@gmail.com> wrote: > > > > From: Sanman Pradhan <sanmanpradhan@meta.com> > > > > 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@meta.com> > > > > --- > > v6: > > - Add get_sensor implementation > > > > v5: https://patchwork.kernel.org/project/netdevbpf/patch/20241009192018.2683416-1-sanman.p211993@gmail.com/ > > > > v4: https://patchwork.kernel.org/project/netdevbpf/patch/20241008143212.2354554-1-sanman.p211993@gmail.com/ > > > > v3: https://patchwork.kernel.org/project/netdevbpf/patch/20241004204953.2223536-1-sanman.p211993@gmail.com/ > > > > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241003173618.2479520-1-sanman.p211993@gmail.com/ > > > > v1: https://lore.kernel.org/netdev/153c5be4-158e-421a-83a5-5632a9263e87@roeck-us.net/T/ > > > > --- > > drivers/net/ethernet/meta/fbnic/Makefile | 1 + > > drivers/net/ethernet/meta/fbnic/fbnic.h | 5 ++ > > drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 7 ++ > > drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c | 81 +++++++++++++++++++ > > drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 23 ++++++ > > drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 7 ++ > > drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 3 + > > 7 files changed, 127 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..ff0ff012c8d6 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; > > @@ -30,6 +31,7 @@ struct fbnic_dev { > > > > struct fbnic_fw_mbx mbx[FBNIC_IPC_MBX_INDICES]; > > struct fbnic_fw_cap fw_cap; > > + struct fbnic_fw_completion *cmpl_data; > > /* Lock protecting Tx Mailbox queue to prevent possible races */ > > spinlock_t fw_tx_lock; > > > > @@ -127,6 +129,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_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > > index 221faf8c6756..7cd8841920e4 100644 > > --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > > @@ -44,6 +44,13 @@ struct fbnic_fw_cap { > > u8 link_fec; > > }; > > > > +struct fbnic_fw_completion { > > + struct { > > + s32 millivolts; > > + s32 millidegrees; > > + } tsene; > > +}; > > + > > void fbnic_mbx_init(struct fbnic_dev *fbd); > > void fbnic_mbx_clean(struct fbnic_dev *fbd); > > void fbnic_mbx_poll(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.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > > index 7b654d0a6dac..aabfb0b72f52 100644 > > --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > > @@ -686,6 +686,28 @@ fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset, > > MAC_STAT_TX_BROADCAST); > > } > > > > +static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id, long *val) > > +{ > > + struct fbnic_fw_completion fw_cmpl; > > + int err = 0; > [Kalesh] No need of local variable "rc" Sorry for the typo, I meant "err" > > + s32 *sensor; > > + > > + switch (id) { > > + case FBNIC_SENSOR_TEMP: > > + sensor = &fw_cmpl.tsene.millidegrees; > > + break; > > + case FBNIC_SENSOR_VOLTAGE: > > + sensor = &fw_cmpl.tsene.millivolts; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + *val = *sensor; > > + > > + return err; > > +} > > + > > static const struct fbnic_mac fbnic_mac_asic = { > > .init_regs = fbnic_mac_init_regs, > > .pcs_enable = fbnic_pcs_enable_asic, > > @@ -695,6 +717,7 @@ static const struct fbnic_mac fbnic_mac_asic = { > > .get_eth_mac_stats = fbnic_mac_get_eth_mac_stats, > > .link_down = fbnic_mac_link_down_asic, > > .link_up = fbnic_mac_link_up_asic, > > + .get_sensor = fbnic_mac_get_sensor_asic, > > }; > > > > /** > > 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); > > }; > > > > 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
On Mon, 14 Oct 2024 at 01:25, Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> wrote: > > On Mon, Oct 14, 2024 at 8:49 AM Kalesh Anakkur Purayil > <kalesh-anakkur.purayil@broadcom.com> wrote: > > > > One minor nit in line. LGTM otherwise. > > > > Thanks for taking care of the comments. > > > > On Sat, Oct 12, 2024 at 5:29 AM Sanman Pradhan <sanman.p211993@gmail.com> wrote: > > > > > > From: Sanman Pradhan <sanmanpradhan@meta.com> > > > > > > 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@meta.com> > > > > > > --- > > > v6: > > > - Add get_sensor implementation > > > > > > v5: https://patchwork.kernel.org/project/netdevbpf/patch/20241009192018.2683416-1-sanman.p211993@gmail.com/ > > > > > > v4: https://patchwork.kernel.org/project/netdevbpf/patch/20241008143212.2354554-1-sanman.p211993@gmail.com/ > > > > > > v3: https://patchwork.kernel.org/project/netdevbpf/patch/20241004204953.2223536-1-sanman.p211993@gmail.com/ > > > > > > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241003173618.2479520-1-sanman.p211993@gmail.com/ > > > > > > v1: https://lore.kernel.org/netdev/153c5be4-158e-421a-83a5-5632a9263e87@roeck-us.net/T/ > > > > > > --- > > > drivers/net/ethernet/meta/fbnic/Makefile | 1 + > > > drivers/net/ethernet/meta/fbnic/fbnic.h | 5 ++ > > > drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 7 ++ > > > drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c | 81 +++++++++++++++++++ > > > drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 23 ++++++ > > > drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 7 ++ > > > drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 3 + > > > 7 files changed, 127 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..ff0ff012c8d6 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; > > > @@ -30,6 +31,7 @@ struct fbnic_dev { > > > > > > struct fbnic_fw_mbx mbx[FBNIC_IPC_MBX_INDICES]; > > > struct fbnic_fw_cap fw_cap; > > > + struct fbnic_fw_completion *cmpl_data; > > > /* Lock protecting Tx Mailbox queue to prevent possible races */ > > > spinlock_t fw_tx_lock; > > > > > > @@ -127,6 +129,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_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > > > index 221faf8c6756..7cd8841920e4 100644 > > > --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h > > > @@ -44,6 +44,13 @@ struct fbnic_fw_cap { > > > u8 link_fec; > > > }; > > > > > > +struct fbnic_fw_completion { > > > + struct { > > > + s32 millivolts; > > > + s32 millidegrees; > > > + } tsene; > > > +}; > > > + > > > void fbnic_mbx_init(struct fbnic_dev *fbd); > > > void fbnic_mbx_clean(struct fbnic_dev *fbd); > > > void fbnic_mbx_poll(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.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > > > index 7b654d0a6dac..aabfb0b72f52 100644 > > > --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > > > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c > > > @@ -686,6 +686,28 @@ fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset, > > > MAC_STAT_TX_BROADCAST); > > > } > > > > > > +static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id, long *val) > > > +{ > > > + struct fbnic_fw_completion fw_cmpl; > > > + int err = 0; > > [Kalesh] No need of local variable "rc" > Sorry for the typo, I meant "err" > > > + s32 *sensor; > > > + > > > + switch (id) { > > > + case FBNIC_SENSOR_TEMP: > > > + sensor = &fw_cmpl.tsene.millidegrees; > > > + break; > > > + case FBNIC_SENSOR_VOLTAGE: > > > + sensor = &fw_cmpl.tsene.millivolts; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + *val = *sensor; > > > + > > > + return err; > > > +} > > > + > > > static const struct fbnic_mac fbnic_mac_asic = { > > > .init_regs = fbnic_mac_init_regs, > > > .pcs_enable = fbnic_pcs_enable_asic, > > > @@ -695,6 +717,7 @@ static const struct fbnic_mac fbnic_mac_asic = { > > > .get_eth_mac_stats = fbnic_mac_get_eth_mac_stats, > > > .link_down = fbnic_mac_link_down_asic, > > > .link_up = fbnic_mac_link_up_asic, > > > + .get_sensor = fbnic_mac_get_sensor_asic, > > > }; > > > > > > /** > > > 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); > > > }; > > > > > > 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 > > > > -- > Regards, > Kalesh A P Thanks Kalesh for reviewing the patch, I've submitted v7 addressing your comment. Thank you. Regards, Sanman Pradhan
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..ff0ff012c8d6 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; @@ -30,6 +31,7 @@ struct fbnic_dev { struct fbnic_fw_mbx mbx[FBNIC_IPC_MBX_INDICES]; struct fbnic_fw_cap fw_cap; + struct fbnic_fw_completion *cmpl_data; /* Lock protecting Tx Mailbox queue to prevent possible races */ spinlock_t fw_tx_lock; @@ -127,6 +129,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_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h index 221faf8c6756..7cd8841920e4 100644 --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h @@ -44,6 +44,13 @@ struct fbnic_fw_cap { u8 link_fec; }; +struct fbnic_fw_completion { + struct { + s32 millivolts; + s32 millidegrees; + } tsene; +}; + void fbnic_mbx_init(struct fbnic_dev *fbd); void fbnic_mbx_clean(struct fbnic_dev *fbd); void fbnic_mbx_poll(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.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c index 7b654d0a6dac..aabfb0b72f52 100644 --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c @@ -686,6 +686,28 @@ fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset, MAC_STAT_TX_BROADCAST); } +static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id, long *val) +{ + struct fbnic_fw_completion fw_cmpl; + int err = 0; + s32 *sensor; + + switch (id) { + case FBNIC_SENSOR_TEMP: + sensor = &fw_cmpl.tsene.millidegrees; + break; + case FBNIC_SENSOR_VOLTAGE: + sensor = &fw_cmpl.tsene.millivolts; + break; + default: + return -EINVAL; + } + + *val = *sensor; + + return err; +} + static const struct fbnic_mac fbnic_mac_asic = { .init_regs = fbnic_mac_init_regs, .pcs_enable = fbnic_pcs_enable_asic, @@ -695,6 +717,7 @@ static const struct fbnic_mac fbnic_mac_asic = { .get_eth_mac_stats = fbnic_mac_get_eth_mac_stats, .link_down = fbnic_mac_link_down_asic, .link_up = fbnic_mac_link_up_asic, + .get_sensor = fbnic_mac_get_sensor_asic, }; /** 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); }; 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);