lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH-L+nN8UQeznWkJrZn4nTNeA+OW6MBq8RhCb0Nn0Q2C+uM8Gw@mail.gmail.com>
Date: Mon, 14 Oct 2024 08:49:06 +0530
From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.com>
To: Sanman Pradhan <sanman.p211993@...il.com>
Cc: netdev@...r.kernel.org, alexanderduyck@...com, kuba@...nel.org, 
	kernel-team@...a.com, davem@...emloft.net, edumazet@...gle.com, 
	pabeni@...hat.com, jdelvare@...e.com, linux@...ck-us.net, horms@...nel.org, 
	mohsin.bashr@...il.com, sanmanpradhan@...a.com, andrew@...n.ch, 
	linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v6] eth: fbnic: Add hardware monitoring support
 via HWMON interface

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@...il.com> wrote:
>
> From: Sanman Pradhan <sanmanpradhan@...a.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@...a.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
>


-- 
Regards,
Kalesh A P

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4239 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