* [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y
@ 2023-12-22 1:33 baneric926
2023-12-22 1:33 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: baneric926 @ 2023-12-22 1:33 UTC (permalink / raw)
To: jdelvare, linux, robh+dt, krzysztof.kozlowski+dt, conor+dt,
corbet
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu,
kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai
From: Ban Feng <kcfeng0@nuvoton.com>
NCT7363Y is an I2C based hardware monitoring chip from Nuvoton.
Changes since version 2:
- Cherry-pick the fan-common.yaml in [1]
- Fix nct736x typo and add unevaluatedProperties
[1]: https://patchwork.kernel.org/project/linux-hwmon/patch/
20231107105025.1480561-2-billy_tsai@aspeedtech.com/
Changes since version 1:
- Modify NCT736X(nct736x) to NCT7363Y(nct7363)
- Convert to devm_hwmon_device_register_with_info API
- All ID tables are next to each other and should be consistent
between i2c_device_id and of_device_id
- Ref. fan-common.yaml and modify properties (nuvoton,pwm-mask/
nuvoton,fanin-mask) to (pwms/tach-ch)
- Convert to devm_regmap_init_i2c API
- Remove unused function (watchdog timer)
- Fix uninitialized symbol which is reported by kernel test robot
Ban Feng (2):
dt-bindings: hwmon: Add NCT7363Y documentation
hwmon: Driver for Nuvoton NCT7363Y
Naresh Solanki (1):
dt-bindings: hwmon: fan: Add fan binding to schema
.../devicetree/bindings/hwmon/fan-common.yaml | 76 +++
.../bindings/hwmon/nuvoton,nct7363.yaml | 63 +++
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/nct7363.rst | 33 ++
MAINTAINERS | 8 +
drivers/hwmon/Kconfig | 11 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/nct7363.c | 515 ++++++++++++++++++
8 files changed, 708 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml
create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml
create mode 100644 Documentation/hwmon/nct7363.rst
create mode 100644 drivers/hwmon/nct7363.c
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2023-12-22 1:33 [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 @ 2023-12-22 1:33 ` baneric926 2024-01-04 0:15 ` Rob Herring 2024-02-03 15:09 ` Guenter Roeck 2023-12-22 1:33 ` [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation baneric926 2023-12-22 1:33 ` [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2 siblings, 2 replies; 11+ messages in thread From: baneric926 @ 2023-12-22 1:33 UTC (permalink / raw) To: jdelvare, linux, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai From: Naresh Solanki <naresh.solanki@9elements.com> Add common fan properties bindings to a schema. Bindings for fan controllers can reference the common schema for the fan child nodes: patternProperties: "^fan@[0-2]": type: object $ref: fan-common.yaml# unevaluatedProperties: false Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> --- .../devicetree/bindings/hwmon/fan-common.yaml | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml diff --git a/Documentation/devicetree/bindings/hwmon/fan-common.yaml b/Documentation/devicetree/bindings/hwmon/fan-common.yaml new file mode 100644 index 000000000000..ba7d6531f01d --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml @@ -0,0 +1,76 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/fan-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common Fan Properties + +maintainers: + - Naresh Solanki <naresh.solanki@9elements.com> + - Billy Tsai <billy_tsai@aspeedtech.com> + +properties: + max-rpm: + description: + Max RPM supported by fan. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 100000 + + min-rpm: + description: + Min RPM supported by fan. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 1000 + + pulses-per-revolution: + description: + The number of pulse from fan sensor per revolution. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 4 + + tach-div: + description: + Divisor for the tach sampling clock, which determines the sensitivity of the tach pin. + $ref: /schemas/types.yaml#/definitions/uint32 + + target-rpm: + description: + The default desired fan speed in RPM. + $ref: /schemas/types.yaml#/definitions/uint32 + + fan-driving-mode: + description: + Select the driving mode of the fan.(DC, PWM and so on) + $ref: /schemas/types.yaml#/definitions/string + + pwms: + description: + PWM provider. + maxItems: 1 + + "#cooling-cells": + const: 2 + + cooling-levels: + description: + The control value which correspond to thermal cooling states. + $ref: /schemas/types.yaml#/definitions/uint32-array + + tach-ch: + description: + The tach channel used for the fan. + $ref: /schemas/types.yaml#/definitions/uint8-array + + label: + description: + Optional fan label + + fan-supply: + description: + Power supply for fan. + + reg: + maxItems: 1 + +additionalProperties: true -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2023-12-22 1:33 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926 @ 2024-01-04 0:15 ` Rob Herring 2024-01-09 9:00 ` Ban Feng 2024-02-03 15:09 ` Guenter Roeck 1 sibling, 1 reply; 11+ messages in thread From: Rob Herring @ 2024-01-04 0:15 UTC (permalink / raw) To: baneric926 Cc: jdelvare, linux, krzysztof.kozlowski+dt, conor+dt, corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai On Fri, Dec 22, 2023 at 09:33:50AM +0800, baneric926@gmail.com wrote: > From: Naresh Solanki <naresh.solanki@9elements.com> > > Add common fan properties bindings to a schema. > > Bindings for fan controllers can reference the common schema for the > fan > > child nodes: > > patternProperties: > "^fan@[0-2]": > type: object > $ref: fan-common.yaml# > unevaluatedProperties: false > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > --- > .../devicetree/bindings/hwmon/fan-common.yaml | 76 +++++++++++++++++++ > 1 file changed, 76 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml Please implement my comments on v10. Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2024-01-04 0:15 ` Rob Herring @ 2024-01-09 9:00 ` Ban Feng 0 siblings, 0 replies; 11+ messages in thread From: Ban Feng @ 2024-01-09 9:00 UTC (permalink / raw) To: Rob Herring Cc: jdelvare, linux, krzysztof.kozlowski+dt, conor+dt, corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai On Thu, Jan 4, 2024 at 8:15 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, Dec 22, 2023 at 09:33:50AM +0800, baneric926@gmail.com wrote: > > From: Naresh Solanki <naresh.solanki@9elements.com> > > > > Add common fan properties bindings to a schema. > > > > Bindings for fan controllers can reference the common schema for the > > fan > > > > child nodes: > > > > patternProperties: > > "^fan@[0-2]": > > type: object > > $ref: fan-common.yaml# > > unevaluatedProperties: false > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > > --- > > .../devicetree/bindings/hwmon/fan-common.yaml | 76 +++++++++++++++++++ > > 1 file changed, 76 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml > > Please implement my comments on v10. > Hi Rob, I saw Aspeed Billy has already added enum to below patch: https://patchwork.kernel.org/project/linux-hwmon/patch/20240108074348.735014-2-billy_tsai@aspeedtech.com/ Thanks, Ban ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2023-12-22 1:33 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926 2024-01-04 0:15 ` Rob Herring @ 2024-02-03 15:09 ` Guenter Roeck 2024-02-21 2:22 ` Ban Feng 1 sibling, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2024-02-03 15:09 UTC (permalink / raw) To: baneric926 Cc: jdelvare, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai On Fri, Dec 22, 2023 at 09:33:50AM +0800, baneric926@gmail.com wrote: > From: Naresh Solanki <naresh.solanki@9elements.com> > > Add common fan properties bindings to a schema. > > Bindings for fan controllers can reference the common schema for the > fan > > child nodes: > > patternProperties: > "^fan@[0-2]": > type: object > $ref: fan-common.yaml# > unevaluatedProperties: false > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> Unfortunately the dt maintainer's Reviewed-by: tag on the latest version of the fan schema patch got lost. I am not sure if I can add that back in on my own without violating some rules. That will need to get resolved before we can move forward with these patches. Guenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2024-02-03 15:09 ` Guenter Roeck @ 2024-02-21 2:22 ` Ban Feng 0 siblings, 0 replies; 11+ messages in thread From: Ban Feng @ 2024-02-21 2:22 UTC (permalink / raw) To: Guenter Roeck Cc: jdelvare, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai Hi Guenter, On Sat, Feb 3, 2024 at 11:09 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Fri, Dec 22, 2023 at 09:33:50AM +0800, baneric926@gmail.com wrote: > > From: Naresh Solanki <naresh.solanki@9elements.com> > > > > Add common fan properties bindings to a schema. > > > > Bindings for fan controllers can reference the common schema for the > > fan > > > > child nodes: > > > > patternProperties: > > "^fan@[0-2]": > > type: object > > $ref: fan-common.yaml# > > unevaluatedProperties: false > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > > Unfortunately the dt maintainer's Reviewed-by: tag on the latest version > of the fan schema patch got lost. I am not sure if I can add that back > in on my own without violating some rules. That will need to get resolved > before we can move forward with these patches. > > Guenter Owner will add 'Reviewed-by: tag from Rob' in the next version. Thanks, Ban ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation 2023-12-22 1:33 [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2023-12-22 1:33 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926 @ 2023-12-22 1:33 ` baneric926 2024-01-04 0:21 ` Rob Herring 2023-12-22 1:33 ` [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2 siblings, 1 reply; 11+ messages in thread From: baneric926 @ 2023-12-22 1:33 UTC (permalink / raw) To: jdelvare, linux, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai From: Ban Feng <kcfeng0@nuvoton.com> Adding bindings for the Nuvoton NCT7363Y Fan Controller Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> --- .../bindings/hwmon/nuvoton,nct7363.yaml | 63 +++++++++++++++++++ MAINTAINERS | 6 ++ 2 files changed, 69 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml new file mode 100644 index 000000000000..1a9d9a5d614e --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7363.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NCT7363Y Hardware Monitoring IC + +maintainers: + - Ban Feng <kcfeng0@nuvoton.com> + +description: | + The NCT7363Y is a Fan controller which provides up to 16 independent + FAN input monitors, and up to 16 independent PWM output with SMBus interface. + +properties: + compatible: + enum: + - nuvoton,nct7363 + + reg: + maxItems: 1 + + "#pwm-cells": + const: 2 + +patternProperties: + "^fan-[0-9]+$": + $ref: fan-common.yaml# + unevaluatedProperties: false + required: + - pwms + - tach-ch + +required: + - compatible + - reg + - "#pwm-cells" + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + hwmon: hwmon@22 { + compatible = "nuvoton,nct7363"; + reg = <0x22>; + #pwm-cells = <2>; + + fan-0 { + pwms = <&hwmon 0 50000>; + tach-ch = /bits/ 8 <0x00>; + }; + fan-1 { + pwms = <&hwmon 1 50000>; + tach-ch = /bits/ 8 <0x01>; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 7cef2d2ef8d7..53cfcc629aa1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14837,6 +14837,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,nct7363.yaml + NETDEVSIM M: Jakub Kicinski <kuba@kernel.org> S: Maintained -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation 2023-12-22 1:33 ` [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation baneric926 @ 2024-01-04 0:21 ` Rob Herring 0 siblings, 0 replies; 11+ messages in thread From: Rob Herring @ 2024-01-04 0:21 UTC (permalink / raw) To: baneric926 Cc: conor+dt, krzysztof.kozlowski+dt, Bonnie_Lo, robh+dt, naresh.solanki, billy_tsai, corbet, jdelvare, linux-hwmon, openbmc, linux, linux-doc, kwliu, kcfeng0, linux-kernel, devicetree, DELPHINE_CHIU On Fri, 22 Dec 2023 09:33:51 +0800, baneric926@gmail.com wrote: > From: Ban Feng <kcfeng0@nuvoton.com> > > Adding bindings for the Nuvoton NCT7363Y Fan Controller > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > --- > .../bindings/hwmon/nuvoton,nct7363.yaml | 63 +++++++++++++++++++ > MAINTAINERS | 6 ++ > 2 files changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y 2023-12-22 1:33 [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2023-12-22 1:33 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926 2023-12-22 1:33 ` [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation baneric926 @ 2023-12-22 1:33 ` baneric926 2024-02-03 15:06 ` Guenter Roeck 2 siblings, 1 reply; 11+ messages in thread From: baneric926 @ 2023-12-22 1:33 UTC (permalink / raw) To: jdelvare, linux, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai From: Ban Feng <kcfeng0@nuvoton.com> NCT7363Y is an I2C based hardware monitoring chip from Nuvoton. Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> --- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/nct7363.rst | 33 ++ MAINTAINERS | 2 + drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/nct7363.c | 515 ++++++++++++++++++++++++++++++++ 6 files changed, 563 insertions(+) create mode 100644 Documentation/hwmon/nct7363.rst create mode 100644 drivers/hwmon/nct7363.c diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index 72f4e6065bae..178d3cae95de 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -161,6 +161,7 @@ Hardware Monitoring Kernel Drivers mp5023 nct6683 nct6775 + nct7363 nct7802 nct7904 npcm750-pwm-fan diff --git a/Documentation/hwmon/nct7363.rst b/Documentation/hwmon/nct7363.rst new file mode 100644 index 000000000000..89699c95aa4b --- /dev/null +++ b/Documentation/hwmon/nct7363.rst @@ -0,0 +1,33 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Kernel driver nct7363 +===================== + +Supported chip: + + * Nuvoton NCT7363Y + + Prefix: nct7363 + + Addresses: I2C 0x20, 0x21, 0x22, 0x23 + +Author: Ban Feng <kcfeng0@nuvoton.com> + + +Description +----------- + +The NCT7363Y is a Fan controller which provides up to 16 independent +FAN input monitors, and up to 16 independent PWM output with SMBus interface. + + +Sysfs entries +------------- + +Currently, the driver supports the following features: + +======================= ======================================================= +fanX_input provide current fan rotation value in RPM + +pwmX get or set PWM fan control value. +======================= ======================================================= diff --git a/MAINTAINERS b/MAINTAINERS index 53cfcc629aa1..e39c4fc01a3b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14842,6 +14842,8 @@ M: Ban Feng <kcfeng0@nuvoton.com> L: linux-hwmon@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml +F: Documentation/hwmon/nct7363.rst +F: drivers/hwmon/nct7363.c NETDEVSIM M: Jakub Kicinski <kuba@kernel.org> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index cf27523eed5a..a0229851fc64 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1605,6 +1605,17 @@ config SENSORS_NCT6775_I2C This driver can also be built as a module. If so, the module will be called nct6775-i2c. +config SENSORS_NCT7363 + tristate "Nuvoton NCT7363Y" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get support for the Nuvoton NCT7363Y, + hardware monitoring chip. + + This driver can also be built as a module. If so, the module + will be called nct7363. + config SENSORS_NCT7802 tristate "Nuvoton NCT7802Y" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index e84bd9685b5c..dd794aa06209 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -166,6 +166,7 @@ obj-$(CONFIG_SENSORS_NCT6775_CORE) += nct6775-core.o nct6775-objs := nct6775-platform.o obj-$(CONFIG_SENSORS_NCT6775) += nct6775.o obj-$(CONFIG_SENSORS_NCT6775_I2C) += nct6775-i2c.o +obj-$(CONFIG_SENSORS_NCT7363) += nct7363.o obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NPCM7XX) += npcm750-pwm-fan.o diff --git a/drivers/hwmon/nct7363.c b/drivers/hwmon/nct7363.c new file mode 100644 index 000000000000..1bf6e83afd7f --- /dev/null +++ b/drivers/hwmon/nct7363.c @@ -0,0 +1,515 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2023 Nuvoton Technology corporation. + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/err.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/i2c.h> +#include <linux/jiffies.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#define NCT7363_REG_GPIO_0_3 0x20 +#define NCT7363_REG_GPIO_4_7 0x21 +#define NCT7363_REG_GPIO_10_13 0x22 +#define NCT7363_REG_GPIO_14_17 0x23 +#define NCT7363_REG_PWMEN_0_7 0x38 +#define NCT7363_REG_PWMEN_8_15 0x39 +#define NCT7363_REG_FANINEN_0_7 0x41 +#define NCT7363_REG_FANINEN_8_15 0x42 +#define NCT7363_REG_FANINx_HVAL(x) (0x48 + ((x) * 2)) +#define NCT7363_REG_FANINx_LVAL(x) (0x49 + ((x) * 2)) +#define NCT7363_REG_FSCPxDUTY(x) (0x90 + ((x) * 2)) +#define NCT7363_REG_VENDOR_ID 0xFD +#define NCT7363_REG_CHIP_ID 0xFE +#define NCT7363_REG_DEVICE_ID 0xFF + +#define NUVOTON_ID 0x49 +#define CHIP_ID 0x19 +#define DEVICE_ID 0x88 + +#define PWM_SEL(x) (BIT(0) << ((x % 4) * 2)) +#define FANIN_SEL(x) (BIT(1) << ((x % 4) * 2)) +#define BIT_CHECK(x) (BIT(0) << x) + +#define NCT7363_FANINx_LVAL_MASK GENMASK(4, 0) +#define NCT7363_FANIN_MASK GENMASK(12, 0) + +#define NCT7363_PWM_COUNT 16 +#define NCT7363_FANIN_COUNT 16 + +#define REFRESH_INTERVAL (2 * HZ) + +static inline unsigned long FAN_FROM_REG(u16 val) +{ + if ((val >= NCT7363_FANIN_MASK) || (val == 0)) + return 0; + + return (1350000UL / val); +} + +static const unsigned short normal_i2c[] = { + 0x20, 0x21, 0x22, 0x23, I2C_CLIENT_END +}; + +enum chips { nct7363 }; + +static const struct i2c_device_id nct7363_id[] = { + { "nct7363", nct7363 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, nct7363_id); + +static const struct of_device_id nct7363_of_match[] = { + { .compatible = "nuvoton,nct7363", .data = (void *)nct7363 }, + { }, +}; +MODULE_DEVICE_TABLE(of, nct7363_of_match); + +struct nct7363_data { + struct regmap *regmap; + struct mutex update_lock; + bool valid; + unsigned long last_updated; /* In jiffies */ + + u16 fanin_mask; + u16 fan[NCT7363_FANIN_COUNT]; + u16 pwm_mask; + u8 pwm[NCT7363_PWM_COUNT]; +}; + +static struct nct7363_data *nct7363_update_device(struct device *dev) +{ + struct nct7363_data *data = dev_get_drvdata(dev); + unsigned int hi, lo, regval; + int i, ret = 0; + + mutex_lock(&data->update_lock); + + if (!(time_after(jiffies, data->last_updated + REFRESH_INTERVAL) + || !data->valid)) + goto no_sensor_update; + + for (i = 0; i < ARRAY_SIZE(data->fan); i++) { + if (!(data->fanin_mask & BIT_CHECK(i))) + continue; + + /* + * High-byte register should be read first to latch + * synchronous low-byte value + */ + ret = regmap_read(data->regmap, + NCT7363_REG_FANINx_HVAL(i), &hi); + if (ret) + goto error; + + ret = regmap_read(data->regmap, + NCT7363_REG_FANINx_LVAL(i), &lo); + if (ret) + goto error; + + data->fan[i] = (hi << 5) | (lo & NCT7363_FANINx_LVAL_MASK); + } + + for (i = 0; i < ARRAY_SIZE(data->pwm); i++) { + if (!(data->pwm_mask & BIT_CHECK(i))) + continue; + + ret = regmap_read(data->regmap, + NCT7363_REG_FSCPxDUTY(i), ®val); + if (ret) + goto error; + + data->pwm[i] = regval; + } + + data->last_updated = jiffies; + data->valid = true; + +error: + if (ret) + data = ERR_PTR(ret); + +no_sensor_update: + mutex_unlock(&data->update_lock); + + return data; +} + +static int nct7363_read_fan(struct device *dev, u32 attr, int channel, + long *val) +{ + struct nct7363_data *data = nct7363_update_device(dev); + u16 cnt, rpm; + + if (IS_ERR(data)) + return PTR_ERR(data); + + switch (attr) { + case hwmon_fan_input: + cnt = data->fan[channel] & NCT7363_FANIN_MASK; + rpm = FAN_FROM_REG(cnt); + *val = (long)rpm; + return 0; + default: + return -EOPNOTSUPP; + } +} + +static umode_t nct7363_fan_is_visible(const void *_data, u32 attr, int channel) +{ + const struct nct7363_data *data = _data; + + switch (attr) { + case hwmon_fan_input: + if (data->fanin_mask & BIT_CHECK(channel)) + return 0444; + break; + default: + break; + } + + return 0; +} + +static int nct7363_read_pwm(struct device *dev, u32 attr, int channel, + long *val) +{ + struct nct7363_data *data = nct7363_update_device(dev); + u16 ret; + + if (IS_ERR(data)) + return PTR_ERR(data); + + switch (attr) { + case hwmon_pwm_input: + ret = data->pwm[channel]; + *val = (long)ret; + return 0; + default: + return -EOPNOTSUPP; + } +} + +static int nct7363_write_pwm(struct device *dev, u32 attr, int channel, + long val) +{ + struct nct7363_data *data = nct7363_update_device(dev); + int ret; + + if (IS_ERR(data)) + return PTR_ERR(data); + + switch (attr) { + case hwmon_pwm_input: + if (val < 0 || val > 255) + return -EINVAL; + mutex_lock(&data->update_lock); + ret = regmap_write(data->regmap, + NCT7363_REG_FSCPxDUTY(channel), val); + if (ret == 0) + data->pwm[channel] = val; + mutex_unlock(&data->update_lock); + return ret; + + default: + return -EOPNOTSUPP; + } +} + +static umode_t nct7363_pwm_is_visible(const void *_data, u32 attr, int channel) +{ + const struct nct7363_data *data = _data; + + switch (attr) { + case hwmon_pwm_input: + if (data->pwm_mask & BIT_CHECK(channel)) + return 0644; + break; + default: + break; + } + + return 0; +} + +static int nct7363_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + switch (type) { + case hwmon_fan: + return nct7363_read_fan(dev, attr, channel, val); + case hwmon_pwm: + return nct7363_read_pwm(dev, attr, channel, val); + default: + return -EOPNOTSUPP; + } +} + +static int nct7363_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + switch (type) { + case hwmon_pwm: + return nct7363_write_pwm(dev, attr, channel, val); + default: + return -EOPNOTSUPP; + } +} + +static umode_t nct7363_is_visible(const void *data, + enum hwmon_sensor_types type, + u32 attr, int channel) +{ + switch (type) { + case hwmon_fan: + return nct7363_fan_is_visible(data, attr, channel); + case hwmon_pwm: + return nct7363_pwm_is_visible(data, attr, channel); + default: + return 0; + } +} + +static const struct hwmon_channel_info *nct7363_info[] = { + HWMON_CHANNEL_INFO(fan, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT, + HWMON_F_INPUT), + HWMON_CHANNEL_INFO(pwm, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT, + HWMON_PWM_INPUT), + NULL +}; + +static const struct hwmon_ops nct7363_hwmon_ops = { + .is_visible = nct7363_is_visible, + .read = nct7363_read, + .write = nct7363_write, +}; + +static const struct hwmon_chip_info nct7363_chip_info = { + .ops = &nct7363_hwmon_ops, + .info = nct7363_info, +}; + +/* Return 0 if detection is successful, -ENODEV otherwise */ +static int nct7363_detect(struct i2c_client *client, + struct i2c_board_info *info) +{ + struct i2c_adapter *adapter = client->adapter; + int vendor, chip, device; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) + return -ENODEV; + + vendor = i2c_smbus_read_byte_data(client, NCT7363_REG_VENDOR_ID); + if (vendor != NUVOTON_ID) + return -ENODEV; + + chip = i2c_smbus_read_byte_data(client, NCT7363_REG_CHIP_ID); + if (chip != CHIP_ID) + return -ENODEV; + + device = i2c_smbus_read_byte_data(client, NCT7363_REG_DEVICE_ID); + if (device != DEVICE_ID) + return -ENODEV; + + strscpy(info->type, "nct7363", I2C_NAME_SIZE); + + return 0; +} + +static int nct7363_init_chip(struct nct7363_data *data) +{ + u8 i, gpio0_3 = 0, gpio4_7 = 0, gpio10_13 = 0, gpio14_17 = 0; + int ret; + + for (i = 0; i < NCT7363_PWM_COUNT; i++) { + if (i < 4) { + if (data->pwm_mask & BIT_CHECK(i)) + gpio0_3 |= PWM_SEL(i); + if (data->fanin_mask & BIT_CHECK(i)) + gpio10_13 |= FANIN_SEL(i); + } else if (i < 8) { + if (data->pwm_mask & BIT_CHECK(i)) + gpio4_7 |= PWM_SEL(i); + if (data->fanin_mask & BIT_CHECK(i)) + gpio14_17 |= FANIN_SEL(i); + } else if (i < 12) { + if (data->pwm_mask & BIT_CHECK(i)) + gpio10_13 |= PWM_SEL(i); + if (data->fanin_mask & BIT_CHECK(i)) + gpio0_3 |= FANIN_SEL(i); + } else { + if (data->pwm_mask & BIT_CHECK(i)) + gpio14_17 |= PWM_SEL(i); + if (data->fanin_mask & BIT_CHECK(i)) + gpio4_7 |= FANIN_SEL(i); + } + } + + /* Pin Function Configuration */ + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_0_3, gpio0_3); + if (ret < 0) + return ret; + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_4_7, gpio4_7); + if (ret < 0) + return ret; + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_10_13, gpio10_13); + if (ret < 0) + return ret; + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_14_17, gpio14_17); + if (ret < 0) + return ret; + + /* PWM and FANIN Monitoring Enable */ + ret = regmap_write(data->regmap, NCT7363_REG_PWMEN_0_7, + data->pwm_mask & 0xff); + if (ret < 0) + return ret; + ret = regmap_write(data->regmap, NCT7363_REG_PWMEN_8_15, + (data->pwm_mask >> 8) & 0xff); + if (ret < 0) + return ret; + ret = regmap_write(data->regmap, NCT7363_REG_FANINEN_0_7, + data->fanin_mask & 0xff); + if (ret < 0) + return ret; + ret = regmap_write(data->regmap, NCT7363_REG_FANINEN_8_15, + (data->fanin_mask >> 8) & 0xff); + if (ret < 0) + return ret; + + return 0; +} + +static int nct7363_present_pwm_fanin(struct device *dev, + struct device_node *child, + struct nct7363_data *data) +{ + struct of_phandle_args args; + int ret, fanin_cnt; + u8 *fanin_ch; + u8 ch, index; + + ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", + 0, &args); + if (ret) + return ret; + + data->pwm_mask |= BIT(args.args[0]); + + fanin_cnt = of_property_count_u8_elems(child, "tach-ch"); + if (fanin_cnt < 1) + return -EINVAL; + + fanin_ch = devm_kcalloc(dev, fanin_cnt, sizeof(*fanin_ch), GFP_KERNEL); + if (!fanin_ch) + return -ENOMEM; + + ret = of_property_read_u8_array(child, "tach-ch", fanin_ch, fanin_cnt); + if (ret) + return ret; + + for (ch = 0; ch < fanin_cnt; ch++) { + index = fanin_ch[ch]; + data->fanin_mask |= BIT(index); + } + + return 0; +} + +static const struct regmap_config nct7363_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int nct7363_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct device_node *child; + struct nct7363_data *data; + struct device *hwmon_dev; + int ret; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->regmap = devm_regmap_init_i2c(client, &nct7363_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + + mutex_init(&data->update_lock); + + for_each_child_of_node(dev->of_node, child) { + ret = nct7363_present_pwm_fanin(dev, child, data); + if (ret) { + of_node_put(child); + return ret; + } + } + + /* Initialize the chip */ + ret = nct7363_init_chip(data); + if (ret) + return ret; + + hwmon_dev = + devm_hwmon_device_register_with_info(dev, client->name, data, + &nct7363_chip_info, NULL); + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static struct i2c_driver nct7363_driver = { + .class = I2C_CLASS_HWMON, + .driver = { + .name = "nct7363", + .of_match_table = nct7363_of_match, + }, + .probe = nct7363_probe, + .id_table = nct7363_id, + .detect = nct7363_detect, + .address_list = normal_i2c, +}; + +module_i2c_driver(nct7363_driver); + +MODULE_AUTHOR("CW Ho <cwho@nuvoton.com>"); +MODULE_AUTHOR("Ban Feng <kcfeng0@nuvoton.com>"); +MODULE_DESCRIPTION("NCT7363 Hardware Monitoring Driver"); +MODULE_LICENSE("GPL"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y 2023-12-22 1:33 ` [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 @ 2024-02-03 15:06 ` Guenter Roeck 2024-02-21 1:26 ` Ban Feng 0 siblings, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2024-02-03 15:06 UTC (permalink / raw) To: baneric926 Cc: jdelvare, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai On Fri, Dec 22, 2023 at 09:33:52AM +0800, baneric926@gmail.com wrote: > From: Ban Feng <kcfeng0@nuvoton.com> > > NCT7363Y is an I2C based hardware monitoring chip from Nuvoton. > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> Sorry for the late reply. I was waiting for the fan schema to be accepted, but it looks like that may take more time. Please fix the various issues reported by checkpatch --strict. Additional comments inline. > --- > Documentation/hwmon/index.rst | 1 + > Documentation/hwmon/nct7363.rst | 33 ++ > MAINTAINERS | 2 + > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/nct7363.c | 515 ++++++++++++++++++++++++++++++++ > 6 files changed, 563 insertions(+) > create mode 100644 Documentation/hwmon/nct7363.rst > create mode 100644 drivers/hwmon/nct7363.c > > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst > index 72f4e6065bae..178d3cae95de 100644 > --- a/Documentation/hwmon/index.rst > +++ b/Documentation/hwmon/index.rst > @@ -161,6 +161,7 @@ Hardware Monitoring Kernel Drivers > mp5023 > nct6683 > nct6775 > + nct7363 > nct7802 > nct7904 > npcm750-pwm-fan > diff --git a/Documentation/hwmon/nct7363.rst b/Documentation/hwmon/nct7363.rst > new file mode 100644 > index 000000000000..89699c95aa4b > --- /dev/null > +++ b/Documentation/hwmon/nct7363.rst > @@ -0,0 +1,33 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +Kernel driver nct7363 > +===================== > + > +Supported chip: > + > + * Nuvoton NCT7363Y > + > + Prefix: nct7363 > + > + Addresses: I2C 0x20, 0x21, 0x22, 0x23 > + > +Author: Ban Feng <kcfeng0@nuvoton.com> > + > + > +Description > +----------- > + > +The NCT7363Y is a Fan controller which provides up to 16 independent > +FAN input monitors, and up to 16 independent PWM output with SMBus interface. > + > + > +Sysfs entries > +------------- > + > +Currently, the driver supports the following features: > + > +======================= ======================================================= > +fanX_input provide current fan rotation value in RPM > + > +pwmX get or set PWM fan control value. > +======================= ======================================================= > diff --git a/MAINTAINERS b/MAINTAINERS > index 53cfcc629aa1..e39c4fc01a3b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14842,6 +14842,8 @@ M: Ban Feng <kcfeng0@nuvoton.com> > L: linux-hwmon@vger.kernel.org > S: Maintained > F: Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml > +F: Documentation/hwmon/nct7363.rst > +F: drivers/hwmon/nct7363.c > > NETDEVSIM > M: Jakub Kicinski <kuba@kernel.org> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index cf27523eed5a..a0229851fc64 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1605,6 +1605,17 @@ config SENSORS_NCT6775_I2C > This driver can also be built as a module. If so, the module > will be called nct6775-i2c. > > +config SENSORS_NCT7363 > + tristate "Nuvoton NCT7363Y" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for the Nuvoton NCT7363Y, > + hardware monitoring chip. > + > + This driver can also be built as a module. If so, the module > + will be called nct7363. > + > config SENSORS_NCT7802 > tristate "Nuvoton NCT7802Y" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index e84bd9685b5c..dd794aa06209 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -166,6 +166,7 @@ obj-$(CONFIG_SENSORS_NCT6775_CORE) += nct6775-core.o > nct6775-objs := nct6775-platform.o > obj-$(CONFIG_SENSORS_NCT6775) += nct6775.o > obj-$(CONFIG_SENSORS_NCT6775_I2C) += nct6775-i2c.o > +obj-$(CONFIG_SENSORS_NCT7363) += nct7363.o > obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o > obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o > obj-$(CONFIG_SENSORS_NPCM7XX) += npcm750-pwm-fan.o > diff --git a/drivers/hwmon/nct7363.c b/drivers/hwmon/nct7363.c > new file mode 100644 > index 000000000000..1bf6e83afd7f > --- /dev/null > +++ b/drivers/hwmon/nct7363.c > @@ -0,0 +1,515 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2023 Nuvoton Technology corporation. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/err.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/i2c.h> > +#include <linux/jiffies.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#define NCT7363_REG_GPIO_0_3 0x20 > +#define NCT7363_REG_GPIO_4_7 0x21 > +#define NCT7363_REG_GPIO_10_13 0x22 > +#define NCT7363_REG_GPIO_14_17 0x23 > +#define NCT7363_REG_PWMEN_0_7 0x38 > +#define NCT7363_REG_PWMEN_8_15 0x39 > +#define NCT7363_REG_FANINEN_0_7 0x41 > +#define NCT7363_REG_FANINEN_8_15 0x42 > +#define NCT7363_REG_FANINx_HVAL(x) (0x48 + ((x) * 2)) > +#define NCT7363_REG_FANINx_LVAL(x) (0x49 + ((x) * 2)) > +#define NCT7363_REG_FSCPxDUTY(x) (0x90 + ((x) * 2)) > +#define NCT7363_REG_VENDOR_ID 0xFD > +#define NCT7363_REG_CHIP_ID 0xFE > +#define NCT7363_REG_DEVICE_ID 0xFF > + > +#define NUVOTON_ID 0x49 > +#define CHIP_ID 0x19 > +#define DEVICE_ID 0x88 > + > +#define PWM_SEL(x) (BIT(0) << ((x % 4) * 2)) > +#define FANIN_SEL(x) (BIT(1) << ((x % 4) * 2)) (x). checkpatch -strict does report this. > +#define BIT_CHECK(x) (BIT(0) << x) This is identical to BIT(x) so I really don't get the point. > + > +#define NCT7363_FANINx_LVAL_MASK GENMASK(4, 0) No CamelCase defines or variables, please. > +#define NCT7363_FANIN_MASK GENMASK(12, 0) > + > +#define NCT7363_PWM_COUNT 16 > +#define NCT7363_FANIN_COUNT 16 > + > +#define REFRESH_INTERVAL (2 * HZ) > + > +static inline unsigned long FAN_FROM_REG(u16 val) > +{ > + if ((val >= NCT7363_FANIN_MASK) || (val == 0)) Unnecessary () as reported by checkpatch. Too bad the datasheet isn't public. It would be useful to know what it means if the upper bits are set. Unconditionally reporting a fan speed of 0 if any of the bits are set seems excessive unless there is a good reason for it. If those bits indicate fan faults, it might make sense to implement respective attributes. [ Why do you keep this chip and NCT7362Y so secretive ? ] > + return 0; > + > + return (1350000UL / val); > +} > + > +static const unsigned short normal_i2c[] = { > + 0x20, 0x21, 0x22, 0x23, I2C_CLIENT_END > +}; > + > +enum chips { nct7363 }; Unnecessary. If there are plans for this driver to support more chips in the future, the enum can be added at that time. Yes, I can see in Nuvoton's 2024 product selection document that there is a NCT7362Y with the same number of fan and pwm channels, but that doesn't help if the chips are kept under wrap. Searching for the chips on the internet, I am reminded of https://lore.kernel.org/lkml/20230607101827.8544-4-zev@bewilderbeest.net/T/ which introduces a quite similar driver with more functionality for NCT7362Y. That submission even mentions a NCT7360 with 8 channels. Quite frankly I am inclined to reject both drivers (not that I ever got an update for the NCT7362Y driver, but anyway) until I get a better understanding of the similarities and differences between NCT7362Y and NCT7363Y. Please consider sending me datasheets for those chips. > + > +static const struct i2c_device_id nct7363_id[] = { > + { "nct7363", nct7363 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, nct7363_id); > + > +static const struct of_device_id nct7363_of_match[] = { > + { .compatible = "nuvoton,nct7363", .data = (void *)nct7363 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, nct7363_of_match); > + > +struct nct7363_data { > + struct regmap *regmap; > + struct mutex update_lock; > + bool valid; > + unsigned long last_updated; /* In jiffies */ > + > + u16 fanin_mask; > + u16 fan[NCT7363_FANIN_COUNT]; > + u16 pwm_mask; > + u8 pwm[NCT7363_PWM_COUNT]; > +}; > + > +static struct nct7363_data *nct7363_update_device(struct device *dev) Personally I recommend to drop all local caching and just read values through regmap as requested. > +{ > + struct nct7363_data *data = dev_get_drvdata(dev); > + unsigned int hi, lo, regval; > + int i, ret = 0; > + > + mutex_lock(&data->update_lock); > + > + if (!(time_after(jiffies, data->last_updated + REFRESH_INTERVAL) > + || !data->valid)) > + goto no_sensor_update; > + > + for (i = 0; i < ARRAY_SIZE(data->fan); i++) { > + if (!(data->fanin_mask & BIT_CHECK(i))) > + continue; > + > + /* > + * High-byte register should be read first to latch > + * synchronous low-byte value > + */ > + ret = regmap_read(data->regmap, > + NCT7363_REG_FANINx_HVAL(i), &hi); > + if (ret) > + goto error; > + > + ret = regmap_read(data->regmap, > + NCT7363_REG_FANINx_LVAL(i), &lo); > + if (ret) > + goto error; > + > + data->fan[i] = (hi << 5) | (lo & NCT7363_FANINx_LVAL_MASK); > + } > + > + for (i = 0; i < ARRAY_SIZE(data->pwm); i++) { > + if (!(data->pwm_mask & BIT_CHECK(i))) > + continue; > + > + ret = regmap_read(data->regmap, > + NCT7363_REG_FSCPxDUTY(i), ®val); > + if (ret) > + goto error; > + > + data->pwm[i] = regval; > + } > + > + data->last_updated = jiffies; > + data->valid = true; > + > +error: > + if (ret) > + data = ERR_PTR(ret); > + > +no_sensor_update: > + mutex_unlock(&data->update_lock); > + > + return data; > +} > + > +static int nct7363_read_fan(struct device *dev, u32 attr, int channel, > + long *val) > +{ > + struct nct7363_data *data = nct7363_update_device(dev); > + u16 cnt, rpm; > + > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + switch (attr) { > + case hwmon_fan_input: > + cnt = data->fan[channel] & NCT7363_FANIN_MASK; > + rpm = FAN_FROM_REG(cnt); > + *val = (long)rpm; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static umode_t nct7363_fan_is_visible(const void *_data, u32 attr, int channel) > +{ > + const struct nct7363_data *data = _data; > + > + switch (attr) { > + case hwmon_fan_input: > + if (data->fanin_mask & BIT_CHECK(channel)) > + return 0444; > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static int nct7363_read_pwm(struct device *dev, u32 attr, int channel, > + long *val) > +{ > + struct nct7363_data *data = nct7363_update_device(dev); > + u16 ret; > + > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + switch (attr) { > + case hwmon_pwm_input: > + ret = data->pwm[channel]; > + *val = (long)ret; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int nct7363_write_pwm(struct device *dev, u32 attr, int channel, > + long val) > +{ > + struct nct7363_data *data = nct7363_update_device(dev); > + int ret; > + > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + switch (attr) { > + case hwmon_pwm_input: > + if (val < 0 || val > 255) > + return -EINVAL; > + mutex_lock(&data->update_lock); > + ret = regmap_write(data->regmap, > + NCT7363_REG_FSCPxDUTY(channel), val); > + if (ret == 0) > + data->pwm[channel] = val; > + mutex_unlock(&data->update_lock); > + return ret; > + > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static umode_t nct7363_pwm_is_visible(const void *_data, u32 attr, int channel) > +{ > + const struct nct7363_data *data = _data; > + > + switch (attr) { > + case hwmon_pwm_input: > + if (data->pwm_mask & BIT_CHECK(channel)) > + return 0644; > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static int nct7363_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + switch (type) { > + case hwmon_fan: > + return nct7363_read_fan(dev, attr, channel, val); > + case hwmon_pwm: > + return nct7363_read_pwm(dev, attr, channel, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int nct7363_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + switch (type) { > + case hwmon_pwm: > + return nct7363_write_pwm(dev, attr, channel, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static umode_t nct7363_is_visible(const void *data, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + case hwmon_fan: > + return nct7363_fan_is_visible(data, attr, channel); > + case hwmon_pwm: > + return nct7363_pwm_is_visible(data, attr, channel); > + default: > + return 0; > + } > +} > + > +static const struct hwmon_channel_info *nct7363_info[] = { > + HWMON_CHANNEL_INFO(fan, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT, > + HWMON_F_INPUT), > + HWMON_CHANNEL_INFO(pwm, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT), > + NULL > +}; > + > +static const struct hwmon_ops nct7363_hwmon_ops = { > + .is_visible = nct7363_is_visible, > + .read = nct7363_read, > + .write = nct7363_write, > +}; > + > +static const struct hwmon_chip_info nct7363_chip_info = { > + .ops = &nct7363_hwmon_ops, > + .info = nct7363_info, > +}; > + > +/* Return 0 if detection is successful, -ENODEV otherwise */ > +static int nct7363_detect(struct i2c_client *client, > + struct i2c_board_info *info) > +{ > + struct i2c_adapter *adapter = client->adapter; > + int vendor, chip, device; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > + return -ENODEV; > + > + vendor = i2c_smbus_read_byte_data(client, NCT7363_REG_VENDOR_ID); > + if (vendor != NUVOTON_ID) > + return -ENODEV; > + > + chip = i2c_smbus_read_byte_data(client, NCT7363_REG_CHIP_ID); > + if (chip != CHIP_ID) > + return -ENODEV; > + > + device = i2c_smbus_read_byte_data(client, NCT7363_REG_DEVICE_ID); > + if (device != DEVICE_ID) > + return -ENODEV; > + > + strscpy(info->type, "nct7363", I2C_NAME_SIZE); > + > + return 0; > +} Chip auto-detection is undesirable for new drivers since it increases boot time. Please consider dropping this if it is not really needed for some actual use case. > + > +static int nct7363_init_chip(struct nct7363_data *data) > +{ > + u8 i, gpio0_3 = 0, gpio4_7 = 0, gpio10_13 = 0, gpio14_17 = 0; > + int ret; > + > + for (i = 0; i < NCT7363_PWM_COUNT; i++) { > + if (i < 4) { > + if (data->pwm_mask & BIT_CHECK(i)) > + gpio0_3 |= PWM_SEL(i); > + if (data->fanin_mask & BIT_CHECK(i)) > + gpio10_13 |= FANIN_SEL(i); > + } else if (i < 8) { > + if (data->pwm_mask & BIT_CHECK(i)) > + gpio4_7 |= PWM_SEL(i); > + if (data->fanin_mask & BIT_CHECK(i)) > + gpio14_17 |= FANIN_SEL(i); > + } else if (i < 12) { > + if (data->pwm_mask & BIT_CHECK(i)) > + gpio10_13 |= PWM_SEL(i); > + if (data->fanin_mask & BIT_CHECK(i)) > + gpio0_3 |= FANIN_SEL(i); > + } else { > + if (data->pwm_mask & BIT_CHECK(i)) > + gpio14_17 |= PWM_SEL(i); > + if (data->fanin_mask & BIT_CHECK(i)) > + gpio4_7 |= FANIN_SEL(i); > + } > + } > + > + /* Pin Function Configuration */ > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_0_3, gpio0_3); > + if (ret < 0) > + return ret; > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_4_7, gpio4_7); > + if (ret < 0) > + return ret; > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_10_13, gpio10_13); > + if (ret < 0) > + return ret; > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_14_17, gpio14_17); > + if (ret < 0) > + return ret; > + > + /* PWM and FANIN Monitoring Enable */ > + ret = regmap_write(data->regmap, NCT7363_REG_PWMEN_0_7, > + data->pwm_mask & 0xff); > + if (ret < 0) > + return ret; > + ret = regmap_write(data->regmap, NCT7363_REG_PWMEN_8_15, > + (data->pwm_mask >> 8) & 0xff); > + if (ret < 0) > + return ret; > + ret = regmap_write(data->regmap, NCT7363_REG_FANINEN_0_7, > + data->fanin_mask & 0xff); > + if (ret < 0) > + return ret; > + ret = regmap_write(data->regmap, NCT7363_REG_FANINEN_8_15, > + (data->fanin_mask >> 8) & 0xff); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > +static int nct7363_present_pwm_fanin(struct device *dev, > + struct device_node *child, > + struct nct7363_data *data) > +{ > + struct of_phandle_args args; > + int ret, fanin_cnt; > + u8 *fanin_ch; > + u8 ch, index; > + > + ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", > + 0, &args); > + if (ret) > + return ret; > + > + data->pwm_mask |= BIT(args.args[0]); > + > + fanin_cnt = of_property_count_u8_elems(child, "tach-ch"); > + if (fanin_cnt < 1) > + return -EINVAL; > + > + fanin_ch = devm_kcalloc(dev, fanin_cnt, sizeof(*fanin_ch), GFP_KERNEL); > + if (!fanin_ch) > + return -ENOMEM; > + > + ret = of_property_read_u8_array(child, "tach-ch", fanin_ch, fanin_cnt); > + if (ret) > + return ret; > + > + for (ch = 0; ch < fanin_cnt; ch++) { > + index = fanin_ch[ch]; > + data->fanin_mask |= BIT(index); > + } > + > + return 0; > +} I didn't check if this aligns with the fan bindings schema. Unfortunately the dt maintainer's reviewed-by: tag got lost with https://patchwork.kernel.org/project/linux-hwmon/patch/20231222013352.3873689-2-kcfeng0@nuvoton.com/ o we can not really move forward until that is resolved. > + > +static const struct regmap_config nct7363_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > +static int nct7363_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct device_node *child; > + struct nct7363_data *data; > + struct device *hwmon_dev; > + int ret; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->regmap = devm_regmap_init_i2c(client, &nct7363_regmap_config); > + if (IS_ERR(data->regmap)) > + return PTR_ERR(data->regmap); > + > + mutex_init(&data->update_lock); > + > + for_each_child_of_node(dev->of_node, child) { > + ret = nct7363_present_pwm_fanin(dev, child, data); > + if (ret) { > + of_node_put(child); > + return ret; > + } > + } > + > + /* Initialize the chip */ > + ret = nct7363_init_chip(data); > + if (ret) > + return ret; > + > + hwmon_dev = > + devm_hwmon_device_register_with_info(dev, client->name, data, > + &nct7363_chip_info, NULL); > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static struct i2c_driver nct7363_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = "nct7363", > + .of_match_table = nct7363_of_match, > + }, > + .probe = nct7363_probe, > + .id_table = nct7363_id, > + .detect = nct7363_detect, > + .address_list = normal_i2c, > +}; > + > +module_i2c_driver(nct7363_driver); > + > +MODULE_AUTHOR("CW Ho <cwho@nuvoton.com>"); > +MODULE_AUTHOR("Ban Feng <kcfeng0@nuvoton.com>"); > +MODULE_DESCRIPTION("NCT7363 Hardware Monitoring Driver"); > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y 2024-02-03 15:06 ` Guenter Roeck @ 2024-02-21 1:26 ` Ban Feng 0 siblings, 0 replies; 11+ messages in thread From: Ban Feng @ 2024-02-21 1:26 UTC (permalink / raw) To: Guenter Roeck Cc: jdelvare, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, openbmc, kwliu, kcfeng0, DELPHINE_CHIU, Bonnie_Lo, naresh.solanki, billy_tsai Hi Guenter, On Sat, Feb 3, 2024 at 11:06 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Fri, Dec 22, 2023 at 09:33:52AM +0800, baneric926@gmail.com wrote: > > From: Ban Feng <kcfeng0@nuvoton.com> > > > > NCT7363Y is an I2C based hardware monitoring chip from Nuvoton. > > > > Signed-off-by: Ban Feng <kcfeng0@nuvoton.com> > > Sorry for the late reply. I was waiting for the fan schema to be accepted, > but it looks like that may take more time. > > Please fix the various issues reported by checkpatch --strict. > Additional comments inline. ok, fix in v4 > > > --- > > Documentation/hwmon/index.rst | 1 + > > Documentation/hwmon/nct7363.rst | 33 ++ > > MAINTAINERS | 2 + > > drivers/hwmon/Kconfig | 11 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/nct7363.c | 515 ++++++++++++++++++++++++++++++++ > > 6 files changed, 563 insertions(+) > > create mode 100644 Documentation/hwmon/nct7363.rst > > create mode 100644 drivers/hwmon/nct7363.c > > > > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst > > index 72f4e6065bae..178d3cae95de 100644 > > --- a/Documentation/hwmon/index.rst > > +++ b/Documentation/hwmon/index.rst > > @@ -161,6 +161,7 @@ Hardware Monitoring Kernel Drivers > > mp5023 > > nct6683 > > nct6775 > > + nct7363 > > nct7802 > > nct7904 > > npcm750-pwm-fan > > diff --git a/Documentation/hwmon/nct7363.rst b/Documentation/hwmon/nct7363.rst > > new file mode 100644 > > index 000000000000..89699c95aa4b > > --- /dev/null > > +++ b/Documentation/hwmon/nct7363.rst > > @@ -0,0 +1,33 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +Kernel driver nct7363 > > +===================== > > + > > +Supported chip: > > + > > + * Nuvoton NCT7363Y > > + > > + Prefix: nct7363 > > + > > + Addresses: I2C 0x20, 0x21, 0x22, 0x23 > > + > > +Author: Ban Feng <kcfeng0@nuvoton.com> > > + > > + > > +Description > > +----------- > > + > > +The NCT7363Y is a Fan controller which provides up to 16 independent > > +FAN input monitors, and up to 16 independent PWM output with SMBus interface. > > + > > + > > +Sysfs entries > > +------------- > > + > > +Currently, the driver supports the following features: > > + > > +======================= ======================================================= > > +fanX_input provide current fan rotation value in RPM > > + > > +pwmX get or set PWM fan control value. > > +======================= ======================================================= > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 53cfcc629aa1..e39c4fc01a3b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -14842,6 +14842,8 @@ M: Ban Feng <kcfeng0@nuvoton.com> > > L: linux-hwmon@vger.kernel.org > > S: Maintained > > F: Documentation/devicetree/bindings/hwmon/nuvoton,nct7363.yaml > > +F: Documentation/hwmon/nct7363.rst > > +F: drivers/hwmon/nct7363.c > > > > NETDEVSIM > > M: Jakub Kicinski <kuba@kernel.org> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index cf27523eed5a..a0229851fc64 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -1605,6 +1605,17 @@ config SENSORS_NCT6775_I2C > > This driver can also be built as a module. If so, the module > > will be called nct6775-i2c. > > > > +config SENSORS_NCT7363 > > + tristate "Nuvoton NCT7363Y" > > + depends on I2C > > + select REGMAP_I2C > > + help > > + If you say yes here you get support for the Nuvoton NCT7363Y, > > + hardware monitoring chip. > > + > > + This driver can also be built as a module. If so, the module > > + will be called nct7363. > > + > > config SENSORS_NCT7802 > > tristate "Nuvoton NCT7802Y" > > depends on I2C > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > > index e84bd9685b5c..dd794aa06209 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -166,6 +166,7 @@ obj-$(CONFIG_SENSORS_NCT6775_CORE) += nct6775-core.o > > nct6775-objs := nct6775-platform.o > > obj-$(CONFIG_SENSORS_NCT6775) += nct6775.o > > obj-$(CONFIG_SENSORS_NCT6775_I2C) += nct6775-i2c.o > > +obj-$(CONFIG_SENSORS_NCT7363) += nct7363.o > > obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o > > obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o > > obj-$(CONFIG_SENSORS_NPCM7XX) += npcm750-pwm-fan.o > > diff --git a/drivers/hwmon/nct7363.c b/drivers/hwmon/nct7363.c > > new file mode 100644 > > index 000000000000..1bf6e83afd7f > > --- /dev/null > > +++ b/drivers/hwmon/nct7363.c > > @@ -0,0 +1,515 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2023 Nuvoton Technology corporation. > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/bits.h> > > +#include <linux/err.h> > > +#include <linux/hwmon.h> > > +#include <linux/hwmon-sysfs.h> > > +#include <linux/i2c.h> > > +#include <linux/jiffies.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/regmap.h> > > +#include <linux/slab.h> > > + > > +#define NCT7363_REG_GPIO_0_3 0x20 > > +#define NCT7363_REG_GPIO_4_7 0x21 > > +#define NCT7363_REG_GPIO_10_13 0x22 > > +#define NCT7363_REG_GPIO_14_17 0x23 > > +#define NCT7363_REG_PWMEN_0_7 0x38 > > +#define NCT7363_REG_PWMEN_8_15 0x39 > > +#define NCT7363_REG_FANINEN_0_7 0x41 > > +#define NCT7363_REG_FANINEN_8_15 0x42 > > +#define NCT7363_REG_FANINx_HVAL(x) (0x48 + ((x) * 2)) > > +#define NCT7363_REG_FANINx_LVAL(x) (0x49 + ((x) * 2)) > > +#define NCT7363_REG_FSCPxDUTY(x) (0x90 + ((x) * 2)) > > +#define NCT7363_REG_VENDOR_ID 0xFD > > +#define NCT7363_REG_CHIP_ID 0xFE > > +#define NCT7363_REG_DEVICE_ID 0xFF > > + > > +#define NUVOTON_ID 0x49 > > +#define CHIP_ID 0x19 > > +#define DEVICE_ID 0x88 > > + > > +#define PWM_SEL(x) (BIT(0) << ((x % 4) * 2)) > > +#define FANIN_SEL(x) (BIT(1) << ((x % 4) * 2)) > > (x). checkpatch -strict does report this. ok, fix in v4 > > > +#define BIT_CHECK(x) (BIT(0) << x) > > This is identical to BIT(x) so I really don't get the point. ok, fix in v4 > > > + > > +#define NCT7363_FANINx_LVAL_MASK GENMASK(4, 0) > > No CamelCase defines or variables, please. ok, fix in v4 > > > +#define NCT7363_FANIN_MASK GENMASK(12, 0) > > + > > +#define NCT7363_PWM_COUNT 16 > > +#define NCT7363_FANIN_COUNT 16 > > + > > +#define REFRESH_INTERVAL (2 * HZ) > > + > > +static inline unsigned long FAN_FROM_REG(u16 val) > > +{ > > + if ((val >= NCT7363_FANIN_MASK) || (val == 0)) > > Unnecessary () as reported by checkpatch. ok, fix in v4 > > Too bad the datasheet isn't public. It would be useful to know what > it means if the upper bits are set. Unconditionally reporting a fan > speed of 0 if any of the bits are set seems excessive unless there > is a good reason for it. If those bits indicate fan faults, > it might make sense to implement respective attributes. > > [ Why do you keep this chip and NCT7362Y so secretive ? ] It's only a double check for the bit range from 0 to 12. I'll consider if this is still needed after dropping the local caching. > > > + return 0; > > + > > + return (1350000UL / val); > > +} > > + > > +static const unsigned short normal_i2c[] = { > > + 0x20, 0x21, 0x22, 0x23, I2C_CLIENT_END > > +}; > > + > > +enum chips { nct7363 }; > > Unnecessary. If there are plans for this driver to support more chips > in the future, the enum can be added at that time. Yes, I can see in > Nuvoton's 2024 product selection document that there is a NCT7362Y > with the same number of fan and pwm channels, but that doesn't help > if the chips are kept under wrap. so far only focus on NCT7363Y, I'll fix in v4 > > Searching for the chips on the internet, I am reminded of > https://lore.kernel.org/lkml/20230607101827.8544-4-zev@bewilderbeest.net/T/ > which introduces a quite similar driver with more functionality for NCT7362Y. > That submission even mentions a NCT7360 with 8 channels. Quite frankly > I am inclined to reject both drivers (not that I ever got an update for > the NCT7362Y driver, but anyway) until I get a better understanding > of the similarities and differences between NCT7362Y and NCT7363Y. > Please consider sending me datasheets for those chips. Sure, I'll send the relevant datasheet to you. > > > + > > +static const struct i2c_device_id nct7363_id[] = { > > + { "nct7363", nct7363 }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(i2c, nct7363_id); > > + > > +static const struct of_device_id nct7363_of_match[] = { > > + { .compatible = "nuvoton,nct7363", .data = (void *)nct7363 }, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, nct7363_of_match); > > + > > +struct nct7363_data { > > + struct regmap *regmap; > > + struct mutex update_lock; > > + bool valid; > > + unsigned long last_updated; /* In jiffies */ > > + > > + u16 fanin_mask; > > + u16 fan[NCT7363_FANIN_COUNT]; > > + u16 pwm_mask; > > + u8 pwm[NCT7363_PWM_COUNT]; > > +}; > > + > > +static struct nct7363_data *nct7363_update_device(struct device *dev) > > Personally I recommend to drop all local caching and just read values > through regmap as requested. ok, fix in v4 > > > +{ > > + struct nct7363_data *data = dev_get_drvdata(dev); > > + unsigned int hi, lo, regval; > > + int i, ret = 0; > > + > > + mutex_lock(&data->update_lock); > > + > > + if (!(time_after(jiffies, data->last_updated + REFRESH_INTERVAL) > > + || !data->valid)) > > + goto no_sensor_update; > > + > > + for (i = 0; i < ARRAY_SIZE(data->fan); i++) { > > + if (!(data->fanin_mask & BIT_CHECK(i))) > > + continue; > > + > > + /* > > + * High-byte register should be read first to latch > > + * synchronous low-byte value > > + */ > > + ret = regmap_read(data->regmap, > > + NCT7363_REG_FANINx_HVAL(i), &hi); > > + if (ret) > > + goto error; > > + > > + ret = regmap_read(data->regmap, > > + NCT7363_REG_FANINx_LVAL(i), &lo); > > + if (ret) > > + goto error; > > + > > + data->fan[i] = (hi << 5) | (lo & NCT7363_FANINx_LVAL_MASK); > > + } > > + > > + for (i = 0; i < ARRAY_SIZE(data->pwm); i++) { > > + if (!(data->pwm_mask & BIT_CHECK(i))) > > + continue; > > + > > + ret = regmap_read(data->regmap, > > + NCT7363_REG_FSCPxDUTY(i), ®val); > > + if (ret) > > + goto error; > > + > > + data->pwm[i] = regval; > > + } > > + > > + data->last_updated = jiffies; > > + data->valid = true; > > + > > +error: > > + if (ret) > > + data = ERR_PTR(ret); > > + > > +no_sensor_update: > > + mutex_unlock(&data->update_lock); > > + > > + return data; > > +} > > + > > +static int nct7363_read_fan(struct device *dev, u32 attr, int channel, > > + long *val) > > +{ > > + struct nct7363_data *data = nct7363_update_device(dev); > > + u16 cnt, rpm; > > + > > + if (IS_ERR(data)) > > + return PTR_ERR(data); > > + > > + switch (attr) { > > + case hwmon_fan_input: > > + cnt = data->fan[channel] & NCT7363_FANIN_MASK; > > + rpm = FAN_FROM_REG(cnt); > > + *val = (long)rpm; > > + return 0; > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static umode_t nct7363_fan_is_visible(const void *_data, u32 attr, int channel) > > +{ > > + const struct nct7363_data *data = _data; > > + > > + switch (attr) { > > + case hwmon_fan_input: > > + if (data->fanin_mask & BIT_CHECK(channel)) > > + return 0444; > > + break; > > + default: > > + break; > > + } > > + > > + return 0; > > +} > > + > > +static int nct7363_read_pwm(struct device *dev, u32 attr, int channel, > > + long *val) > > +{ > > + struct nct7363_data *data = nct7363_update_device(dev); > > + u16 ret; > > + > > + if (IS_ERR(data)) > > + return PTR_ERR(data); > > + > > + switch (attr) { > > + case hwmon_pwm_input: > > + ret = data->pwm[channel]; > > + *val = (long)ret; > > + return 0; > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int nct7363_write_pwm(struct device *dev, u32 attr, int channel, > > + long val) > > +{ > > + struct nct7363_data *data = nct7363_update_device(dev); > > + int ret; > > + > > + if (IS_ERR(data)) > > + return PTR_ERR(data); > > + > > + switch (attr) { > > + case hwmon_pwm_input: > > + if (val < 0 || val > 255) > > + return -EINVAL; > > + mutex_lock(&data->update_lock); > > + ret = regmap_write(data->regmap, > > + NCT7363_REG_FSCPxDUTY(channel), val); > > + if (ret == 0) > > + data->pwm[channel] = val; > > + mutex_unlock(&data->update_lock); > > + return ret; > > + > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static umode_t nct7363_pwm_is_visible(const void *_data, u32 attr, int channel) > > +{ > > + const struct nct7363_data *data = _data; > > + > > + switch (attr) { > > + case hwmon_pwm_input: > > + if (data->pwm_mask & BIT_CHECK(channel)) > > + return 0644; > > + break; > > + default: > > + break; > > + } > > + > > + return 0; > > +} > > + > > +static int nct7363_read(struct device *dev, enum hwmon_sensor_types type, > > + u32 attr, int channel, long *val) > > +{ > > + switch (type) { > > + case hwmon_fan: > > + return nct7363_read_fan(dev, attr, channel, val); > > + case hwmon_pwm: > > + return nct7363_read_pwm(dev, attr, channel, val); > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static int nct7363_write(struct device *dev, enum hwmon_sensor_types type, > > + u32 attr, int channel, long val) > > +{ > > + switch (type) { > > + case hwmon_pwm: > > + return nct7363_write_pwm(dev, attr, channel, val); > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > > + > > +static umode_t nct7363_is_visible(const void *data, > > + enum hwmon_sensor_types type, > > + u32 attr, int channel) > > +{ > > + switch (type) { > > + case hwmon_fan: > > + return nct7363_fan_is_visible(data, attr, channel); > > + case hwmon_pwm: > > + return nct7363_pwm_is_visible(data, attr, channel); > > + default: > > + return 0; > > + } > > +} > > + > > +static const struct hwmon_channel_info *nct7363_info[] = { > > + HWMON_CHANNEL_INFO(fan, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT, > > + HWMON_F_INPUT), > > + HWMON_CHANNEL_INFO(pwm, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT, > > + HWMON_PWM_INPUT), > > + NULL > > +}; > > + > > +static const struct hwmon_ops nct7363_hwmon_ops = { > > + .is_visible = nct7363_is_visible, > > + .read = nct7363_read, > > + .write = nct7363_write, > > +}; > > + > > +static const struct hwmon_chip_info nct7363_chip_info = { > > + .ops = &nct7363_hwmon_ops, > > + .info = nct7363_info, > > +}; > > + > > +/* Return 0 if detection is successful, -ENODEV otherwise */ > > +static int nct7363_detect(struct i2c_client *client, > > + struct i2c_board_info *info) > > +{ > > + struct i2c_adapter *adapter = client->adapter; > > + int vendor, chip, device; > > + > > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > > + return -ENODEV; > > + > > + vendor = i2c_smbus_read_byte_data(client, NCT7363_REG_VENDOR_ID); > > + if (vendor != NUVOTON_ID) > > + return -ENODEV; > > + > > + chip = i2c_smbus_read_byte_data(client, NCT7363_REG_CHIP_ID); > > + if (chip != CHIP_ID) > > + return -ENODEV; > > + > > + device = i2c_smbus_read_byte_data(client, NCT7363_REG_DEVICE_ID); > > + if (device != DEVICE_ID) > > + return -ENODEV; > > + > > + strscpy(info->type, "nct7363", I2C_NAME_SIZE); > > + > > + return 0; > > +} > > Chip auto-detection is undesirable for new drivers since it increases > boot time. Please consider dropping this if it is not really needed > for some actual use case. ok, fix in v4 > > > + > > +static int nct7363_init_chip(struct nct7363_data *data) > > +{ > > + u8 i, gpio0_3 = 0, gpio4_7 = 0, gpio10_13 = 0, gpio14_17 = 0; > > + int ret; > > + > > + for (i = 0; i < NCT7363_PWM_COUNT; i++) { > > + if (i < 4) { > > + if (data->pwm_mask & BIT_CHECK(i)) > > + gpio0_3 |= PWM_SEL(i); > > + if (data->fanin_mask & BIT_CHECK(i)) > > + gpio10_13 |= FANIN_SEL(i); > > + } else if (i < 8) { > > + if (data->pwm_mask & BIT_CHECK(i)) > > + gpio4_7 |= PWM_SEL(i); > > + if (data->fanin_mask & BIT_CHECK(i)) > > + gpio14_17 |= FANIN_SEL(i); > > + } else if (i < 12) { > > + if (data->pwm_mask & BIT_CHECK(i)) > > + gpio10_13 |= PWM_SEL(i); > > + if (data->fanin_mask & BIT_CHECK(i)) > > + gpio0_3 |= FANIN_SEL(i); > > + } else { > > + if (data->pwm_mask & BIT_CHECK(i)) > > + gpio14_17 |= PWM_SEL(i); > > + if (data->fanin_mask & BIT_CHECK(i)) > > + gpio4_7 |= FANIN_SEL(i); > > + } > > + } > > + > > + /* Pin Function Configuration */ > > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_0_3, gpio0_3); > > + if (ret < 0) > > + return ret; > > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_4_7, gpio4_7); > > + if (ret < 0) > > + return ret; > > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_10_13, gpio10_13); > > + if (ret < 0) > > + return ret; > > + ret = regmap_write(data->regmap, NCT7363_REG_GPIO_14_17, gpio14_17); > > + if (ret < 0) > > + return ret; > > + > > + /* PWM and FANIN Monitoring Enable */ > > + ret = regmap_write(data->regmap, NCT7363_REG_PWMEN_0_7, > > + data->pwm_mask & 0xff); > > + if (ret < 0) > > + return ret; > > + ret = regmap_write(data->regmap, NCT7363_REG_PWMEN_8_15, > > + (data->pwm_mask >> 8) & 0xff); > > + if (ret < 0) > > + return ret; > > + ret = regmap_write(data->regmap, NCT7363_REG_FANINEN_0_7, > > + data->fanin_mask & 0xff); > > + if (ret < 0) > > + return ret; > > + ret = regmap_write(data->regmap, NCT7363_REG_FANINEN_8_15, > > + (data->fanin_mask >> 8) & 0xff); > > + if (ret < 0) > > + return ret; > > + > > + return 0; > > +} > > + > > +static int nct7363_present_pwm_fanin(struct device *dev, > > + struct device_node *child, > > + struct nct7363_data *data) > > +{ > > + struct of_phandle_args args; > > + int ret, fanin_cnt; > > + u8 *fanin_ch; > > + u8 ch, index; > > + > > + ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", > > + 0, &args); > > + if (ret) > > + return ret; > > + > > + data->pwm_mask |= BIT(args.args[0]); > > + > > + fanin_cnt = of_property_count_u8_elems(child, "tach-ch"); > > + if (fanin_cnt < 1) > > + return -EINVAL; > > + > > + fanin_ch = devm_kcalloc(dev, fanin_cnt, sizeof(*fanin_ch), GFP_KERNEL); > > + if (!fanin_ch) > > + return -ENOMEM; > > + > > + ret = of_property_read_u8_array(child, "tach-ch", fanin_ch, fanin_cnt); > > + if (ret) > > + return ret; > > + > > + for (ch = 0; ch < fanin_cnt; ch++) { > > + index = fanin_ch[ch]; > > + data->fanin_mask |= BIT(index); > > + } > > + > > + return 0; > > +} > > I didn't check if this aligns with the fan bindings schema. Unfortunately > the dt maintainer's reviewed-by: tag got lost with > https://patchwork.kernel.org/project/linux-hwmon/patch/20231222013352.3873689-2-kcfeng0@nuvoton.com/ > > o we can not really move forward until that is resolved. ok, I'll keep monitoring the fan binding schema. Thanks, Ban > > > + > > +static const struct regmap_config nct7363_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > +}; > > + > > +static int nct7363_probe(struct i2c_client *client) > > +{ > > + struct device *dev = &client->dev; > > + struct device_node *child; > > + struct nct7363_data *data; > > + struct device *hwmon_dev; > > + int ret; > > + > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + data->regmap = devm_regmap_init_i2c(client, &nct7363_regmap_config); > > + if (IS_ERR(data->regmap)) > > + return PTR_ERR(data->regmap); > > + > > + mutex_init(&data->update_lock); > > + > > + for_each_child_of_node(dev->of_node, child) { > > + ret = nct7363_present_pwm_fanin(dev, child, data); > > + if (ret) { > > + of_node_put(child); > > + return ret; > > + } > > + } > > + > > + /* Initialize the chip */ > > + ret = nct7363_init_chip(data); > > + if (ret) > > + return ret; > > + > > + hwmon_dev = > > + devm_hwmon_device_register_with_info(dev, client->name, data, > > + &nct7363_chip_info, NULL); > > + return PTR_ERR_OR_ZERO(hwmon_dev); > > +} > > + > > +static struct i2c_driver nct7363_driver = { > > + .class = I2C_CLASS_HWMON, > > + .driver = { > > + .name = "nct7363", > > + .of_match_table = nct7363_of_match, > > + }, > > + .probe = nct7363_probe, > > + .id_table = nct7363_id, > > + .detect = nct7363_detect, > > + .address_list = normal_i2c, > > +}; > > + > > +module_i2c_driver(nct7363_driver); > > + > > +MODULE_AUTHOR("CW Ho <cwho@nuvoton.com>"); > > +MODULE_AUTHOR("Ban Feng <kcfeng0@nuvoton.com>"); > > +MODULE_DESCRIPTION("NCT7363 Hardware Monitoring Driver"); > > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-02-21 2:22 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-22 1:33 [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2023-12-22 1:33 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926 2024-01-04 0:15 ` Rob Herring 2024-01-09 9:00 ` Ban Feng 2024-02-03 15:09 ` Guenter Roeck 2024-02-21 2:22 ` Ban Feng 2023-12-22 1:33 ` [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation baneric926 2024-01-04 0:21 ` Rob Herring 2023-12-22 1:33 ` [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2024-02-03 15:06 ` Guenter Roeck 2024-02-21 1:26 ` Ban Feng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).