Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-08 Thread Laxman Dewangan



On Thursday 08 March 2018 08:01 PM, Guenter Roeck wrote:

On 03/07/2018 10:06 PM, Laxman Dewangan wrote:



The RPM is measured speed via PWM signal capture  which is output 
from fan.

So should we have the fan[1..n]_output_rpm?



No. I hear you clearly that you for some reason dislike fan[1..n]_input.
While ABIs are not always to our liking, that doesn't mean that we get
to change them at our whim. If that is not acceptable for you, I can't
help you. And you can't change inX_input to inX_voltage either, sorry.


My opinion is only to not use "input" as this is not really the input to 
fan.


Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-08 Thread Guenter Roeck

On 03/07/2018 11:57 PM, Mikko Perttunen wrote:



On 07.03.2018 16:20, Guenter Roeck wrote:

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:



On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:

On 02/27/2018 11:03 PM, Mikko Perttunen wrote:

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the
speed of
a fan and exposes it in roatations per minute (RPM) to the user
space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112
+
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan.
It uses the
+generic PWM interface and can be used on SoCs as along as the
SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the
Fan speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the
user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
    If you say yes here you get support for the temperature
    and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
    comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    +=
wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o
    obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
    ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights
reserved.
+ *
+ * This program is free software; you can redistribute it
and/or modify it
+ * under the terms and conditions of the GNU General Public
License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful,
but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct
device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm = 0;
+
+    err = pwm_capture(pwm, &result, 0);
+    if (err < 0) {
+    dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+    return err;
+    }
+
+    if (result.period)
+    rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+    result.period);
+
+    return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, s

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-08 Thread Guenter Roeck

On 03/07/2018 10:06 PM, Laxman Dewangan wrote:



On Wednesday 07 March 2018 07:50 PM, Guenter Roeck wrote:

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:




While I am not opposed to ABI changes, the merits of those would need to be
discussed on the mailing list. But replacing "fan1_input" with "rpm" is
not an acceptable ABI change, even if it may measure something that turns
but isn't a fan.

If this _is_ in fact supposed to be used for something else but fans, we
would have to discuss what that might be, and if hwmon is the appropriate
subsystem to measure and report it. This does to some degree lead back to
my concern of having the "fan" part of this patch series in the pwm core.
I am still not sure if that makes sense.

Thanks,
Guenter

I am planning to add tachometer support in pwm-fan.c driver (drivers/hwmon/) 
instead of adding new generic-pwm-tachometer.c driver. Measuring RPM value will 
be done in pwm-fan driver itself using pwm capture feature and will add new 
sysfs attributes under this driver to report rpm value of fan.


There is an existing attribute to report the RPM of fans. It is called 
fan[1..n]_input.

"replacing "fan1_input" with "rpm" is not an acceptable ABI change"

Preemptive NACK.


The RPM is measured speed via PWM signal capture  which is output from fan.
So should we have the fan[1..n]_output_rpm?



No. I hear you clearly that you for some reason dislike fan[1..n]_input.
While ABIs are not always to our liking, that doesn't mean that we get
to change them at our whim. If that is not acceptable for you, I can't
help you. And you can't change inX_input to inX_voltage either, sorry.

Guenter


Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-07 Thread Mikko Perttunen



On 07.03.2018 16:20, Guenter Roeck wrote:

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:



On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:

On 02/27/2018 11:03 PM, Mikko Perttunen wrote:

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the
speed of
a fan and exposes it in roatations per minute (RPM) to the user
space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112
+
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan.
It uses the
+generic PWM interface and can be used on SoCs as along as the
SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the
Fan speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the
user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+=
wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights
reserved.
+ *
+ * This program is free software; you can redistribute it
and/or modify it
+ * under the terms and conditions of the GNU General Public
License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful,
but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+struct device*dev;
+struct pwm_device*pwm;
+struct device*hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct
device_attribute *attr,
+char *buf)
+{
+struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+struct pwm_device *pwm = ptt->pwm;
+struct pwm_capture result;
+int err;
+unsigned int rpm = 0;
+
+err = pwm_capture(pwm, &result, 0);
+if (err < 0) {
+dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+return err;
+}
+
+if (result.period)
+rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+result.period);
+
+return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *p

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-07 Thread Laxman Dewangan



On Wednesday 07 March 2018 07:50 PM, Guenter Roeck wrote:

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:




While I am not opposed to ABI changes, the merits of those would 
need to be

discussed on the mailing list. But replacing "fan1_input" with "rpm" is
not an acceptable ABI change, even if it may measure something that 
turns

but isn't a fan.

If this _is_ in fact supposed to be used for something else but 
fans, we
would have to discuss what that might be, and if hwmon is the 
appropriate
subsystem to measure and report it. This does to some degree lead 
back to
my concern of having the "fan" part of this patch series in the pwm 
core.

I am still not sure if that makes sense.

Thanks,
Guenter
I am planning to add tachometer support in pwm-fan.c driver 
(drivers/hwmon/) instead of adding new generic-pwm-tachometer.c 
driver. Measuring RPM value will be done in pwm-fan driver itself 
using pwm capture feature and will add new sysfs attributes under 
this driver to report rpm value of fan.


There is an existing attribute to report the RPM of fans. It is called 
fan[1..n]_input.


"replacing "fan1_input" with "rpm" is not an acceptable ABI change"

Preemptive NACK.


The RPM is measured speed via PWM signal capture  which is output from fan.
So should we have the fan[1..n]_output_rpm?




Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-07 Thread Guenter Roeck

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:



On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:

On 02/27/2018 11:03 PM, Mikko Perttunen wrote:

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 +
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
    If you say yes here you get support for the temperature
    and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
    comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    += wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o
    obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
    ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm = 0;
+
+    err = pwm_capture(pwm, &result, 0);
+    if (err < 0) {
+    dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+    return err;
+    }
+
+    if (result.period)
+    rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+    result.period);
+
+    return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+    &sensor_dev_attr_r

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-07 Thread Rajkumar Rampelli



On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:

On 02/27/2018 11:03 PM, Mikko Perttunen wrote:

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the 
speed of
a fan and exposes it in roatations per minute (RPM) to the user 
space

by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 
+

  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer

new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. 
It uses the
+generic PWM interface and can be used on SoCs as along as the 
SoC supports

+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan 
speed using
+PWM module and Tachometer controller. It requests period value 
through PWM
+capture interface to Tachometer and measures the Rotations per 
minute using
+received period value. It exposes the Fan speed in RPM to the 
user space by

+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= 
wm8350-hwmon.o

  obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c

new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights 
reserved.

+ *
+ * This program is free software; you can redistribute it 
and/or modify it
+ * under the terms and conditions of the GNU General Public 
License,

+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, 
but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General 
Public License for

+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+struct device*dev;
+struct pwm_device*pwm;
+struct device*hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct 
device_attribute *attr,

+char *buf)
+{
+struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+struct pwm_device *pwm = ptt->pwm;
+struct pwm_capture result;
+int err;
+unsigned int rpm = 0;
+
+err = pwm_capture(pwm, &result, 0);
+if (err < 0) {
+dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+return err;
+}
+
+if (result.period)
+rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+result.period);
+
+return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+&sensor_dev_attr_rpm.dev_attr.attr,
+   

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-03-01 Thread Rob Herring
On Wed, Feb 21, 2018 at 05:20:29PM +0200, Mikko Perttunen wrote:
> On 21.02.2018 16:46, Guenter Roeck wrote:
> > On 02/20/2018 11:15 PM, Mikko Perttunen wrote:
> > > AIUI, the PWM framework already exposes a sysfs node with period
> > > information. We should just use that instead of adding a new driver
> > > for this.
> > > 
> > 
> > I am kind of lost. Please explain.
> > 
> > Are you saying that we should drop the pwm-fan driver as well (which goes
> > the opposite way), as well as any other drivers doing anything with pwm
> > signals,
> > because after all those signals are already exposed to userspace a sysfs
> > attributes,
> > and a kernel driver to abstract those values is thus not needed ?
> 
> The only thing this driver does is do a constant division in kernelspace.
> I'm not really seeing why that couldn't be done in userspace. But if you
> think it's appropriate to do the RPM conversion in kernelspace then I'm not
> greatly opposed to that.
> 
> > 
> > > In any case, we cannot add something like this to device tree since
> > > it's not a hardware device.
> > > 
> > 
> > So you are saying there is no means to express in devicetree that
> > a pwm input is connected to a fan ? How is that not hardware ?
> > 
> > If so, how do you express in devicetree that a pwm signal is connected
> > to anything ?
> 
> If we want to describe that the tachometer is connected to a fan, then we
> should have a fan node in the board's device tree. We don't have a chip that
> has a thing called "generic-pwm-tachometer" attached to it. (We have chips
> that have a "nvidia,tegra186-tachometer", so it's proper to have that.)

We already have some fan control bindings in the tree. Follow those. 
There's only so many ways to control fans, so lets have some alignment. 
And yes, we should have a fan node. 

Rob


Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-28 Thread Guenter Roeck

On 02/27/2018 11:03 PM, Mikko Perttunen wrote:

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 +
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
    If you say yes here you get support for the temperature
    and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
    comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    += wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o
    obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
    ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm = 0;
+
+    err = pwm_capture(pwm, &result, 0);
+    if (err < 0) {
+    dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+    return err;
+    }
+
+    if (result.period)
+    rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+    result.period);
+
+    return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+    &sensor_dev_attr_rpm.dev_attr.attr,
+    NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single st

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-27 Thread Mikko Perttunen

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 
+

  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer

