From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13605C48BDF for ; Thu, 24 Jun 2021 12:44:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DF7CA61220 for ; Thu, 24 Jun 2021 12:44:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231432AbhFXMqu (ORCPT ); Thu, 24 Jun 2021 08:46:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48790 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231405AbhFXMqt (ORCPT ); Thu, 24 Jun 2021 08:46:49 -0400 Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 94C7CC061574; Thu, 24 Jun 2021 05:44:30 -0700 (PDT) Received: by mail-ot1-x332.google.com with SMTP id v5-20020a0568301bc5b029045c06b14f83so5374889ota.13; Thu, 24 Jun 2021 05:44:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Q3gvZmypEjLXLqsFmPmNg0fnsAU683BrMuDR/uus3S8=; b=TUytgdOpb5Dh7U9LBK+OVwUmgXNPe/LpMGCGsLMVUVefURAC/FGHmidrYT3x56hMlv kUOiDdl+iw+9OiLKEue02u+DwW//WcvApLPcvEzBhZ+N3oLYkX7gMEhRV/yav4NvgsiN K0EFn3fWEKCQNBwlCYzzat8iK7cBbEmS0rVBmSoGoxTonFxC3kLa+PkbWsC5ITeCqWVT MjDmZrxd3X0tvMk27+Chkb/jQ9HrHLP0qK1elHqVuOPQvN5JvKvXuZ3BNh1bcF4LYLxe ZoC1u3wgfRs6SEmzFrG0ZEn07yYcQvcpMbGIuA7oK+x20vdgCwgWurHLuMWc9GlNgzPJ HKfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=Q3gvZmypEjLXLqsFmPmNg0fnsAU683BrMuDR/uus3S8=; b=IBSGO6ManwFXQOQHe53ldZzEOzfhdyRaL48mvOpNOUyqTQNPTIIsuW9Qr33J8obkfa bKg2Hs29QpFK8xhpbQ7z1cSStXRQeItM/u1tFJbJtkfhBHPJUIvBn92YjRoXtk/rokED 4kf2vxu2lAcZw/DrIacaLWnhu5qhQYUXlJiHZbQiF7oz02hF+VuSfFn5n70VrPJEOs6D SOi2QvKGPEY0Z7QncsfG3SRzZ6Gcpmh+gGp5MWtbp+WGxbuyNGcLGX/LFlniGxBKpmtC xd3SGwX2MfWLq3KfLrm+gxtSTx4hVtaAVm4O8E1Qm75kg2yvkUOrgSZIHGEpenPVqoV3 eQwQ== X-Gm-Message-State: AOAM531aU4pW8qOs+94hAwY2Q7q4VYMvBxXMa4xu3BYnz317eZnZ03bw E9Af2OGxl7i7q+XP45xrM1aVCsaDEYo= X-Google-Smtp-Source: ABdhPJyGU8PRH2pP3pbdqSGcc0Y7/MlzY7Ggy4CwdK//7tZYzApvAnUs/5/Br9kykr9JEZOqHdR5hA== X-Received: by 2002:a05:6830:154b:: with SMTP id l11mr4598072otp.66.1624538669960; Thu, 24 Jun 2021 05:44:29 -0700 (PDT) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id h27sm642857ote.79.2021.06.24.05.44.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Jun 2021 05:44:29 -0700 (PDT) Sender: Guenter Roeck Date: Thu, 24 Jun 2021 05:44:28 -0700 From: Guenter Roeck To: Billy Tsai Cc: jdelvare@suse.com, joel@jms.id.au, andrew@aj.id.au, linux-hwmon@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, BMC-SW@aspeedtech.com Subject: Re: [PATCH] hwmon: (aspeed-pwm-tacho) Using falling edge. Message-ID: <20210624124428.GB1670703@roeck-us.net> References: <20210624035821.25375-1-billy_tsai@aspeedtech.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210624035821.25375-1-billy_tsai@aspeedtech.com> Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org On Thu, Jun 24, 2021 at 11:58:21AM +0800, Billy Tsai wrote: > The tach shouldn't use both edges to measure. When the tach input > duty cycle isn't 50% the return value will inaccurate. > A tachometer doesn't have a duty cycle. A pwm has a duty cycle, but that is completely independent of the pwm duty cycle used to set the fan speed. So this patch does not really make sense with the above explanation. The impact of this patch is likely that the reported fan speed is reduced by 50%. It may well be that the driver currently reports twice the real fan speed. I have no idea if that is the case, but if it is it should not be conditional. The description above states "when the tach input cycle isn't 50%", suggesting that this is conditional on some other configuration. I don't know what that might be either. So, sorry, I can't accept this patch without a more detailed and accurate description and explanation why it is needed. > Signed-off-by: Billy Tsai > --- > drivers/hwmon/aspeed-pwm-tacho.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c > index 3d8239fd66ed..0a70a0e22acf 100644 > --- a/drivers/hwmon/aspeed-pwm-tacho.c > +++ b/drivers/hwmon/aspeed-pwm-tacho.c > @@ -158,7 +158,7 @@ > * 10: both > * 11: reserved. > */ > -#define M_TACH_MODE 0x02 /* 10b */ > +#define M_TACH_MODE 0x00 /* 10b */ That comment is now wrong. Guenter > #define M_TACH_UNIT 0x0210 > #define INIT_FAN_CTRL 0xFF > > -- > 2.25.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 79CD9C48BDF for ; Thu, 24 Jun 2021 12:46:05 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 43E48613EC for ; Thu, 24 Jun 2021 12:46:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 43E48613EC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ce0yxMQ1mimYUNTT9w6zxCGfKZTovx5CZbNj9eN+aqc=; b=VLxM6bGjmHbXVq zVG/lifUzGH4iYP9uDMtLtDNd6Ommxar/5KbvAG0uP2iqSNvCGUYAANbncr8FkSK+zf+6puCf5q5b R6ssoJhtX/I/cSCTR8f0HfsxLD1eNZL8ptHEEXi//Wtc07ItWvYyTdv6CkZucfFFaW+y1RiDzpMCK 4vLd3VkK7MXZDzMYDgmEDPTv8AMYtRDzhUpNhcXOpAoCqYTdFvLRqZaB5F6QjsT6+dbPLa2rhVP/S O+OaAbfbW5QQPAgpNf+panNrsqTK4OH91yvCXgd4cnpl9jQq8C/H3toAsseA3sbgBpM3RxihhlQiO 8GiDLPpUozphEBqBCe0g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lwOih-00Ef7v-Bq; Thu, 24 Jun 2021 12:44:35 +0000 Received: from mail-ot1-x32d.google.com ([2607:f8b0:4864:20::32d]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lwOid-00Ef6p-It for linux-arm-kernel@lists.infradead.org; Thu, 24 Jun 2021 12:44:33 +0000 Received: by mail-ot1-x32d.google.com with SMTP id w23-20020a9d5a970000b02903d0ef989477so5398015oth.9 for ; Thu, 24 Jun 2021 05:44:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Q3gvZmypEjLXLqsFmPmNg0fnsAU683BrMuDR/uus3S8=; b=TUytgdOpb5Dh7U9LBK+OVwUmgXNPe/LpMGCGsLMVUVefURAC/FGHmidrYT3x56hMlv kUOiDdl+iw+9OiLKEue02u+DwW//WcvApLPcvEzBhZ+N3oLYkX7gMEhRV/yav4NvgsiN K0EFn3fWEKCQNBwlCYzzat8iK7cBbEmS0rVBmSoGoxTonFxC3kLa+PkbWsC5ITeCqWVT MjDmZrxd3X0tvMk27+Chkb/jQ9HrHLP0qK1elHqVuOPQvN5JvKvXuZ3BNh1bcF4LYLxe ZoC1u3wgfRs6SEmzFrG0ZEn07yYcQvcpMbGIuA7oK+x20vdgCwgWurHLuMWc9GlNgzPJ HKfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=Q3gvZmypEjLXLqsFmPmNg0fnsAU683BrMuDR/uus3S8=; b=tfRYuwubD3GMTfQOXTTd5iG0tXxecK5aSdecOTL4w6BjNnrWvaR1MYlunk5K7nhoni NBWH6LztMvwpxB07x7vC4Q0AHBhhBgNYDBxASA6U2Yj4yjswwmT0PTzRZfXPexyvPAS5 EiGHIcFI+N/6SlgSPmdODEAJjz9AFP/jpltI331fB/UK6DhXgGG2VU33bWUoaxUOcf05 x/pxHMKSclASYFtv+yxa9t+Ffb14kTav00LSCtnk0Xn5cDnpEqmIiZQCE6smXTYLN67z lulccBiA1j/DYkz/VhIzlMxWSj13ZEknUk3412JY62XngXrU4KLZPq3azNjK5PiQ1xyL Tkvg== X-Gm-Message-State: AOAM530qR4EgjHk6SdhMzbiY3HWHhflQQqBFyBVWpL6aJudB7rfyXj5B nW1RgcrsVAi6KLSAd/Wt7pU= X-Google-Smtp-Source: ABdhPJyGU8PRH2pP3pbdqSGcc0Y7/MlzY7Ggy4CwdK//7tZYzApvAnUs/5/Br9kykr9JEZOqHdR5hA== X-Received: by 2002:a05:6830:154b:: with SMTP id l11mr4598072otp.66.1624538669960; Thu, 24 Jun 2021 05:44:29 -0700 (PDT) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id h27sm642857ote.79.2021.06.24.05.44.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Jun 2021 05:44:29 -0700 (PDT) Date: Thu, 24 Jun 2021 05:44:28 -0700 From: Guenter Roeck To: Billy Tsai Cc: jdelvare@suse.com, joel@jms.id.au, andrew@aj.id.au, linux-hwmon@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, BMC-SW@aspeedtech.com Subject: Re: [PATCH] hwmon: (aspeed-pwm-tacho) Using falling edge. Message-ID: <20210624124428.GB1670703@roeck-us.net> References: <20210624035821.25375-1-billy_tsai@aspeedtech.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210624035821.25375-1-billy_tsai@aspeedtech.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210624_054431_678337_C830FA49 X-CRM114-Status: GOOD ( 23.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jun 24, 2021 at 11:58:21AM +0800, Billy Tsai wrote: > The tach shouldn't use both edges to measure. When the tach input > duty cycle isn't 50% the return value will inaccurate. > A tachometer doesn't have a duty cycle. A pwm has a duty cycle, but that is completely independent of the pwm duty cycle used to set the fan speed. So this patch does not really make sense with the above explanation. The impact of this patch is likely that the reported fan speed is reduced by 50%. It may well be that the driver currently reports twice the real fan speed. I have no idea if that is the case, but if it is it should not be conditional. The description above states "when the tach input cycle isn't 50%", suggesting that this is conditional on some other configuration. I don't know what that might be either. So, sorry, I can't accept this patch without a more detailed and accurate description and explanation why it is needed. > Signed-off-by: Billy Tsai > --- > drivers/hwmon/aspeed-pwm-tacho.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c > index 3d8239fd66ed..0a70a0e22acf 100644 > --- a/drivers/hwmon/aspeed-pwm-tacho.c > +++ b/drivers/hwmon/aspeed-pwm-tacho.c > @@ -158,7 +158,7 @@ > * 10: both > * 11: reserved. > */ > -#define M_TACH_MODE 0x02 /* 10b */ > +#define M_TACH_MODE 0x00 /* 10b */ That comment is now wrong. Guenter > #define M_TACH_UNIT 0x0210 > #define INIT_FAN_CTRL 0xFF > > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel