[PATCH v2 1/2] docs/core-api: memory-allocation: remove uses of c:func:

[Chris Packham]

These are no longer needed as the documentation build will automatically
add the cross references.

Signed-off-by: Chris Packham 
---

Notes:
It should be noted that kvmalloc() and kmem_cache_destroy() lack a
kerneldoc header, a side-effect of this change is that the :c:func:
fallback of making them bold is lost. This is probably best fixed by
adding a kerneldoc header to their source.

Changes in v2:
- new

 Documentation/core-api/memory-allocation.rst | 49 +---
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/Documentation/core-api/memory-allocation.rst 
b/Documentation/core-api/memory-allocation.rst
index e59779aa7615..14e22accdee7 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -88,39 +88,36 @@ Selecting memory allocator
 ==
 
 The most straightforward way to allocate memory is to use a function
-from the :c:func:`kmalloc` family. And, to be on the safe side it's
-best to use routines that set memory to zero, like
-:c:func:`kzalloc`. If you need to allocate memory for an array, there
-are :c:func:`kmalloc_array` and :c:func:`kcalloc` helpers.
+from the kmalloc() family. And, to be on the safe side it's best to use
+routines that set memory to zero, like kzalloc(). If you need to
+allocate memory for an array, there are kmalloc_array() and kcalloc()
+helpers.
 
 The maximal size of a chunk that can be allocated with `kmalloc` is
 limited. The actual limit depends on the hardware and the kernel
 configuration, but it is a good practice to use `kmalloc` for objects
 smaller than page size.
 
-For large allocations you can use :c:func:`vmalloc` and
-:c:func:`vzalloc`, or directly request pages from the page
-allocator. The memory allocated by `vmalloc` and related functions is
-not physically contiguous.
+For large allocations you can use vmalloc() and vzalloc(), or directly
+request pages from the page allocator. The memory allocated by `vmalloc`
+and related functions is not physically contiguous.
 
 If you are not sure whether the allocation size is too large for
-`kmalloc`, it is possible to use :c:func:`kvmalloc` and its
-derivatives. It will try to allocate memory with `kmalloc` and if the
-allocation fails it will be retried with `vmalloc`. There are
-restrictions on which GFP flags can be used with `kvmalloc`; please
-see :c:func:`kvmalloc_node` reference documentation. Note that
-`kvmalloc` may return memory that is not physically contiguous.
+`kmalloc`, it is possible to use kvmalloc() and its derivatives. It will
+try to allocate memory with `kmalloc` and if the allocation fails it
+will be retried with `vmalloc`. There are restrictions on which GFP
+flags can be used with `kvmalloc`; please see kvmalloc_node() reference
+documentation. Note that `kvmalloc` may return memory that is not
+physically contiguous.
 
 If you need to allocate many identical objects you can use the slab
-cache allocator. The cache should be set up with
-:c:func:`kmem_cache_create` or :c:func:`kmem_cache_create_usercopy`
-before it can be used. The second function should be used if a part of
-the cache might be copied to the userspace.  After the cache is
-created :c:func:`kmem_cache_alloc` and its convenience wrappers can
-allocate memory from that cache.
-
-When the allocated memory is no longer needed it must be freed. You
-can use :c:func:`kvfree` for the memory allocated with `kmalloc`,
-`vmalloc` and `kvmalloc`. The slab caches should be freed with
-:c:func:`kmem_cache_free`. And don't forget to destroy the cache with
-:c:func:`kmem_cache_destroy`.
+cache allocator. The cache should be set up with kmem_cache_create() or
+kmem_cache_create_usercopy() before it can be used. The second function
+should be used if a part of the cache might be copied to the userspace.
+After the cache is created kmem_cache_alloc() and its convenience
+wrappers can allocate memory from that cache.
+
+When the allocated memory is no longer needed it must be freed. You can
+use kvfree() for the memory allocated with `kmalloc`, `vmalloc` and
+`kvmalloc`. The slab caches should be freed with kmem_cache_free(). And
+don't forget to destroy the cache with kmem_cache_destroy().
-- 
2.23.0

[PATCH v2 0/2] docs/core-api: memory-allocation: minor cleanups

[Chris Packham]

Clean up some formatting and add references to helpers for calculating sizes
safely.

Chris Packham (2):
  docs/core-api: memory-allocation: remove uses of c:func:
  docs/core-api: memory-allocation: mention size helpers

 Documentation/core-api/memory-allocation.rst | 50 ++--
 1 file changed, 24 insertions(+), 26 deletions(-)

-- 
2.23.0

[PATCH v2 2/2] docs/core-api: memory-allocation: mention size helpers

[Chris Packham]

Mention struct_size(), array_size() and array3_size() in the same place
as kmalloc() and friends.

Signed-off-by: Chris Packham 
---

Notes:
Changes in v2:
- Drop use of c:func:

 Documentation/core-api/memory-allocation.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/core-api/memory-allocation.rst 
b/Documentation/core-api/memory-allocation.rst
index 14e22accdee7..5c9dd70b0115 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -91,7 +91,8 @@ The most straightforward way to allocate memory is to use a 
function
 from the kmalloc() family. And, to be on the safe side it's best to use
 routines that set memory to zero, like kzalloc(). If you need to
 allocate memory for an array, there are kmalloc_array() and kcalloc()
-helpers.
+helpers. The helpers struct_size(), array_size() and array3_size() can
+be used to safely calculate object sizes without overflowing.
 
 The maximal size of a chunk that can be allocated with `kmalloc` is
 limited. The actual limit depends on the hardware and the kernel
-- 
2.23.0

Re: [PATCH] docs/core-api: memory-allocation: mention size helpers

[Chris Packham]

Hi Jon,

On Tue, 2019-10-22 at 07:19 -0600, Jonathan Corbet wrote:
> On Tue, 22 Oct 2019 10:27:47 +1300
> Chris Packham  wrote:
> 
> > Mention struct_size(), array_size() and array3_size() in the same place
> > as kmalloc() and friends.
> > 
> > Signed-off-by: Chris Packham 
> > ---
> >  Documentation/core-api/memory-allocation.rst | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/core-api/memory-allocation.rst 
> > b/Documentation/core-api/memory-allocation.rst
> > index e59779aa7615..6a131767becd 100644
> > --- a/Documentation/core-api/memory-allocation.rst
> > +++ b/Documentation/core-api/memory-allocation.rst
> > @@ -91,7 +91,9 @@ The most straightforward way to allocate memory is to use 
> > a function
> >  from the :c:func:`kmalloc` family. And, to be on the safe side it's
> >  best to use routines that set memory to zero, like
> >  :c:func:`kzalloc`. If you need to allocate memory for an array, there
> > -are :c:func:`kmalloc_array` and :c:func:`kcalloc` helpers.
> > +are :c:func:`kmalloc_array` and :c:func:`kcalloc` helpers. The helpers
> > +:c:func:`struct_size`, :c:func:`array_size` and :c:func:`array3_size` can 
> > be
> > +used to safely calculate object sizes without overflowing.
> 
> Quick comment: we don't need :c:func: anymore; the markup happens anyway.
> So rather than adding more of them, could I ask you to please take out the
> ones that are there now?

So just with backquotes i.e. :c:func:`kmalloc` becomes `kmalloc`?

> 
> Thanks,
> 
> jon

[PATCH] docs/core-api: memory-allocation: mention size helpers

[Chris Packham]

Mention struct_size(), array_size() and array3_size() in the same place
as kmalloc() and friends.

Signed-off-by: Chris Packham 
---
 Documentation/core-api/memory-allocation.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/core-api/memory-allocation.rst 
b/Documentation/core-api/memory-allocation.rst
index e59779aa7615..6a131767becd 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -91,7 +91,9 @@ The most straightforward way to allocate memory is to use a 
function
 from the :c:func:`kmalloc` family. And, to be on the safe side it's
 best to use routines that set memory to zero, like
 :c:func:`kzalloc`. If you need to allocate memory for an array, there
-are :c:func:`kmalloc_array` and :c:func:`kcalloc` helpers.
+are :c:func:`kmalloc_array` and :c:func:`kcalloc` helpers. The helpers
+:c:func:`struct_size`, :c:func:`array_size` and :c:func:`array3_size` can be
+used to safely calculate object sizes without overflowing.
 
 The maximal size of a chunk that can be allocated with `kmalloc` is
 limited. The actual limit depends on the hardware and the kernel
-- 
2.23.0

[PATCH] docs: ioctl: fix typo

[Chris Packham]

"pointres" should be "pointers".

Signed-off-by: Chris Packham 
---
 Documentation/ioctl/botching-up-ioctls.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/ioctl/botching-up-ioctls.rst 
b/Documentation/ioctl/botching-up-ioctls.rst
index ac697fef3545..2d4829b2fb09 100644
--- a/Documentation/ioctl/botching-up-ioctls.rst
+++ b/Documentation/ioctl/botching-up-ioctls.rst
@@ -46,7 +46,7 @@ will need to add a 32-bit compat layer:
conversion or worse, fiddle the raw __u64 through your code since that
diminishes the checking tools like sparse can provide. The macro
u64_to_user_ptr can be used in the kernel to avoid warnings about integers
-   and pointres of different sizes.
+   and pointers of different sizes.
 
 
 Basics
-- 
2.23.0

[PATCH] docs/core-api: memory-allocation: fix typo

[Chris Packham]

"on the safe size" should be "on the safe side".

Signed-off-by: Chris Packham 
---
 Documentation/core-api/memory-allocation.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/core-api/memory-allocation.rst 
b/Documentation/core-api/memory-allocation.rst
index 7744aa3bf2e0..e59779aa7615 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -88,7 +88,7 @@ Selecting memory allocator
 ==
 
 The most straightforward way to allocate memory is to use a function