new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It 
uses the
+generic PWM interface and can be used on SoCs as along as the SoC 
supports

+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan 
speed using
+PWM module and Tachometer controller. It requests period value 
through PWM
+capture interface to Tachometer and measures the Rotations per 
minute using
+received period value. It exposes the Fan speed in RPM to the user 
space by

+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
    If you say yes here you get support for the temperature
    and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
    comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    += wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o
    obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
    ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c

new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or 
modify it

+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but 
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
License for

+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct 
device_attribute *attr,

+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm = 0;
+
+    err = pwm_capture(pwm, &result, 0);
+    if (err < 0) {
+    dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+    return err;
+    }
+
+    if (result.period)
+    rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+    result.period);
+
+    return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+    &sensor_dev_attr_rpm.dev_attr.attr,
+    NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single standard hwmon sysfs attribute

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-27 Thread Rajkumar Rampelli


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 
+

  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer

new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It 
uses the
+generic PWM interface and can be used on SoCs as along as the SoC 
supports

+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan 
speed using
+PWM module and Tachometer controller. It requests period value 
through PWM
+capture interface to Tachometer and measures the Rotations per 
minute using
+received period value. It exposes the Fan speed in RPM to the user 
space by

+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c

new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or 
modify it

+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but 
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
License for

+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+struct device*dev;
+struct pwm_device*pwm;
+struct device*hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct 
device_attribute *attr,

+char *buf)
+{
+struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+struct pwm_device *pwm = ptt->pwm;
+struct pwm_capture result;
+int err;
+unsigned int rpm = 0;
+
+err = pwm_capture(pwm, &result, 0);
+if (err < 0) {
+dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+return err;
+}
+
+if (result.period)
+rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+result.period);
+
+return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+&sensor_dev_attr_rpm.dev_attr.attr,
+NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single standard hwmon sysfs attribute, having a hwmon driver is 
pointless.

Guenter Ro

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-27 Thread Guenter Roeck

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 +
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
    If you say yes here you get support for the temperature
    and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
    comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    += wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o
    obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
    ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm = 0;
+
+    err = pwm_capture(pwm, &result, 0);
+    if (err < 0) {
+    dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+    return err;
+    }
+
+    if (result.period)
+    rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+    result.period);
+
+    return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+    &sensor_dev_attr_rpm.dev_attr.attr,
+    NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single standard hwmon sysfs attribute, having a hwmon driver is pointless.

Guenter Roeck,
I will define a new hwmon sysfs attribute node called 
"hwmon_tachometer_attrib

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-27 Thread Rajkumar Rampelli


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 
+

  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer

new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It 
uses the
+generic PWM interface and can be used on SoCs as along as the SoC 
supports

+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan 
speed using
+PWM module and Tachometer controller. It requests period value 
through PWM
+capture interface to Tachometer and measures the Rotations per 
minute using
+received period value. It exposes the Fan speed in RPM to the user 
space by

