Received: by 10.223.185.116 with SMTP id b49csp815833wrg; Wed, 21 Feb 2018 07:22:09 -0800 (PST) X-Google-Smtp-Source: AH8x225d5qtzq/8T2sqK+YqYmsxvCGZeAAhQ4sAYVWpImFy5F2UJ/W8y+L9cN5ncpzRyFhx8WwQu X-Received: by 10.98.16.13 with SMTP id y13mr1919153pfi.188.1519226529174; Wed, 21 Feb 2018 07:22:09 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519226529; cv=none; d=google.com; s=arc-20160816; b=nFhDICjBaodwxqkSps/Pg4Jm/lFVbGJ6iyQNmt9oHL9kdK3aXh1VKuLzsbu9xFFJFz 53YBEH4Y6j7OFMJBlEoDbwcBjo8lXL/8aavzaWcO2W5Bx4HUifI0H31yz6ibDKyO6CHi ukGOa9P+LyI39wmqA0WhXDIUEz1ay/W4qYiWfsmR63z5ip4LBC3KLxEUK3tYS+G3+V+g 4G7HLQWaBkcAbF90DDBgv6TyuiT49Z27nk7dlWP92tjUOaoxATFFfpRsDzIgCnMh8zGn 3axhcyrFpE2NiSw5Swd3vp7BV41bc+8ytvV7ymOjYRdoWhiONvhg8c18bbKA0zLSA94C wazQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:cc:references:to :subject:dkim-signature:arc-authentication-results; bh=m0RTZVuNuYC6ujWlh0v3d9gADzpm/fzBr2hOCa6VzV8=; b=WF24WJwU4KRdC2LIDybofmIF+d46Kd9omXLFyIlbsdQ2k60mFSAOzOaDjYi9vFsNbN 6dM6E+8EIpVOvvhhO2fqsclfk+PxCNLFgON/HFZP9ha2rves5wFO+FaBxlA/rJa+kWTI oFq6OcGrK/D5FROaHhfZ5CU+lPQ6PqpbP3Guuk2M43UAODeM31ju+cS6XSIQnQaDq6Fm Q/d2P+NsN/xYBobpK8SA8pfKqA+O6rnHbnIVH93P6Jya4ddgPYZ6wlMfmZRl/19t47R6 kMM0VHVpUdPiW+SrEF1BMLVaj8N20/0vW0wl0EMBALn5zz65tCYV6lRqlFGe4fbDwQWu Y/2Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kapsi.fi header.s=20161220 header.b=vjRTniY9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n6-v6si1419350pla.619.2018.02.21.07.21.54; Wed, 21 Feb 2018 07:22:09 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kapsi.fi header.s=20161220 header.b=vjRTniY9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938233AbeBUPUn (ORCPT + 99 others); Wed, 21 Feb 2018 10:20:43 -0500 Received: from mail.kapsi.fi ([91.232.154.25]:52363 "EHLO mail.kapsi.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S938217AbeBUPUl (ORCPT ); Wed, 21 Feb 2018 10:20:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=kapsi.fi; s=20161220; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:Cc:References:To:Subject; bh=m0RTZVuNuYC6ujWlh0v3d9gADzpm/fzBr2hOCa6VzV8=; b=vjRTniY9w+Pe2Kh4xgrNwwACcpuHYSIyf0ZXM7+Lu9pzJK5MNM8cMWrYlKNBJZYREqN2NwFFvPDbu7QxREmeo+Sh18rKVQBNAs3HZptZTE1JlVMnpg3FyVUer+Xkiv4GK7Mc/9hCmlZbiyVoglctKzjqyCp4mXliP0OYaWRsAc1XtTxQRjO3r5eCr5ZxN17dDEty3ofYpYbkTwscFy1uFKy1oON58q2fjMRCoc5EYDrO7kqTJJHbYOC8XjrEQ8b68hu5e4rV1irfN7km/snZBq9UbOL3ASBT+IMHAeRpWr112zubc+tKVcZD2LCl5rHPXgui4onlo0vJgl9G+5jjsQ==; Received: from [62.209.167.43] (helo=[10.21.26.144]) by mail.kapsi.fi with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1eoWC6-00077o-KC; Wed, 21 Feb 2018 17:20:30 +0200 Subject: Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer To: Guenter Roeck , Rajkumar Rampelli , robh+dt@kernel.org, mark.rutland@arm.com, thierry.reding@gmail.com, jonathanh@nvidia.com, jdelvare@suse.com, corbet@lwn.net, catalin.marinas@arm.com, will.deacon@arm.com, kstewart@linuxfoundation.org, gregkh@linuxfoundation.org, pombredanne@nexb.com, mmaddireddy@nvidia.com, mperttunen@nvidia.com, arnd@arndb.de, gregory.clement@free-electrons.com, timur@codeaurora.org, andy.gross@linaro.org, xuwei5@hisilicon.com, elder@linaro.org, heiko@sntech.de, krzk@kernel.org, ard.biesheuvel@linaro.org References: <1519196339-9377-1-git-send-email-rrajk@nvidia.com> <1519196339-9377-6-git-send-email-rrajk@nvidia.com> Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org, linux-tegra@vger.kernel.org, linux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, ldewangan@nvidia.com From: Mikko Perttunen Message-ID: Date: Wed, 21 Feb 2018 17:20:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 62.209.167.43 X-SA-Exim-Mail-From: cyndis@kapsi.fi X-SA-Exim-Scanned: No (on mail.kapsi.fi); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21.02.2018 16:46, Guenter Roeck wrote: > On 02/20/2018 11:15 PM, Mikko Perttunen wrote: >> AIUI, the PWM framework already exposes a sysfs node with period >> information. We should just use that instead of adding a new driver >> for this. >> > > I am kind of lost. Please explain. > > Are you saying that we should drop the pwm-fan driver as well (which goes > the opposite way), as well as any other drivers doing anything with pwm > signals, > because after all those signals are already exposed to userspace a sysfs > attributes, > and a kernel driver to abstract those values is thus not needed ? The only thing this driver does is do a constant division in kernelspace. I'm not really seeing why that couldn't be done in userspace. But if you think it's appropriate to do the RPM conversion in kernelspace then I'm not greatly opposed to that. > >> In any case, we cannot add something like this to device tree since >> it's not a hardware device. >> > > So you are saying there is no means to express in devicetree that > a pwm input is connected to a fan ? How is that not hardware ? > > If so, how do you express in devicetree that a pwm signal is connected > to anything ? If we want to describe that the tachometer is connected to a fan, then we should have a fan node in the board's device tree. We don't have a chip that has a thing called "generic-pwm-tachometer" attached to it. (We have chips that have a "nvidia,tegra186-tachometer", so it's proper to have that.) Thanks, Mikko > > Guenter > >> Mikko >> >> On 21.02.2018 08:58, Rajkumar Rampelli wrote: >>> Add generic PWM based tachometer driver via HWMON interface >>> to report the RPM of motor. This drivers get the period/duty >>> cycle from PWM IP which captures the motor PWM output. >>> >>> This driver implements a simple interface for monitoring the speed of >>> a fan and exposes it in roatations per minute (RPM) to the user space >>> by using the hwmon's sysfs interface >>> >>> Signed-off-by: Rajkumar Rampelli >>> --- >>> Documentation/hwmon/generic-pwm-tachometer | 17 +++++ >>> drivers/hwmon/Kconfig | 10 +++ >>> drivers/hwmon/Makefile | 1 + >>> drivers/hwmon/generic-pwm-tachometer.c | 112 >>> +++++++++++++++++++++++++++++ >>> 4 files changed, 140 insertions(+) >>> create mode 100644 Documentation/hwmon/generic-pwm-tachometer >>> create mode 100644 drivers/hwmon/generic-pwm-tachometer.c >>> >>> diff --git a/Documentation/hwmon/generic-pwm-tachometer >>> b/Documentation/hwmon/generic-pwm-tachometer >>> new file mode 100644 >>> index 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 >>> + >>> +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 >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +struct pwm_hwmon_tach { >>> + struct device *dev; >>> + struct pwm_device *pwm; >>> + struct device *hwmon; >>> +}; >>> + >>> +static ssize_t show_rpm(struct device *dev, struct device_attribute >>> *attr, >>> + char *buf) >>> +{ >>> + struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev); >>> + struct pwm_device *pwm = ptt->pwm; >>> + struct pwm_capture result; >>> + int err; >>> + unsigned int rpm = 0; >>> + >>> + err = pwm_capture(pwm, &result, 0); >>> + if (err < 0) { >>> + dev_err(ptt->dev, "Failed to capture PWM: %d\n", err); >>> + return err; >>> + } >>> + >>> + if (result.period) >>> + rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC, >>> + result.period); >>> + >>> + return sprintf(buf, "%u\n", rpm); >>> +} >>> + >>> +static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0); >>> + >>> +static struct attribute *pwm_tach_attrs[] = { >>> + &sensor_dev_attr_rpm.dev_attr.attr, >>> + NULL, >>> +}; >>> + >>> +ATTRIBUTE_GROUPS(pwm_tach); >>> + >>> +static int pwm_tach_probe(struct platform_device *pdev) >>> +{ >>> + struct pwm_hwmon_tach *ptt; >>> + int err; >>> + >>> + ptt = devm_kzalloc(&pdev->dev, sizeof(*ptt), GFP_KERNEL); >>> + if (!ptt) >>> + return -ENOMEM; >>> + >>> + ptt->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 "); >>> +MODULE_AUTHOR("Rajkumar Rampelli "); >>> +MODULE_LICENSE("GPL v2"); >>> >> >