-from the :c:func:`kmalloc` family. And, to be on the safe size it's
+from the :c:func:`kmalloc` family. And, to be on the safe side it's
 best to use routines that set memory to zero, like
 :c:func:`kzalloc`. If you need to allocate memory for an array, there
 are :c:func:`kmalloc_array` and :c:func:`kcalloc` helpers.
-- 
2.23.0

[PATCH] hwmon: (adt7475) document mapping of sysfs entries to inputs

[Chris Packham]

As per the usual standard with hwmon drivers the mapping to sysfs
entries follows the register map of the device e.g. in0_input
corresponds to the register 0x20, in1_input corresponds to 0x21 etc.

Hardware designers tend to work with input pins instead of registers
which is where things start to get confusing. A hardware designer might
say "the 1.5V rail is connected to the VCCP pin" leaving the software
designer none the wiser as to which of the sysfs entries should be
associated with the label "1.5V".

Try to bridge the gap by documenting the mapping of sysfs entries to
the corresponding pins. This should allow someone to create a
configuration file or other mapping without needing to dive into the
code and ADT datasheets.

Signed-off-by: Chris Packham 
---
After being given numerous hardware designs over the years using ADT chips for
hardware monitoring I found myself always having to read the schematic, ADT
datasheet and the adt7475.c driver.

I was about to document this mapping on an internal wiki page but I figured
it's probably of interest to the wider community. Hence this patch.

I've only documented the voltage mapping as the pwm and temperature and
tachometer pins appear to have reasonably sensible naming.

 Documentation/hwmon/adt7475 | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 09d73a10644c..01b46b290532 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -79,6 +79,18 @@ ADT7490:
   * 2 GPIO pins (not implemented)
   * system acoustics optimizations (not implemented)
 
+Sysfs Mapping
+-
+
+ ADT7490 ADT7476 ADT7475   ADT7473
+ --- --- ---   ---
+in0  2.5VIN (22) 2.5VIN (22) - -
+in1  VCCP   (23) VCCP   (23) VCCP (14) VCCP (14)
+in2  VCC(4)  VCC(4)  VCC  (4)  VCC  (3)
+in3  5VIN   (20) 5VIN   (20)
+in4  12VIN  (21) 12VIN  (21)
+in5  VTT(8)
+
 Special Features
 
 
-- 
2.19.1

[PATCH] switchdev: documentation: minor typo fixes

[Chris Packham]

Two typos in switchdev.txt

Signed-off-by: Chris Packham 
---
 Documentation/networking/switchdev.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/switchdev.txt 
b/Documentation/networking/switchdev.txt
index 3e7b946dea27..5e40e1f68873 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -228,7 +228,7 @@ Learning on the device port should be enabled, as well as 
learning_sync:
bridge link set dev DEV learning on self
bridge link set dev DEV learning_sync on self
 
-Learning_sync attribute enables syncing of the learned/forgotton FDB entry to
+Learning_sync attribute enables syncing of the learned/forgotten FDB entry to
 the bridge's FDB.  It's possible, but not optimal, to enable learning on the
 device port and on the bridge port, and disable learning_sync.
 
@@ -245,7 +245,7 @@ the responsibility of the port driver/device to age out 
these entries.  If the
 port device supports ageing, when the FDB entry expires, it will notify the
 driver which in turn will notify the bridge with SWITCHDEV_FDB_DEL.  If the
 device does not support ageing, the driver can simulate ageing using a
-garbage collection timer to monitor FBD entries.  Expired entries will be
+garbage collection timer to monitor FDB entries.  Expired entries will be
 notified to the bridge using SWITCHDEV_FDB_DEL.  See rocker driver for
 example of driver running ageing timer.
 
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] switchdev: documentation: minor typo fixes

[Chris Packham]

Two typos in switchdev.txt

Signed-off-by: Chris Packham 
---
 Documentation/networking/switchdev.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/switchdev.txt 
b/Documentation/networking/switchdev.txt
index 3e7b946dea27..5e40e1f68873 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -228,7 +228,7 @@ Learning on the device port should be enabled, as well as 
learning_sync:
bridge link set dev DEV learning on self
bridge link set dev DEV learning_sync on self
 
-Learning_sync attribute enables syncing of the learned/forgotton FDB entry to
+Learning_sync attribute enables syncing of the learned/forgotten FDB entry to
 the bridge's FDB.  It's possible, but not optimal, to enable learning on the
 device port and on the bridge port, and disable learning_sync.
 
@@ -245,7 +245,7 @@ the responsibility of the port driver/device to age out 
these entries.  If the
 port device supports ageing, when the FDB entry expires, it will notify the
 driver which in turn will notify the bridge with SWITCHDEV_FDB_DEL.  If the
 device does not support ageing, the driver can simulate ageing using a
-garbage collection timer to monitor FBD entries.  Expired entries will be
+garbage collection timer to monitor FDB entries.  Expired entries will be
 notified to the bridge using SWITCHDEV_FDB_DEL.  See rocker driver for
 example of driver running ageing timer.
 
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kbuild-all] [PATCH v4 2/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

On 17/05/17 15:09, Ye Xiaolong wrote:
> On 05/16, Chris Packham wrote:
>> On 16/05/17 20:23, kbuild test robot wrote:
>>> Hi Chris,
>>>
>>> [auto build test ERROR on hwmon/hwmon-next]
>>> [also build test ERROR on v4.12-rc1 next-20170516]
>>> [if your patch is applied to the wrong git tree, please drop us a note to 
>>> help improve the system]
>>>
>>> url:
>>> https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
>>> base:   
>>> https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
>>> hwmon-next
>>> config: x86_64-rhel (attached as .config)
>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>> reproduce:
>>> # save the attached .config to linux build tree
>>> make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 
>>>>> 'find_closest_descending' [-Werror=implicit-function-declaration]
>>>   val = find_closest_descending(val, ad7475_st_map,
>>> ^~~
>>>cc1: some warnings being treated as errors
>>>
>>> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>>>
>>>616  shift = 4;
>>>617  idx = 1;
>>>618  break;
>>>619  }
>>>620  
>>>621  if (val > 0) {
>>>  > 622  val = find_closest_descending(val, 
>>> ad7475_st_map,
>>>623
>>> ARRAY_SIZE(ad7475_st_map));
>>>624  val |= 0x8;
>>>625  }
>>>
>>> ---
>>> 0-DAY kernel test infrastructureOpen Source Technology 
>>> Center
>>> https://lists.01.org/pipermail/kbuild-all   Intel 
>>> Corporation
>>>
>>
>> I'm not sure how this is failing. find_closest_descending() is a macro
>> defined in linux/util_macros.h which is explicitly included in
>> drivers/hwmon/adt7475.c. Aside from the include guards there's nothing
>> conditional about it.
>
> Hi,
>
> 0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add 
> client driver for IR35221"),
> is it wrong or you have some prerequisite patches?
>

Looks like it's missing 
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=bbb4dd0ff
 
which was part of the series but was applied after v3 so I didn't send 
it out with v4.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

On 16/05/17 20:23, kbuild test robot wrote:
> Hi Chris,
>
> [auto build test ERROR on hwmon/hwmon-next]
> [also build test ERROR on v4.12-rc1 next-20170516]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
> hwmon-next
> config: x86_64-rhel (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
>drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 
>>> 'find_closest_descending' [-Werror=implicit-function-declaration]
>   val = find_closest_descending(val, ad7475_st_map,
> ^~~
>cc1: some warnings being treated as errors
>
> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>
>616shift = 4;
>617idx = 1;
>618break;
>619}
>620
>621if (val > 0) {
>  > 622val = find_closest_descending(val, 
> ad7475_st_map,
>623  
> ARRAY_SIZE(ad7475_st_map));
>624val |= 0x8;
>625}
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>

I'm not sure how this is failing. find_closest_descending() is a macro 
defined in linux/util_macros.h which is explicitly included in 
drivers/hwmon/adt7475.c. Aside from the include guards there's nothing 
conditional about it.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/3] hwmon: (adt7475) fan stall prevention

[Chris Packham]

By default adt7475 will stop the fans (pwm duty cycle 0%) when the
temperature drops past Tmin - hysteresis. Some systems want to keep the
fans moving even when the temperature drops so add new sysfs attributes
that configure the enhanced acoustics min 1-3 which allows the fans to
run at the minimum configure pwm duty cycle.

Signed-off-by: Chris Packham 
---
Changes in v2:
- use pwmN_stall_dis as the attribute name. I think this describes the purpose
  pretty well. I went with a new attribute instead of overloading
  pwmN_auto_point1_pwm so this doesn't affect existing users.
Changes in v3:
- Fix grammar.
- change enh_acou to enh_acoustics
Changes in v4:
- Change sysfs attribute to pwmN_stall_disable

 Documentation/hwmon/adt7475 |  5 +
 drivers/hwmon/adt7475.c | 50 +
 2 files changed, 55 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 0502f2b464e1..dc0b55794c47 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 
255 (full speed).
 Fan speed may be set to maximum when the temperature sensor associated with
 the PWM control exceeds temp#_max.
 
+At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
+minimum (i.e. auto_point1_pwm). This behaviour can be configured using the
+pwm[1-*]_stall_disable sysfs attribute. A value of 0 means the fans will shut
+off. A value of 1 means the fans will run at auto_point1_pwm.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index ec0c43fbcdce..3eb8c5c2f8af 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -79,6 +79,9 @@
 
 #define REG_TEMP_TRANGE_BASE   0x5F
 
+#define REG_ENHANCE_ACOUSTICS1 0x62
+#define REG_ENHANCE_ACOUSTICS2 0x63
+
 #define REG_PWM_MIN_BASE   0x64
 
 #define REG_TEMP_TMIN_BASE 0x67
@@ -209,6 +212,7 @@ struct adt7475_data {
u8 range[3];
u8 pwmctl[3];
u8 pwmchan[3];
+   u8 enh_acoustics[2];
 
u8 vid;
u8 vrm;
@@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct 
device_attribute *attr,
data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
i2c_smbus_write_byte_data(client, reg,
  data->pwm[sattr->nr][sattr->index]);
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
+static ssize_t show_stall_disable(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   u8 mask = BIT(5 + sattr->index);
+
+   return sprintf(buf, "%d\n", !!(data->enh_acoustics[0] & mask));
+}
+
+static ssize_t set_stall_disable(struct device *dev,
+struct device_attribute *attr, const char *buf,
+size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   long val;
+   u8 mask = BIT(5 + sattr->index);
+
+   if (kstrtol(buf, 10, &val))
+   return -EINVAL;
+
+   mutex_lock(&data->lock);
+
+   data->enh_acoustics[0] &= ~mask;
+   if (val)
+   data->enh_acoustics[0] |= mask;
+
+   i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
+ data->enh_acoustics[0]);
 
mutex_unlock(&data->lock);
 
@@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 0);
 static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 0);
