From 50fe870273543a821f25e7f78ea7389183d819e7 Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Thu, 5 May 2022 15:46:07 +0100 Subject: [PATCH] hwmon: emc2305: fixups for driver submitted to mailing lists The driver had a number of issues, checkpatch warnings/errors, and other limitations, so fix these up to make it usable. Signed-off-by: Phil Elwell Signed-off-by: Dave Stevenson --- drivers/hwmon/emc2305.c | 86 ++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 36 deletions(-) --- a/drivers/hwmon/emc2305.c +++ b/drivers/hwmon/emc2305.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * Hardware monitoring driver for EMC2305 fan controller + * Hardware monitoring driver for Microchip EMC2305 fan controller * * Copyright (C) 2022 Nvidia Technologies Ltd and Delta Networks, Inc. */ @@ -17,17 +17,17 @@ static const unsigned short emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_END }; +#define EMC2305_REG_FAN_STATUS 0x24 +#define EMC2305_REG_FAN_STALL_STATUS 0x25 #define EMC2305_REG_DRIVE_FAIL_STATUS 0x27 -#define EMC2305_REG_DEVICE 0xfd #define EMC2305_REG_VENDOR 0xfe #define EMC2305_FAN_MAX 0xff #define EMC2305_FAN_MIN 0x00 #define EMC2305_FAN_MAX_STATE 10 -#define EMC2305_DEVICE 0x34 #define EMC2305_VENDOR 0x5d #define EMC2305_REG_PRODUCT_ID 0xfd #define EMC2305_TACH_REGS_UNUSE_BITS 3 -#define EMC2305_TACH_CNT_MULTIPLIER 0x02 +#define EMC2305_TACH_CNT_MULTIPLIER 1 /* Set by FAN_CFG RNGx bits */ #define EMC2305_PWM_MAX 5 #define EMC2305_PWM_CHNL_CMN 0 @@ -42,6 +42,7 @@ emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2 #define EMC2305_RPM_FACTOR 3932160 #define EMC2305_REG_FAN_DRIVE(n) (0x30 + 0x10 * (n)) +#define EMC2305_REG_FAN_CFG(n) (0x32 + 0x10 * (n)) #define EMC2305_REG_FAN_MIN_DRIVE(n) (0x38 + 0x10 * (n)) #define EMC2305_REG_FAN_TACH(n) (0x3e + 0x10 * (n)) @@ -62,7 +63,8 @@ static const struct i2c_device_id emc230 MODULE_DEVICE_TABLE(i2c, emc2305_ids); static const struct of_device_id emc2305_dt_ids[] = { - { .compatible = "smsc,emc2305" }, + { .compatible = "microchip,emc2305" }, + { .compatible = "microchip,emc2301" }, { } }; MODULE_DEVICE_TABLE(of, emc2305_dt_ids); @@ -187,7 +189,7 @@ static int emc2305_set_cur_state(struct struct emc2305_data *data = cdev->devdata; struct i2c_client *client = data->client; u8 val, i; - + if (state > data->max_state) return -EINVAL; @@ -200,19 +202,20 @@ static int emc2305_set_cur_state(struct state = max_t(unsigned long, state, data->cdev_data[cdev_idx].last_hwmon_state); val = EMC2305_PWM_STATE2DUTY(state, data->max_state, data->pwm_max); + if (val > EMC2305_FAN_MAX) return -EINVAL; data->cdev_data[cdev_idx].cur_state = state; if (data->pwm_channel == EMC2305_PWM_CHNL_CMN) - /* Set the same PWM value in all channels + /* Set the same PWM value in all channels * if common PWM channel is used. */ for (i = 0; i < data->pwm_num; i++) i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(i), val); else i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(cdev_idx), val); - + return 0; } @@ -274,7 +277,7 @@ static int emc2305_get_tz_of(struct devi struct emc2305_data *data = dev_get_drvdata(dev); int ret = 0; - /* OF parameters are optional - overwrite default setting + /* OF parameters are optional - overwrite default setting * if some of them are provided. */ @@ -296,8 +299,8 @@ static int emc2305_get_tz_of(struct devi return ret; } - /* Not defined or 0 means one thermal zone over all colling devices. - * Otherwise - separted thermal zones for each PWM channel. + /* Not defined or 0 means one thermal zone over all cooling devices. + * Otherwise - separated thermal zones for each PWM channel. */ if (of_find_property(np, "emc2305,pwm-channel", NULL)) { ret = of_property_read_u8(np, "emc2305,pwm-channel", &data->pwm_channel); @@ -313,7 +316,7 @@ static int emc2305_set_single_tz(struct struct emc2305_data *data = dev_get_drvdata(dev); long pwm = data->pwm_max; int cdev_idx; - + cdev_idx = (idx) ? idx - 1 : 0; if (dev->of_node) @@ -323,7 +326,7 @@ static int emc2305_set_single_tz(struct &emc2305_cooling_ops); else data->cdev_data[cdev_idx].cdev = - thermal_cooling_device_register(emc2305_fan_name[idx], data, + thermal_cooling_device_register(emc2305_fan_name[idx], data, &emc2305_cooling_ops); if (IS_ERR(data->cdev_data[cdev_idx].cdev)) { @@ -339,6 +342,22 @@ static int emc2305_set_single_tz(struct return 0; } +static void emc2305_unset_tz(struct device *dev) +{ + struct emc2305_data *data = dev_get_drvdata(dev); + int i; + + /* Unregister cooling device in case they have been registered by + * thermal_cooling_device_unregister(). No need for clean-up flow in case they + * have been registered by devm_thermal_of_cooling_device_register() + */ + if (!dev->of_node) { + for (i = 0; i < EMC2305_PWM_MAX; i++) + if (data->cdev_data[i].cdev) + thermal_cooling_device_unregister(data->cdev_data[i].cdev); + } +} + static int emc2305_set_tz(struct device *dev) { struct emc2305_data *data = dev_get_drvdata(dev); @@ -359,22 +378,6 @@ thermal_cooling_device_register_fail: return ret; } -static void emc2305_unset_tz(struct device *dev) -{ - struct emc2305_data *data = dev_get_drvdata(dev); - int i; - - /* Unregister cooling device in case they have been registered by - * thermal_cooling_device_unregister(). No need for clean-up flow in case they - * have been registered by devm_thermal_of_cooling_device_register() - */ - if (!dev->of_node) { - for (i = 0; i < EMC2305_PWM_MAX; i++) - if (data->cdev_data[i].cdev) - thermal_cooling_device_unregister(data->cdev_data[i].cdev); - } -} - static umode_t emc2305_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel) { @@ -548,7 +551,7 @@ static int emc2305_probe(struct i2c_clie struct i2c_adapter *adapter = client->adapter; struct device *dev = &client->dev; struct emc2305_data *data; - int vendor, device; + int vendor; int ret; int i; @@ -559,10 +562,6 @@ static int emc2305_probe(struct i2c_clie if (vendor != EMC2305_VENDOR) return -ENODEV; - device = i2c_smbus_read_byte_data(client, EMC2305_REG_DEVICE); - if (device != EMC2305_DEVICE) - return -ENODEV; - data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; @@ -595,8 +594,23 @@ static int emc2305_probe(struct i2c_clie return ret; } - for (i = 0; i < data->pwm_num; i++) - i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_MIN_DRIVE(i), data->pwm_min); + for (i = 0; i < data->pwm_num; i++) { + u8 val; + + i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_MIN_DRIVE(i), + data->pwm_min); + + val = i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_CFG(i)); + /* Set RNGx so minRPM=500 */ + val &= ~0x60; + i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_CFG(i), val); + } + + /* Acknowledge any existing faults. Stops the device responding on the + * SMBus alert address. + */ + i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_STALL_STATUS); + i2c_smbus_read_byte_data(client, EMC2305_REG_FAN_STATUS); return 0; }