On Mon, Aug 14, 2023 at 09:56:56PM -0700, Michael Chan wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@xxxxxxxxxxxx>
>
> The use of hwmon_device_register_with_groups() is deprecated.
> Modified the driver to use hwmon_device_register_with_info().
>
> Driver currently exports only temp1_input through hwmon sysfs
> interface. But FW has been modified to report more threshold
> temperatures and driver want to report them through the
> hwmon interface.
>
> Cc: Jean Delvare <jdelvare@xxxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Cc: linux-hwmon@xxxxxxxxxxxxxxx
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@xxxxxxxxxxxx>
> Signed-off-by: Michael Chan <michael.chan@xxxxxxxxxxxx>
> ---
> .../net/ethernet/broadcom/bnxt/bnxt_hwmon.c | 72 ++++++++++++++-----
> 1 file changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
> index 476616d97071..20381b7b1d78 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
> @@ -18,34 +18,74 @@
> #include "bnxt_hwrm.h"
> #include "bnxt_hwmon.h"
>
> -static ssize_t bnxt_show_temp(struct device *dev,
> - struct device_attribute *devattr, char *buf)
> +static int bnxt_hwrm_temp_query(struct bnxt *bp, u8 *temp)
> {
> struct hwrm_temp_monitor_query_output *resp;
> struct hwrm_temp_monitor_query_input *req;
> - struct bnxt *bp = dev_get_drvdata(dev);
> - u32 len = 0;
> int rc;
>
> rc = hwrm_req_init(bp, req, HWRM_TEMP_MONITOR_QUERY);
> if (rc)
> return rc;
> resp = hwrm_req_hold(bp, req);
> - rc = hwrm_req_send(bp, req);
> - if (!rc)
> - len = sprintf(buf, "%u\n", resp->temp * 1000); /* display millidegree */
> - hwrm_req_drop(bp, req);
> + rc = hwrm_req_send_silent(bp, req);
> if (rc)
> + goto err;
> +
> + if (temp)
> + *temp = resp->temp;
Why this NULL pointer check ? This is a static function,
and it is never called with a NULL pointer.
> +err:
> + hwrm_req_drop(bp, req);
> + return rc;
> +}
> +
> +static umode_t bnxt_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + if (type != hwmon_temp)
> + return 0;
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> +static int bnxt_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + struct bnxt *bp = dev_get_drvdata(dev);
> + u8 temp = 0;
> + int rc;
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + rc = bnxt_hwrm_temp_query(bp, &temp);
> + if (!rc)
> + *val = temp * 1000;
> return rc;
> - return len;
> + default:
> + return -EOPNOTSUPP;
> + }
> }
> -static SENSOR_DEVICE_ATTR(temp1_input, 0444, bnxt_show_temp, NULL, 0);
>
> -static struct attribute *bnxt_attrs[] = {
> - &sensor_dev_attr_temp1_input.dev_attr.attr,
> +static const struct hwmon_channel_info *bnxt_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT),
Nit: Unnecessary continuation line
> NULL
> };
> -ATTRIBUTE_GROUPS(bnxt);
> +
> +static const struct hwmon_ops bnxt_hwmon_ops = {
> + .is_visible = bnxt_hwmon_is_visible,
> + .read = bnxt_hwmon_read,
> +};
> +
> +static const struct hwmon_chip_info bnxt_hwmon_chip_info = {
> + .ops = &bnxt_hwmon_ops,
> + .info = bnxt_hwmon_info,
> +};
>
> void bnxt_hwmon_uninit(struct bnxt *bp)
> {
> @@ -72,9 +112,9 @@ void bnxt_hwmon_init(struct bnxt *bp)
> if (bp->hwmon_dev)
> return;
>
> - bp->hwmon_dev = hwmon_device_register_with_groups(&pdev->dev,
> - DRV_MODULE_NAME, bp,
> - bnxt_groups);
> + bp->hwmon_dev = hwmon_device_register_with_info(&pdev->dev,
> + DRV_MODULE_NAME, bp,
> + &bnxt_hwmon_chip_info, NULL);
> if (IS_ERR(bp->hwmon_dev)) {
> bp->hwmon_dev = NULL;
> dev_warn(&pdev->dev, "Cannot register hwmon device\n");
> --
> 2.30.1
>