+static SENSOR_DEVICE_ATTR_2(pwm1_stall_disable, S_IRUGO | S_IWUSR,
+   show_stall_disable, set_stall_disable, 0, 0);
 static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
1);
 static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 1);
 static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 1);
+static SENSOR_DEVICE_ATTR_2(pwm2_stall_disable, S_IRUGO | S_IWUSR,
+   show_stall_disable, set_stall_disable, 0, 1);
 static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
2);
 static SENSOR_DEVICE_ATTR_2(pwm3_freq

[PATCH v4 2/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

When enabled temperature smoothing allows ramping the fan speed over a
configurable period of time instead of jumping to the new speed
instantaneously.

Signed-off-by: Chris Packham 
---
Changes in v2:
- use a single tempN_smoothing attribute
Changes in v3:
- change enh_acou to enh_acoustics
- simplify show_temp_st()
Changes in v4:
- removed dead code.
- Make the order of the smoothing attributes match the other temperature
  attributes.

Guenter,

We'd previously discussed making the smoothing values set CONFIG6[SLOW] to
expose the other set of potential values. I wasn't sure where you wanted to go
on that one.

Personally I was on the fence since the difference would only be noticeable for
the higher values. If we do want to add support for the other values it could
be done as a subsequent patch (or a v5 if you want it).

 Documentation/hwmon/adt7475 |  4 ++
 drivers/hwmon/adt7475.c | 91 +
 2 files changed, 95 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index dc0b55794c47..09d73a10644c 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour can be 
configured using the
 pwm[1-*]_stall_disable sysfs attribute. A value of 0 means the fans will shut
 off. A value of 1 means the fans will run at auto_point1_pwm.
 
+The responsiveness of the ADT747x to temperature changes can be configured.
+This allows smoothing of the fan speed transition. To set the transition time
+set the value in ms in the temp[1-*]_smoothing sysfs attribute.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 3eb8c5c2f8af..3056076fae27 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,88 @@ static ssize_t set_temp(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
+/* Assuming CONFIG6[SLOW] is 0 */
+static const int ad7475_st_map[] = {
+   37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
+};
+
+static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   long val;
+
+   switch (sattr->index) {
+   case 0:
+   val = data->enh_acoustics[0] & 0xf;
+   break;
+   case 1:
+   val = (data->enh_acoustics[1] >> 4) & 0xf;
+   break;
+   case 2:
+   default:
+   val = data->enh_acoustics[1] & 0xf;
+   break;
+   }
+
+   if (val & 0x8)
+   return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
+   else
+   return sprintf(buf, "0\n");
+}
+
+static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   unsigned char reg;
+   int shift, idx;
+   ulong val;
+
+   if (kstrtoul(buf, 10, &val))
+   return -EINVAL;
+
+   switch (sattr->index) {
+   case 0:
+   reg = REG_ENHANCE_ACOUSTICS1;
+   shift = 0;
+   idx = 0;
+   break;
+   case 1:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 0;
+   idx = 1;
+   break;
+   case 2:
+   default:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 4;
+   idx = 1;
+   break;
+   }
+
+   if (val > 0) {
+   val = find_closest_descending(val, ad7475_st_map,
+ ARRAY_SIZE(ad7475_st_map));
+   val |= 0x8;
+   }
+
+   mutex_lock(&data->lock);
+
+   data->enh_acoustics[idx] &= ~(0xf << shift);
+   data->enh_acoustics[idx] |= (val << shift);
+
+   i2c_smbus_write_byte_data(client, reg, data->enh_acoustics[idx]);
+
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
 /*
  * Table of autorange values - the user will write the value in millidegrees,
  * and we'll convert it
@@ -1008,6 +1090,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | 
S_IWUSR, show_temp, set_temp,
THERM, 0);
 static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp, HYSTERSIS, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+   set_temp_st, 0, 0);
 stati

Re: [PATCH v3 3/4] hwmon: (adt7475) temperature smoothing

[Chris Packham]

On 15/05/17 03:40, Guenter Roeck wrote:
> On 05/10/2017 08:45 PM, Chris Packham wrote:
>> When enabled temperature smoothing allows ramping the fan speed over a
>> configurable period of time instead of jumping to the new speed
>> instantaneously.
>>
>> Signed-off-by: Chris Packham 
>> ---
>>
>> Changes in v2:
>> - use a single tempN_smoothing attribute
>
> This is a bit confusing. tempN suggests that the attribute would be associated
> with a given temperature, not with fan control. Not that I have a better idea
> for an attribute name, though, so unless you find a better name I am ok with 
> it.
>

The datasheet is a bit confusing in this respect.

 From the description of register 0x62:

"Assuming that PWMx is associated with the Remote 1 temperature channel, 
these bits define the maximum rate of change of the PWMx output for 
Remote 1 temperature related changes. Instead of the fan speed jumping 
instantaneously to its newly determined speed, it ramps
gracefully at the rate determined by these bits. This feature ultimately 
enhances the acoustics of the fan."

Based on my reading it's a property of the temperature input not of the 
PWM. If you changed pwmN_auto_channels_temp this setting would stay with 
the temperature sensor not the PWM.

>> Changes in v3:
>> - change enh_acou to enh_acoustics
>> - simplify show_temp_st()
>>
>>  Documentation/hwmon/adt7475 |  4 ++
>>  drivers/hwmon/adt7475.c | 93 
>> +
>>  2 files changed, 97 insertions(+)
>>
>> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
>> index 3990bae60e78..e82b24ec4b07 100644
>> --- a/Documentation/hwmon/adt7475
>> +++ b/Documentation/hwmon/adt7475
>> @@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour can be 
>> configured using the
>>  pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut 
>> off.
>>  A value of 1 means the fans will run at auto_point1_pwm.
>>
>> +The responsiveness of the ADT747x to temperature changes can be configured.
>> +This allows smoothing of the fan speed transition. To set the transition 
>> time
>> +set the value in ms in the temp[1-*]_smoothing sysfs attribute.
>> +
>>  Notes
>>  -
>>
>> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
>> index 4d6c625fec70..f7322330789c 100644
>> --- a/drivers/hwmon/adt7475.c
>> +++ b/drivers/hwmon/adt7475.c
>> @@ -526,6 +526,90 @@ static ssize_t set_temp(struct device *dev, struct 
>> device_attribute *attr,
>>  return count;
>>  }
>>
>> +/* Assuming CONFIG6[SLOW] is 0 */
>> +static const int ad7475_st_map[] = {
>> +37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
>> +};
>> +
>> +static ssize_t show_temp_st(struct device *dev, struct device_attribute 
>> *attr,
>> +  char *buf)
>> +{
>> +struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +struct i2c_client *client = to_i2c_client(dev);
>> +struct adt7475_data *data = i2c_get_clientdata(client);
>> +long val;
>> +
>> +switch (sattr->index) {
>> +case 0:
>> +val = data->enh_acoustics[0] & 0xf;
>> +break;
>> +case 1:
>> +val = (data->enh_acoustics[1] >> 4) & 0xf;
>> +break;
>> +case 2:
>> +val = data->enh_acoustics[1] & 0xf;
>> +break;
>> +default:
>> +return -EINVAL;
>
> This will never happen and, if it does, would indicate a bug, not invalid 
> input.
> I kind of dislike dead code; it just bloats the kernel. Please either use
> default: for or together with case 2:, or make it if/else.
>

Will combine default and case 2.

>> +}
>> +
>> +if (val & 0x8)
>> +return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
>> +else
>> +return sprintf(buf, "0\n");
>> +}
>> +
>> +static ssize_t set_temp_st(struct device *dev, struct device_attribute 
>> *attr,
>> + const char *buf, size_t count)
>> +{
>> +struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +struct i2c_client *client = to_i2c_client(dev);
>> +struct adt7475_data *data = i2c_get_clientdata(client);
>> +unsigned char reg;
>> +int shift, idx;
>> +ulong val;
>> +
>> +if (kstrtoul(buf, 10, &val))
>> +re

Re: [PATCH v3 2/4] hwmon: (adt7475) fan stall prevention

[Chris Packham]

On 15/05/17 02:54, Guenter Roeck wrote:
> On 05/10/2017 08:45 PM, Chris Packham wrote:
>> By default adt7475 will stop the fans (pwm duty cycle 0%) when the
>> temperature drops past Tmin - hysteresis. Some systems want to keep the
>> fans moving even when the temperature drops so add new sysfs attributes
>> that configure the enhanced acoustics min 1-3 which allows the fans to
>> run at the minimum configure pwm duty cycle.
>>
>> Signed-off-by: Chris Packham 
>> ---
>>
>> Changes in v2:
>> - use pwmN_stall_dis as the attribute name. I think this describes the 
>> purpose
>>   pretty well. I went with a new attribute instead of overloading
>
> Almost agree. Can we use pwmN_stall_disable ?
>

Sold. I'll send a v4 of this patch with the name changed.

> Thanks,
> Guenter
>
>
>>   pwmN_auto_point1_pwm so this doesn't affect existing users.
>> Changes in v3:
>> - Fix grammar.
>> - change enh_acou to enh_acoustics

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/4] hwmon: (adt7475) fan stall prevention

[Chris Packham]

By default adt7475 will stop the fans (pwm duty cycle 0%) when the
temperature drops past Tmin - hysteresis. Some systems want to keep the
fans moving even when the temperature drops so add new sysfs attributes
that configure the enhanced acoustics min 1-3 which allows the fans to
run at the minimum configure pwm duty cycle.

Signed-off-by: Chris Packham 
---

Changes in v2:
- use pwmN_stall_dis as the attribute name. I think this describes the purpose
  pretty well. I went with a new attribute instead of overloading
  pwmN_auto_point1_pwm so this doesn't affect existing users.
Changes in v3:
- Fix grammar.
- change enh_acou to enh_acoustics

 Documentation/hwmon/adt7475 |  5 +
 drivers/hwmon/adt7475.c | 50 +
 2 files changed, 55 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 0502f2b464e1..3990bae60e78 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 
255 (full speed).
 Fan speed may be set to maximum when the temperature sensor associated with
 the PWM control exceeds temp#_max.
 
+At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
+minimum (i.e. auto_point1_pwm). This behaviour can be configured using the
+pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
+A value of 1 means the fans will run at auto_point1_pwm.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index ec0c43fbcdce..4d6c625fec70 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -79,6 +79,9 @@
 
 #define REG_TEMP_TRANGE_BASE   0x5F
 
+#define REG_ENHANCE_ACOUSTICS1 0x62
+#define REG_ENHANCE_ACOUSTICS2 0x63
+
 #define REG_PWM_MIN_BASE   0x64
 
 #define REG_TEMP_TMIN_BASE 0x67
@@ -209,6 +212,7 @@ struct adt7475_data {
u8 range[3];
u8 pwmctl[3];
u8 pwmchan[3];
+   u8 enh_acoustics[2];
 
u8 vid;
u8 vrm;
@@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct 
device_attribute *attr,
data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
i2c_smbus_write_byte_data(client, reg,
  data->pwm[sattr->nr][sattr->index]);
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
+
+static ssize_t show_stall_dis(struct device *dev, struct device_attribute 
*attr,
+ char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   u8 mask = BIT(5 + sattr->index);
+
+   return sprintf(buf, "%d\n", !!(data->enh_acoustics[0] & mask));
+}
+
+static ssize_t set_stall_dis(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   long val;
+   u8 mask = BIT(5 + sattr->index);
+
+   if (kstrtol(buf, 10, &val))
+   return -EINVAL;
+
+   mutex_lock(&data->lock);
+
+   data->enh_acoustics[0] &= ~mask;
+   if (val)
+   data->enh_acoustics[0] |= mask;
+
+   i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
+ data->enh_acoustics[0]);
 
mutex_unlock(&data->lock);
 
@@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 0);
 static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 0);
+static SENSOR_DEVICE_ATTR_2(pwm1_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+   set_stall_dis, 0, 0);
 static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
1);
 static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 1);
 static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 1);