+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c

new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or 
modify it

+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but 
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of 
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
License for

+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+struct device*dev;
+struct pwm_device*pwm;
+struct device*hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute 
*attr,

+char *buf)
+{
+struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+struct pwm_device *pwm = ptt->pwm;
+struct pwm_capture result;
+int err;
+unsigned int rpm = 0;
+
+err = pwm_capture(pwm, &result, 0);
+if (err < 0) {
+dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+return err;
+}
+
+if (result.period)
+rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+result.period);
+
+return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+&sensor_dev_attr_rpm.dev_attr.attr,
+NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single standard hwmon sysfs attribute, having a hwmon driver is 
pointless.

Guenter Roeck,
I will define a new hwmon sysfs attribute node called 
"hwmon_tachometer_attributes" in hwmon.h like below a

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-23 Thread RAJKUMAR


On Wednesday 21 February 2018 09:38 PM, Guenter Roeck wrote:

On Wed, Feb 21, 2018 at 05:20:29PM +0200, Mikko Perttunen wrote:

On 21.02.2018 16:46, Guenter Roeck wrote:

On 02/20/2018 11:15 PM, Mikko Perttunen wrote:

AIUI, the PWM framework already exposes a sysfs node with period
information. We should just use that instead of adding a new driver
for this.


I am kind of lost. Please explain.

Are you saying that we should drop the pwm-fan driver as well (which goes
the opposite way), as well as any other drivers doing anything with pwm
signals,
because after all those signals are already exposed to userspace a sysfs
attributes,
and a kernel driver to abstract those values is thus not needed ?

The only thing this driver does is do a constant division in kernelspace.
I'm not really seeing why that couldn't be done in userspace. But if you
think it's appropriate to do the RPM conversion in kernelspace then I'm not
greatly opposed to that.


Isn't that true for any conversion from and to pwm values ? Voltages,
for example ? Should those be handled in userspace as well ?

Note that I am not entirely convinced that the approach suggested in this
patch series makes sense. Patch 4 seems to move the notion of a tachometer
into the pwm subsystem. I am not really convinced that this makes sense
(maybe all that code should be in the hwmon driver instead, or in a thermal
driver if the author prefers). But that is a different issue. The question
you raised is if there should be any abstraction to or from raw pwm values
in the kernel.
Since tachometer driver implements PWM capture callback feature, I have 
added it under pwm driver as pwm-tegra-tachometer.c driver.
If the best approach is to have this tachometer driver under hwmon 
driver then I will move it to hwmon driver.



In any case, we cannot add something like this to device tree since
it's not a hardware device.


So you are saying there is no means to express in devicetree that
a pwm input is connected to a fan ? How is that not hardware ?

If so, how do you express in devicetree that a pwm signal is connected
to anything ?

