Messages in this thread | | | Subject | Re: [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable | From | Guenter Roeck <> | Date | Sun, 10 Oct 2021 21:08:57 -0700 |
| |
On 10/10/21 6:53 PM, Oskar Senft wrote: > I wasn't sure if submitting JUST a v7 of nuvoton,nct7802.yaml (i.e. > the 1/2 in this series) would be the right thing to do and thought it > would be easier to follow if I dropped both. I couldn't find > documentation on what's the right thing to do in that case. Would it > have been better to only submit nuvoton,nct7802.yaml as "PATCH v7" ? >
First, please don't top-post.
Second, no, submitting both patches was the right thing to do, but you should not drop a Reviewed-by: tag if there was no change to the patch.
Guenter
> Oskar. > > On Sun, Oct 10, 2021 at 9:49 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 10/10/21 6:22 PM, Oskar Senft wrote: >>> This change allows LTD and RTD inputs to be configured via >>> device tree bindings. If the DT bindings are not present or >>> invalid, the input configuration is not modified and left at >>> HW defaults. >>> >>> Signed-off-by: Oskar Senft <osk@google.com> >> >> I sent a Reviewed-by: for v6 of this patch. >> >>> --- >>> Changes from PATCH v6: >>> - None (resubmitted due to changes in nuvoton,nct7802.yaml). >>> >> >> Why did you drop it if there was no change ? >> >> Guenter >> >>> Changes from PATCH v5: >>> - Removed unused "found_channel_config" variable. >>> - Initialize mode_mask and mode_val to defaults. >>> --- >>> drivers/hwmon/nct7802.c | 129 ++++++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 125 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c >>> index 604af2f6103a..d56f78327619 100644 >>> --- a/drivers/hwmon/nct7802.c >>> +++ b/drivers/hwmon/nct7802.c >>> @@ -51,6 +51,23 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = { >>> #define REG_CHIP_ID 0xfe >>> #define REG_VERSION_ID 0xff >>> >>> +/* >>> + * Resistance temperature detector (RTD) modes according to 7.2.32 Mode >>> + * Selection Register >>> + */ >>> +#define RTD_MODE_CURRENT 0x1 >>> +#define RTD_MODE_THERMISTOR 0x2 >>> +#define RTD_MODE_VOLTAGE 0x3 >>> + >>> +#define MODE_RTD_MASK 0x3 >>> +#define MODE_LTD_EN 0x40 >>> + >>> +/* >>> + * Bit offset for sensors modes in REG_MODE. >>> + * Valid for index 0..2, indicating RTD1..3. >>> + */ >>> +#define MODE_BIT_OFFSET_RTD(index) ((index) * 2) >>> + >>> /* >>> * Data structures and manipulation thereof >>> */ >>> @@ -1038,7 +1055,112 @@ static const struct regmap_config nct7802_regmap_config = { >>> .volatile_reg = nct7802_regmap_is_volatile, >>> }; >>> >>> -static int nct7802_init_chip(struct nct7802_data *data) >>> +static int nct7802_get_channel_config(struct device *dev, >>> + struct device_node *node, u8 *mode_mask, >>> + u8 *mode_val) >>> +{ >>> + u32 reg; >>> + const char *type_str, *md_str; >>> + u8 md; >>> + >>> + if (!node->name || of_node_cmp(node->name, "channel")) >>> + return 0; >>> + >>> + if (of_property_read_u32(node, "reg", ®)) { >>> + dev_err(dev, "Could not read reg value for '%s'\n", >>> + node->full_name); >>> + return -EINVAL; >>> + } >>> + >>> + if (reg > 3) { >>> + dev_err(dev, "Invalid reg (%u) in '%s'\n", reg, >>> + node->full_name); >>> + return -EINVAL; >>> + } >>> + >>> + if (reg == 0) { >>> + if (!of_device_is_available(node)) >>> + *mode_val &= ~MODE_LTD_EN; >>> + else >>> + *mode_val |= MODE_LTD_EN; >>> + *mode_mask |= MODE_LTD_EN; >>> + return 0; >>> + } >>> + >>> + /* At this point we have reg >= 1 && reg <= 3 */ >>> + >>> + if (!of_device_is_available(node)) { >>> + *mode_val &= ~(MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1)); >>> + *mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1); >>> + return 0; >>> + } >>> + >>> + if (of_property_read_string(node, "sensor-type", &type_str)) { >>> + dev_err(dev, "No type for '%s'\n", node->full_name); >>> + return -EINVAL; >>> + } >>> + >>> + if (!strcmp(type_str, "voltage")) { >>> + *mode_val |= (RTD_MODE_VOLTAGE & MODE_RTD_MASK) >>> + << MODE_BIT_OFFSET_RTD(reg - 1); >>> + *mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1); >>> + return 0; >>> + } >>> + >>> + if (strcmp(type_str, "temperature")) { >>> + dev_err(dev, "Invalid type '%s' for '%s'\n", type_str, >>> + node->full_name); >>> + return -EINVAL; >>> + } >>> + >>> + if (reg == 3) { >>> + /* RTD3 only supports thermistor mode */ >>> + md = RTD_MODE_THERMISTOR; >>> + } else { >>> + if (of_property_read_string(node, "temperature-mode", >>> + &md_str)) { >>> + dev_err(dev, "No mode for '%s'\n", node->full_name); >>> + return -EINVAL; >>> + } >>> + >>> + if (!strcmp(md_str, "thermal-diode")) >>> + md = RTD_MODE_CURRENT; >>> + else if (!strcmp(md_str, "thermistor")) >>> + md = RTD_MODE_THERMISTOR; >>> + else { >>> + dev_err(dev, "Invalid mode '%s' for '%s'\n", md_str, >>> + node->full_name); >>> + return -EINVAL; >>> + } >>> + } >>> + >>> + *mode_val |= (md & MODE_RTD_MASK) << MODE_BIT_OFFSET_RTD(reg - 1); >>> + *mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1); >>> + >>> + return 0; >>> +} >>> + >>> +static int nct7802_configure_channels(struct device *dev, >>> + struct nct7802_data *data) >>> +{ >>> + /* Enable local temperature sensor by default */ >>> + u8 mode_mask = MODE_LTD_EN, mode_val = MODE_LTD_EN; >>> + struct device_node *node; >>> + int err; >>> + >>> + if (dev->of_node) { >>> + for_each_child_of_node(dev->of_node, node) { >>> + err = nct7802_get_channel_config(dev, node, &mode_mask, >>> + &mode_val); >>> + if (err) >>> + return err; >>> + } >>> + } >>> + >>> + return regmap_update_bits(data->regmap, REG_MODE, mode_mask, mode_val); >>> +} >>> + >>> +static int nct7802_init_chip(struct device *dev, struct nct7802_data *data) >>> { >>> int err; >>> >>> @@ -1047,8 +1169,7 @@ static int nct7802_init_chip(struct nct7802_data *data) >>> if (err) >>> return err; >>> >>> - /* Enable local temperature sensor */ >>> - err = regmap_update_bits(data->regmap, REG_MODE, 0x40, 0x40); >>> + err = nct7802_configure_channels(dev, data); >>> if (err) >>> return err; >>> >>> @@ -1074,7 +1195,7 @@ static int nct7802_probe(struct i2c_client *client) >>> mutex_init(&data->access_lock); >>> mutex_init(&data->in_alarm_lock); >>> >>> - ret = nct7802_init_chip(data); >>> + ret = nct7802_init_chip(dev, data); >>> if (ret < 0) >>> return ret; >>> >>> >>
| |