+static SENSOR_DEVICE_ATTR_2(pwm2_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+   set_stall_dis, 0, 1);
 static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
2);
 static SENSOR_DEVICE_ATTR_2(pwm3_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1052,6 +1097,8 @@ static SENSOR_DEVICE_ATTR_2(pwm3_auto_point1_pwm, S_IRUGO 
| S_IWUSR, 

[PATCH v3 3/4] hwmon: (adt7475) temperature smoothing

[Chris Packham]

When enabled temperature smoothing allows ramping the fan speed over a
configurable period of time instead of jumping to the new speed
instantaneously.

Signed-off-by: Chris Packham 
---

Changes in v2:
- use a single tempN_smoothing attribute
Changes in v3:
- change enh_acou to enh_acoustics
- simplify show_temp_st()

 Documentation/hwmon/adt7475 |  4 ++
 drivers/hwmon/adt7475.c | 93 +
 2 files changed, 97 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 3990bae60e78..e82b24ec4b07 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour can be 
configured using the
 pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
 A value of 1 means the fans will run at auto_point1_pwm.
 
+The responsiveness of the ADT747x to temperature changes can be configured.
+This allows smoothing of the fan speed transition. To set the transition time
+set the value in ms in the temp[1-*]_smoothing sysfs attribute.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 4d6c625fec70..f7322330789c 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,90 @@ static ssize_t set_temp(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
+/* Assuming CONFIG6[SLOW] is 0 */
+static const int ad7475_st_map[] = {
+   37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
+};
+
+static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   long val;
+
+   switch (sattr->index) {
+   case 0:
+   val = data->enh_acoustics[0] & 0xf;
+   break;
+   case 1:
+   val = (data->enh_acoustics[1] >> 4) & 0xf;
+   break;
+   case 2:
+   val = data->enh_acoustics[1] & 0xf;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   if (val & 0x8)
+   return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
+   else
+   return sprintf(buf, "0\n");
+}
+
+static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   unsigned char reg;
+   int shift, idx;
+   ulong val;
+
+   if (kstrtoul(buf, 10, &val))
+   return -EINVAL;
+
+   switch (sattr->index) {
+   case 0:
+   reg = REG_ENHANCE_ACOUSTICS1;
+   shift = 0;
+   idx = 0;
+   break;
+   case 1:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 4;
+   idx = 1;
+   break;
+   case 2:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 0;
+   idx = 1;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   if (val > 0) {
+   val = find_closest_descending(val, ad7475_st_map,
+ ARRAY_SIZE(ad7475_st_map));
+   val |= 0x8;
+   }
+
+   mutex_lock(&data->lock);
+
+   data->enh_acoustics[idx] &= ~(0xf << shift);
+   data->enh_acoustics[idx] |= (val << shift);
+
+   i2c_smbus_write_byte_data(client, reg, data->enh_acoustics[idx]);
+
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
 /*
  * Table of autorange values - the user will write the value in millidegrees,
  * and we'll convert it
@@ -1008,6 +1092,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | 
S_IWUSR, show_temp, set_temp,
THERM, 0);
 static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp, HYSTERSIS, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+   set_temp_st, 0, 0);
 static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, INPUT, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_alarm, S_IRUGO, show_temp, NULL, ALARM, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR, show_temp, set_temp,
@@ -1024,6 +1110,8 @@ static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO | 
S_IWUSR, show_temp, set_temp,
THERM, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
  

Re: [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

On 04/05/17 04:30, Guenter Roeck wrote:
> On Wed, May 03, 2017 at 12:40:09PM +1200, Chris Packham wrote:
>> When enabled temperature smoothing allows ramping the fan speed over a
>> configurable period of time instead of jumping to the new speed
>> instantaneously.
>>
>> Signed-off-by: Chris Packham 
>> ---
>> Changes in v2:
>> - use a single tempN_smoothing attribute
>>
>>  Documentation/hwmon/adt7475 |  4 ++
>>  drivers/hwmon/adt7475.c | 99 
>> +
>>  2 files changed, 103 insertions(+)
>>
>> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
>> index 63507402cd4f..dd6433d819d0 100644
>> --- a/Documentation/hwmon/adt7475
>> +++ b/Documentation/hwmon/adt7475
>> @@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour be 
>> configured using the
>>  pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut 
>> off.
>>  A value of 1 means the fans will run at auto_point1_pwm.
>>
>> +The responsiveness of the ADT747x to temperature changes can be configured.
>> +This allows smoothing of the fan speed transition. To set the transition 
>> time
>> +set the value in ms in the temp[1-*]_smoothing sysfs attribute.
>> +
>>  Notes
>>  -
>>
>> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
>> index 85957324cd85..41342de6e20c 100644
>> --- a/drivers/hwmon/adt7475.c
>> +++ b/drivers/hwmon/adt7475.c
>> @@ -526,6 +526,96 @@ static ssize_t set_temp(struct device *dev, struct 
>> device_attribute *attr,
>>  return count;
>>  }
>>
>> +/* Assuming CONFIG6[SLOW] is 0 */
>
> Can you take that into account and calculate a "best fit" based on the
> available options ?

Do you mean check CONFIG6[SLOW] and choose between 1 of 2 possible maps? 
Or have a unified map and choose both the SLOW and ACOU values?

I briefly considered the latter but things soon started to get 
complicated. It would look something like this (please excuse using a 
MUA as a code editor).

static const int ad7475_st_map[] = {
52200, 37500, 26100, 18800, 17400, 12500, 10400, 7500,
6500, 4700, 4400, 3100, 2200, 1600, 1100, 800,
};

i = find_closest_descending(val, ad7475_st_map,
ARRAY_SIZE(ad7475_st_map));
acou = i / 2;
slow = i % 2;

Going in reverse then gets really fun

slow = !!(i2c_smbus_read_byte_data(client, REG_CONFIG6) & BIT(PWMx))
acou = data->enh_acou[idx];

i = (acou * 2) + slow;

return sprintf(buf, "%d\n", ad7475_st_map[i]);

I could probably make the above work I just wasn't sure it was worth the 
hassle. Only the higher ranges would really be noticeable to anyone.

>
>> +static const int ad7475_st_map[] = {
>> +37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
>> +};
>> +
>> +static ssize_t show_temp_st(struct device *dev, struct device_attribute 
>> *attr,
>> +  char *buf)
>> +{
>> +struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +struct i2c_client *client = to_i2c_client(dev);
>> +struct adt7475_data *data = i2c_get_clientdata(client);
>> +int shift, idx;
>> +long val;
>> +
>> +switch (sattr->index) {
>> +case 0:
>> +shift = 0;
>> +idx = 0;
>> +break;
>> +case 1:
>> +shift = 4;
>> +idx = 1;
>> +break;
>> +case 2:
>> +shift = 0;
>> +idx = 1;
>> +break;
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +val = data->enh_acou[idx] >> shift;
>> +if (val & 0x8) {
>> +return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
>> +} else {
>> +return sprintf(buf, "0\n");
>> +}
>> +}
>> +
>> +static ssize_t set_temp_st(struct device *dev, struct device_attribute 
>> *attr,
>> + const char *buf, size_t count)
>> +{
>> +struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +struct i2c_client *client = to_i2c_client(dev);
>> +struct adt7475_data *data = i2c_get_clientdata(client);
>> +unsigned char reg;
>> +int shift, idx;
>> +ulong val;
>> +
>> +if (kstrtoul(buf, 10, &val))
>> +return -EINVAL;
>> +
>> +switch (sattr->index) {
>> +case 0:
>> +reg = REG_ENHANCE_ACOUSTICS

Re: [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention

[Chris Packham]

On 04/05/17 04:10, Guenter Roeck wrote:
> On Wed, May 03, 2017 at 12:40:08PM +1200, Chris Packham wrote:
>> By default adt7475 will stop the fans (pwm duty cycle 0%) when the
>> temperature drops past Tmin - hysteresis. Some systems want to keep the
>> fans moving even when the temperature drops so add new sysfs attributes
>> that configure the enhanced acoustics min 1-3 which allows the fans to
>> run at the minimum configure pwm duty cycle.
>>
>> Signed-off-by: Chris Packham 
>> ---
>> Changes in v2:
>> - use pwmN_stall_dis as the attribute name. I think this describes the 
>> purpose
>>   pretty well. I went with a new attribute instead of overloading
>>   pwmN_auto_point1_pwm so this doesn't affect existing users.
>>
>>  Documentation/hwmon/adt7475 |  5 +
>>  drivers/hwmon/adt7475.c | 50 
>> +
>>  2 files changed, 55 insertions(+)
>>
>> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
>> index 0502f2b464e1..63507402cd4f 100644
>> --- a/Documentation/hwmon/adt7475
>> +++ b/Documentation/hwmon/adt7475
>> @@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 
>> 255 (full speed).
>>  Fan speed may be set to maximum when the temperature sensor associated with
>>  the PWM control exceeds temp#_max.
>>
>> +At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at 
>> the
>> +minimum (i.e. auto_point1_pwm). This behaviour be configured using the
>> +pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut 
>> off.
>
> That is really an awkward attribute name. I'll have to think about this some
> more.

I agree. The other thing I considered was "halt" and using inverted 
logic so halt == 1 reflected the HW default of allowing fans to stop.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

On 03/05/17 12:40, Chris Packham wrote:
> When enabled temperature smoothing allows ramping the fan speed over a
> configurable period of time instead of jumping to the new speed
> instantaneously.
>
> Signed-off-by: Chris Packham 
> ---
> Changes in v2:
> - use a single tempN_smoothing attribute
>
>  Documentation/hwmon/adt7475 |  4 ++
>  drivers/hwmon/adt7475.c | 99 
> +
>  2 files changed, 103 insertions(+)

I've had some feedback internally on this patch around making 
show_temp_st() smaller and using "enh_acoustics" instead of "enh_acou". 
I'll wait for a while to see if there's any more feedback from the list 
before sending out v3.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

When enabled temperature smoothing allows ramping the fan speed over a
configurable period of time instead of jumping to the new speed
instantaneously.

Signed-off-by: Chris Packham 
---
Changes in v2:
- use a single tempN_smoothing attribute

 Documentation/hwmon/adt7475 |  4 ++
 drivers/hwmon/adt7475.c | 99 +
 2 files changed, 103 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 63507402cd4f..dd6433d819d0 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour be 
configured using the
 pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
 A value of 1 means the fans will run at auto_point1_pwm.
 
+The responsiveness of the ADT747x to temperature changes can be configured.
+This allows smoothing of the fan speed transition. To set the transition time
+set the value in ms in the temp[1-*]_smoothing sysfs attribute.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 85957324cd85..41342de6e20c 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,96 @@ static ssize_t set_temp(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
+/* Assuming CONFIG6[SLOW] is 0 */
+static const int ad7475_st_map[] = {
+   37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
+};
+
+static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   int shift, idx;
+   long val;
+
+   switch (sattr->index) {
+   case 0:
+   shift = 0;
+   idx = 0;
+   break;
+   case 1:
+   shift = 4;
+   idx = 1;
+   break;
+   case 2:
+   shift = 0;
+   idx = 1;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   val = data->enh_acou[idx] >> shift;
+   if (val & 0x8) {
+   return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
+   } else {
+   return sprintf(buf, "0\n");
+   }
+}
+
+static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   unsigned char reg;
+   int shift, idx;
+   ulong val;
+
+   if (kstrtoul(buf, 10, &val))
+   return -EINVAL;
+
+   switch (sattr->index) {
+   case 0:
+   reg = REG_ENHANCE_ACOUSTICS1;
+   shift = 0;
+   idx = 0;
+   break;
+   case 1:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 4;
+   idx = 1;
+   break;
+   case 2:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 0;
+   idx = 1;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   if (val > 0) {
+   val = find_closest_descending(val, ad7475_st_map,
+ ARRAY_SIZE(ad7475_st_map));
+   val |= 0x8;
+   }
+
+   mutex_lock(&data->lock);
+
+   data->enh_acou[idx] &= ~(0xf << shift);
+   data->enh_acou[idx] |= (val << shift);
+
+   i2c_smbus_write_byte_data(client, reg, data->enh_acou[idx]);
+
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
 /*
  * Table of autorange values - the user will write the value in millidegrees,
  * and we'll convert it
@@ -1008,6 +1098,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | 
S_IWUSR, show_temp, set_temp,
THERM, 0);
 static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp, HYSTERSIS, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+   set_temp_st, 0, 0);
 static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, INPUT, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_alarm, S_IRUGO, show_temp, NULL, ALARM, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR, show_temp, set_temp,
@@ -1024,6 +1116,8 @@ static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO | 
S_IWUSR, show_temp, set_temp,
THERM, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp

[RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention

[Chris Packham]

By default adt7475 will stop the fans (pwm duty cycle 0%) when the
temperature drops past Tmin - hysteresis. Some systems want to keep the
fans moving even when the temperature drops so add new sysfs attributes
that configure the enhanced acoustics min 1-3 which allows the fans to
run at the minimum configure pwm duty cycle.

Signed-off-by: Chris Packham 
---
Changes in v2:
- use pwmN_stall_dis as the attribute name. I think this describes the purpose
  pretty well. I went with a new attribute instead of overloading
  pwmN_auto_point1_pwm so this doesn't affect existing users.

 Documentation/hwmon/adt7475 |  5 +
 drivers/hwmon/adt7475.c | 50 +
 2 files changed, 55 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 0502f2b464e1..63507402cd4f 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 
255 (full speed).
 Fan speed may be set to maximum when the temperature sensor associated with
 the PWM control exceeds temp#_max.
 
+At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
+minimum (i.e. auto_point1_pwm). This behaviour be configured using the
+pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
+A value of 1 means the fans will run at auto_point1_pwm.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index ec0c43fbcdce..85957324cd85 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -79,6 +79,9 @@
 
 #define REG_TEMP_TRANGE_BASE   0x5F
 
+#define REG_ENHANCE_ACOUSTICS1 0x62
+#define REG_ENHANCE_ACOUSTICS2 0x63
+
 #define REG_PWM_MIN_BASE   0x64
 
 #define REG_TEMP_TMIN_BASE 0x67
@@ -209,6 +212,7 @@ struct adt7475_data {
u8 range[3];
u8 pwmctl[3];
u8 pwmchan[3];
+   u8 enh_acou[2];
 
u8 vid;
u8 vrm;
@@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct 
device_attribute *attr,
data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
i2c_smbus_write_byte_data(client, reg,
  data->pwm[sattr->nr][sattr->index]);
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
+
+static ssize_t show_stall_dis(struct device *dev, struct device_attribute 
*attr,
+ char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   u8 mask = BIT(5 + sattr->index);
+
+   return sprintf(buf, "%d\n", !!(data->enh_acou[0] & mask));
+}
+
+static ssize_t set_stall_dis(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   long val;
+   u8 mask = BIT(5 + sattr->index);
+
+   if (kstrtol(buf, 10, &val))
+   return -EINVAL;
+
+   mutex_lock(&data->lock);
+
+   data->enh_acou[0] &= ~mask;
+   if (val)
+   data->enh_acou[0] |= mask;
+
+   i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
+ data->enh_acou[0]);
 
mutex_unlock(&data->lock);
 
@@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 0);
 static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 0);
+static SENSOR_DEVICE_ATTR_2(pwm1_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+   set_stall_dis, 0, 0);
 static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
1);
 static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 1);
 static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 1);
+static SENSOR_DEVICE_ATTR_2(pwm2_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+   set_stall_dis, 0, 1);
 static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
2);
 static SENSOR_DEVICE_ATTR_2(pwm3_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1052,6 +1097,8 @@ static SENSOR_DEVICE_ATTR_2(pwm3_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 2);
 static SENSOR_DEVICE_ATTR_2(pwm3_auto_point2_pwm

Re: [PATCH 2/3] hwmon: (adt7475) fan stall prevention

[Chris Packham]

On 03/05/17 06:07, Guenter Roeck wrote:
> On Tue, May 02, 2017 at 05:45:35PM +1200, Chris Packham wrote:
>> By default adt7475 will stop the fans (pwm duty cycle 0%) when the
>> temperature drops past Tmin - hysteresis. Some systems want to keep the
>> fans moving even when the temperature drops so add new sysfs attributes
>> that configure the enhanced acoustics min 1-3 which allows the fans to
>> run at the minimum configure pwm duty cycle.
>>
>> Signed-off-by: Chris Packham 
>> ---
>> pwmN_min is a horrible name but I really can't think of anything better.
>> I'm biased a little because that is essentially the name of the bits in
>> the datasheet. I'm open to suggestions.
>
> pwmX_min is also traditionally the mimimum permitted pwm value,
> not a boolean. This would be more appropriate to reflect the PWMmin
> register values (0x64 to 0x66). Similar for pwmX_max if you want to
> add support for it.

For the adt7476 driver these are used as pwmN_auto_point[12]_pwm.

> It might make sense to combine pwmX_min==0 with clearing the
> respective bit in the REG_ENHANCE_ACOUSTICS[12] register. This way
> we would only need one attribute to support both.

I could add code such that if pwmN_auto_point1_pwm > 0. The bit in 
REG_ENHANCE_ACOUSTICS is set but that would be a change in existing 
behaviour.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

On 03/05/17 07:14, Guenter Roeck wrote:
> On Tue, May 02, 2017 at 05:45:36PM +1200, Chris Packham wrote:
>> When enabled temperature smoothing allows ramping the fan speed over a
>> configurable period of time instead of jumping to the new speed
>> instantaneously.
>>
>> Signed-off-by: Chris Packham 
>> ---
>>  Documentation/hwmon/adt7475 |   5 ++
>>  drivers/hwmon/adt7475.c | 121 
>> 
>>  2 files changed, 126 insertions(+)
>>
>> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
>> index 85dc9e17bdee..31b15cb910ea 100644
>> --- a/Documentation/hwmon/adt7475
>> +++ b/Documentation/hwmon/adt7475
>> @@ -114,6 +114,11 @@ at the minimum (i.e. auto_point1_pwm). This can be 
>> configured using the
>>  pwm[1-*]_min sysfs attribute. A value of 0 means the fans will shut off. A
>>  value of 1 means the fans will run at auto_point1_pwm.
>>
>> +The responsiveness of the ADT747x to temperature changes can be configured.
>> +This allows smoothing of the fan speed transition. To enable temperature
>> +smoothing used the temp[1-*]_smoothing_enable sysfs attribute. To set the
>> +transition time set the value in ms in the temp[1-*]_smoothing sysfs 
>> attribute.
>> +
> Does this require two attributes, or can setting '0' for temp[1-*]_smoothing
> be used to disable it ?
>

One attribute could be made to work. I was following asc7621.c but from 
a usability perspective having to set the two attributes is not 
particularly convenient. The only argument for separating them is to 
allow smoothing at whatever the hardware default is.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] hwmon: (adt7475) fan stall prevention

[Chris Packham]

By default adt7475 will stop the fans (pwm duty cycle 0%) when the
temperature drops past Tmin - hysteresis. Some systems want to keep the
fans moving even when the temperature drops so add new sysfs attributes
that configure the enhanced acoustics min 1-3 which allows the fans to
run at the minimum configure pwm duty cycle.

Signed-off-by: Chris Packham 
---
pwmN_min is a horrible name but I really can't think of anything better.
I'm biased a little because that is essentially the name of the bits in
the datasheet. I'm open to suggestions.

 Documentation/hwmon/adt7475 |  5 +
 drivers/hwmon/adt7475.c | 50 +
 2 files changed, 55 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 0502f2b464e1..85dc9e17bdee 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 
255 (full speed).
 Fan speed may be set to maximum when the temperature sensor associated with
 the PWM control exceeds temp#_max.
 
+At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or
+at the minimum (i.e. auto_point1_pwm). This can be configured using the
+pwm[1-*]_min sysfs attribute. A value of 0 means the fans will shut off. A
+value of 1 means the fans will run at auto_point1_pwm.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index ec0c43fbcdce..53f25bda0919 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -79,6 +79,9 @@
 
 #define REG_TEMP_TRANGE_BASE   0x5F
 
+#define REG_ENHANCE_ACOUSTICS1 0x62
+#define REG_ENHANCE_ACOUSTICS2 0x63
+
 #define REG_PWM_MIN_BASE   0x64
 
 #define REG_TEMP_TMIN_BASE 0x67
@@ -209,6 +212,7 @@ struct adt7475_data {
u8 range[3];
u8 pwmctl[3];
u8 pwmchan[3];
+   u8 enh_acou[2];
 
u8 vid;
u8 vrm;
@@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct 
device_attribute *attr,
data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
i2c_smbus_write_byte_data(client, reg,
  data->pwm[sattr->nr][sattr->index]);
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
+
+static ssize_t show_pwm_min(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   u8 mask = BIT(5 + sattr->index);
+
+   return sprintf(buf, "%d\n", !!(data->enh_acou[0] & mask));
+}
+
+static ssize_t set_pwm_min(struct device *dev, struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   long val;
+   u8 mask = BIT(5 + sattr->index);
+
+   if (kstrtol(buf, 10, &val))
+   return -EINVAL;
+
+   mutex_lock(&data->lock);
+
+   data->enh_acou[0] &= ~mask;
+   if (val)
+   data->enh_acou[0] |= mask;
+
+   i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
+ data->enh_acou[0]);
 
mutex_unlock(&data->lock);
 
@@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 0);
 static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 0);
+static SENSOR_DEVICE_ATTR_2(pwm1_min, S_IRUGO | S_IWUSR, show_pwm_min,
+   set_pwm_min, 0, 0);
 static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
1);
 static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 1);
 static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 1);
+static SENSOR_DEVICE_ATTR_2(pwm2_min, S_IRUGO | S_IWUSR, show_pwm_min,
+   set_pwm_min, 0, 1);
 static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
2);
 static SENSOR_DEVICE_ATTR_2(pwm3_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1052,6 +1097,8 @@ static SENSOR_DEVICE_ATTR_2(pwm3_auto_point1_pwm, S_IRUGO 
| S_IWUSR, show_pwm,
set_pwm, MIN, 2);
 static SENSOR_DEVICE_ATTR_2(pwm3_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
  

[RFC PATCH 3/3] hwmon: (adt7475) temperature smoothing

[Chris Packham]

When enabled temperature smoothing allows ramping the fan speed over a
configurable period of time instead of jumping to the new speed
instantaneously.

Signed-off-by: Chris Packham 
---
 Documentation/hwmon/adt7475 |   5 ++
 drivers/hwmon/adt7475.c | 121 
 2 files changed, 126 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 85dc9e17bdee..31b15cb910ea 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,11 @@ at the minimum (i.e. auto_point1_pwm). This can be 
configured using the
 pwm[1-*]_min sysfs attribute. A value of 0 means the fans will shut off. A
 value of 1 means the fans will run at auto_point1_pwm.
 
+The responsiveness of the ADT747x to temperature changes can be configured.
+This allows smoothing of the fan speed transition. To enable temperature
+smoothing used the temp[1-*]_smoothing_enable sysfs attribute. To set the
+transition time set the value in ms in the temp[1-*]_smoothing sysfs attribute.
+
 Notes
 -
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 53f25bda0919..e1299eef7c51 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,109 @@ static ssize_t set_temp(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
+/* Assuming CONFIG6[SLOW] is 0 */
+static const int ad7475_st_map[] = {
+   37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
+};
+
+static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   int shift, idx;
+   long val;
+
+   switch (sattr->index) {
+   case 0:
+   shift = 0;
+   idx = 0;
+   break;
+   case 1:
+   shift = 4;
+   idx = 1;
+   break;
+   case 2:
+   shift = 0;
+   idx = 1;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   switch (sattr->nr) {
+   case CONTROL:
+   val = (data->enh_acou[idx] >> shift) & 0x8;
+   return sprintf(buf, "%d\n", !!val);
+   case MIN:
+   val = (data->enh_acou[idx] >> shift) & 0x7;
+   return sprintf(buf, "%d\n", ad7475_st_map[val]);
+   default:
+   return -EINVAL;
+   }
+}
+
+static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   struct i2c_client *client = to_i2c_client(dev);
+   struct adt7475_data *data = i2c_get_clientdata(client);
+   unsigned char reg;
+   int shift, idx;
+   int mask;
+   long val;
+
+   if (kstrtol(buf, 10, &val))
+   return -EINVAL;
+
+   switch (sattr->index) {
+   case 0:
+   reg = REG_ENHANCE_ACOUSTICS1;
+   shift = 0;
+   idx = 0;
+   break;
+   case 1:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 4;
+   idx = 1;
+   break;
+   case 2:
+   reg = REG_ENHANCE_ACOUSTICS2;
+   shift = 0;
+   idx = 1;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   switch (sattr->nr) {
+   case CONTROL:
+   val = !!val << 3;
+   mask = 0x8;
+   break;
+   case MIN:
+   val = find_closest_descending(val, ad7475_st_map,
+ ARRAY_SIZE(ad7475_st_map));
+   mask = 0x7;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   mutex_lock(&data->lock);
+
+   data->enh_acou[idx] &= ~(mask << shift);
+   data->enh_acou[idx] |= (val << shift);
+
+   i2c_smbus_write_byte_data(client, reg, data->enh_acou[idx]);
+
+   mutex_unlock(&data->lock);
+
+   return count;
+}
+
 /*
  * Table of autorange values - the user will write the value in millidegrees,
  * and we'll convert it
@@ -1008,6 +,10 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | 
S_IWUSR, show_temp, set_temp,
THERM, 0);
 static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp, HYSTERSIS, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+   set_temp_st, MIN, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing_enable, S_IRUGO | S_IWUSR,
+   

[PATCH] docs: hwmon: Fix typo "Microship" should be "Microchip"

[Chris Packham]

Signed-off-by: Chris Packham 
---
 Documentation/hwmon/tc654 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/hwmon/tc654 b/Documentation/hwmon/tc654
index 91a2843f5f98..47636a8077b4 100644
--- a/Documentation/hwmon/tc654
+++ b/Documentation/hwmon/tc654
@@ -2,7 +2,7 @@ Kernel driver tc654
 ===
 
 Supported chips:
-  * Microship TC654 and TC655
+  * Microchip TC654 and TC655
 Prefix: 'tc654'
 Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
 
-- 
2.11.0.24.ge6920cf

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4] hwmon: Add tc654 driver

[Chris Packham]

On 10/13/2016 02:03 AM, Guenter Roeck wrote:
> On Tue, Oct 11, 2016 at 10:26:31AM +1300, Chris Packham wrote:
>> > Add support for the tc654 and tc655 fan controllers from Microchip.
>> >
>> > http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
>> >
>> > Signed-off-by: Chris Packham 
>> > Acked-by: Rob Herring 
> Applied to -next (after fixing continuation line alignments).
>

Sorry about those. I'll try to do better next time.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4] hwmon: Add tc654 driver

[Chris Packham]

Add support for the tc654 and tc655 fan controllers from Microchip.

http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf

Signed-off-by: Chris Packham 
---
Changes in v4:
- tab-align values in #defines
- ensure locking in set_pwm covers updating cached values
- populate the cached value for the config register in tc654_probe()

Changes in v3:
- typofix in documentation
- add missing value to tc654_pwm_map, re-generate based on datasheet.
- remove unnecessary hwmon_dev member from struct tc654_data
- bug fixes in set_fan_min() and show_pwm_mode()
- miscellaneous style fixes
 
Changes in v2:
- Add Documentation/hwmon/tc654
- Incorporate most of the review comments from Guenter. Additional error
  handling is added. Unused/unnecessary code is removed. I decided not
  to go down the regmap path yet. I may circle back to it when I look at
  using regmap in the adm9240 driver.

 .../devicetree/bindings/i2c/trivial-devices.txt|   2 +
 Documentation/hwmon/tc654  |  31 ++
 drivers/hwmon/Kconfig  |  11 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/tc654.c  | 514 +
 5 files changed, 559 insertions(+)
 create mode 100644 Documentation/hwmon/tc654
 create mode 100644 drivers/hwmon/tc654.c

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 1416c6a0d2cd..833fb9f133d3 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -122,6 +122,8 @@ microchip,mcp4662-502   Microchip 8-bit Dual I2C 
Digital Potentiometer with NV Mem
 microchip,mcp4662-103  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (10k)
 microchip,mcp4662-503  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (50k)
 microchip,mcp4662-104  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (100k)
+microchip,tc654PWM Fan Speed Controller With Fan Fault 
Detection
+microchip,tc655PWM Fan Speed Controller With Fan Fault 
Detection
 national,lm63  Temperature sensor with integrated fan control
 national,lm75  I2C TEMP SENSOR
 national,lm80  Serial Interface ACPI-Compatible Microprocessor System 
Hardware Monitor
diff --git a/Documentation/hwmon/tc654 b/Documentation/hwmon/tc654
new file mode 100644
index ..91a2843f5f98
--- /dev/null
+++ b/Documentation/hwmon/tc654
@@ -0,0 +1,31 @@
+Kernel driver tc654
+===
+
+Supported chips:
+  * Microship TC654 and TC655
+Prefix: 'tc654'
+Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
+
+Authors:
+    Chris Packham 
+Masahiko Iwamoto 
+
+Description
+---
+This driver implements support for the Microchip TC654 and TC655.
+
+The TC654 uses the 2-wire interface compatible with the SMBUS 2.0
+specification. The TC654 has two (2) inputs for measuring fan RPM and
+one (1) PWM output which can be used for fan control.
+
+Configuration Notes
+---
+Ordinarily the pwm1_mode ABI is used for controlling the pwm output
+mode.  However, for this chip the output is always pwm, and the
+pwm1_mode determines if the pwm output is controlled via the pwm1 value
+or via the Vin analog input.
+
+
+Setting pwm1_mode to 1 will cause the pwm output to be driven based on
+the pwm1 value. Setting pwm1_mode to 0 will cause the pwm output to be
+driven based on the Vin input.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 45cef3d2c75c..8681bc65cde5 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -907,6 +907,17 @@ config SENSORS_MCP3021
  This driver can also be built as a module.  If so, the module
  will be called mcp3021.
 
+config SENSORS_TC654
+   tristate "Microchip TC654/TC655 and compatibles"
+   depends on I2C
+   help
+ If you say yes here you get support for TC654 and TC655.
+ The TC654 and TC655 are PWM mode fan speed controllers with
+ FanSense technology for use with brushless DC fans.
+
+ This driver can also be built as a module.  If so, the module
+ will be called tc654.
+
 config SENSORS_MENF21BMC_HWMON
tristate "MEN 14F021P00 BMC Hardware Monitoring"
depends on MFD_MENF21BMC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index aecf4ba17460..c651f0f1d047 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -122,6 +122,7 @@ obj-$(CONFIG_SENSORS_MAX6697)   += max6697.o
 obj-$(CONFIG_SENSORS_MAX31790) += max31790.o
 obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
 obj-$(CONFIG_SENSORS_MCP3021)  += mcp3021.o
+obj-$(CONFIG_SENSORS_TC654)+= tc654.o
 obj-$(CONFIG_SENSORS_MENF21BMC_HWMON) += menf21bmc_hwmon.o
 obj-$(CONFIG_SENSORS_NCT6683)  += nct6683.o

Re: [PATCHv3] hwmon: Add tc654 driver

[Chris Packham]

On 10/11/2016 02:22 AM, Guenter Roeck wrote:
>> +if (val)
>> > +  data->config |= TC654_REG_CONFIG_DUTYC;
>> > +  else
>> > +  data->config &= ~TC654_REG_CONFIG_DUTYC;
> I just realized that this won't work as intended. Problem is that you
> only fill data->config when reading an attribute. So, if a set function
> is called prior to reading an attribute, data->config will be 0, and
> you end up overwriting the original configuration.
>

Should I just read it in the probe function or fill it in with the 
documented hardware defaults?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3] hwmon: Add tc654 driver

[Chris Packham]

Add support for the tc654 and tc655 fan controllers from Microchip.

http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf

Signed-off-by: Chris Packham 
---
Changes in v3:
- typofix in documentation
- add missing value to tc654_pwm_map, re-generate based on datasheet.
- remove unnecessary hwmon_dev member from struct tc654_data
- bug fixes in set_fan_min() and show_pwm_mode()
- miscellaneous style fixes

Changes in v2:
- Add Documentation/hwmon/tc654
- Incorporate most of the review comments from Guenter. Additional error
  handling is added. Unused/unnecessary code is removed. I decided not
  to go down the regmap path yet. I may circle back to it when I look at
  using regmap in the adm9240 driver.

 .../devicetree/bindings/i2c/trivial-devices.txt|   2 +
 Documentation/hwmon/tc654  |  31 ++
 drivers/hwmon/Kconfig  |  11 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/tc654.c  | 509 +
 5 files changed, 554 insertions(+)
 create mode 100644 Documentation/hwmon/tc654
 create mode 100644 drivers/hwmon/tc654.c

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 1416c6a0d2cd..833fb9f133d3 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -122,6 +122,8 @@ microchip,mcp4662-502   Microchip 8-bit Dual I2C 
Digital Potentiometer with NV Mem
 microchip,mcp4662-103  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (10k)
 microchip,mcp4662-503  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (50k)
 microchip,mcp4662-104  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (100k)
+microchip,tc654PWM Fan Speed Controller With Fan Fault 
Detection
+microchip,tc655PWM Fan Speed Controller With Fan Fault 
Detection
 national,lm63  Temperature sensor with integrated fan control
 national,lm75  I2C TEMP SENSOR
 national,lm80  Serial Interface ACPI-Compatible Microprocessor System 
Hardware Monitor
diff --git a/Documentation/hwmon/tc654 b/Documentation/hwmon/tc654
new file mode 100644
index ..91a2843f5f98
--- /dev/null
+++ b/Documentation/hwmon/tc654
@@ -0,0 +1,31 @@
+Kernel driver tc654
+===
+
+Supported chips:
+  * Microship TC654 and TC655
+Prefix: 'tc654'
+Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
+
+Authors:
+    Chris Packham 
+Masahiko Iwamoto 
+
+Description
+---
+This driver implements support for the Microchip TC654 and TC655.
+
+The TC654 uses the 2-wire interface compatible with the SMBUS 2.0
+specification. The TC654 has two (2) inputs for measuring fan RPM and
+one (1) PWM output which can be used for fan control.
+
+Configuration Notes
+---
+Ordinarily the pwm1_mode ABI is used for controlling the pwm output
+mode.  However, for this chip the output is always pwm, and the
+pwm1_mode determines if the pwm output is controlled via the pwm1 value
+or via the Vin analog input.
+
+
+Setting pwm1_mode to 1 will cause the pwm output to be driven based on
+the pwm1 value. Setting pwm1_mode to 0 will cause the pwm output to be
+driven based on the Vin input.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 45cef3d2c75c..8681bc65cde5 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -907,6 +907,17 @@ config SENSORS_MCP3021
  This driver can also be built as a module.  If so, the module
  will be called mcp3021.
 
+config SENSORS_TC654
+   tristate "Microchip TC654/TC655 and compatibles"
+   depends on I2C
+   help
+ If you say yes here you get support for TC654 and TC655.
+ The TC654 and TC655 are PWM mode fan speed controllers with
+ FanSense technology for use with brushless DC fans.
+
+ This driver can also be built as a module.  If so, the module
+ will be called tc654.
+
 config SENSORS_MENF21BMC_HWMON
tristate "MEN 14F021P00 BMC Hardware Monitoring"
depends on MFD_MENF21BMC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index aecf4ba17460..c651f0f1d047 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -122,6 +122,7 @@ obj-$(CONFIG_SENSORS_MAX6697)   += max6697.o
 obj-$(CONFIG_SENSORS_MAX31790) += max31790.o
 obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
 obj-$(CONFIG_SENSORS_MCP3021)  += mcp3021.o
+obj-$(CONFIG_SENSORS_TC654)+= tc654.o
 obj-$(CONFIG_SENSORS_MENF21BMC_HWMON) += menf21bmc_hwmon.o
 obj-$(CONFIG_SENSORS_NCT6683)  += nct6683.o
 obj-$(CONFIG_SENSORS_NCT6775)  += nct6775.o
diff --git a/drivers/hwmon/tc654.c b/drivers/hwmon/tc654.c
new file mode 100644
index ..456e0bb9f94f
--- /dev/null
+++ b

Re: [PATCHv2] hwmon: Add tc654 driver

[Chris Packham]

Hi Gunter,

Thanks for the review. v3 on it's way some responses below.

On 10/08/2016 07:29 AM, Guenter Roeck wrote:
> On Fri, Oct 07, 2016 at 02:38:44PM +1300, Chris Packham wrote:
>> Add support for the tc654 and tc655 fan controllers from Microchip.
>>
>> http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
>>
>> Signed-off-by: Chris Packham 
>> ---
>>
>> Changes in v2:
>> - Add Documentation/hwmon/tc654
>> - Incorporate most of the review comments from Guenter. Additional error
>>   handling is added. Unused/unnecessary code is removed. I decided not
>>   to go down the regmap path yet. I may circle back to it when I look at
>>   using regmap in the adm9240 driver.
>>
>>  .../devicetree/bindings/i2c/trivial-devices.txt|   2 +
>>  Documentation/hwmon/tc654  |  26 ++
>>  drivers/hwmon/Kconfig  |  11 +
>>  drivers/hwmon/Makefile |   1 +
>>  drivers/hwmon/tc654.c  | 513 
>> +
>>  5 files changed, 553 insertions(+)
>>  create mode 100644 Documentation/hwmon/tc654
>>  create mode 100644 drivers/hwmon/tc654.c
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
>> b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> index 1416c6a0d2cd..833fb9f133d3 100644
>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> @@ -122,6 +122,8 @@ microchip,mcp4662-502Microchip 8-bit Dual I2C 
>> Digital Potentiometer with NV Mem
>>  microchip,mcp4662-103   Microchip 8-bit Dual I2C Digital Potentiometer 
>> with NV Memory (10k)
>>  microchip,mcp4662-503   Microchip 8-bit Dual I2C Digital Potentiometer 
>> with NV Memory (50k)
>>  microchip,mcp4662-104   Microchip 8-bit Dual I2C Digital Potentiometer 
>> with NV Memory (100k)
>> +microchip,tc654 PWM Fan Speed Controller With Fan Fault 
>> Detection
>> +microchip,tc655 PWM Fan Speed Controller With Fan Fault 
>> Detection
>>  national,lm63   Temperature sensor with integrated fan control
>>  national,lm75   I2C TEMP SENSOR
>>  national,lm80   Serial Interface ACPI-Compatible Microprocessor 
>> System Hardware Monitor
>> diff --git a/Documentation/hwmon/tc654 b/Documentation/hwmon/tc654
>> new file mode 100644
>> index ..93796c5c7e79
>> --- /dev/null
>> +++ b/Documentation/hwmon/tc654
>> @@ -0,0 +1,26 @@
>> +Kernel driver tc654
>> +===
>> +
>> +Supported chips:
>> +  * Microship TC654 and TC655
>> +Prefix: 'tc654'
>> +Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
>> +
>> +Authors:
>> +Chris Packham 
>> +Masahiko Iwamoto 
>> +
>> +Description
>> +---
>> +This driver implements support for the Microchip TC654 and TC655.
>> +
>> +The TC654 used the 2-wire interface compatible with the SMBUS 2.0
>
> uses
>

Done.

>> +specification. The TC654 has two (2) inputs for measuring fan RPM and
>> +one (1) PWM output which can be used for fan control.
>> +
>> +Configuration Notes
>> +---
>> +Ordinarily the pwm1_mode ABI is used for controlling the pwm output
>> +mode.  However, for this chip the output is always pwm, and the
>> +pwm1_mode determines if the pwm output is controlled via the pwm1 value
>> +or via the Vin analog input.
>
> Please describe the supported values here.
>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 45cef3d2c75c..8681bc65cde5 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -907,6 +907,17 @@ config SENSORS_MCP3021
>>This driver can also be built as a module.  If so, the module
>>will be called mcp3021.
>>
>> +config SENSORS_TC654
>> +tristate "Microchip TC654/TC655 and compatibles"
>> +depends on I2C
>> +help
>> +  If you say yes here you get support for TC654 and TC655.
>> +  The TC654 and TC655 are PWM mode fan speed controllers with
>> +  FanSense technology for use with brushless DC fans.
>> +
>> +  This driver can also be built as a module.  If so, the module
>> +  will be called tc654.
>> +
>>  config SENSORS_MENF21BMC_HWMON
>>  tristate "MEN 14F021P00 BMC Hardware Monitoring"
>>  dep

[PATCHv2] hwmon: Add tc654 driver

[Chris Packham]

Add support for the tc654 and tc655 fan controllers from Microchip.

http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf

Signed-off-by: Chris Packham 
---

Changes in v2:
- Add Documentation/hwmon/tc654
- Incorporate most of the review comments from Guenter. Additional error
  handling is added. Unused/unnecessary code is removed. I decided not
  to go down the regmap path yet. I may circle back to it when I look at
  using regmap in the adm9240 driver.

 .../devicetree/bindings/i2c/trivial-devices.txt|   2 +
 Documentation/hwmon/tc654  |  26 ++
 drivers/hwmon/Kconfig  |  11 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/tc654.c  | 513 +
 5 files changed, 553 insertions(+)
 create mode 100644 Documentation/hwmon/tc654
 create mode 100644 drivers/hwmon/tc654.c

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 1416c6a0d2cd..833fb9f133d3 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -122,6 +122,8 @@ microchip,mcp4662-502   Microchip 8-bit Dual I2C 
Digital Potentiometer with NV Mem
 microchip,mcp4662-103  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (10k)
 microchip,mcp4662-503  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (50k)
 microchip,mcp4662-104  Microchip 8-bit Dual I2C Digital Potentiometer with NV 
Memory (100k)
+microchip,tc654PWM Fan Speed Controller With Fan Fault 
Detection
+microchip,tc655PWM Fan Speed Controller With Fan Fault 
Detection
 national,lm63  Temperature sensor with integrated fan control
 national,lm75  I2C TEMP SENSOR
 national,lm80  Serial Interface ACPI-Compatible Microprocessor System 
Hardware Monitor
diff --git a/Documentation/hwmon/tc654 b/Documentation/hwmon/tc654
new file mode 100644
index ..93796c5c7e79
--- /dev/null
+++ b/Documentation/hwmon/tc654
@@ -0,0 +1,26 @@
+Kernel driver tc654
+===
+
+Supported chips:
+  * Microship TC654 and TC655
+Prefix: 'tc654'
+Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/20001734C.pdf
+
+Authors:
+    Chris Packham 
+Masahiko Iwamoto 
+
+Description
+---
+This driver implements support for the Microchip TC654 and TC655.
+
+The TC654 used the 2-wire interface compatible with the SMBUS 2.0
+specification. The TC654 has two (2) inputs for measuring fan RPM and
+one (1) PWM output which can be used for fan control.
+
+Configuration Notes
+---
+Ordinarily the pwm1_mode ABI is used for controlling the pwm output
+mode.  However, for this chip the output is always pwm, and the
+pwm1_mode determines if the pwm output is controlled via the pwm1 value
+or via the Vin analog input.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 45cef3d2c75c..8681bc65cde5 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -907,6 +907,17 @@ config SENSORS_MCP3021
  This driver can also be built as a module.  If so, the module
  will be called mcp3021.
 
+config SENSORS_TC654
+   tristate "Microchip TC654/TC655 and compatibles"
+   depends on I2C
+   help
+ If you say yes here you get support for TC654 and TC655.
+ The TC654 and TC655 are PWM mode fan speed controllers with
+ FanSense technology for use with brushless DC fans.
+
+ This driver can also be built as a module.  If so, the module
+ will be called tc654.
+
 config SENSORS_MENF21BMC_HWMON
tristate "MEN 14F021P00 BMC Hardware Monitoring"
depends on MFD_MENF21BMC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index aecf4ba17460..c651f0f1d047 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -122,6 +122,7 @@ obj-$(CONFIG_SENSORS_MAX6697)   += max6697.o
 obj-$(CONFIG_SENSORS_MAX31790) += max31790.o
 obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
 obj-$(CONFIG_SENSORS_MCP3021)  += mcp3021.o
+obj-$(CONFIG_SENSORS_TC654)+= tc654.o
 obj-$(CONFIG_SENSORS_MENF21BMC_HWMON) += menf21bmc_hwmon.o
 obj-$(CONFIG_SENSORS_NCT6683)  += nct6683.o
 obj-$(CONFIG_SENSORS_NCT6775)  += nct6775.o
diff --git a/drivers/hwmon/tc654.c b/drivers/hwmon/tc654.c
new file mode 100644
index ..cba31cbd3383
--- /dev/null
+++ b/drivers/hwmon/tc654.c
@@ -0,0 +1,513 @@
+/*
+ * tc654.c - Linux kernel modules for fan speed controller
+ *
+ * Copyright (C) 2016 Allied Telesis Labs NZ
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This prog