If we want to describe that the tachometer is connected to a fan, then we
should have a fan node in the board's device tree. We don't have a chip that
has a thing called "generic-pwm-tachometer" attached to it. (We have chips
that have a "nvidia,tegra186-tachometer", so it's proper to have that.)


So you are concerned about the property name ? I don't really care how
it is called.

Guenter


Thanks,
Mikko


Guenter


Mikko

On 21.02.2018 08:58, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
Documentation/hwmon/generic-pwm-tachometer |  17 +
drivers/hwmon/Kconfig  |  10 +++
drivers/hwmon/Makefile |   1 +
drivers/hwmon/generic-pwm-tachometer.c | 112
+
4 files changed, 140 insertions(+)
create mode 100644 Documentation/hwmon/generic-pwm-tachometer
create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It
uses the
+generic PWM interface and can be used on SoCs as along as the SoC
supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan
speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the user
space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
   If you say yes here you get support for the temperature
   and power sensors for APM X-Gene SoC.

+config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
if ACPI

comment "

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-21 Thread Guenter Roeck
On Wed, Feb 21, 2018 at 05:20:29PM +0200, Mikko Perttunen wrote:
> On 21.02.2018 16:46, Guenter Roeck wrote:
> >On 02/20/2018 11:15 PM, Mikko Perttunen wrote:
> >>AIUI, the PWM framework already exposes a sysfs node with period
> >>information. We should just use that instead of adding a new driver
> >>for this.
> >>
> >
> >I am kind of lost. Please explain.
> >
> >Are you saying that we should drop the pwm-fan driver as well (which goes
> >the opposite way), as well as any other drivers doing anything with pwm
> >signals,
> >because after all those signals are already exposed to userspace a sysfs
> >attributes,
> >and a kernel driver to abstract those values is thus not needed ?
> 
> The only thing this driver does is do a constant division in kernelspace.
> I'm not really seeing why that couldn't be done in userspace. But if you
> think it's appropriate to do the RPM conversion in kernelspace then I'm not
> greatly opposed to that.
> 
Isn't that true for any conversion from and to pwm values ? Voltages,
for example ? Should those be handled in userspace as well ?

Note that I am not entirely convinced that the approach suggested in this
patch series makes sense. Patch 4 seems to move the notion of a tachometer
into the pwm subsystem. I am not really convinced that this makes sense
(maybe all that code should be in the hwmon driver instead, or in a thermal
driver if the author prefers). But that is a different issue. The question
you raised is if there should be any abstraction to or from raw pwm values
in the kernel.

> >
> >>In any case, we cannot add something like this to device tree since
> >>it's not a hardware device.
> >>
> >
> >So you are saying there is no means to express in devicetree that
> >a pwm input is connected to a fan ? How is that not hardware ?
> >
> >If so, how do you express in devicetree that a pwm signal is connected
> >to anything ?
> 
> If we want to describe that the tachometer is connected to a fan, then we
> should have a fan node in the board's device tree. We don't have a chip that
> has a thing called "generic-pwm-tachometer" attached to it. (We have chips
> that have a "nvidia,tegra186-tachometer", so it's proper to have that.)
> 
So you are concerned about the property name ? I don't really care how
it is called.

Guenter

> Thanks,
> Mikko
> 
> >
> >Guenter
> >
> >>Mikko
> >>
> >>On 21.02.2018 08:58, Rajkumar Rampelli wrote:
> >>>Add generic PWM based tachometer driver via HWMON interface
> >>>to report the RPM of motor. This drivers get the period/duty
> >>>cycle from PWM IP which captures the motor PWM output.
> >>>
> >>>This driver implements a simple interface for monitoring the speed of
> >>>a fan and exposes it in roatations per minute (RPM) to the user space
> >>>by using the hwmon's sysfs interface
> >>>
> >>>Signed-off-by: Rajkumar Rampelli 
> >>>---
> >>> Documentation/hwmon/generic-pwm-tachometer |  17 +
> >>> drivers/hwmon/Kconfig  |  10 +++
> >>> drivers/hwmon/Makefile |   1 +
> >>> drivers/hwmon/generic-pwm-tachometer.c | 112
> >>>+
> >>> 4 files changed, 140 insertions(+)
> >>> create mode 100644 Documentation/hwmon/generic-pwm-tachometer
> >>> create mode 100644 drivers/hwmon/generic-pwm-tachometer.c
> >>>
> >>>diff --git a/Documentation/hwmon/generic-pwm-tachometer
> >>>b/Documentation/hwmon/generic-pwm-tachometer
> >>>new file mode 100644
> >>>index 000..e0713ee
> >>>--- /dev/null
> >>>+++ b/Documentation/hwmon/generic-pwm-tachometer
> >>>@@ -0,0 +1,17 @@
> >>>+Kernel driver generic-pwm-tachometer
> >>>+
> >>>+
> >>>+This driver enables the use of a PWM module to monitor a fan. It
> >>>uses the
> >>>+generic PWM interface and can be used on SoCs as along as the SoC
> >>>supports
> >>>+Tachometer controller that moniors the Fan speed in periods.
> >>>+
> >>>+Author: Rajkumar Rampelli 
> >>>+
> >>>+Description
> >>>+---
> >>>+
> >>>+The driver implements a simple interface for monitoring the Fan
> >>>speed using
> >>>+PWM module and Tachometer controller. It requests period value
> >>>through PWM
> >>>+capture interface to Tachometer and measures the Rotations per
> >>>minute using
> >>>+received period value. It exposes the Fan speed in RPM to the user
> >>>space by
> >>>+using the hwmon's sysfs interface.
> >>>diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> >>>index ef23553..8912dcb 100644
> >>>--- a/drivers/hwmon/Kconfig
> >>>+++ b/drivers/hwmon/Kconfig
> >>>@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
> >>>   If you say yes here you get support for the temperature
> >>>   and power sensors for APM X-Gene SoC.
> >>>
> >>>+config GENERIC_PWM_TACHOMETER
> >>>+tristate "Generic PWM based tachometer driver"
> >>>+depends on PWM
> >>>+help
> >>>+  Enables a driver to use PWM signal from motor to use
> >>>+  for measuring the motor speed. The RPM is captured by
> >>>+  PWM modules which has 

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-21 Thread Mikko Perttunen

On 21.02.2018 16:46, Guenter Roeck wrote:

On 02/20/2018 11:15 PM, Mikko Perttunen wrote:

AIUI, the PWM framework already exposes a sysfs node with period
information. We should just use that instead of adding a new driver
for this.



I am kind of lost. Please explain.

Are you saying that we should drop the pwm-fan driver as well (which goes
the opposite way), as well as any other drivers doing anything with pwm
signals,
because after all those signals are already exposed to userspace a sysfs
attributes,
and a kernel driver to abstract those values is thus not needed ?


The only thing this driver does is do a constant division in 
kernelspace. I'm not really seeing why that couldn't be done in 
userspace. But if you think it's appropriate to do the RPM conversion in 
kernelspace then I'm not greatly opposed to that.





In any case, we cannot add something like this to device tree since
it's not a hardware device.



So you are saying there is no means to express in devicetree that
a pwm input is connected to a fan ? How is that not hardware ?

If so, how do you express in devicetree that a pwm signal is connected
to anything ?


If we want to describe that the tachometer is connected to a fan, then 
we should have a fan node in the board's device tree. We don't have a 
chip that has a thing called "generic-pwm-tachometer" attached to it. 
(We have chips that have a "nvidia,tegra186-tachometer", so it's proper 
to have that.)


Thanks,
Mikko



Guenter


Mikko

On 21.02.2018 08:58, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
 Documentation/hwmon/generic-pwm-tachometer |  17 +
 drivers/hwmon/Kconfig  |  10 +++
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/generic-pwm-tachometer.c | 112
+
 4 files changed, 140 insertions(+)
 create mode 100644 Documentation/hwmon/generic-pwm-tachometer
 create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It
uses the
+generic PWM interface and can be used on SoCs as along as the SoC
supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan
speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the user
space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
   If you say yes here you get support for the temperature
   and power sensors for APM X-Gene SoC.

+config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
 if ACPI

 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o

 obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o

 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG

diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
WITHOUT
+ * ANY WARRANTY; without ev

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-21 Thread Guenter Roeck

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 +
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
  If you say yes here you get support for the temperature
  and power sensors for APM X-Gene SoC.
  
+config GENERIC_PWM_TACHOMETER

+   tristate "Generic PWM based tachometer driver"
+   depends on PWM
+   help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
  if ACPI
  
  comment "ACPI drivers"

diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)   += xgene-hwmon.o
  
  obj-$(CONFIG_PMBUS)		+= pmbus/

+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
  
  ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  
diff --git a/drivers/hwmon/generic-pwm-tachometer.c b/drivers/hwmon/generic-pwm-tachometer.c

new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+   struct device   *dev;
+   struct pwm_device   *pwm;
+   struct device   *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+   struct pwm_device *pwm = ptt->pwm;
+   struct pwm_capture result;
+   int err;
+   unsigned int rpm = 0;
+
+   err = pwm_capture(pwm, &result, 0);
+   if (err < 0) {
+   dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+   return err;
+   }
+
+   if (result.period)
+   rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+   result.period);
+
+   return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+   &sensor_dev_attr_rpm.dev_attr.attr,
+   NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single standard hwmon sysfs attribute, having a hwmon driver is pointless.


