* [PATCH 01/10] pwm: core: Add support for PWM HW driver with pwm capture only
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 02/10] dt-bindings: Tegra186 tachometer device tree bindings Rajkumar Rampelli
` (8 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
Add support for pwm HW driver which has only capture functionality.
This helps to implement the PWM based Tachometer driver which reads
the PWM output signals from electronic fans.
PWM Tachometer captures the period and duty cycle of the PWM signal
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
drivers/pwm/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 1581f6a..87d14c9 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -246,6 +246,10 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
if (ops->apply)
return true;
+ /* driver supports capture operation */
+ if (ops->capture)
+ return true;
+
return false;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 02/10] dt-bindings: Tegra186 tachometer device tree bindings
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 01/10] pwm: core: Add support for PWM HW driver with pwm capture only Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 03/10] pwm: tegra: Add PWM based Tachometer driver Rajkumar Rampelli
` (7 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
Supply Device tree binding documentation for the NVIDIA
Tegra186 SoC's Tachometer Controller
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
.../bindings/pwm/pwm-tegra-tachometer.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
diff --git a/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt b/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
new file mode 100644
index 0000000..4a7ead4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
@@ -0,0 +1,31 @@
+Bindings for a PWM based Tachometer driver
+
+Required properties:
+- compatible: Must be "nvidia,tegra186-pwm-tachometer"
+- reg: physical base addresses of the controller and length of
+ memory mapped region.
+- #pwm-cells: should be 2. See pwm.txt in this directory for a
+ description of the cells format.
+- clocks: phandle list of tachometer clocks
+- clock-names: should be "tachometer". See clock-bindings.txt in documentations
+- resets: phandle to the reset controller for the Tachometer IP
+- reset-names: should be "tachometer". See reset.txt in documentations
+- nvidia,pulse-per-rev: Integer, pulses per revolution of a Fan. This value
+ obtained from Fan specification document.
+- nvidia,capture-window-len: Integer, window of the Fan Tach monitor, it indicates
+ that how many period of the input fan tach signal will the FAN TACH logic
+ monitor. Valid values are 1, 2, 4 and 8 only.
+
+Example:
+ tegra_tachometer: tachometer@39c0000 {
+ compatible = "nvidia,tegra186-pwm-tachometer";
+ reg = <0x0 0x039c0000 0x0 0x10>;
+ #pwm-cells = <2>;
+ clocks = <&tegra_car TEGRA186_CLK_TACH>;
+ clock-names = "tachometer";
+ resets = <&tegra_car TEGRA186_RESET_TACH>;
+ reset-names = "tachometer";
+ nvidia,pulse-per-rev = <2>;
+ nvidia,capture-window-len = <2>;
+ status = "disabled";
+ };
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 03/10] pwm: tegra: Add PWM based Tachometer driver
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 01/10] pwm: core: Add support for PWM HW driver with pwm capture only Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 02/10] dt-bindings: Tegra186 tachometer device tree bindings Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 04/10] hwmon: generic-pwm-tachometer: Add DT binding details Rajkumar Rampelli
` (6 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
PWM Tachometer driver capture the PWM signal which is output of FAN
in general and provide the period of PWM signal which is converted to
RPM by SW.
Add Tegra Tachometer driver which implements the pwm-capture to
measure period.
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
drivers/pwm/Kconfig | 10 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-tegra-tachometer.c | 303 +++++++++++++++++++++++++++++++++++++
3 files changed, 314 insertions(+)
create mode 100644 drivers/pwm/pwm-tegra-tachometer.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 763ee50..29aeeeb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -454,6 +454,16 @@ config PWM_TEGRA
To compile this driver as a module, choose M here: the module
will be called pwm-tegra.
+config PWM_TEGRA_TACHOMETER
+ tristate "NVIDIA Tegra Tachometer PWM driver"
+ depends on ARCH_TEGRA
+ help
+ NVIDIA Tegra Tachometer reads the PWM signal and reports the PWM
+ signal periods. This helps in measuring the fan speed where Fan
+ output for speed is PWM signal.
+
+ This driver support the Tachometer driver in PWM framework.
+
config PWM_TIECAP
tristate "ECAP PWM support"
depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0258a74..14c183e 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o
obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
+obj-$(CONFIG_PWM_TEGRA_TACHOMETER) += pwm-tegra-tachometer.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TIPWMSS) += pwm-tipwmss.o
diff --git a/drivers/pwm/pwm-tegra-tachometer.c b/drivers/pwm/pwm-tegra-tachometer.c
new file mode 100644
index 0000000..1304e47
--- /dev/null
+++ b/drivers/pwm/pwm-tegra-tachometer.c
@@ -0,0 +1,303 @@
+/*
+ * Tegra Tachometer Pulse-Width-Modulation driver
+ *
+ * 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 <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+/* Since oscillator clock (38.4MHz) serves as a clock source for
+ * the tach input controller, 1.0105263MHz (i.e. 38.4/38) has to be
+ * used as a clock value in the RPM calculations
+ */
+#define TACH_COUNTER_CLK 1010526
+
+#define TACH_FAN_TACH0 0x0
+#define TACH_FAN_TACH0_PERIOD_MASK 0x7FFFF
+#define TACH_FAN_TACH0_PERIOD_MAX 0x7FFFF
+#define TACH_FAN_TACH0_PERIOD_MIN 0x0
+#define TACH_FAN_TACH0_WIN_LENGTH_SHIFT 25
+#define TACH_FAN_TACH0_WIN_LENGTH_MASK 0x3
+#define TACH_FAN_TACH0_OVERFLOW_MASK BIT(24)
+
+#define TACH_FAN_TACH1 0x4
+#define TACH_FAN_TACH1_HI_MASK 0x7FFFF
+/*
+ * struct pwm_tegra_tach - Tegra tachometer object
+ * @dev: device providing the Tachometer
+ * @pulse_per_rev: Pulses per revolution of a Fan
+ * @capture_window_len: Defines the window of the FAN TACH monitor
+ * @regs: physical base addresses of the controller
+ * @clk: phandle list of tachometer clocks
+ * @rst: phandle to reset the controller
+ * @chip: PWM chip providing this PWM device
+ */
+struct pwm_tegra_tach {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *clk;
+ struct reset_control *rst;
+ u32 pulse_per_rev;
+ u32 capture_window_len;
+ struct pwm_chip chip;
+};
+
+static struct pwm_tegra_tach *to_tegra_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct pwm_tegra_tach, chip);
+}
+
+static u32 tachometer_readl(struct pwm_tegra_tach *ptt, unsigned long reg)
+{
+ return readl(ptt->regs + reg);
+}
+
+static inline void tachometer_writel(struct pwm_tegra_tach *ptt, u32 val,
+ unsigned long reg)
+{
+ writel(val, ptt->regs + reg);
+}
+
+static int pwm_tegra_tach_set_wlen(struct pwm_tegra_tach *ptt,
+ u32 window_length)
+{
+ u32 tach0, wlen;
+
+ /*
+ * As per FAN Spec, the window length value should be greater than or
+ * equal to Pulses Per Revolution value to measure the time period
+ * values accurately.
+ */
+ if (ptt->pulse_per_rev > ptt->capture_window_len) {
+ dev_err(ptt->dev,
+ "Window length value < pulses per revolution value\n");
+ return -EINVAL;
+ }
+
+ if (hweight8(window_length) != 1) {
+ dev_err(ptt->dev,
+ "Valid value of window length is {1, 2, 4 or 8}\n");
+ return -EINVAL;
+ }
+
+ wlen = ffs(window_length) - 1;
+ tach0 = tachometer_readl(ptt, TACH_FAN_TACH0);
+ tach0 &= ~(TACH_FAN_TACH0_WIN_LENGTH_MASK <<
+ TACH_FAN_TACH0_WIN_LENGTH_SHIFT);
+ tach0 |= wlen << TACH_FAN_TACH0_WIN_LENGTH_SHIFT;
+ tachometer_writel(ptt, tach0, TACH_FAN_TACH0);
+
+ return 0;
+}
+
+static int pwm_tegra_tach_capture(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ struct pwm_capture *result,
+ unsigned long timeout)
+{
+ struct pwm_tegra_tach *ptt = to_tegra_pwm_chip(chip);
+ unsigned long period;
+ u32 tach;
+
+ tach = tachometer_readl(ptt, TACH_FAN_TACH1);
+ result->duty_cycle = tach & TACH_FAN_TACH1_HI_MASK;
+
+ tach = tachometer_readl(ptt, TACH_FAN_TACH0);
+ if (tach & TACH_FAN_TACH0_OVERFLOW_MASK) {
+ /* Fan is stalled, clear overflow state by writing 1 */
+ dev_dbg(ptt->dev, "Tachometer Overflow is detected\n");
+ tachometer_writel(ptt, tach, TACH_FAN_TACH0);
+ }
+
+ period = tach & TACH_FAN_TACH0_PERIOD_MASK;
+ if ((period == TACH_FAN_TACH0_PERIOD_MIN) ||
+ (period == TACH_FAN_TACH0_PERIOD_MAX)) {
+ dev_dbg(ptt->dev, "Period set to min/max 0x%lx, Invalid RPM\n",
+ period);
+ result->period = 0;
+ result->duty_cycle = 0;
+ return 0;
+ }
+
+ period = period + 1;
+
+ period = DIV_ROUND_CLOSEST_ULL(period * ptt->pulse_per_rev * 1000000ULL,
+ ptt->capture_window_len *
+ TACH_COUNTER_CLK);
+
+ /*
+ * period & duty cycle values are in units of micro seconds.
+ * Hence, convert them into nano seconds and store.
+ */
+ result->period = period * 1000;
+ result->duty_cycle = result->duty_cycle * 1000;
+
+ return 0;
+}
+
+static const struct pwm_ops pwm_tegra_tach_ops = {
+ .capture = pwm_tegra_tach_capture,
+ .owner = THIS_MODULE,
+};
+
+static int pwm_tegra_tach_read_platform_data(struct pwm_tegra_tach *ptt)
+{
+ struct device_node *np = ptt->dev->of_node;
+ u32 pval;
+ int err = 0;
+
+ err = of_property_read_u32(np, "nvidia,pulse-per-rev", &pval);
+ if (err < 0) {
+ dev_err(ptt->dev,
+ "\"nvidia,pulse-per-rev\" property is missing\n");
+ return err;
+ }
+ ptt->pulse_per_rev = pval;
+
+ err = of_property_read_u32(np, "nvidia,capture-window-len", &pval);
+ if (err < 0) {
+ dev_err(ptt->dev,
+ "\"nvidia,capture-window-len\" property is missing\n");
+ return err;
+ }
+ ptt->capture_window_len = pval;
+
+ return err;
+}
+
+static int pwm_tegra_tach_probe(struct platform_device *pdev)
+{
+ struct pwm_tegra_tach *ptt;
+ struct resource *res;
+ int err = 0;
+
+ ptt = devm_kzalloc(&pdev->dev, sizeof(*ptt), GFP_KERNEL);
+ if (!ptt)
+ return -ENOMEM;
+
+ ptt->dev = &pdev->dev;
+
+ err = pwm_tegra_tach_read_platform_data(ptt);
+ if (err < 0)
+ return err;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ptt->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ptt->regs)) {
+ dev_err(&pdev->dev, "Failed to remap I/O memory\n");
+ return PTR_ERR(ptt->regs);
+ }
+
+ platform_set_drvdata(pdev, ptt);
+
+ ptt->clk = devm_clk_get(&pdev->dev, "tach");
+ if (IS_ERR(ptt->clk)) {
+ err = PTR_ERR(ptt->clk);
+ dev_err(&pdev->dev, "Failed to get Tachometer clk: %d\n", err);
+ return err;
+ }
+
+ ptt->rst = devm_reset_control_get(&pdev->dev, "tach");
+ if (IS_ERR(ptt->rst)) {
+ err = PTR_ERR(ptt->rst);
+ dev_err(&pdev->dev, "Failed to get reset handle: %d\n", err);
+ return err;
+ }
+
+ err = clk_prepare_enable(ptt->clk);
+ if (err < 0) {
+ dev_err(&pdev->dev, "Failed to prepare clock: %d\n", err);
+ return err;
+ }
+
+ err = clk_set_rate(ptt->clk, TACH_COUNTER_CLK);
+ if (err < 0) {
+ dev_err(&pdev->dev, "Failed to set clock rate %d: %d\n",
+ TACH_COUNTER_CLK, err);
+ goto clk_unprep;
+ }
+
+ reset_control_reset(ptt->rst);
+
+ ptt->chip.dev = &pdev->dev;
+ ptt->chip.ops = &pwm_tegra_tach_ops;
+ ptt->chip.base = -1;
+ ptt->chip.npwm = 1;
+
+ err = pwmchip_add(&ptt->chip);
+ if (err < 0) {
+ dev_err(&pdev->dev, "Failed to add tachometer PWM: %d\n", err);
+ goto reset_assert;
+ }
+
+ err = pwm_tegra_tach_set_wlen(ptt, ptt->capture_window_len);
+ if (err < 0) {
+ dev_err(ptt->dev, "Failed to set window length: %d\n", err);
+ goto pwm_remove;
+ }
+
+ return 0;
+
+pwm_remove:
+ pwmchip_remove(&ptt->chip);
+
+reset_assert:
+ reset_control_assert(ptt->rst);
+
+clk_unprep:
+ clk_disable_unprepare(ptt->clk);
+
+ return err;
+}
+
+static int pwm_tegra_tach_remove(struct platform_device *pdev)
+{
+ struct pwm_tegra_tach *ptt = platform_get_drvdata(pdev);
+
+ reset_control_assert(ptt->rst);
+
+ clk_disable_unprepare(ptt->clk);
+
+ return pwmchip_remove(&ptt->chip);
+}
+
+static const struct of_device_id pwm_tegra_tach_of_match[] = {
+ { .compatible = "nvidia,tegra186-pwm-tachometer" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pwm_tegra_tach_of_match);
+
+static struct platform_driver tegra_tach_driver = {
+ .driver = {
+ .name = "pwm-tegra-tachometer",
+ .of_match_table = pwm_tegra_tach_of_match,
+ },
+ .probe = pwm_tegra_tach_probe,
+ .remove = pwm_tegra_tach_remove,
+};
+
+module_platform_driver(tegra_tach_driver);
+
+MODULE_DESCRIPTION("PWM based NVIDIA Tegra Tachometer driver");
+MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 04/10] hwmon: generic-pwm-tachometer: Add DT binding details
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (2 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 03/10] pwm: tegra: Add PWM based Tachometer driver Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer Rajkumar Rampelli
` (5 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
Add DT binding details for the PWM based generic tachometer
driver which gets the period of the PWM tach-output from Fan
via PWM IP having capability of capturing the signal.
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
.../bindings/hwmon/generic-pwm-tachometer.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/generic-pwm-tachometer.txt
diff --git a/Documentation/devicetree/bindings/hwmon/generic-pwm-tachometer.txt b/Documentation/devicetree/bindings/hwmon/generic-pwm-tachometer.txt
new file mode 100644
index 0000000..3541fe5
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/generic-pwm-tachometer.txt
@@ -0,0 +1,25 @@
+Device tree bindings for fan tach output connected to PWM controller with
+PWM capture capability.
+
+Required properties:
+- compatible : Should be "generic-pwm-tachometer"
+- pwms : PWM handle. Please refer pwm.txt DT binding for more details.
+
+Example:
+ tegra_tachometer: tachometer@39c0000 {
+ compatible = "nvidia,tegra186-pwm-tachometer";
+ reg = <0x0 0x039c0000 0x0 0x10>;
+ clocks = <&bpmp_clks TEGRA194_CLK_TACH>;
+ clock-names = "tachometer";
+ resets = <&bpmp_resets TEGRA194_RESET_TACH>;
+ reset-names = "tachometer";
+ nvidia,pulse-per-rev = <2>;
+ nvidia,sampling-window = <2>;
+ status = "okay";
+ };
+
+ generic_pwm_tachometer {
+ compatible = "generic-pwm-tachometer";
+ pwms = <&tegra_tachometer 0 1000000>;
+ status = "okay";
+ };
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (3 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 04/10] hwmon: generic-pwm-tachometer: Add DT binding details Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 7:15 ` Mikko Perttunen
2018-02-21 14:50 ` Guenter Roeck
2018-02-21 6:58 ` [PATCH 06/10] arm64: tegra: Add Tachometer Controller on Tegra186 Rajkumar Rampelli
` (4 subsequent siblings)
9 siblings, 2 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
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 <rrajk@nvidia.com>
---
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 0000000..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 <rrajk@nvidia.com>
+
+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 0000000..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 <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/pwm.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+
+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->dev = &pdev->dev;
+
+ platform_set_drvdata(pdev, ptt);
+ dev_set_drvdata(&pdev->dev, ptt);
+
+ ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
+ if (IS_ERR(ptt->pwm)) {
+ err = PTR_ERR(ptt->pwm);
+ dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
+ err);
+ return err;
+ }
+
+ ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
+ "pwm_tach", ptt, pwm_tach_groups);
+ if (IS_ERR(ptt->hwmon)) {
+ err = PTR_ERR_OR_ZERO(ptt->hwmon);
+ dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
+ err);
+ return err;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id pwm_tach_of_match[] = {
+ { .compatible = "generic-pwm-tachometer" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
+
+static struct platform_driver pwm_tach_driver = {
+ .driver = {
+ .name = "generic-pwm-tachometer",
+ .of_match_table = pwm_tach_of_match,
+ },
+ .probe = pwm_tach_probe,
+};
+
+module_platform_driver(pwm_tach_driver);
+
+MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 6:58 ` [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer Rajkumar Rampelli
@ 2018-02-21 7:15 ` Mikko Perttunen
2018-02-21 14:46 ` Guenter Roeck
2018-02-21 14:50 ` Guenter Roeck
1 sibling, 1 reply; 31+ messages in thread
From: Mikko Perttunen @ 2018-02-21 7:15 UTC (permalink / raw)
To: Rajkumar Rampelli, robh+dt, mark.rutland, thierry.reding,
jonathanh, jdelvare, linux, corbet, catalin.marinas, will.deacon,
kstewart, gregkh, pombredanne, mmaddireddy, mperttunen, arnd,
gregory.clement, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
> ---
> 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 0000000..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 <rrajk@nvidia.com>
> +
> +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 0000000..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 <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/pwm.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +
> +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->dev = &pdev->dev;
> +
> + platform_set_drvdata(pdev, ptt);
> + dev_set_drvdata(&pdev->dev, ptt);
> +
> + ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
> + if (IS_ERR(ptt->pwm)) {
> + err = PTR_ERR(ptt->pwm);
> + dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
> + err);
> + return err;
> + }
> +
> + ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
> + "pwm_tach", ptt, pwm_tach_groups);
> + if (IS_ERR(ptt->hwmon)) {
> + err = PTR_ERR_OR_ZERO(ptt->hwmon);
> + dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
> + err);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pwm_tach_of_match[] = {
> + { .compatible = "generic-pwm-tachometer" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
> +
> +static struct platform_driver pwm_tach_driver = {
> + .driver = {
> + .name = "generic-pwm-tachometer",
> + .of_match_table = pwm_tach_of_match,
> + },
> + .probe = pwm_tach_probe,
> +};
> +
> +module_platform_driver(pwm_tach_driver);
> +
> +MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
> +MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 7:15 ` Mikko Perttunen
@ 2018-02-21 14:46 ` Guenter Roeck
2018-02-21 15:20 ` Mikko Perttunen
0 siblings, 1 reply; 31+ messages in thread
From: Guenter Roeck @ 2018-02-21 14:46 UTC (permalink / raw)
To: Mikko Perttunen, Rajkumar Rampelli, robh+dt, mark.rutland,
thierry.reding, jonathanh, jdelvare, corbet, catalin.marinas,
will.deacon, kstewart, gregkh, pombredanne, mmaddireddy,
mperttunen, arnd, gregory.clement, timur, andy.gross, xuwei5,
elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>> ---
>> 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 0000000..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 <rrajk@nvidia.com>
>> +
>> +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 0000000..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 <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/err.h>
>> +#include <linux/pwm.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +
>> +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->dev = &pdev->dev;
>> +
>> + platform_set_drvdata(pdev, ptt);
>> + dev_set_drvdata(&pdev->dev, ptt);
>> +
>> + ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
>> + if (IS_ERR(ptt->pwm)) {
>> + err = PTR_ERR(ptt->pwm);
>> + dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
>> + err);
>> + return err;
>> + }
>> +
>> + ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
>> + "pwm_tach", ptt, pwm_tach_groups);
>> + if (IS_ERR(ptt->hwmon)) {
>> + err = PTR_ERR_OR_ZERO(ptt->hwmon);
>> + dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
>> + err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id pwm_tach_of_match[] = {
>> + { .compatible = "generic-pwm-tachometer" },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
>> +
>> +static struct platform_driver pwm_tach_driver = {
>> + .driver = {
>> + .name = "generic-pwm-tachometer",
>> + .of_match_table = pwm_tach_of_match,
>> + },
>> + .probe = pwm_tach_probe,
>> +};
>> +
>> +module_platform_driver(pwm_tach_driver);
>> +
>> +MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
>> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
>> +MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
>> +MODULE_LICENSE("GPL v2");
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 14:46 ` Guenter Roeck
@ 2018-02-21 15:20 ` Mikko Perttunen
2018-02-21 16:08 ` Guenter Roeck
2018-03-01 22:08 ` Rob Herring
0 siblings, 2 replies; 31+ messages in thread
From: Mikko Perttunen @ 2018-02-21 15:20 UTC (permalink / raw)
To: Guenter Roeck, Rajkumar Rampelli, robh+dt, mark.rutland,
thierry.reding, jonathanh, jdelvare, corbet, catalin.marinas,
will.deacon, kstewart, gregkh, pombredanne, mmaddireddy,
mperttunen, arnd, gregory.clement, timur, andy.gross, xuwei5,
elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>> ---
>>> 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 0000000..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 <rrajk@nvidia.com>
>>> +
>>> +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 0000000..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 <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/pwm.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +
>>> +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->dev = &pdev->dev;
>>> +
>>> + platform_set_drvdata(pdev, ptt);
>>> + dev_set_drvdata(&pdev->dev, ptt);
>>> +
>>> + ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
>>> + if (IS_ERR(ptt->pwm)) {
>>> + err = PTR_ERR(ptt->pwm);
>>> + dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
>>> + err);
>>> + return err;
>>> + }
>>> +
>>> + ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
>>> + "pwm_tach", ptt, pwm_tach_groups);
>>> + if (IS_ERR(ptt->hwmon)) {
>>> + err = PTR_ERR_OR_ZERO(ptt->hwmon);
>>> + dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
>>> + err);
>>> + return err;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id pwm_tach_of_match[] = {
>>> + { .compatible = "generic-pwm-tachometer" },
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
>>> +
>>> +static struct platform_driver pwm_tach_driver = {
>>> + .driver = {
>>> + .name = "generic-pwm-tachometer",
>>> + .of_match_table = pwm_tach_of_match,
>>> + },
>>> + .probe = pwm_tach_probe,
>>> +};
>>> +
>>> +module_platform_driver(pwm_tach_driver);
>>> +
>>> +MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
>>> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
>>> +MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 15:20 ` Mikko Perttunen
@ 2018-02-21 16:08 ` Guenter Roeck
2018-02-23 11:45 ` RAJKUMAR
2018-03-01 22:08 ` Rob Herring
1 sibling, 1 reply; 31+ messages in thread
From: Guenter Roeck @ 2018-02-21 16:08 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Rajkumar Rampelli, robh+dt, mark.rutland, thierry.reding,
jonathanh, jdelvare, corbet, catalin.marinas, will.deacon,
kstewart, gregkh, pombredanne, mmaddireddy, mperttunen, arnd,
gregory.clement, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel, devicetree, linux-kernel, linux-pwm, linux-tegra,
linux-hwmon, linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
> >>>---
> >>> 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 0000000..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 <rrajk@nvidia.com>
> >>>+
> >>>+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 0000000..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 <linux/module.h>
> >>>+#include <linux/platform_device.h>
> >>>+#include <linux/err.h>
> >>>+#include <linux/pwm.h>
> >>>+#include <linux/hwmon.h>
> >>>+#include <linux/hwmon-sysfs.h>
> >>>+
> >>>+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->dev = &pdev->dev;
> >>>+
> >>>+ platform_set_drvdata(pdev, ptt);
> >>>+ dev_set_drvdata(&pdev->dev, ptt);
> >>>+
> >>>+ ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
> >>>+ if (IS_ERR(ptt->pwm)) {
> >>>+ err = PTR_ERR(ptt->pwm);
> >>>+ dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
> >>>+ err);
> >>>+ return err;
> >>>+ }
> >>>+
> >>>+ ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
> >>>+ "pwm_tach", ptt, pwm_tach_groups);
> >>>+ if (IS_ERR(ptt->hwmon)) {
> >>>+ err = PTR_ERR_OR_ZERO(ptt->hwmon);
> >>>+ dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
> >>>+ err);
> >>>+ return err;
> >>>+ }
> >>>+
> >>>+ return 0;
> >>>+}
> >>>+
> >>>+static const struct of_device_id pwm_tach_of_match[] = {
> >>>+ { .compatible = "generic-pwm-tachometer" },
> >>>+ {}
> >>>+};
> >>>+MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
> >>>+
> >>>+static struct platform_driver pwm_tach_driver = {
> >>>+ .driver = {
> >>>+ .name = "generic-pwm-tachometer",
> >>>+ .of_match_table = pwm_tach_of_match,
> >>>+ },
> >>>+ .probe = pwm_tach_probe,
> >>>+};
> >>>+
> >>>+module_platform_driver(pwm_tach_driver);
> >>>+
> >>>+MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
> >>>+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
> >>>+MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
> >>>+MODULE_LICENSE("GPL v2");
> >>>
> >>
> >
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 16:08 ` Guenter Roeck
@ 2018-02-23 11:45 ` RAJKUMAR
0 siblings, 0 replies; 31+ messages in thread
From: RAJKUMAR @ 2018-02-23 11:45 UTC (permalink / raw)
To: Guenter Roeck, Mikko Perttunen
Cc: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel,
devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>> ---
>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>> +
>>>>> +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 0000000..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 <linux/module.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/pwm.h>
>>>>> +#include <linux/hwmon.h>
>>>>> +#include <linux/hwmon-sysfs.h>
>>>>> +
>>>>> +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->dev = &pdev->dev;
>>>>> +
>>>>> + platform_set_drvdata(pdev, ptt);
>>>>> + dev_set_drvdata(&pdev->dev, ptt);
>>>>> +
>>>>> + ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
>>>>> + if (IS_ERR(ptt->pwm)) {
>>>>> + err = PTR_ERR(ptt->pwm);
>>>>> + dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
>>>>> + err);
>>>>> + return err;
>>>>> + }
>>>>> +
>>>>> + ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
>>>>> + "pwm_tach", ptt, pwm_tach_groups);
>>>>> + if (IS_ERR(ptt->hwmon)) {
>>>>> + err = PTR_ERR_OR_ZERO(ptt->hwmon);
>>>>> + dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
>>>>> + err);
>>>>> + return err;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id pwm_tach_of_match[] = {
>>>>> + { .compatible = "generic-pwm-tachometer" },
>>>>> + {}
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
>>>>> +
>>>>> +static struct platform_driver pwm_tach_driver = {
>>>>> + .driver = {
>>>>> + .name = "generic-pwm-tachometer",
>>>>> + .of_match_table = pwm_tach_of_match,
>>>>> + },
>>>>> + .probe = pwm_tach_probe,
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(pwm_tach_driver);
>>>>> +
>>>>> +MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
>>>>> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
>>>>> +MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 15:20 ` Mikko Perttunen
2018-02-21 16:08 ` Guenter Roeck
@ 2018-03-01 22:08 ` Rob Herring
1 sibling, 0 replies; 31+ messages in thread
From: Rob Herring @ 2018-03-01 22:08 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Guenter Roeck, Rajkumar Rampelli, mark.rutland, thierry.reding,
jonathanh, jdelvare, corbet, catalin.marinas, will.deacon,
kstewart, gregkh, pombredanne, mmaddireddy, mperttunen, arnd,
gregory.clement, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel, devicetree, linux-kernel, linux-pwm, linux-tegra,
linux-hwmon, linux-doc, linux-arm-kernel, ldewangan
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 6:58 ` [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer Rajkumar Rampelli
2018-02-21 7:15 ` Mikko Perttunen
@ 2018-02-21 14:50 ` Guenter Roeck
2018-02-28 5:38 ` Rajkumar Rampelli
1 sibling, 1 reply; 31+ messages in thread
From: Guenter Roeck @ 2018-02-21 14:50 UTC (permalink / raw)
To: Rajkumar Rampelli, robh+dt, mark.rutland, thierry.reding,
jonathanh, jdelvare, corbet, catalin.marinas, will.deacon,
kstewart, gregkh, pombredanne, mmaddireddy, mperttunen, arnd,
gregory.clement, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
> ---
> 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 0000000..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 <rrajk@nvidia.com>
> +
> +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 0000000..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 <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/pwm.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +
> +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 platform_device *pdev)
> +{
> + struct pwm_hwmon_tach *ptt;
> + int err;
> +
> + ptt = devm_kzalloc(&pdev->dev, sizeof(*ptt), GFP_KERNEL);
> + if (!ptt)
> + return -ENOMEM;
> +
> + ptt->dev = &pdev->dev;
> +
> + platform_set_drvdata(pdev, ptt);
> + dev_set_drvdata(&pdev->dev, ptt);
> +
None of those is used.
> + ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
> + if (IS_ERR(ptt->pwm)) {
> + err = PTR_ERR(ptt->pwm);
> + dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
> + err);
> + return err;
> + }
> +
> + ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
> + "pwm_tach", ptt, pwm_tach_groups);
Please use the new API - devm_hwmon_device_register_with_info -
for new drivers.
> + if (IS_ERR(ptt->hwmon)) {
> + err = PTR_ERR_OR_ZERO(ptt->hwmon);
> + dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
> + err);
> + return err;
> + }
Please no noise on failure. The failure is going to be a memory allocation failure,
which is reported already, and user space will be informed with the error code.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pwm_tach_of_match[] = {
> + { .compatible = "generic-pwm-tachometer" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, pwm_tach_of_match);
> +
> +static struct platform_driver pwm_tach_driver = {
> + .driver = {
> + .name = "generic-pwm-tachometer",
> + .of_match_table = pwm_tach_of_match,
> + },
> + .probe = pwm_tach_probe,
> +};
> +
> +module_platform_driver(pwm_tach_driver);
> +
> +MODULE_DESCRIPTION("PWM based Generic Tachometer driver");
> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
> +MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-21 14:50 ` Guenter Roeck
@ 2018-02-28 5:38 ` Rajkumar Rampelli
2018-02-28 5:58 ` Guenter Roeck
0 siblings, 1 reply; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-28 5:38 UTC (permalink / raw)
To: Guenter Roeck, robh+dt, mark.rutland, thierry.reding, jonathanh,
jdelvare, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, timur, andy.gross,
xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>> ---
>> 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 0000000..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 <rrajk@nvidia.com>
>> +
>> +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 0000000..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 <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/err.h>
>> +#include <linux/pwm.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +
>> +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 and update the same
in tachometer hwmon driver. Is it fine ?
enum hwmon_tachometer_attributes {
hwmon_tachometer_rpm,
};
>> +
>> +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->dev = &pdev->dev;
>> +
>> + platform_set_drvdata(pdev, ptt);
>> + dev_set_drvdata(&pdev->dev, ptt);
>> +
>
> None of those is used.
I will take care in next version of patches.
>
>> + ptt->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
>> + if (IS_ERR(ptt->pwm)) {
>> + err = PTR_ERR(ptt->pwm);
>> + dev_err(&pdev->dev, "Failed to get pwm handle, err: %d\n",
>> + err);
>> + return err;
>> + }
>> +
>> + ptt->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
>> + "pwm_tach", ptt, pwm_tach_groups);
>
> Please use the new API - devm_hwmon_device_register_with_info -
> for new drivers.
Sure, will take care.
>
>> + if (IS_ERR(ptt->hwmon)) {
>> + err = PTR_ERR_OR_ZERO(ptt->hwmon);
>> + dev_err(&pdev->dev, "Failed to register hwmon device: %d\n",
>> + err);
>> + return err;
>> + }
>
> Please no noise on failure. The failure is going to be a memory
> allocation failure,
> which is reported already, and user space will be informed with the
> error code.
I will take care.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-28 5:38 ` Rajkumar Rampelli
@ 2018-02-28 5:58 ` Guenter Roeck
2018-02-28 6:07 ` Rajkumar Rampelli
2018-02-28 6:12 ` Rajkumar Rampelli
0 siblings, 2 replies; 31+ messages in thread
From: Guenter Roeck @ 2018-02-28 5:58 UTC (permalink / raw)
To: Rajkumar Rampelli, robh+dt, mark.rutland, thierry.reding,
jonathanh, jdelvare, corbet, catalin.marinas, will.deacon,
kstewart, gregkh, pombredanne, mmaddireddy, mperttunen, arnd,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>> ---
>>> 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 0000000..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 <rrajk@nvidia.com>
>>> +
>>> +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 0000000..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 <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/pwm.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +
>>> +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 and update the same in tachometer hwmon driver. Is it fine ?
> enum hwmon_tachometer_attributes {
Are you kidding me ?
Guenter
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-28 5:58 ` Guenter Roeck
@ 2018-02-28 6:07 ` Rajkumar Rampelli
2018-02-28 6:12 ` Rajkumar Rampelli
1 sibling, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-28 6:07 UTC (permalink / raw)
To: Guenter Roeck, robh+dt, mark.rutland, thierry.reding, jonathanh,
jdelvare, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, timur, andy.gross,
xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
[-- Attachment #1: Type: text/plain, Size: 6382 bytes --]
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 <rrajk@nvidia.com>
>>>> ---
>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>> +
>>>> +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 0000000..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 <linux/module.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/pwm.h>
>>>> +#include <linux/hwmon.h>
>>>> +#include <linux/hwmon-sysfs.h>
>>>> +
>>>> +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 and update the
>> same in tachometer hwmon driver. Is it fine ?
>> enum hwmon_tachometer_attributes {
>
> Are you kidding me ?
>
> Guenter
Sorry, I just wanted to confirm that whether my understanding is correct
or not before implementing it actually.
Or shall I add this attribute as a part of fan attributes with
"hwmon_fan_rpm"? or anyother way to do it ? I need your inputs in fixing it.
[-- Attachment #2: Type: text/html, Size: 10908 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-28 5:58 ` Guenter Roeck
2018-02-28 6:07 ` Rajkumar Rampelli
@ 2018-02-28 6:12 ` Rajkumar Rampelli
2018-02-28 7:03 ` Mikko Perttunen
1 sibling, 1 reply; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-28 6:12 UTC (permalink / raw)
To: Guenter Roeck, robh+dt, mark.rutland, thierry.reding, jonathanh,
jdelvare, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, timur, andy.gross,
xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>> ---
>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>> +
>>>> +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 0000000..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 <linux/module.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/pwm.h>
>>>> +#include <linux/hwmon.h>
>>>> +#include <linux/hwmon-sysfs.h>
>>>> +
>>>> +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 and update the
>> same in tachometer hwmon driver. Is it fine ?
>> enum hwmon_tachometer_attributes {
>
> Are you kidding me ?
>
> Guenter
Sorry, I just wanted to confirm whether my understanding is correct or
not before implementing it actually.
Or, shall I add this attribute as a part of fan attributes with
"hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in
fixing this.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-28 6:12 ` Rajkumar Rampelli
@ 2018-02-28 7:03 ` Mikko Perttunen
2018-02-28 14:29 ` Guenter Roeck
0 siblings, 1 reply; 31+ messages in thread
From: Mikko Perttunen @ 2018-02-28 7:03 UTC (permalink / raw)
To: Rajkumar Rampelli, Guenter Roeck, robh+dt, mark.rutland,
thierry.reding, jonathanh, jdelvare, corbet, catalin.marinas,
will.deacon, kstewart, gregkh, pombredanne, mmaddireddy,
mperttunen, arnd, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>> ---
>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>> +
>>>>> +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 0000000..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 <linux/module.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/pwm.h>
>>>>> +#include <linux/hwmon.h>
>>>>> +#include <linux/hwmon-sysfs.h>
>>>>> +
>>>>> +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 and update the
>>> same in tachometer hwmon driver. Is it fine ?
>>> enum hwmon_tachometer_attributes {
>>
>> Are you kidding me ?
>>
>> Guenter
> Sorry, I just wanted to confirm whether my understanding is correct or
> not before implementing it actually.
> Or, shall I add this attribute as a part of fan attributes with
> "hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in
> fixing this.
I think he wants the attribute to be named according to the properties
in this document:
https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface. I guess
the attribute would then map to fan1_input, though I'm not sure if
that's 100% correct either since this could technically be attached to
something other than a fan. But I would think in practice that's not a
big concern.
Guenter,
Please correct me as well if I'm wrong.
Thank you,
Mikko
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-28 7:03 ` Mikko Perttunen
@ 2018-02-28 14:29 ` Guenter Roeck
2018-03-07 9:47 ` Rajkumar Rampelli
0 siblings, 1 reply; 31+ messages in thread
From: Guenter Roeck @ 2018-02-28 14:29 UTC (permalink / raw)
To: Mikko Perttunen, Rajkumar Rampelli, robh+dt, mark.rutland,
thierry.reding, jonathanh, jdelvare, corbet, catalin.marinas,
will.deacon, kstewart, gregkh, pombredanne, mmaddireddy,
mperttunen, arnd, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>>> ---
>>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>>> +
>>>>>> +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 0000000..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 <linux/module.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +#include <linux/err.h>
>>>>>> +#include <linux/pwm.h>
>>>>>> +#include <linux/hwmon.h>
>>>>>> +#include <linux/hwmon-sysfs.h>
>>>>>> +
>>>>>> +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 and update the same in tachometer hwmon driver. Is it fine ?
>>>> enum hwmon_tachometer_attributes {
>>>
>>> Are you kidding me ?
>>>
>>> Guenter
>> Sorry, I just wanted to confirm whether my understanding is correct or not before implementing it actually.
>> Or, shall I add this attribute as a part of fan attributes with "hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in fixing this.
>
> I think he wants the attribute to be named according to the properties in this document: https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface. I guess the attribute would then map to fan1_input, though I'm not sure if that's 100% correct either since this could technically be attached to something other than a fan. But I would think in practice that's not a big concern.
>
> Guenter,
> Please correct me as well if I'm wrong.
>
You are absolutely correct.
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
> Thank you,
> Mikko
>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-02-28 14:29 ` Guenter Roeck
@ 2018-03-07 9:47 ` Rajkumar Rampelli
2018-03-07 14:20 ` Guenter Roeck
0 siblings, 1 reply; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-03-07 9:47 UTC (permalink / raw)
To: Guenter Roeck, Mikko Perttunen, robh+dt, mark.rutland,
thierry.reding, jonathanh, jdelvare, corbet, catalin.marinas,
will.deacon, kstewart, gregkh, pombredanne, mmaddireddy,
mperttunen, arnd, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>>>> ---
>>>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>>>> +
>>>>>>> +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 0000000..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 <linux/module.h>
>>>>>>> +#include <linux/platform_device.h>
>>>>>>> +#include <linux/err.h>
>>>>>>> +#include <linux/pwm.h>
>>>>>>> +#include <linux/hwmon.h>
>>>>>>> +#include <linux/hwmon-sysfs.h>
>>>>>>> +
>>>>>>> +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 and update the
>>>>> same in tachometer hwmon driver. Is it fine ?
>>>>> enum hwmon_tachometer_attributes {
>>>>
>>>> Are you kidding me ?
>>>>
>>>> Guenter
>>> Sorry, I just wanted to confirm whether my understanding is correct
>>> or not before implementing it actually.
>>> Or, shall I add this attribute as a part of fan attributes with
>>> "hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in
>>> fixing this.
>>
>> I think he wants the attribute to be named according to the
>> properties in this document:
>> https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface. I
>> guess the attribute would then map to fan1_input, though I'm not sure
>> if that's 100% correct either since this could technically be
>> attached to something other than a fan. But I would think in practice
>> that's not a big concern.
>>
>> Guenter,
>> Please correct me as well if I'm wrong.
>>
> You are absolutely correct.
>
> 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.
>
>> Thank you,
>> Mikko
>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-tegra" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-hwmon" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-03-07 9:47 ` Rajkumar Rampelli
@ 2018-03-07 14:20 ` Guenter Roeck
2018-03-08 6:06 ` Laxman Dewangan
2018-03-08 7:57 ` Mikko Perttunen
0 siblings, 2 replies; 31+ messages in thread
From: Guenter Roeck @ 2018-03-07 14:20 UTC (permalink / raw)
To: Rajkumar Rampelli, Mikko Perttunen, robh+dt, mark.rutland,
thierry.reding, jonathanh, jdelvare, corbet, catalin.marinas,
will.deacon, kstewart, gregkh, pombredanne, mmaddireddy,
mperttunen, arnd, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>>>>> ---
>>>>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>>>>> +
>>>>>>>> +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 0000000..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 <linux/module.h>
>>>>>>>> +#include <linux/platform_device.h>
>>>>>>>> +#include <linux/err.h>
>>>>>>>> +#include <linux/pwm.h>
>>>>>>>> +#include <linux/hwmon.h>
>>>>>>>> +#include <linux/hwmon-sysfs.h>
>>>>>>>> +
>>>>>>>> +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 and update the same in tachometer hwmon driver. Is it fine ?
>>>>>> enum hwmon_tachometer_attributes {
>>>>>
>>>>> Are you kidding me ?
>>>>>
>>>>> Guenter
>>>> Sorry, I just wanted to confirm whether my understanding is correct or not before implementing it actually.
>>>> Or, shall I add this attribute as a part of fan attributes with "hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in fixing this.
>>>
>>> I think he wants the attribute to be named according to the properties in this document: https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface. I guess the attribute would then map to fan1_input, though I'm not sure if that's 100% correct either since this could technically be attached to something other than a fan. But I would think in practice that's not a big concern.
>>>
>>> Guenter,
>>> Please correct me as well if I'm wrong.
>>>
>> You are absolutely correct.
>>
>> 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.
Guenter
>>
>>> Thank you,
>>> Mikko
>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-03-07 14:20 ` Guenter Roeck
@ 2018-03-08 6:06 ` Laxman Dewangan
2018-03-08 14:31 ` Guenter Roeck
2018-03-08 7:57 ` Mikko Perttunen
1 sibling, 1 reply; 31+ messages in thread
From: Laxman Dewangan @ 2018-03-08 6:06 UTC (permalink / raw)
To: Guenter Roeck, Rajkumar Rampelli, Mikko Perttunen, robh+dt,
mark.rutland, thierry.reding, jonathanh, jdelvare, corbet,
catalin.marinas, will.deacon, kstewart, gregkh, pombredanne,
mmaddireddy, mperttunen, arnd, timur, andy.gross, xuwei5, elder,
heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel
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?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-03-08 6:06 ` Laxman Dewangan
@ 2018-03-08 14:31 ` Guenter Roeck
2018-03-08 15:21 ` Laxman Dewangan
0 siblings, 1 reply; 31+ messages in thread
From: Guenter Roeck @ 2018-03-08 14:31 UTC (permalink / raw)
To: Laxman Dewangan, Rajkumar Rampelli, Mikko Perttunen, robh+dt,
mark.rutland, thierry.reding, jonathanh, jdelvare, corbet,
catalin.marinas, will.deacon, kstewart, gregkh, pombredanne,
mmaddireddy, mperttunen, arnd, timur, andy.gross, xuwei5, elder,
heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-03-08 14:31 ` Guenter Roeck
@ 2018-03-08 15:21 ` Laxman Dewangan
0 siblings, 0 replies; 31+ messages in thread
From: Laxman Dewangan @ 2018-03-08 15:21 UTC (permalink / raw)
To: Guenter Roeck, Rajkumar Rampelli, Mikko Perttunen, robh+dt,
mark.rutland, thierry.reding, jonathanh, jdelvare, corbet,
catalin.marinas, will.deacon, kstewart, gregkh, pombredanne,
mmaddireddy, mperttunen, arnd, timur, andy.gross, xuwei5, elder,
heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-03-07 14:20 ` Guenter Roeck
2018-03-08 6:06 ` Laxman Dewangan
@ 2018-03-08 7:57 ` Mikko Perttunen
2018-03-08 14:33 ` Guenter Roeck
1 sibling, 1 reply; 31+ messages in thread
From: Mikko Perttunen @ 2018-03-08 7:57 UTC (permalink / raw)
To: Guenter Roeck, Rajkumar Rampelli, Mikko Perttunen, robh+dt,
mark.rutland, thierry.reding, jonathanh, jdelvare, corbet,
catalin.marinas, will.deacon, kstewart, gregkh, pombredanne,
mmaddireddy, arnd, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>>>>>> ---
>>>>>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>>>>>> +
>>>>>>>>> +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 0000000..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 <linux/module.h>
>>>>>>>>> +#include <linux/platform_device.h>
>>>>>>>>> +#include <linux/err.h>
>>>>>>>>> +#include <linux/pwm.h>
>>>>>>>>> +#include <linux/hwmon.h>
>>>>>>>>> +#include <linux/hwmon-sysfs.h>
>>>>>>>>> +
>>>>>>>>> +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 and update
>>>>>>> the same in tachometer hwmon driver. Is it fine ?
>>>>>>> enum hwmon_tachometer_attributes {
>>>>>>
>>>>>> Are you kidding me ?
>>>>>>
>>>>>> Guenter
>>>>> Sorry, I just wanted to confirm whether my understanding is correct
>>>>> or not before implementing it actually.
>>>>> Or, shall I add this attribute as a part of fan attributes with
>>>>> "hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in
>>>>> fixing this.
>>>>
>>>> I think he wants the attribute to be named according to the
>>>> properties in this document:
>>>> https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface. I
>>>> guess the attribute would then map to fan1_input, though I'm not
>>>> sure if that's 100% correct either since this could technically be
>>>> attached to something other than a fan. But I would think in
>>>> practice that's not a big concern.
>>>>
>>>> Guenter,
>>>> Please correct me as well if I'm wrong.
>>>>
>>> You are absolutely correct.
>>>
>>> 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.
>
> Guenter
>
I think the idea here (based on personal discussion with Rajkumar) was
to add a "new" sysfs attribute the driver exports, but that attribute
would be the existing fan[1..n]_input.
Mikko
>>>
>>>> Thank you,
>>>> Mikko
>>>>
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe
>>>>> linux-tegra" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe
>>>> linux-hwmon" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
2018-03-08 7:57 ` Mikko Perttunen
@ 2018-03-08 14:33 ` Guenter Roeck
0 siblings, 0 replies; 31+ messages in thread
From: Guenter Roeck @ 2018-03-08 14:33 UTC (permalink / raw)
To: Mikko Perttunen, Rajkumar Rampelli, Mikko Perttunen, robh+dt,
mark.rutland, thierry.reding, jonathanh, jdelvare, corbet,
catalin.marinas, will.deacon, kstewart, gregkh, pombredanne,
mmaddireddy, arnd, timur, andy.gross, xuwei5, elder, heiko, krzk,
ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan
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 <rrajk@nvidia.com>
>>>>>>>>>> ---
>>>>>>>>>> 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 0000000..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 <rrajk@nvidia.com>
>>>>>>>>>> +
>>>>>>>>>> +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 0000000..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 <linux/module.h>
>>>>>>>>>> +#include <linux/platform_device.h>
>>>>>>>>>> +#include <linux/err.h>
>>>>>>>>>> +#include <linux/pwm.h>
>>>>>>>>>> +#include <linux/hwmon.h>
>>>>>>>>>> +#include <linux/hwmon-sysfs.h>
>>>>>>>>>> +
>>>>>>>>>> +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 and update
>>>>>>>> the same in tachometer hwmon driver. Is it fine ?
>>>>>>>> enum hwmon_tachometer_attributes {
>>>>>>>
>>>>>>> Are you kidding me ?
>>>>>>>
>>>>>>> Guenter
>>>>>> Sorry, I just wanted to confirm whether my understanding is correct
>>>>>> or not before implementing it actually.
>>>>>> Or, shall I add this attribute as a part of fan attributes with
>>>>>> "hwmon_fan_rpm" ? or any other way to do it ? I need your inputs in
>>>>>> fixing this.
>>>>>
>>>>> I think he wants the attribute to be named according to the
>>>>> properties in this document:
>>>>> https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface. I
>>>>> guess the attribute would then map to fan1_input, though I'm not
>>>>> sure if that's 100% correct either since this could technically be
>>>>> attached to something other than a fan. But I would think in
>>>>> practice that's not a big concern.
>>>>>
>>>>> Guenter,
>>>>> Please correct me as well if I'm wrong.
>>>>>
>>>> You are absolutely correct.
>>>>
>>>> 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.
>>
>> Guenter
>>
>
> I think the idea here (based on personal discussion with Rajkumar) was to add a "new" sysfs attribute the driver exports, but that attribute would be the existing fan[1..n]_input.
>
Ah, sorry, in this case misunderstanding on my side.
Guenter
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/10] arm64: tegra: Add Tachometer Controller on Tegra186
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (4 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 07/10] arm64: tegra: Add PWM based Tachometer support " Rajkumar Rampelli
` (3 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
The NVIDIA Tegra186 SoC has a Tachometer Controller that analyzes the
PWM signal of a Fan and reports the period value through pwm interface.
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts | 5 +++++
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 +++++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts b/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
index bd5305a..13c3e59 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
@@ -172,4 +172,9 @@
vin-supply = <&vdd_5v0_sys>;
};
};
+
+ tachometer@39c0000 {
+ nvidia,pulse-per-rev = <2>;
+ nvidia,capture-window-len = <2>;
+ };
};
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index b762227..8f2d598 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1031,4 +1031,15 @@
(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
interrupt-parent = <&gic>;
};
+
+ tegra_tachometer: tachometer@39c0000 {
+ compatible = "nvidia,tegra186-pwm-tachometer";
+ reg = <0x0 0x039c0000 0x0 0x10>;
+ #pwm-cells = <2>;
+ clocks = <&bpmp TEGRA186_CLK_TACH>;
+ clock-names = "tachometer";
+ resets = <&bpmp TEGRA186_RESET_TACH>;
+ reset-names = "tachometer";
+ status = "disabled";
+ };
};
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 07/10] arm64: tegra: Add PWM based Tachometer support on Tegra186
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (5 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 06/10] arm64: tegra: Add Tachometer Controller on Tegra186 Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 08/10] arm64: defconfig: enable Nvidia Tegra Tachometer as a module Rajkumar Rampelli
` (2 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
Add PWM based Tachometer support on Tegra186 to measure
number of rotations of a Fan per minute by using PWM
capture interface
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 8f2d598..37149e9 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1042,4 +1042,10 @@
reset-names = "tachometer";
status = "disabled";
};
+
+ generic_pwm_tachometer {
+ compatible = "generic-pwm-tachometer";
+ pwms = <&tegra_tachometer 0 1000000>;
+ status = "disabled";
+ };
};
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 08/10] arm64: defconfig: enable Nvidia Tegra Tachometer as a module
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (6 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 07/10] arm64: tegra: Add PWM based Tachometer support " Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 09/10] arm64: defconfig: Enable Generic PWM based Tachometer driver Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 10/10] arm64: tegra: Add PWM controller on Tegra186 soc Rajkumar Rampelli
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
Tegra Tachometer driver implements PWM capture to measure
period. Enable this driver as a module in the ARM64 defconfig.
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 634b373..8b2bda7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -550,6 +550,7 @@ CONFIG_PWM_MESON=m
CONFIG_PWM_ROCKCHIP=y
CONFIG_PWM_SAMSUNG=y
CONFIG_PWM_TEGRA=m
+CONFIG_PWM_TEGRA_TACHOMETER=m
CONFIG_PHY_RCAR_GEN3_USB2=y
CONFIG_PHY_HI6220_USB=y
CONFIG_PHY_QCOM_USB_HS=y
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 09/10] arm64: defconfig: Enable Generic PWM based Tachometer driver
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (7 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 08/10] arm64: defconfig: enable Nvidia Tegra Tachometer as a module Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
2018-02-21 6:58 ` [PATCH 10/10] arm64: tegra: Add PWM controller on Tegra186 soc Rajkumar Rampelli
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
Enable Generic PWM based Tachometer driver which implements a simple
interface for monitoring the speed of a fan in roatations per minute,
and exposes it to the user space by using the hwmon's sysfs interface.
Enable this driver as a module in the ARM64 defconfig.
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8b2bda7..1b29109 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -321,6 +321,7 @@ CONFIG_BATTERY_BQ27XXX=y
CONFIG_SENSORS_ARM_SCPI=y
CONFIG_SENSORS_LM90=m
CONFIG_SENSORS_INA2XX=m
+CONFIG_GENERIC_PWM_TACHOMETER=m
CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
CONFIG_CPU_THERMAL=y
CONFIG_THERMAL_EMULATION=y
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 10/10] arm64: tegra: Add PWM controller on Tegra186 soc
2018-02-21 6:58 [PATCH 00/10] Implementation of Tegra Tachometer driver Rajkumar Rampelli
` (8 preceding siblings ...)
2018-02-21 6:58 ` [PATCH 09/10] arm64: defconfig: Enable Generic PWM based Tachometer driver Rajkumar Rampelli
@ 2018-02-21 6:58 ` Rajkumar Rampelli
9 siblings, 0 replies; 31+ messages in thread
From: Rajkumar Rampelli @ 2018-02-21 6:58 UTC (permalink / raw)
To: robh+dt, mark.rutland, thierry.reding, jonathanh, jdelvare,
linux, corbet, catalin.marinas, will.deacon, kstewart, gregkh,
pombredanne, mmaddireddy, mperttunen, arnd, gregory.clement,
timur, andy.gross, xuwei5, elder, heiko, krzk, ard.biesheuvel
Cc: devicetree, linux-kernel, linux-pwm, linux-tegra, linux-hwmon,
linux-doc, linux-arm-kernel, ldewangan, rrajk
The NVIDIA Tegra186 SoC has a PWM controller which is
used in FAN control use case.
Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 37149e9..c6f154e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1032,6 +1032,17 @@
interrupt-parent = <&gic>;
};
+ pwm@c340000 {
+ compatible = "nvidia,tegra186-pwm";
+ reg = <0x0 0xc340000 0x0 0x10000>;
+ clocks = <&bpmp TEGRA186_CLK_PWM4>;
+ clock-names = "pwm";
+ #pwm-cells = <2>;
+ resets = <&bpmp TEGRA186_RESET_PWM4>;
+ reset-names = "pwm";
+ status = "disabled";
+ };
+
tegra_tachometer: tachometer@39c0000 {
compatible = "nvidia,tegra186-pwm-tachometer";
reg = <0x0 0x039c0000 0x0 0x10>;
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread