Message ID | 20231116022330.2696-2-xingtong_wu@163.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b909:0:b0:403:3b70:6f57 with SMTP id t9csp2944216vqg; Wed, 15 Nov 2023 18:26:21 -0800 (PST) X-Google-Smtp-Source: AGHT+IG9vea9CyyD6KiKg4si+gugMcxDOcpkeszIbFWJUAMgr1xy5Rl/j2i+OqTDR2NWx172FMeS X-Received: by 2002:a05:6358:e4a1:b0:16b:c810:667b with SMTP id by33-20020a056358e4a100b0016bc810667bmr7266086rwb.2.1700101580860; Wed, 15 Nov 2023 18:26:20 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700101580; cv=none; d=google.com; s=arc-20160816; b=cxS2bVsHslAQoEKtTMREGbSLbpqRbMOFi5xJa+AD8ilkcqTIRllysHO1TNeEf4wH+w goPgBqmoL1WqoFrwmpA3sLJVQl1em+DNIMphiMiaIXJ2l887Ls5y3orPp9FLj7CLswjz +L6Df5fQ3baBSisH2OUhAduq768BVeNf4ZFuULyVWBmIXJf3dSfsPpSuBVYyT3CUddhV nlKjq1jEXnqRKmrAJtZz3pytAdPkW7iIlzrRMQW1C8PaBj1ggmwBauPXYPkaQGkhBk4U kLq9vTrgdxqbZeJmhFt6e/iLMfZ3s5IepDTCv1gNl3hASCyp8k+rFwgYX8ahAvjrbHTP bfQA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=hijftSCcjG/9boz0oEYP+nVxe7rLA+Dx/w2RlM3IhcM=; fh=brTD9uWwHSBEuDMsNSIyrWZVEeJXt4Ae/wkRCvfJiZk=; b=eQEQ/IxbpsNwQOpa0aOy/7pT/QEKdHfp8PdQHw8bnENS72QYDdOUBXfy5efs57vVlb vyG8vDk7Q5KHJAm43NQpoH+ET2mYQwuevS27UF1O0oO7bOdsDDGUM3Avg/25VptGvAP/ C26Z56xbMkibPrdeL0zDh2X9YeEKa5qpCimSgK0JQZzcVe4vv/ePkOoN3GzZcVARgVrg Gy3tB4cVmczxFJd3/aM4Lez+tFuHotGrfcHcSgO3EiTKCo3lwGTUHQitstxtRapwiMj6 0bjszvq5W6csrHWhkXSH9eBmsBYt3gQZd6UTPUPOTlCCdIkuMjcii6o4JotTu7DCyxbG JgHQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@163.com header.s=s110527 header.b=QgdDwbXn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=163.com Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id c14-20020a63ea0e000000b0056da0ae25d1si10534381pgi.831.2023.11.15.18.26.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Nov 2023 18:26:20 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@163.com header.s=s110527 header.b=QgdDwbXn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=163.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id EA338809B6E6; Wed, 15 Nov 2023 18:26:00 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344578AbjKPCZE (ORCPT <rfc822;lhua1029@gmail.com> + 28 others); Wed, 15 Nov 2023 21:25:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43064 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235725AbjKPCYx (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 15 Nov 2023 21:24:53 -0500 Received: from m12.mail.163.com (m12.mail.163.com [220.181.12.196]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id DDA841B2; Wed, 15 Nov 2023 18:24:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=hijft SCcjG/9boz0oEYP+nVxe7rLA+Dx/w2RlM3IhcM=; b=QgdDwbXnbs69uyuHTiUNJ 9QLs+wt31XmvrnsGpX600Qy2/SACwcUHp3oPVFnAu8ZnVns2jlP/ldoe4VqxnAgt Oo2oW+GOOWmr2GsYzZPUy+jtC0zn0OoVTcqalz/geFI44Uli0IQKu7MBUvBRLely hFfmysW/EC8bqBShNZUv9Q= Received: from localhost.localdomain (unknown [39.144.137.125]) by zwqz-smtp-mta-g4-4 (Coremail) with SMTP id _____wD3H2U+fVVl5bnXDA--.19779S3; Thu, 16 Nov 2023 10:24:07 +0800 (CST) From: Xing Tong Wu <xingtong_wu@163.com> To: Guenter Roeck <linux@roeck-us.net>, Jean Delvare <jdelvare@suse.com>, linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org Cc: xingtong.wu@siemens.com, tobias.schaffner@siemens.com, gerd.haeussler.ext@siemens.com Subject: [PATCH 1/3] hwmon: (nct6775) Fix incomplete register array Date: Thu, 16 Nov 2023 10:23:28 +0800 Message-Id: <20231116022330.2696-2-xingtong_wu@163.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20231116022330.2696-1-xingtong_wu@163.com> References: <20231116022330.2696-1-xingtong_wu@163.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: _____wD3H2U+fVVl5bnXDA--.19779S3 X-Coremail-Antispam: 1Uf129KBjvJXoW7ur1fXr1Dtry8uF1xur4xXrb_yoW8Gr1kpF ykXrWSy3Wrt3WavF43Ga1ru3W3Cwn7ArWIyw1UCw4SkFn5tFyxXF43KFyqywn0yFWfJa42 9FykJFWYg3Z8CF7anT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jqxRhUUUUU= X-Originating-IP: [39.144.137.125] X-CM-SenderInfo: p0lqw35rqjs4rx6rljoofrz/1tbiEAsq0F8YMgZlaQAAse X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Wed, 15 Nov 2023 18:26:01 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782685715670273977 X-GMAIL-MSGID: 1782685715670273977 |
Series |
*** hwmon: (nct6775) Fix pwm bugs for NCT chips ***
|
|
Commit Message
Xing Tong Wu
Nov. 16, 2023, 2:23 a.m. UTC
From: Xing Tong Wu <xingtong.wu@siemens.com> The nct6116 specification actually includes 5 PWMs, but only 3 PWMs are present in the array. To address this, the missing 2 PWMs have been added to the array. Signed-off-by: Xing Tong Wu <xingtong.wu@siemens.com> --- drivers/hwmon/nct6775-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Thu, Nov 16, 2023 at 10:23:28AM +0800, Xing Tong Wu wrote: > From: Xing Tong Wu <xingtong.wu@siemens.com> > > The nct6116 specification actually includes 5 PWMs, but only 3 > PWMs are present in the array. To address this, the missing 2 > PWMs have been added to the array. > > Signed-off-by: Xing Tong Wu <xingtong.wu@siemens.com> > --- > drivers/hwmon/nct6775-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c > index d928eb8ae5a3..2111f0cd9787 100644 > --- a/drivers/hwmon/nct6775-core.c > +++ b/drivers/hwmon/nct6775-core.c > @@ -769,7 +769,7 @@ static const u16 NCT6106_FAN_PULSE_SHIFT[] = { 0, 2, 4 }; > > static const u8 NCT6106_REG_PWM_MODE[] = { 0xf3, 0xf3, 0xf3 }; > static const u8 NCT6106_PWM_MODE_MASK[] = { 0x01, 0x02, 0x04 }; > -static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c }; > +static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c, 0xd8, 0xd9 }; I'll have to check the datasheet if this is generic, but at the very least it is incomplete and REG_PWM_MODE as well as REG_PWM_MODE_MASK would have to be updated as well. Also, I don't see an update to has_pwm, meaning the two additional pwm controls won't ever be used/enabled. Guenter
On 11/15/23 18:23, Xing Tong Wu wrote: > From: Xing Tong Wu <xingtong.wu@siemens.com> > > The nct6116 specification actually includes 5 PWMs, but only 3 > PWMs are present in the array. To address this, the missing 2 > PWMs have been added to the array. > > Signed-off-by: Xing Tong Wu <xingtong.wu@siemens.com> > --- > drivers/hwmon/nct6775-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c > index d928eb8ae5a3..2111f0cd9787 100644 > --- a/drivers/hwmon/nct6775-core.c > +++ b/drivers/hwmon/nct6775-core.c > @@ -769,7 +769,7 @@ static const u16 NCT6106_FAN_PULSE_SHIFT[] = { 0, 2, 4 }; > > static const u8 NCT6106_REG_PWM_MODE[] = { 0xf3, 0xf3, 0xf3 }; > static const u8 NCT6106_PWM_MODE_MASK[] = { 0x01, 0x02, 0x04 }; > -static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c }; > +static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c, 0xd8, 0xd9 }; I have no idea where you got the above register addresses from. Looking at the datasheet, NCT6116 doesn't use those registers at all, and neither does NCT6106. The PWM registers for NCT6116 are static const u16 NCT6116_REG_PWM[] = { 0x119, 0x129, 0x139, 0x199, 0x1a9 }; > static const u16 NCT6106_REG_FAN_MODE[] = { 0x113, 0x123, 0x133 }; > static const u16 NCT6106_REG_TEMP_SOURCE[] = { > 0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5 }; > @@ -3595,7 +3595,7 @@ int nct6775_probe(struct device *dev, struct nct6775_data *data, > break; > case nct6116: > data->in_num = 9; > - data->pwm_num = 3; > + data->pwm_num = 5; This does look correct, though. Guenter > data->auto_pwm_num = 4; > data->temp_fixed_num = 3; > data->num_temp_alarms = 3;
On 2023/11/17 09:35, Guenter Roeck wrote: > On 11/15/23 18:23, Xing Tong Wu wrote: >> From: Xing Tong Wu <xingtong.wu@siemens.com> >> >> The nct6116 specification actually includes 5 PWMs, but only 3 >> PWMs are present in the array. To address this, the missing 2 >> PWMs have been added to the array. >> >> Signed-off-by: Xing Tong Wu <xingtong.wu@siemens.com> >> --- >> drivers/hwmon/nct6775-core.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c >> index d928eb8ae5a3..2111f0cd9787 100644 >> --- a/drivers/hwmon/nct6775-core.c >> +++ b/drivers/hwmon/nct6775-core.c >> @@ -769,7 +769,7 @@ static const u16 NCT6106_FAN_PULSE_SHIFT[] = { 0, 2, 4 }; >> static const u8 NCT6106_REG_PWM_MODE[] = { 0xf3, 0xf3, 0xf3 }; >> static const u8 NCT6106_PWM_MODE_MASK[] = { 0x01, 0x02, 0x04 }; >> -static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c }; >> +static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c, 0xd8, 0xd9 }; > > I have no idea where you got the above register addresses from. Looking at > the datasheet, NCT6116 doesn't use those registers at all, and neither does > NCT6106. The PWM registers for NCT6116 are > I obtain these registers from the Nuvoton official specification "NCT6116D_Datasheet_V1_0.pdf". There is a table that describes the access for the PWM registers and corresponding fans: Fans: SYSFANOUT, CPUFANOUT, AUXFANOUT0, AUXFANOUT1, AUXFANOUT2 PWM output duty (write): Bank1 Index19 bit[7:0], Bank1 Index29 bit[7:0], Bank1 Index39 bit[7:0], Bank1 Index99 bit[7:0], Bank1 IndexA9 bit[7:0] Current output value (read): Bank0 Index4A, Bank0 Index4B, Bank0 Index4C, Bank0 IndexD8, Bank0 IndexD9 I have checked the NCT6106-NCT6126 series specification(These documents are not publicly available, so I cannot share them with you), there are only 3 fans in NCT6106: SYSFANOUT, CPUFANOUT, AUXFANOU0. However, for NCT6116D-NCT6126D, there are 2 additional fans: AUXFANOUT1, AUXFANOUT2. The registers for these fans are the same. I'll add a new array for NCT6116D's PWM read in my new patch. > static const u16 NCT6116_REG_PWM[] = { 0x119, 0x129, 0x139, 0x199, 0x1a9 }; Therefore, this array is only for writing, we need to add an array of registers for reading. > >> static const u16 NCT6106_REG_FAN_MODE[] = { 0x113, 0x123, 0x133 }; >> static const u16 NCT6106_REG_TEMP_SOURCE[] = { >> 0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5 }; >> @@ -3595,7 +3595,7 @@ int nct6775_probe(struct device *dev, struct nct6775_data *data, >> break; >> case nct6116: >> data->in_num = 9; >> - data->pwm_num = 3; >> + data->pwm_num = 5; > > This does look correct, though. > > Guenter > >> data->auto_pwm_num = 4; >> data->temp_fixed_num = 3; >> data->num_temp_alarms = 3;
On 11/19/23 19:30, xingtong.wu wrote: > On 2023/11/17 09:35, Guenter Roeck wrote: >> On 11/15/23 18:23, Xing Tong Wu wrote: >>> From: Xing Tong Wu <xingtong.wu@siemens.com> >>> >>> The nct6116 specification actually includes 5 PWMs, but only 3 >>> PWMs are present in the array. To address this, the missing 2 >>> PWMs have been added to the array. >>> >>> Signed-off-by: Xing Tong Wu <xingtong.wu@siemens.com> >>> --- >>> drivers/hwmon/nct6775-core.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c >>> index d928eb8ae5a3..2111f0cd9787 100644 >>> --- a/drivers/hwmon/nct6775-core.c >>> +++ b/drivers/hwmon/nct6775-core.c >>> @@ -769,7 +769,7 @@ static const u16 NCT6106_FAN_PULSE_SHIFT[] = { 0, 2, 4 }; >>> static const u8 NCT6106_REG_PWM_MODE[] = { 0xf3, 0xf3, 0xf3 }; >>> static const u8 NCT6106_PWM_MODE_MASK[] = { 0x01, 0x02, 0x04 }; >>> -static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c }; >>> +static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c, 0xd8, 0xd9 }; >> >> I have no idea where you got the above register addresses from. Looking at >> the datasheet, NCT6116 doesn't use those registers at all, and neither does >> NCT6106. The PWM registers for NCT6116 are >> > > I obtain these registers from the Nuvoton official specification > "NCT6116D_Datasheet_V1_0.pdf". There is a table that describes the access for the > PWM registers and corresponding fans: > > Fans: SYSFANOUT, CPUFANOUT, AUXFANOUT0, AUXFANOUT1, AUXFANOUT2 > PWM output duty (write): Bank1 Index19 bit[7:0], Bank1 Index29 bit[7:0], Bank1 Index39 bit[7:0], Bank1 Index99 bit[7:0], Bank1 IndexA9 bit[7:0] > Current output value (read): Bank0 Index4A, Bank0 Index4B, Bank0 Index4C, Bank0 IndexD8, Bank0 IndexD9 > > I have checked the NCT6106-NCT6126 series specification(These documents are not > publicly available, so I cannot share them with you), there are only 3 fans in > NCT6106: SYSFANOUT, CPUFANOUT, AUXFANOU0. However, for NCT6116D-NCT6126D, there > are 2 additional fans: AUXFANOUT1, AUXFANOUT2. The registers for these fans are > the same. I'll add a new array for NCT6116D's PWM read in my new patch. > I wasn't aware of NCT6126D, but I do have datasheets for NCT6102/4/6 and for NCT6112/4/6. There is no need to add a separate array. This is good as-is since the added registers are not used for NCT6102/4/6. >> static const u16 NCT6116_REG_PWM[] = { 0x119, 0x129, 0x139, 0x199, 0x1a9 }; > > Therefore, this array is only for writing, we need to add an array of registers for reading. > Ah, good point. I forgot that the read and write registers are different. Still, you'll need to extend NCT6106_REG_PWM_MODE[] and NCT6106_PWM_MODE_MASK[] as well. As far as I can see, aux1 and aux2 are always in pwm mode, so the register arrays need to be extended with ", 0, 0". Thanks, Guenter
diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c index d928eb8ae5a3..2111f0cd9787 100644 --- a/drivers/hwmon/nct6775-core.c +++ b/drivers/hwmon/nct6775-core.c @@ -769,7 +769,7 @@ static const u16 NCT6106_FAN_PULSE_SHIFT[] = { 0, 2, 4 }; static const u8 NCT6106_REG_PWM_MODE[] = { 0xf3, 0xf3, 0xf3 }; static const u8 NCT6106_PWM_MODE_MASK[] = { 0x01, 0x02, 0x04 }; -static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c }; +static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c, 0xd8, 0xd9 }; static const u16 NCT6106_REG_FAN_MODE[] = { 0x113, 0x123, 0x133 }; static const u16 NCT6106_REG_TEMP_SOURCE[] = { 0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5 }; @@ -3595,7 +3595,7 @@ int nct6775_probe(struct device *dev, struct nct6775_data *data, break; case nct6116: data->in_num = 9; - data->pwm_num = 3; + data->pwm_num = 5; data->auto_pwm_num = 4; data->temp_fixed_num = 3; data->num_temp_alarms = 3;