[{"id":1765871,"web_url":"http://patchwork.ozlabs.org/comment/1765871/","msgid":"<433d70c7-726d-c89e-e90b-262d473058ca@roeck-us.net>","list_archive_url":null,"date":"2017-09-09T21:20:36","subject":"Re: [patch v3 1/2] hwmon: (max6621) Add support for Maxim MAX6621\n\ttemperature sensor","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/05/2017 12:34 AM, Vadim Pasternak wrote:\n> MAX6621 is a PECI-to-I2C translator provides an efficient, low-cost\n> solution for PECI-to-SMBus/I2C protocol conversion. It allows reading the\n> temperature from the PECI-compliant host directly from up to four\n> PECI-enabled CPUs.\n> \n> Signed-off-by: Vadim Pasternak \n> ---\n> v2->v3\n> Comments pointed out by Guenter:\n> - arrange includes in alphabetic order;\n> - use regmap for caching the value of temp_offset to have attribute value\n> matching hardware value.\n> - Add to regmap writable, readable, volatile definitions and add cache\n> initialization to probe;\n> - have label attribute as RO;\n> - change if string layout in max6621_verify_reg_data;\n> - change hwmon_temp_alarm to hwmon_temp_crit_alarm;\n> - in max6621_read for reading hwmon_temp_crit_alarm set *val to 0 at\n> beginning of the case statement to recover from the case, when there is\n> no alert, f.e. reading MAX6621_TEMP_ALERT_CAUSE should result in\n> MAX6621_ALERT_DIS, which will return error, but alarm should be\n> returned as 0 in such case;\n> - clear the alert automatically after reading MAX6621_TEMP_ALERT_CAUSE;\n> - align clear alert on channels, which report alert temperature.\n> It requires access to the same register, but will be aligned with\n> the attribute's definitions;\n> Fixes added by Vadim:\n> - change MIN/MAX temperature defaults to -127000/128000;\n> - add initialization of CONFIG1 register;\n> v1->v2\n> Comments pointed out by Guenter:\n> - arrange includes in alphabetic order;\n> - add include for bitops.h;\n> - remove MAX6621_REG_NON_WRITEABLE_REG;\n> - drop temp_min, temp_max, temp_reset_history attributes;\n> - remove redundant braces in max6621_verify_reg_data;\n> - fix return code in max6621_verify_reg_data;\n> - not report channels which are not physically connected;\n> - use u32 for regval in max6621_read and max6621_write;\n> - provide in temprature offset in milli-degrees C;\n> - drop hwmon_temp_fault attribute;\n> - drop activation point setting in CONFIG2 reg, leave it for user space;\n> Comments pointed out by Jiri:\n> - fixe device name;\n> Fixes added by Vadim:\n> - modify names in max6621_temp_labels;\n> - add hwmon_temp_alarm attribute and hwmon_temp_crit attributes per\n> socket;\n> - fix defines for MIN and MAX temperature;\n> ---\n> drivers/hwmon/Kconfig | 14 ++\n> drivers/hwmon/Makefile | 1 +\n> drivers/hwmon/max6621.c | 602 ++++++++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 617 insertions(+)\n> create mode 100644 drivers/hwmon/max6621.c\n> \n> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig\n> index 5b9a61f..325d161 100644\n> --- a/drivers/hwmon/Kconfig\n> +++ b/drivers/hwmon/Kconfig\n> @@ -855,6 +855,20 @@ tristate \"MAX31722 temperature sensor\"\n> \t This driver can also be built as a module. If so, the module\n> \t will be called max31722.\n> \n> +config SENSORS_MAX6621\n> +\ttristate \"Maxim MAX6621 sensor chip\"\n> +\tdepends on I2C\n> +\tselect REGMAP_I2C\n> +\thelp\n> +\t If you say yes here you get support for MAX6621 sensor chip.\n> +\t MAX6621 is a PECI-to-I2C translator provides an efficient,\n> +\t low-cost solution for PECI-to-SMBus/I2C protocol conversion.\n> +\t It allows reading the temperature from the PECI-compliant\n> +\t host directly from up to four PECI-enabled CPUs.\n> +\n> +\t This driver can also be built as a module. If so, the module\n> +\t will be called max6621.\n> +\n> config SENSORS_MAX6639\n> \ttristate \"Maxim MAX6639 sensor chip\"\n> \tdepends on I2C\n> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile\n> index d4641a9..43333cb 100644\n> --- a/drivers/hwmon/Makefile\n> +++ b/drivers/hwmon/Makefile\n> @@ -116,6 +116,7 @@ obj-$(CONFIG_SENSORS_MAX1619)\t+= max1619.o\n> obj-$(CONFIG_SENSORS_MAX1668)\t+= max1668.o\n> obj-$(CONFIG_SENSORS_MAX197)\t+= max197.o\n> obj-$(CONFIG_SENSORS_MAX31722)\t+= max31722.o\n> +obj-$(CONFIG_SENSORS_MAX6621)\t+= max6621.o\n> obj-$(CONFIG_SENSORS_MAX6639)\t+= max6639.o\n> obj-$(CONFIG_SENSORS_MAX6642)\t+= max6642.o\n> obj-$(CONFIG_SENSORS_MAX6650)\t+= max6650.o\n> diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c\n> new file mode 100644\n> index 0000000..7c0122a\n> --- /dev/null\n> +++ b/drivers/hwmon/max6621.c\n> @@ -0,0 +1,602 @@\n> +/*\n> + * Hardware monitoring driver for Maxim MAX6621\n> + *\n> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.\n> + * Copyright (c) 2017 Vadim Pasternak \n> + *\n> + * This program is free software; you can redistribute it and/or modify\n> + * it under the terms of the GNU General Public License as published by\n> + * the Free Software Foundation; either version 2 of the License, or\n> + * (at your option) any later version.\n> + *\n> + * This program is distributed in the hope that it will be useful,\n> + * but WITHOUT ANY WARRANTY; without even the implied warranty of\n> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n> + * GNU General Public License for more details.\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#define MAX6621_DRV_NAME\t\t\"max6621\"\n> +#define MAX6621_TEMP_INPUT_REG_NUM\t9\n> +#define MAX6621_TEMP_INPUT_MIN\t\t-127000\n> +#define MAX6621_TEMP_INPUT_MAX\t\t128000\n> +\n> +#define MAX6621_TEMP_S0D0_REG\t\t0x00\n> +#define MAX6621_TEMP_S0D1_REG\t\t0x01\n> +#define MAX6621_TEMP_S1D0_REG\t\t0x02\n> +#define MAX6621_TEMP_S1D1_REG\t\t0x03\n> +#define MAX6621_TEMP_S2D0_REG\t\t0x04\n> +#define MAX6621_TEMP_S2D1_REG\t\t0x05\n> +#define MAX6621_TEMP_S3D0_REG\t\t0x06\n> +#define MAX6621_TEMP_S3D1_REG\t\t0x07\n> +#define MAX6621_TEMP_MAX_REG\t\t0x08\n> +#define MAX6621_TEMP_MAX_ADDR_REG\t0x0a\n> +#define MAX6621_TEMP_ALERT_CAUSE_REG\t0x0b\n> +#define MAX6621_CONFIG0_REG\t\t0x0c\n> +#define MAX6621_CONFIG1_REG\t\t0x0d\n> +#define MAX6621_CONFIG2_REG\t\t0x0e\n> +#define MAX6621_CONFIG3_REG\t\t0x0f\n> +#define MAX6621_TEMP_S0_ALERT_REG\t0x10\n> +#define MAX6621_TEMP_S1_ALERT_REG\t0x11\n> +#define MAX6621_TEMP_S2_ALERT_REG\t0x12\n> +#define MAX6621_TEMP_S3_ALERT_REG\t0x13\n> +#define MAX6621_CLEAR_ALERT_REG\t\t0x15\n> +#define MAX6621_REG_MAX\t\t\t(MAX6621_CLEAR_ALERT_REG + 1)\n> +#define MAX6621_REG_TEMP_SHIFT\t\t0x06\n> +\n> +#define MAX6621_ENABLE_TEMP_ALERTS_BIT\t4\n> +#define MAX6621_ENABLE_I2C_CRC_BIT\t5\n> +#define MAX6621_ENABLE_ALTERNATE_DATA\t6\n> +#define MAX6621_ENABLE_LOCKUP_TO\t7\n> +#define MAX6621_ENABLE_S0D0_BIT\t\t8\n> +#define MAX6621_ENABLE_S0D1_BIT\t\t9\n> +#define MAX6621_ENABLE_S1D0_BIT\t\t10\n> +#define MAX6621_ENABLE_S1D1_BIT\t\t11\n> +#define MAX6621_ENABLE_S2D0_BIT\t\t12\n> +#define MAX6621_ENABLE_S2D1_BIT\t\t13\n> +#define MAX6621_ENABLE_S3D0_BIT\t\t14\n> +#define MAX6621_ENABLE_S3D1_BIT\t\t15\n\nMaybe I am missing something, but why not use BIT() here ?\nI don't see the value in having _BIT in some of the defines\n(but not all of them).\n\n> +#define MAX6621_ENABLE_TEMP_ALL\t\tGENMASK(MAX6621_ENABLE_S3D1_BIT, \\\n> +\t\t\t\t\t\tMAX6621_ENABLE_S0D0_BIT)\n> +#define MAX6621_POLL_DELAY_MASK\t\t0x5\n> +#define MAX6621_CONFIG0_INIT\t\t(MAX6621_ENABLE_TEMP_ALL | \\\n> +\t\t\t\t\t BIT(MAX6621_ENABLE_LOCKUP_TO) | \\\n> +\t\t\t\t\t BIT(MAX6621_ENABLE_I2C_CRC_BIT) | \\\n> +\t\t\t\t\t MAX6621_POLL_DELAY_MASK)\n> +#define MAX6621_PECI_BIT_TIME\t\t0x2\n> +#define MAX6621_PECI_RETRY_NUM\t\t0x3\n> +#define MAX6621_CONFIG1_INIT\t\t((MAX6621_PECI_BIT_TIME << 8) | \\\n> +\t\t\t\t\t MAX6621_PECI_RETRY_NUM)\n> +\n> +/* Error codes */\n> +#define MAX6621_TRAN_FAILED\t0x8100\t/*\n> +\t\t\t\t\t * PECI transaction failed for more\n> +\t\t\t\t\t * than the configured number of\n> +\t\t\t\t\t * consecutive retries.\n> +\t\t\t\t\t */\n> +#define MAX6621_POOL_DIS\t0x8101\t/*\n> +\t\t\t\t\t * Polling disabled for requested\n> +\t\t\t\t\t * socket/domain.\n> +\t\t\t\t\t */\n> +#define MAX6621_POOL_UNCOMPLETE\t0x8102\t/*\n> +\t\t\t\t\t * First poll not yet completed for\n> +\t\t\t\t\t * requested socket/domain (on\n> +\t\t\t\t\t * startup).\n> +\t\t\t\t\t */\n> +#define MAX6621_SD_DIS\t\t0x8103\t/*\n> +\t\t\t\t\t * Read maximum temperature requested,\n> +\t\t\t\t\t * but no sockets/domains enabled or\n> +\t\t\t\t\t * all enabled sockets/domains have\n> +\t\t\t\t\t * errors; or read maximum temperature\n> +\t\t\t\t\t * address requested, but read maximum\n> +\t\t\t\t\t * temperature was not called.\n> +\t\t\t\t\t */\n> +#define MAX6621_ALERT_DIS\t0x8104\t/*\n> +\t\t\t\t\t * Get alert socket/domain requested,\n> +\t\t\t\t\t * but no alert active.\n> +\t\t\t\t\t */\n> +#define MAX6621_PECI_ERR_MIN\t0x8000\t/* Intel spec PECI error min value. */\n> +#define MAX6621_PECI_ERR_MAX\t0x80ff\t/* Intel spec PECI error max value. */\n> +\n> +static const u32 max6621_temp_regs[] = {\n> +\tMAX6621_TEMP_MAX_REG, MAX6621_TEMP_S0D0_REG, MAX6621_TEMP_S1D0_REG,\n> +\tMAX6621_TEMP_S2D0_REG, MAX6621_TEMP_S3D0_REG, MAX6621_TEMP_S0D1_REG,\n> +\tMAX6621_TEMP_S1D1_REG, MAX6621_TEMP_S2D1_REG, MAX6621_TEMP_S3D1_REG,\n> +};\n> +\n> +static const char *const max6621_temp_labels[] = {\n> +\t\"maximum\",\n> +\t\"socket0_0\",\n> +\t\"socket1_0\",\n> +\t\"socket2_0\",\n> +\t\"socket3_0\",\n> +\t\"socket0_1\",\n> +\t\"socket1_1\",\n> +\t\"socket2_1\",\n> +\t\"socket3_1\",\n> +};\n> +\n> +static const int max6621_temp_alert_chan2reg[] = {\n> +\tMAX6621_CLEAR_ALERT_REG,\n\nI still don't understand what this is used for.\n\n> +\tMAX6621_TEMP_S0_ALERT_REG,\n> +\tMAX6621_TEMP_S1_ALERT_REG,\n> +\tMAX6621_TEMP_S2_ALERT_REG,\n> +\tMAX6621_TEMP_S3_ALERT_REG,\n> +};\n> +\n> +/**\n> + * struct max6621_data - private data:\n> + *\n> + * @client: I2C client;\n> + * @regmap: register map handle;\n> + * @input_chan2reg: mapping from channel to register;\n> + */\n> +struct max6621_data {\n> +\tstruct i2c_client\t*client;\n> +\tstruct regmap\t\t*regmap;\n> +\tint\t\t\tinput_chan2reg[MAX6621_TEMP_INPUT_REG_NUM + 1];\n> +};\n> +\n> +static long max6621_temp_mc2reg(long val)\n> +{\n> +\treturn (val / 1000L) << MAX6621_REG_TEMP_SHIFT;\n> +}\n> +\n> +static umode_t\n> +max6621_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,\n> +\t\t int channel)\n> +{\n> +\t/* Skip channels which are not physically conncted. */\n> +\tif (((struct max6621_data *)data)->input_chan2reg[channel] < 0)\n> +\t\treturn 0;\n> +\n> +\tswitch (type) {\n> +\tcase hwmon_temp:\n> +\t\tswitch (attr) {\n> +\t\tcase hwmon_temp_input:\n> +\t\tcase hwmon_temp_label:\n> +\t\t\treturn 0444;\n> +\t\tcase hwmon_temp_offset:\n> +\t\tcase hwmon_temp_crit:\n> +\t\tcase hwmon_temp_crit_alarm:\n> +\t\t\treturn 0644;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int max6621_verify_reg_data(struct device *dev, int regval)\n> +{\n> +\tif (regval >= MAX6621_PECI_ERR_MIN &&\n> +\t regval <= MAX6621_PECI_ERR_MAX) {\n> +\t\tdev_dbg(dev, \"PECI error code - err 0x%04x.\\n\",\n> +\t\t\tregval);\n> +\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tswitch (regval) {\n> +\tcase MAX6621_TRAN_FAILED:\n> +\t\tdev_dbg(dev, \"PECI transaction failed - err 0x%04x.\\n\",\n> +\t\t\tregval);\n> +\t\tbreak;\n> +\tcase MAX6621_POOL_DIS:\n> +\t\tdev_dbg(dev, \"Polling disabled - err 0x%04x.\\n\", regval);\n> +\t\tbreak;\n> +\tcase MAX6621_POOL_UNCOMPLETE:\n> +\t\tdev_dbg(dev, \"First poll not completed on startup - err 0x%04x.\\n\",\n> +\t\t\tregval);\n> +\t\tbreak;\n> +\tcase MAX6621_SD_DIS:\n> +\t\tdev_dbg(dev, \"Resource is disabled - err 0x%04x.\\n\", regval);\n> +\t\tbreak;\n> +\tcase MAX6621_ALERT_DIS:\n> +\t\tdev_dbg(dev, \"No alert active - err 0x%04x.\\n\", regval);\n> +\t\tbreak;\n> +\tdefault:\n> +\t\treturn 0;\n> +\t}\n> +\n> +\treturn -EINVAL;\n\n-EINVAL reflects bad user input, which is not the case here. You'll have to find\nsome other error code.\n\n> +}\n> +\n> +static int\n> +max6621_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,\n> +\t int channel, long *val)\n> +{\n> +\tstruct max6621_data *data = dev_get_drvdata(dev);\n> +\tu32 regval;\n> +\tint reg;\n> +\ts8 temp;\n> +\tint ret;\n> +\n> +\tswitch (type) {\n> +\tcase hwmon_temp:\n> +\t\tswitch (attr) {\n> +\t\tcase hwmon_temp_input:\n> +\t\t\treg = data->input_chan2reg[channel];\n> +\t\t\tret = regmap_read(data->regmap, reg, ®val);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tret = max6621_verify_reg_data(dev, regval);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\t/*\n> +\t\t\t * Bit MAX6621_REG_TEMP_SHIFT represents 1 degree step.\n> +\t\t\t * The temperature is given in two's complement and 8\n> +\t\t\t * bits is used for the register conversion.\n> +\t\t\t */\n> +\t\t\ttemp = (regval >> MAX6621_REG_TEMP_SHIFT);\n> +\t\t\t*val = temp * 1000L;\n> +\n> +\t\t\tbreak;\n> +\t\tcase hwmon_temp_offset:\n> +\t\t\tret = regmap_read(data->regmap, MAX6621_CONFIG2_REG,\n> +\t\t\t\t\t ®val);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tret = max6621_verify_reg_data(dev, regval);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\t*val = (regval >> MAX6621_REG_TEMP_SHIFT) *\n> +\t\t\t 1000L;\n> +\n> +\t\t\tbreak;\n> +\t\tcase hwmon_temp_crit:\n> +\t\t\treg = max6621_temp_alert_chan2reg[channel];\n> +\t\t\tret = regmap_read(data->regmap, reg, ®val);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tret = max6621_verify_reg_data(dev, regval);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\t*val = regval * 1000L;\n> +\n> +\t\t\tbreak;\n> +\t\tcase hwmon_temp_crit_alarm:\n> +\t\t\t/*\n> +\t\t\t * Set val to zero to recover the case, when reading\n> +\t\t\t * MAX6621_TEMP_ALERT_CAUSE_REG results in for example\n> +\t\t\t * MAX6621_ALERT_DIS. Reading will return with error,\n> +\t\t\t * but in such case alarm should be returned as 0.\n> +\t\t\t */\n> +\t\t\t*val = 0;\n> +\t\t\tret = regmap_read(data->regmap,\n> +\t\t\t\t\t MAX6621_TEMP_ALERT_CAUSE_REG,\n> +\t\t\t\t\t ®val);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tret = max6621_verify_reg_data(dev, regval);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\treturn ret;\n> +\nmax6621_verify_reg_data() will return an error if MAX6621_ALERT_DIS is set.\nIt won't help that *val is set to 0, since reading the attribute will report\nthe error to user space (the read will fail and errno will be set).\n\n> +\t\t\t/*\n> +\t\t\t * Clear the alert automatically, using send-byte\n> +\t\t\t * smbus protocol for clearing alert.\n> +\t\t\t */\n> +\t\t\tif (regval) {\n> +\t\t\t\tret = i2c_smbus_write_byte(data->client,\n> +\t\t\t\t\t\tMAX6621_CLEAR_ALERT_REG);\n> +\t\t\t\tif (!ret)\n> +\t\t\t\t\treturn ret;\n> +\t\t\t}\n> +\nIf you are doing this, the writable alarm attribute is unnecessary;\nreading the attribute accomplishes the same.\n\n> +\t\t\t*val = !!regval;\n> +\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\treturn -EOPNOTSUPP;\n> +\t\t}\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\treturn -EOPNOTSUPP;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int\n> +max6621_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,\n> +\t int channel, long val)\n> +{\n> +\tstruct max6621_data *data = dev_get_drvdata(dev);\n> +\tu32 reg;\n> +\n> +\tswitch (type) {\n> +\tcase hwmon_temp:\n> +\t\tswitch (attr) {\n> +\t\tcase hwmon_temp_offset:\n> +\t\t\t/* Clamp to allowed range to prevent overflow. */\n> +\t\t\tval = clamp_val(val, MAX6621_TEMP_INPUT_MIN,\n> +\t\t\t\t\tMAX6621_TEMP_INPUT_MAX);\n> +\t\t\tval = max6621_temp_mc2reg(val);\n> +\n> +\t\t\treturn regmap_write(data->regmap,\n> +\t\t\t\t\t MAX6621_CONFIG2_REG, val);\n> +\t\tcase hwmon_temp_crit:\n> +\t\t\treg = max6621_temp_alert_chan2reg[channel];\n> +\t\t\t/* Clamp to allowed range to prevent overflow. */\n> +\t\t\tval = clamp_val(val, MAX6621_TEMP_INPUT_MIN,\n> +\t\t\t\t\tMAX6621_TEMP_INPUT_MAX);\n> +\t\t\tval = val / 1000L;\n> +\n> +\t\t\treturn regmap_write(data->regmap, reg, val);\n> +\t\tcase hwmon_temp_crit_alarm:\n> +\t\t\t/*\n> +\t\t\t * Use i2c_smbus_write_byte, since\n> +\t\t\t * MAX6621_CLEAR_ALERT_REG access expects send-byte\n> +\t\t\t * smbus protocol for clearing alert.\n> +\t\t\t */\n> +\t\t\treturn i2c_smbus_write_byte(data->client,\n> +\t\t\t\t\t\t MAX6621_CLEAR_ALERT_REG);\n> +\t\tdefault:\n> +\t\t\treturn -EOPNOTSUPP;\n> +\t\t}\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\treturn -EOPNOTSUPP;\n> +\t}\n> +\n> +\treturn -EOPNOTSUPP;\n> +}\n> +\n> +static int\n> +max6621_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,\n> +\t\t int channel, const char **str)\n> +{\n> +\tswitch (type) {\n> +\tcase hwmon_temp:\n> +\t\tswitch (attr) {\n> +\t\tcase hwmon_temp_label:\n> +\t\t\t*str = max6621_temp_labels[channel];\n> +\t\t\treturn 0;\n> +\t\tdefault:\n> +\t\t\treturn -EOPNOTSUPP;\n> +\t\t}\n> +\t\tbreak;\n> +\tdefault:\n> +\t\treturn -EOPNOTSUPP;\n> +\t}\n> +\n> +\treturn -EOPNOTSUPP;\n> +}\n> +\n> +static bool max6621_writeable_reg(struct device *dev, unsigned int reg)\n> +{\n> +\tswitch (reg) {\n> +\tcase MAX6621_CONFIG0_REG:\n> +\tcase MAX6621_CONFIG1_REG:\n> +\tcase MAX6621_CONFIG2_REG:\n> +\tcase MAX6621_CONFIG3_REG:\n> +\tcase MAX6621_TEMP_S0_ALERT_REG:\n> +\tcase MAX6621_TEMP_S1_ALERT_REG:\n> +\tcase MAX6621_TEMP_S2_ALERT_REG:\n> +\tcase MAX6621_TEMP_S3_ALERT_REG:\n> +\tcase MAX6621_TEMP_ALERT_CAUSE_REG:\n> +\t\treturn true;\n> +\t}\n> +\treturn false;\n> +}\n> +\n> +static bool max6621_readable_reg(struct device *dev, unsigned int reg)\n> +{\n> +\tswitch (reg) {\n> +\tcase MAX6621_TEMP_S0D0_REG:\n> +\tcase MAX6621_TEMP_S0D1_REG:\n> +\tcase MAX6621_TEMP_S1D0_REG:\n> +\tcase MAX6621_TEMP_S1D1_REG:\n> +\tcase MAX6621_TEMP_S2D0_REG:\n> +\tcase MAX6621_TEMP_S2D1_REG:\n> +\tcase MAX6621_TEMP_S3D0_REG:\n> +\tcase MAX6621_TEMP_S3D1_REG:\n> +\tcase MAX6621_TEMP_MAX_REG:\n> +\tcase MAX6621_TEMP_MAX_ADDR_REG:\n> +\tcase MAX6621_CONFIG0_REG:\n> +\tcase MAX6621_CONFIG1_REG:\n> +\tcase MAX6621_CONFIG2_REG:\n> +\tcase MAX6621_CONFIG3_REG:\n> +\tcase MAX6621_TEMP_S0_ALERT_REG:\n> +\tcase MAX6621_TEMP_S1_ALERT_REG:\n> +\tcase MAX6621_TEMP_S2_ALERT_REG:\n> +\tcase MAX6621_TEMP_S3_ALERT_REG:\n> +\t\treturn true;\n> +\t}\n> +\treturn false;\n> +}\n> +\n> +static bool max6621_volatile_reg(struct device *dev, unsigned int reg)\n> +{\n> +\tswitch (reg) {\n> +\tcase MAX6621_TEMP_S0D0_REG:\n> +\tcase MAX6621_TEMP_S0D1_REG:\n> +\tcase MAX6621_TEMP_S1D0_REG:\n> +\tcase MAX6621_TEMP_S1D1_REG:\n> +\tcase MAX6621_TEMP_S2D0_REG:\n> +\tcase MAX6621_TEMP_S2D1_REG:\n> +\tcase MAX6621_TEMP_S3D0_REG:\n> +\tcase MAX6621_TEMP_S3D1_REG:\n> +\tcase MAX6621_TEMP_MAX_REG:\n> +\tcase MAX6621_TEMP_S0_ALERT_REG:\n> +\tcase MAX6621_TEMP_S1_ALERT_REG:\n> +\tcase MAX6621_TEMP_S2_ALERT_REG:\n> +\tcase MAX6621_TEMP_S3_ALERT_REG:\n> +\tcase MAX6621_TEMP_ALERT_CAUSE_REG:\n> +\t\treturn true;\n> +\t}\n> +\treturn false;\n> +}\n> +\n> +static const struct reg_default max6621_regmap_default[] = {\n> +\t{ MAX6621_CONFIG0_REG, MAX6621_CONFIG0_INIT },\n> +\t{ MAX6621_CONFIG1_REG, MAX6621_CONFIG1_INIT },\n> +};\n> +\n> +static const struct regmap_config max6621_regmap_config = {\n> +\t.reg_bits = 8,\n> +\t.val_bits = 16,\n> +\t.max_register = MAX6621_REG_MAX,\n> +\t.val_format_endian = REGMAP_ENDIAN_LITTLE,\n> +\t.cache_type = REGCACHE_FLAT,\n> +\t.writeable_reg = max6621_writeable_reg,\n> +\t.readable_reg = max6621_readable_reg,\n> +\t.volatile_reg = max6621_volatile_reg,\n> +\t.reg_defaults = max6621_regmap_default,\n> +\t.num_reg_defaults = ARRAY_SIZE(max6621_regmap_default),\n> +};\n> +\n> +static u32 max6621_chip_config[] = {\n> +\tHWMON_C_REGISTER_TZ,\n> +\t0\n> +};\n> +\n> +static const struct hwmon_channel_info max6621_chip = {\n> +\t.type = hwmon_chip,\n> +\t.config = max6621_chip_config,\n> +};\n> +\n> +static const u32 max6621_temp_config[] = {\n> +\tHWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,\n> +\tHWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_CRIT_ALARM | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_CRIT_ALARM | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_CRIT_ALARM | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_CRIT_ALARM | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_LABEL,\n> +\tHWMON_T_INPUT | HWMON_T_LABEL,\n> +\t0\n> +};\n> +\n> +static const struct hwmon_channel_info max6621_temp = {\n> +\t.type = hwmon_temp,\n> +\t.config = max6621_temp_config,\n> +};\n> +\n> +static const struct hwmon_channel_info *max6621_info[] = {\n> +\t&max6621_chip,\n> +\t&max6621_temp,\n> +\tNULL\n> +};\n> +\n> +static const struct hwmon_ops max6621_hwmon_ops = {\n> +\t.read = max6621_read,\n> +\t.write = max6621_write,\n> +\t.read_string = max6621_read_string,\n> +\t.is_visible = max6621_is_visible,\n> +};\n> +\n> +static const struct hwmon_chip_info max6621_chip_info = {\n> +\t.ops = &max6621_hwmon_ops,\n> +\t.info = max6621_info,\n> +};\n> +\n> +static int max6621_probe(struct i2c_client *client,\n> +\t\t\t const struct i2c_device_id *id)\n> +{\n> +\tstruct device *dev = &client->dev;\n> +\tstruct max6621_data *data;\n> +\tstruct device *hwmon_dev;\n> +\tint i;\n> +\tint ret;\n> +\n> +\tdata = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);\n> +\tif (!data)\n> +\t\treturn -ENOMEM;\n> +\n> +\tdata->regmap = devm_regmap_init_i2c(client, &max6621_regmap_config);\n> +\tif (IS_ERR(data->regmap))\n> +\t\treturn PTR_ERR(data->regmap);\n> +\n> +\ti2c_set_clientdata(client, data);\n> +\tdata->client = client;\n> +\n> +\t/* Set CONFIG0 register masking temperature alerts and PEC. */\n> +\tret = regmap_write(data->regmap, MAX6621_CONFIG0_REG,\n> +\t\t\t MAX6621_CONFIG0_INIT);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Set CONFIG1 register for PEC access retry number. */\n> +\tret = regmap_write(data->regmap, MAX6621_CONFIG1_REG,\n> +\t\t\t MAX6621_CONFIG1_INIT);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Sync registers with hardware. */\n> +\tregcache_mark_dirty(data->regmap);\n> +\tret = regcache_sync(data->regmap);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/* Verify which temperature input registers are enabled. */\n> +\tfor (i = 0; i < MAX6621_TEMP_INPUT_REG_NUM; i++) {\n> +\t\tret = i2c_smbus_read_word_data(client, max6621_temp_regs[i]);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t\tret = max6621_verify_reg_data(dev, ret);\n> +\t\tif (ret) {\n> +\t\t\tdata->input_chan2reg[i] = -1;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tdata->input_chan2reg[i] = max6621_temp_regs[i];\n> +\t}\n> +\n> +\thwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,\n> +\t\t\t\t\t\t\t data,\n> +\t\t\t\t\t\t\t &max6621_chip_info,\n> +\t\t\t\t\t\t\t NULL);\n> +\n> +\treturn PTR_ERR_OR_ZERO(hwmon_dev);\n> +}\n> +\n> +static const struct i2c_device_id max6621_id[] = {\n> +\t{ MAX6621_DRV_NAME, 0 },\n> +\t{ }\n> +};\n> +MODULE_DEVICE_TABLE(i2c, max6621_id);\n> +\n> +static const struct of_device_id max6621_of_match[] = {\n> +\t{ .compatible = \"maxim,max6621\" },\n> +\t{ }\n> +};\n> +MODULE_DEVICE_TABLE(of, max6621_of_match);\n> +\n> +static struct i2c_driver max6621_driver = {\n> +\t.class\t\t= I2C_CLASS_HWMON,\n> +\t.driver = {\n> +\t\t.name = MAX6621_DRV_NAME,\n> +\t\t.of_match_table = of_match_ptr(max6621_of_match),\n> +\t},\n> +\t.probe\t\t= max6621_probe,\n> +\t.id_table\t= max6621_id,\n> +};\n> +\n> +module_i2c_driver(max6621_driver);\n> +\n> +MODULE_AUTHOR(\"Vadim Pasternak \");\n> +MODULE_DESCRIPTION(\"Driver for Maxim MAX6621\");\n> +MODULE_LICENSE(\"GPL\");\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"hR76IY+4\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xqRth6Nwcz9t2Z\n\tfor ;\n\tSun, 10 Sep 2017 07:20:44 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751081AbdIIVUl (ORCPT\n\t);\n\tSat, 9 Sep 2017 17:20:41 -0400","from mail-pf0-f194.google.com ([209.85.192.194]:36663 \"EHLO\n\tmail-pf0-f194.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751079AbdIIVUk (ORCPT\n\t); Sat, 9 Sep 2017 17:20:40 -0400","by mail-pf0-f194.google.com with SMTP id f84so3148949pfj.3;\n\tSat, 09 Sep 2017 14:20:40 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tr11sm9882730pfg.180.2017.09.09.14.20.37\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tSat, 09 Sep 2017 14:20:38 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=o6bWuyEXiAU/4ydq8wPyWy6IYi8IGly/gFWAv9N+07Y=;\n\tb=hR76IY+4FACNObFcy2WHtz7O9aHwo30LNHylArmRDAvOyJj9wUWItdCxoWhUslTXKX\n\tzts/Hya1WfcYjohKmUdAsZWTQL4rGu6Vg73YpejvDVgSIZ/tO2XCFOy8xvnDSCtZoOgv\n\t+u3JqS0eRYgmVLWnpOaDh+0dUcw1lSvyZ/edpP5YixRCOXgHtdLs6eCKxmIr2/fQpPO9\n\t+yXeDAvHdAlucr/9I/R+B+p0lKJq6cjuJ0VNTX90rqD3lVHW+zm/eS3b6wp6Zk/4HH4H\n\t0Qc41+jTv+sPAhIGrr/JHVRzcGeAE7sTxxKuwFKneZk299NRqCqp+GOGhQD5OB7NgJac\n\tFVvw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=o6bWuyEXiAU/4ydq8wPyWy6IYi8IGly/gFWAv9N+07Y=;\n\tb=PZ+HL3t5cx/ULQQdHLceaZ10B9bOCEB3JevxytXNYoXbRH0oImsKQqEx1h11qTEPC7\n\tPbEi2eU4RxPJGumsrAcKgBsS7AYJwawSck3Yw83Ok6hl+Nh0aYnWvqd9XbwPBruYbCcM\n\tQ6eloFBRivYFkQYZElz+B6Ou15SrYze9vEJFi9eyGezC/4hLTy6ykCdx7uOVEZ+BjDFO\n\tcLk52Iv2eRmdg9D6PmUu6wxXSh6m40y5QALXyRwFBIWGG61emXD/vDcJbMCLQ686RI/a\n\tu7s5poDsdW6Xsf28N6VwgZZnXNZ4WxPUxUkWN/ZDBwNvhaaPx8Or4u74Ip531Ea3VuHS\n\thGcg==","X-Gm-Message-State":"AHPjjUheheLlW26+u69vq00/CPKzYP1kNR2Mq/CYcd0dgO3D/lGLVjFy\n\tgKtwfkn0UZ3oZCMW","X-Google-Smtp-Source":"ADKCNb4CHjYEKG+qyTu4aY+e7St6j2uPG+B3lQXmlMaz+4OPHvLxc/frxNpm97UXhQlG+i0dY/p+Dg==","X-Received":"by 10.99.163.26 with SMTP id s26mr7303030pge.445.1504992039281; \n\tSat, 09 Sep 2017 14:20:39 -0700 (PDT)","Subject":"Re: [patch v3 1/2] hwmon: (max6621) Add support for Maxim MAX6621\n\ttemperature sensor","To":"Vadim Pasternak , robh+dt@kernel.org","Cc":"linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, jiri@resnulli.us","References":"<1504596848-160952-1-git-send-email-vadimp@mellanox.com>\n\t<1504596848-160952-2-git-send-email-vadimp@mellanox.com>","From":"Guenter Roeck ","Message-ID":"<433d70c7-726d-c89e-e90b-262d473058ca@roeck-us.net>","Date":"Sat, 9 Sep 2017 14:20:36 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504596848-160952-2-git-send-email-vadimp@mellanox.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}}]