Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753446AbbGAD2P (ORCPT ); Tue, 30 Jun 2015 23:28:15 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:56991 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750984AbbGAD2H (ORCPT ); Tue, 30 Jun 2015 23:28:07 -0400 Message-ID: <55935E41.9020107@roeck-us.net> Date: Tue, 30 Jun 2015 20:28:01 -0700 From: Guenter Roeck User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Constantine Shulyupin , jdelvare@suse.de, lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] hwmon: (nct7802) add temperature sensor type attribute References: <1435619148-4929-1-git-send-email-const@MakeLinux.com> In-Reply-To: <1435619148-4929-1-git-send-email-const@MakeLinux.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4675 Lines: 135 Hi Constantine, On 06/29/2015 04:05 PM, Constantine Shulyupin wrote: > From: const > > Sensor type: > 3 diode (current mode), MD=1 > 4 thermistor, MD=2 > > Reference: > Nuvoton Hardware Monitoring IC NCT7802Y > 7.2.32 Mode Selection Register > Location : Index 22h > > Signed-off-by: Constantine Shulyupin > --- > static struct attribute *nct7802_temp_attrs[] = { > + &sensor_dev_attr_temp1_type.dev_attr.attr, > &sensor_dev_attr_temp1_input.dev_attr.attr, > &sensor_dev_attr_temp1_min.dev_attr.attr, > &sensor_dev_attr_temp1_max.dev_attr.attr, > @@ -485,7 +529,9 @@ static struct attribute *nct7802_temp_attrs[] = { > &sensor_dev_attr_temp1_fault.dev_attr.attr, > &sensor_dev_attr_temp1_beep.dev_attr.attr, > > - &sensor_dev_attr_temp2_input.dev_attr.attr, /* 9 */ > + /* 10: */ My problem with this kind of change isn't that much that I wrote the code and I tend to use the other mechanism. My problem is that if I accept it, someone else who adds another attribute may change it back tomorrow, or find yet another means to express the same. And we'd end up with a lot of churn for pretty much personal preference reasons. So, please refrain from making changes like this. > + &sensor_dev_attr_temp2_type.dev_attr.attr, > + &sensor_dev_attr_temp2_input.dev_attr.attr, > &sensor_dev_attr_temp2_min.dev_attr.attr, > &sensor_dev_attr_temp2_max.dev_attr.attr, > &sensor_dev_attr_temp2_crit.dev_attr.attr, > @@ -495,7 +541,9 @@ static struct attribute *nct7802_temp_attrs[] = { > &sensor_dev_attr_temp2_fault.dev_attr.attr, > &sensor_dev_attr_temp2_beep.dev_attr.attr, > > - &sensor_dev_attr_temp3_input.dev_attr.attr, /* 18 */ > + /* 20: */ > + &sensor_dev_attr_temp3_type.dev_attr.attr, > + &sensor_dev_attr_temp3_input.dev_attr.attr, > &sensor_dev_attr_temp3_min.dev_attr.attr, > &sensor_dev_attr_temp3_max.dev_attr.attr, > &sensor_dev_attr_temp3_crit.dev_attr.attr, > @@ -505,7 +553,8 @@ static struct attribute *nct7802_temp_attrs[] = { > &sensor_dev_attr_temp3_fault.dev_attr.attr, > &sensor_dev_attr_temp3_beep.dev_attr.attr, > > - &sensor_dev_attr_temp4_input.dev_attr.attr, /* 27 */ > + /* 30: */ > + &sensor_dev_attr_temp4_input.dev_attr.attr, > &sensor_dev_attr_temp4_min.dev_attr.attr, > &sensor_dev_attr_temp4_max.dev_attr.attr, > &sensor_dev_attr_temp4_crit.dev_attr.attr, > @@ -514,7 +563,8 @@ static struct attribute *nct7802_temp_attrs[] = { > &sensor_dev_attr_temp4_crit_alarm.dev_attr.attr, > &sensor_dev_attr_temp4_beep.dev_attr.attr, > > - &sensor_dev_attr_temp5_input.dev_attr.attr, /* 35 */ > + /* 38: */ > + &sensor_dev_attr_temp5_input.dev_attr.attr, > &sensor_dev_attr_temp5_min.dev_attr.attr, > &sensor_dev_attr_temp5_max.dev_attr.attr, > &sensor_dev_attr_temp5_crit.dev_attr.attr, > @@ -523,7 +573,8 @@ static struct attribute *nct7802_temp_attrs[] = { > &sensor_dev_attr_temp5_crit_alarm.dev_attr.attr, > &sensor_dev_attr_temp5_beep.dev_attr.attr, > > - &sensor_dev_attr_temp6_input.dev_attr.attr, /* 43 */ > + /* 46: */ > + &sensor_dev_attr_temp6_input.dev_attr.attr, > &sensor_dev_attr_temp6_beep.dev_attr.attr, > > NULL > @@ -541,25 +592,31 @@ static umode_t nct7802_temp_is_visible(struct kobject *kobj, > if (err < 0) > return 0; > > - if (index < 9 && > + /* temp1 */ This comment is redundant with RD1. Same for the other temp comments below. Thanks, Guenter > + if (index < 10 && > (reg & 03) != 0x01 && (reg & 0x03) != 0x02) /* RD1 */ > return 0; > - if (index >= 9 && index < 18 && > + > + /* temp2 */ > + if (index >= 10 && index < 20 && > (reg & 0x0c) != 0x04 && (reg & 0x0c) != 0x08) /* RD2 */ > return 0; > - if (index >= 18 && index < 27 && (reg & 0x30) != 0x20) /* RD3 */ > + /* temp3 */ > + if (index >= 20 && index < 30 && (reg & 0x30) != 0x20) /* RD3 */ > return 0; > - if (index >= 27 && index < 35) /* local */ > + > + /* temp4, local */ > + if (index >= 30 && index < 38) > return attr->mode; > > err = regmap_read(data->regmap, REG_PECI_ENABLE, ®); > if (err < 0) > return 0; > > - if (index >= 35 && index < 43 && !(reg & 0x01)) /* PECI 0 */ > + if (index >= 38 && index < 46 && !(reg & 0x01)) /* PECI 0 */ > return 0; > > - if (index >= 0x43 && (!(reg & 0x02))) /* PECI 1 */ > + if (index >= 0x46 && (!(reg & 0x02))) /* PECI 1 */ > return 0; > > return attr->mode; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/