+
+ATTRIBUTE_GROUPS(pwm_tach);
+
+static int pwm_tach_probe(struct p

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-21 Thread Guenter Roeck

On 02/20/2018 11:15 PM, Mikko Perttunen wrote:

AIUI, the PWM framework already exposes a sysfs node with period information. 
We should just use that instead of adding a new driver for this.



I am kind of lost. Please explain.

Are you saying that we should drop the pwm-fan driver as well (which goes
the opposite way), as well as any other drivers doing anything with pwm signals,
because after all those signals are already exposed to userspace a sysfs 
attributes,
and a kernel driver to abstract those values is thus not needed ?


In any case, we cannot add something like this to device tree since it's not a 
hardware device.



So you are saying there is no means to express in devicetree that
a pwm input is connected to a fan ? How is that not hardware ?

If so, how do you express in devicetree that a pwm signal is connected
to anything ?

Guenter


Mikko

On 21.02.2018 08:58, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
 Documentation/hwmon/generic-pwm-tachometer |  17 +
 drivers/hwmon/Kconfig  |  10 +++
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/generic-pwm-tachometer.c | 112 +
 4 files changed, 140 insertions(+)
 create mode 100644 Documentation/hwmon/generic-pwm-tachometer
 create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
   If you say yes here you get support for the temperature
   and power sensors for APM X-Gene SoC.

+config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
 if ACPI

 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    += wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o

 obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o

 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG

diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-20 Thread Mikko Perttunen
AIUI, the PWM framework already exposes a sysfs node with period 
information. We should just use that instead of adding a new driver for 
this.


In any case, we cannot add something like this to device tree since it's 
not a hardware device.


Mikko

On 21.02.2018 08:58, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
 Documentation/hwmon/generic-pwm-tachometer |  17 +
 drivers/hwmon/Kconfig  |  10 +++
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/generic-pwm-tachometer.c | 112 +
 4 files changed, 140 insertions(+)
 create mode 100644 Documentation/hwmon/generic-pwm-tachometer
 create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
  If you say yes here you get support for the temperature
  and power sensors for APM X-Gene SoC.

+config GENERIC_PWM_TACHOMETER
+   tristate "Generic PWM based tachometer driver"
+   depends on PWM
+   help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
 if ACPI

 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o

 obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o

 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG

diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+   struct device   *dev;
+   struct pwm_device   *pwm;
+   struct device   *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+   struct pwm_device *pwm = ptt->pwm;
+   struct pwm_capture result;
+   int err;
+   unsigned int rpm = 0;
+
+   err = pwm_capture(pwm, &result, 0);
+   if (err < 0) {
+   dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+   return err;
+   }
+
+   if (result.period)
+   rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+   result.period);
+
+   return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+   &sensor_dev_attr_rpm.dev_attr.attr,

[PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

2018-02-20 Thread Rajkumar Rampelli
Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
 Documentation/hwmon/generic-pwm-tachometer |  17 +
 drivers/hwmon/Kconfig  |  10 +++
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/generic-pwm-tachometer.c | 112 +
 4 files changed, 140 insertions(+)
 create mode 100644 Documentation/hwmon/generic-pwm-tachometer
 create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
  If you say yes here you get support for the temperature
  and power sensors for APM X-Gene SoC.
 
+config GENERIC_PWM_TACHOMETER
+   tristate "Generic PWM based tachometer driver"
+   depends on PWM
+   help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
 
 obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
 
 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
 
diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+   struct device   *dev;
+   struct pwm_device   *pwm;
+   struct device   *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+   struct pwm_device *pwm = ptt->pwm;
+   struct pwm_capture result;
+   int err;
+   unsigned int rpm = 0;
+
+   err = pwm_capture(pwm, &result, 0);
+   if (err < 0) {
+   dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+   return err;
+   }
+
+   if (result.period)
+   rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+   result.period);
+
+   return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+   &sensor_dev_attr_rpm.dev_attr.attr,
+   NULL,
+};
+
+ATTRIBUTE_GROUPS(pwm_tach);
+
+static int pwm_tach_probe(struct platform_device *pdev)
+{
+   struct pwm_hwmon_tach *ptt;
+   int err;
+
+   ptt = devm_kzalloc(&pdev->dev, sizeof(*ptt), GFP_KERNEL);
+   if (!ptt)
+   return -ENOMEM;
+
+   ptt