Message ID | 20231204055650.788388-2-kcfeng0@nuvoton.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp2570505vqy; Sun, 3 Dec 2023 21:57:59 -0800 (PST) X-Google-Smtp-Source: AGHT+IGUoIORks7peyNPG5JL9q+KM/6ohLw1i5xWW0ttAlQo16carUuoBpQ7hn1tlvE929WWR42l X-Received: by 2002:a17:90b:4a83:b0:286:964d:c with SMTP id lp3-20020a17090b4a8300b00286964d000cmr1376553pjb.86.1701669479603; Sun, 03 Dec 2023 21:57:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701669479; cv=none; d=google.com; s=arc-20160816; b=BJK4lc16BoZALsXsorRSdiAXhLeRinJ+r9wj73b+WLzNlo9Xcq5jCfoRdp5l6oSTi/ VnTBRjLJwvqUMUnBoQJDROtgOzNQDWm8f7ucxxOPBkUBijokaFqSOQjQ1milku4xDD55 nlPdUoM4FjDxx4vq169jIRta5pF46GAz1M/pNPjicSRNwiu2Hk8M4H06Df4zzvH4VXMz j1t2rS6IKSz3InFWET9JrIboGo9a5jqe6LDYc0N5CW84p+OSfQdZQ5c3aCUYjsqkOxjF FJXv7XiNmORvbDiOhAiS8UekWeYqobFTGCLTyZcYmzdBE8OaQO9pB3iSRawcpYt7eaYl +zRA== 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=lAIrBJ++PnmJ5yex7aR18QSXrWBXVdHB6cliHdL5dig=; fh=Y4+mEyvR/BEIHiyHXlW90rR+mRf7YZhfurnX+jddbzU=; b=wdgv2mCBNGEWSKgKHITvFamQ4K0HlI3BDdSo5IPWHNL3Lj9KK3LojIx3ao5zPXdqRV zemSaPLbI4H5V6fA7lMmqeT95PSry8uTNkHmc+VonfIV9iab8Di7nfDt3gGofRsH+A6S VjJPpsutuYNUwHpr2jPGbVUC0yDXE4EiUzthIxQvGUY2iUJus8lJgN7+rIi+NnvuEIE5 bBCFTmYfBRFycfRuZ8CFBWZ/c3afAnhNLUGgFQZOSrxf5BfBX9K7VQ6P0NBdHgYRuS0s aBKnns042U2cwWmXDOctCCiOpPivFWrRhK4lSjVmrro+YKP76KvTB+AaxcWgp680xOdM DsQg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=edd8WCcb; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from morse.vger.email (morse.vger.email. [2620:137:e000::3:1]) by mx.google.com with ESMTPS id u18-20020a170903125200b001cfaeeb91a3si3799283plh.474.2023.12.03.21.57.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 Dec 2023 21:57:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=edd8WCcb; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id E370B8068E23; Sun, 3 Dec 2023 21:57:56 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234511AbjLDF5p (ORCPT <rfc822;chrisfriedt@gmail.com> + 99 others); Mon, 4 Dec 2023 00:57:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57816 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjLDF5n (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 4 Dec 2023 00:57:43 -0500 Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D617BD7; Sun, 3 Dec 2023 21:57:49 -0800 (PST) Received: by mail-pj1-x1032.google.com with SMTP id 98e67ed59e1d1-2866951b6e0so1921020a91.2; Sun, 03 Dec 2023 21:57:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701669469; x=1702274269; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=lAIrBJ++PnmJ5yex7aR18QSXrWBXVdHB6cliHdL5dig=; b=edd8WCcbr7gTKoFSTvIgKVSKDdHimMf2192fyhW+IBhqJ3suDpR4Oab/wcqPGc4si+ Vaap7BjwjaiXDWbUMdnymDB6Rrz51c7j1CrlYdLvZTE6zIAe7OMSF7qG5ldsOEl79z7o M9vzNOJnFT67P2Cc3ZYwvJSroztWVG0D8zMJHUF0bL18hlWmyqMzMaWzPGmZSaf/TfHN 4j4cF3FRUk1pQz90MHJPocVfcICSjEsepBmgkyAOX43j/7YPmanuU8sEvuC3bURPqkYf 1iBIasoY9bfmTwHT+1z5QLxxqKjbJ8kRjAcSqNwN6GosVl5iVgQ8e8OOTsLY5ujZCZkY cAWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701669469; x=1702274269; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lAIrBJ++PnmJ5yex7aR18QSXrWBXVdHB6cliHdL5dig=; b=cEPXGcXw/DtECzePIb6WkrjwZsrKrQCBMP5C5Qfc97e3byc2m+3E8sw5KqkDHSx1An 507aUoCHV6T/WWV5Knj9a3m9MJ3yipmmxdr2uT/ZGplXKtZQ0M+BPRGRRZbMQDSDDaON bjeufQgZY4Lixp7r52CiOqbf/v1y5ltJT/QPKEeDt1AHOJALW8Esxv47pRRiMDxCvSKo zV11wAR+PNdM6wPBiUoHPIPVnzpIZFy1n1A2kebh+bT8umsXmDl/8Ui1g7hWMMi+HAsM 5LUA/2EHYhDxdEJ5Wqi0VxPUOXQEUavBnzXV5CKVnKcAx1K3vlF/wYzBqBEElucLetVX Zbvw== X-Gm-Message-State: AOJu0YxExGBprXkY3TGWhSi03WcK7K9BYbng1B27t3NRhPPLNxrZL2QT OuswhQkVJ/hcjuBcPB6M9lYyFt2i/7vGvA== X-Received: by 2002:a17:90b:1b01:b0:286:5965:1f68 with SMTP id nu1-20020a17090b1b0100b0028659651f68mr2395855pjb.4.1701669469231; Sun, 03 Dec 2023 21:57:49 -0800 (PST) Received: from hcdev-d520mt2.. (60-250-192-107.hinet-ip.hinet.net. [60.250.192.107]) by smtp.gmail.com with ESMTPSA id 95-20020a17090a09e800b0028651ea5e7csm6113523pjo.33.2023.12.03.21.57.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 Dec 2023 21:57:49 -0800 (PST) From: baneric926@gmail.com X-Google-Original-From: kcfeng0@nuvoton.com To: jdelvare@suse.com, linux@roeck-us.net, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, corbet@lwn.net Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, openbmc@lists.ozlabs.org, kwliu@nuvoton.com, kcfeng0@nuvoton.com, DELPHINE_CHIU@wiwynn.com, Bonnie_Lo@wiwynn.com Subject: [PATCH v1 1/2] dt-bindings: hwmon: Add nct736x bindings Date: Mon, 4 Dec 2023 13:56:49 +0800 Message-Id: <20231204055650.788388-2-kcfeng0@nuvoton.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231204055650.788388-1-kcfeng0@nuvoton.com> References: <20231204055650.788388-1-kcfeng0@nuvoton.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 morse.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 (morse.vger.email [0.0.0.0]); Sun, 03 Dec 2023 21:57:57 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784329776362762971 X-GMAIL-MSGID: 1784329776362762971 |
Series |
hwmon: Driver for Nuvoton NCT736X
|
|
Commit Message
Ban Feng
Dec. 4, 2023, 5:56 a.m. UTC
From: Ban Feng <kcfeng0@nuvoton.com> This change documents the device tree bindings for the Nuvoton NCT7362Y, NCT7363Y driver. Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> --- .../bindings/hwmon/nuvoton,nct736x.yaml | 80 +++++++++++++++++++ MAINTAINERS | 6 ++ 2 files changed, 86 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml
Comments
On 04/12/2023 06:56, baneric926@gmail.com wrote: > From: Ban Feng <kcfeng0@nuvoton.com> > > This change documents the device tree bindings for the Nuvoton > NCT7362Y, NCT7363Y driver. > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > --- > .../bindings/hwmon/nuvoton,nct736x.yaml | 80 +++++++++++++++++++ > MAINTAINERS | 6 ++ > 2 files changed, 86 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > new file mode 100644 > index 000000000000..f98fd260a20f > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > @@ -0,0 +1,80 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > + > +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct736x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton NCT736X Hardware Monitoring IC > + > +maintainers: > + - Ban Feng <kcfeng0@nuvoton.com> > + > +description: | > + The NCT736X is a Fan controller which provides up to 16 independent > + FAN input monitors, and up to 16 independent PWM output with SMBus interface. > + Besides, NCT7363Y has a built-in watchdog timer which is used for > + conditionally generating a system reset output (INT#). > + > +additionalProperties: false Please place it just like other bindings are placing it. Not in some random order. See example-schema. You should use common fan properties. If it was not merged yet, you must rebase on patchset on LKML and mention the dependency in the change log (---). > + > +properties: > + compatible: > + enum: > + - nuvoton,nct7362 > + - nuvoton,nct7363 > + > + reg: > + maxItems: 1 > + > + nuvoton,pwm-mask: > + description: | > + each bit means PWMx enable/disable setting, where x = 0~15. > + 0: disabled, 1: enabled > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 0x0 > + maximum: 0xFFFF > + default: 0x0 Use pwms, not own property for this. > + > + nuvoton,fanin-mask: > + description: | > + each bit means FANINx monitoring enable/disable setting, > + where x = 0~15. > + 0: disabled, 1: enabled > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 0x0 > + maximum: 0xFFFF > + default: 0x0 Use properties from common fan bindings. > + > + nuvoton,wdt-timeout: > + description: | > + Watchdog Timer time configuration for NCT7363Y, as below > + 0: 15 sec (default) > + 1: 7.5 sec > + 2: 3.75 sec > + 3: 30 sec > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + default: 0 Nope, reference watchdog.yaml and use its properties. See other watchdog bindings for examples. > + > +required: > + - compatible > + - reg > + - nuvoton,pwm-mask > + - nuvoton,fanin-mask > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + nct7363@22 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Best regards, Krzysztof
Hi Krzysztof, On Mon, Dec 4, 2023 at 4:04 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 04/12/2023 06:56, baneric926@gmail.com wrote: > > From: Ban Feng <kcfeng0@nuvoton.com> > > > > This change documents the device tree bindings for the Nuvoton > > NCT7362Y, NCT7363Y driver. > > > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > > --- > > .../bindings/hwmon/nuvoton,nct736x.yaml | 80 +++++++++++++++++++ > > MAINTAINERS | 6 ++ > > 2 files changed, 86 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > > new file mode 100644 > > index 000000000000..f98fd260a20f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > > @@ -0,0 +1,80 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > + > > +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct736x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Nuvoton NCT736X Hardware Monitoring IC > > + > > +maintainers: > > + - Ban Feng <kcfeng0@nuvoton.com> > > + > > +description: | > > + The NCT736X is a Fan controller which provides up to 16 independent > > + FAN input monitors, and up to 16 independent PWM output with SMBus interface. > > + Besides, NCT7363Y has a built-in watchdog timer which is used for > > + conditionally generating a system reset output (INT#). > > + > > +additionalProperties: false > > Please place it just like other bindings are placing it. Not in some > random order. See example-schema. ok, I'll move additionalProperties to the correct place. > > You should use common fan properties. If it was not merged yet, you must > rebase on patchset on LKML and mention the dependency in the change log > (---). please kindly see below > > > + > > +properties: > > + compatible: > > + enum: > > + - nuvoton,nct7362 > > + - nuvoton,nct7363 > > + > > + reg: > > + maxItems: 1 > > + > > + nuvoton,pwm-mask: > > + description: | > > + each bit means PWMx enable/disable setting, where x = 0~15. > > + 0: disabled, 1: enabled > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0x0 > > + maximum: 0xFFFF > > + default: 0x0 > > Use pwms, not own property for this. NCT736X has 16 funtion pins, they can be GPIO/PWM/FANIN/Reserved_or_ALERT#, and default is GPIO. We would like to add such a property that can configure the function pin for pin0~7 and pin10~17. My original design is only for PWM/FANIN, however, I noticed that we can change the design to "nuvoton,pin[0-7]$" and "nuvoton,pin[10-17]$", like example in adt7475.yaml. I'm not sure if this can be accepted or not, please kindly review this. > > > + > > + nuvoton,fanin-mask: > > + description: | > > + each bit means FANINx monitoring enable/disable setting, > > + where x = 0~15. > > + 0: disabled, 1: enabled > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0x0 > > + maximum: 0xFFFF > > + default: 0x0 > > Use properties from common fan bindings. Ditto > > > + > > + nuvoton,wdt-timeout: > > + description: | > > + Watchdog Timer time configuration for NCT7363Y, as below > > + 0: 15 sec (default) > > + 1: 7.5 sec > > + 2: 3.75 sec > > + 3: 30 sec > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3] > > + default: 0 > > Nope, reference watchdog.yaml and use its properties. See other watchdog > bindings for examples. NCT7363 has a built-in watchdog timer which is used for conditionally generating a system reset output (INT#) if the microcontroller or microprocessor stops to periodically send a pulse signal or interface communication access signal like SCL signal from SMBus interface. We only consider "Watchdog Timer Configuration Register" enable bit and timeout setting. Should we still need to add struct watchdog_device to struct nct736x_data? > > > + > > +required: > > + - compatible > > + - reg > > + - nuvoton,pwm-mask > > + - nuvoton,fanin-mask > > + > > +examples: > > + - | > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + nct7363@22 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation ok, I'll change nct7363@22 to hwmon@22. Thanks, Ban
On 05/12/2023 10:31, Ban Feng wrote: > Hi Krzysztof, > > On Mon, Dec 4, 2023 at 4:04 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 04/12/2023 06:56, baneric926@gmail.com wrote: >>> From: Ban Feng <kcfeng0@nuvoton.com> >>> >>> This change documents the device tree bindings for the Nuvoton >>> NCT7362Y, NCT7363Y driver. >>> >>> Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> >>> --- >>> .../bindings/hwmon/nuvoton,nct736x.yaml | 80 +++++++++++++++++++ >>> MAINTAINERS | 6 ++ >>> 2 files changed, 86 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml >>> new file mode 100644 >>> index 000000000000..f98fd260a20f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml >>> @@ -0,0 +1,80 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> + >>> +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct736x.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Nuvoton NCT736X Hardware Monitoring IC >>> + >>> +maintainers: >>> + - Ban Feng <kcfeng0@nuvoton.com> >>> + >>> +description: | >>> + The NCT736X is a Fan controller which provides up to 16 independent >>> + FAN input monitors, and up to 16 independent PWM output with SMBus interface. >>> + Besides, NCT7363Y has a built-in watchdog timer which is used for >>> + conditionally generating a system reset output (INT#). >>> + >>> +additionalProperties: false >> >> Please place it just like other bindings are placing it. Not in some >> random order. See example-schema. > > ok, I'll move additionalProperties to the correct place. > >> >> You should use common fan properties. If it was not merged yet, you must >> rebase on patchset on LKML and mention the dependency in the change log >> (---). > > please kindly see below > >> >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - nuvoton,nct7362 >>> + - nuvoton,nct7363 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + nuvoton,pwm-mask: >>> + description: | >>> + each bit means PWMx enable/disable setting, where x = 0~15. >>> + 0: disabled, 1: enabled >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + minimum: 0x0 >>> + maximum: 0xFFFF >>> + default: 0x0 >> >> Use pwms, not own property for this. > > NCT736X has 16 funtion pins, they can be > GPIO/PWM/FANIN/Reserved_or_ALERT#, and default is GPIO. > We would like to add such a property that can configure the function > pin for pin0~7 and pin10~17. It looks you are writing custom pinctrl instead of using standard bindings and frameworks. > > My original design is only for PWM/FANIN, however, > I noticed that we can change the design to "nuvoton,pin[0-7]$" and > "nuvoton,pin[10-17]$", like example in adt7475.yaml. > I'm not sure if this can be accepted or not, please kindly review this. It looks like the same problem as with other fan/Nuvoton bindings we discussed some time ago on the lists. Please do not duplicate threads, work and reviews: https://lore.kernel.org/all/20230607101827.8544-5-zev@bewilderbeest.net/ I already gave same comments there. >>> + nuvoton,wdt-timeout: >>> + description: | >>> + Watchdog Timer time configuration for NCT7363Y, as below >>> + 0: 15 sec (default) >>> + 1: 7.5 sec >>> + 2: 3.75 sec >>> + 3: 30 sec >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [0, 1, 2, 3] >>> + default: 0 >> >> Nope, reference watchdog.yaml and use its properties. See other watchdog >> bindings for examples. > > NCT7363 has a built-in watchdog timer which is used for conditionally > generating a system reset > output (INT#) if the microcontroller or microprocessor stops to > periodically send a pulse signal or > interface communication access signal like SCL signal from SMBus interface. > > We only consider "Watchdog Timer Configuration Register" enable bit > and timeout setting. > Should we still need to add struct watchdog_device to struct nct736x_data? I do not see correlation between watchdog.yaml and some struct. I did not write anything about driver, so I don't understand your comments. Anyway, I don't like that we are duplicating entire effort and our review. Please join existing discussions, threads and build on top of it. Best regards, Krzysztof
Hi Krzysztof, On Tuesday, December 5, 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 05/12/2023 10:31, Ban Feng wrote: > > Hi Krzysztof, > > > > On Mon, Dec 4, 2023 at 4:04 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 04/12/2023 06:56, baneric926@gmail.com wrote: > >>> From: Ban Feng <kcfeng0@nuvoton.com> > >>> > >>> This change documents the device tree bindings for the Nuvoton > >>> NCT7362Y, NCT7363Y driver. > >>> > >>> Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > >>> --- > >>> .../bindings/hwmon/nuvoton,nct736x.yaml | 80 +++++++++++++++++++ > >>> MAINTAINERS | 6 ++ > >>> 2 files changed, 86 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > >>> new file mode 100644 > >>> index 000000000000..f98fd260a20f > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml > >>> @@ -0,0 +1,80 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> + > >>> +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct736x.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Nuvoton NCT736X Hardware Monitoring IC > >>> + > >>> +maintainers: > >>> + - Ban Feng <kcfeng0@nuvoton.com> > >>> + > >>> +description: | > >>> + The NCT736X is a Fan controller which provides up to 16 independent > >>> + FAN input monitors, and up to 16 independent PWM output with SMBus interface. > >>> + Besides, NCT7363Y has a built-in watchdog timer which is used for > >>> + conditionally generating a system reset output (INT#). > >>> + > >>> +additionalProperties: false > >> > >> Please place it just like other bindings are placing it. Not in some > >> random order. See example-schema. > > > > ok, I'll move additionalProperties to the correct place. > > > >> > >> You should use common fan properties. If it was not merged yet, you must > >> rebase on patchset on LKML and mention the dependency in the change log > >> (---). > > > > please kindly see below > > > >> > >>> + > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - nuvoton,nct7362 > >>> + - nuvoton,nct7363 > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + nuvoton,pwm-mask: > >>> + description: | > >>> + each bit means PWMx enable/disable setting, where x = 0~15. > >>> + 0: disabled, 1: enabled > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + minimum: 0x0 > >>> + maximum: 0xFFFF > >>> + default: 0x0 > >> > >> Use pwms, not own property for this. > > > > NCT736X has 16 funtion pins, they can be > > GPIO/PWM/FANIN/Reserved_or_ALERT#, and default is GPIO. > > We would like to add such a property that can configure the function > > pin for pin0~7 and pin10~17. > > It looks you are writing custom pinctrl instead of using standard > bindings and frameworks. Please kindly see below > > > > > > My original design is only for PWM/FANIN, however, > > I noticed that we can change the design to "nuvoton,pin[0-7]$" and > > "nuvoton,pin[10-17]$", like example in adt7475.yaml. > > I'm not sure if this can be accepted or not, please kindly review this. > > It looks like the same problem as with other fan/Nuvoton bindings we > discussed some time ago on the lists. > > Please do not duplicate threads, work and reviews: > https://lore.kernel.org/all/20230607101827.8544-5-zev@bewilderbeest.net/ > > I already gave same comments there. Thanks for your kindly sharing. I noticed that [1] defines some useful properties, pwms and tach-ch, like you mentioned. Therefore, I'll modify our design to follow the common fan properties in v2. [1] https://lists.openwall.net/linux-kernel/2023/11/07/406 > > > >>> + nuvoton,wdt-timeout: > >>> + description: | > >>> + Watchdog Timer time configuration for NCT7363Y, as below > >>> + 0: 15 sec (default) > >>> + 1: 7.5 sec > >>> + 2: 3.75 sec > >>> + 3: 30 sec > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + enum: [0, 1, 2, 3] > >>> + default: 0 > >> > >> Nope, reference watchdog.yaml and use its properties. See other watchdog > >> bindings for examples. > > > > NCT7363 has a built-in watchdog timer which is used for conditionally > > generating a system reset > > output (INT#) if the microcontroller or microprocessor stops to > > periodically send a pulse signal or > > interface communication access signal like SCL signal from SMBus interface. > > > > We only consider "Watchdog Timer Configuration Register" enable bit > > and timeout setting. > > Should we still need to add struct watchdog_device to struct nct736x_data? > > I do not see correlation between watchdog.yaml and some struct. I did > not write anything about driver, so I don't understand your comments. > > Anyway, I don't like that we are duplicating entire effort and our > review. Please join existing discussions, threads and build on top of it. > Thanks, I'll remove unused function in hwmon subsystem, and consider a watchdog subsystem design if necessary. > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml new file mode 100644 index 000000000000..f98fd260a20f --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct736x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NCT736X Hardware Monitoring IC + +maintainers: + - Ban Feng <kcfeng0@nuvoton.com> + +description: | + The NCT736X is a Fan controller which provides up to 16 independent + FAN input monitors, and up to 16 independent PWM output with SMBus interface. + Besides, NCT7363Y has a built-in watchdog timer which is used for + conditionally generating a system reset output (INT#). + +additionalProperties: false + +properties: + compatible: + enum: + - nuvoton,nct7362 + - nuvoton,nct7363 + + reg: + maxItems: 1 + + nuvoton,pwm-mask: + description: | + each bit means PWMx enable/disable setting, where x = 0~15. + 0: disabled, 1: enabled + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0x0 + maximum: 0xFFFF + default: 0x0 + + nuvoton,fanin-mask: + description: | + each bit means FANINx monitoring enable/disable setting, + where x = 0~15. + 0: disabled, 1: enabled + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0x0 + maximum: 0xFFFF + default: 0x0 + + nuvoton,wdt-timeout: + description: | + Watchdog Timer time configuration for NCT7363Y, as below + 0: 15 sec (default) + 1: 7.5 sec + 2: 3.75 sec + 3: 30 sec + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + default: 0 + +required: + - compatible + - reg + - nuvoton,pwm-mask + - nuvoton,fanin-mask + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + nct7363@22 { + compatible = "nuvoton,nct7363"; + reg = <0x22>; + + nuvoton,pwm-mask = <0x003F>; + nuvoton,fanin-mask = <0x003F>; + nuvoton,wdt-timeout = <0x3>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 012df8ccf34e..eef44c13278c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14900,6 +14900,12 @@ S: Maintained F: Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml F: drivers/hwmon/nct6775-i2c.c +NCT736X HARDWARE MONITOR DRIVER +M: Ban Feng <kcfeng0@nuvoton.com> +L: linux-hwmon@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/hwmon/nuvoton,nct736x.yaml + NETDEVSIM M: Jakub Kicinski <kuba@kernel.org> S: Maintained