* [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y
@ 2023-12-22 1:33 ` baneric926
0 siblings, 0 replies; 23+ 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] 23+ messages in thread* [PATCH v3 0/3] hwmon: Driver for Nuvoton NCT7363Y @ 2023-12-22 1:33 ` baneric926 0 siblings, 0 replies; 23+ 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, kcfeng0, kwliu, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, 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] 23+ messages in thread
* [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2023-12-22 1:33 ` baneric926 @ 2023-12-22 1:33 ` baneric926 -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema @ 2023-12-22 1:33 ` baneric926 0 siblings, 0 replies; 23+ 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, kcfeng0, kwliu, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, 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] 23+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2023-12-22 1:33 ` baneric926 @ 2024-01-04 0:15 ` Rob Herring -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema @ 2024-01-04 0:15 ` Rob Herring 0 siblings, 0 replies; 23+ messages in thread From: Rob Herring @ 2024-01-04 0:15 UTC (permalink / raw) To: baneric926 Cc: linux-hwmon, devicetree, conor+dt, jdelvare, corbet, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, krzysztof.kozlowski+dt, kwliu, linux 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] 23+ 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 -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema @ 2024-01-09 9:00 ` Ban Feng 0 siblings, 0 replies; 23+ messages in thread From: Ban Feng @ 2024-01-09 9:00 UTC (permalink / raw) To: Rob Herring Cc: linux-hwmon, devicetree, conor+dt, jdelvare, corbet, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, krzysztof.kozlowski+dt, kwliu, linux 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] 23+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2023-12-22 1:33 ` baneric926 @ 2024-02-03 15:09 ` Guenter Roeck -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema @ 2024-02-03 15:09 ` Guenter Roeck 0 siblings, 0 replies; 23+ messages in thread From: Guenter Roeck @ 2024-02-03 15:09 UTC (permalink / raw) To: baneric926 Cc: linux-hwmon, devicetree, conor+dt, jdelvare, corbet, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, robh+dt, krzysztof.kozlowski+dt, kwliu 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] 23+ 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 -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema @ 2024-02-21 2:22 ` Ban Feng 0 siblings, 0 replies; 23+ messages in thread From: Ban Feng @ 2024-02-21 2:22 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, devicetree, conor+dt, jdelvare, corbet, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, robh+dt, krzysztof.kozlowski+dt, kwliu 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] 23+ messages in thread
* [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation 2023-12-22 1:33 ` baneric926 @ 2023-12-22 1:33 ` baneric926 -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation @ 2023-12-22 1:33 ` baneric926 0 siblings, 0 replies; 23+ 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, kcfeng0, kwliu, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, 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] 23+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation 2023-12-22 1:33 ` baneric926 @ 2024-01-04 0:21 ` Rob Herring -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation @ 2024-01-04 0:21 ` Rob Herring 0 siblings, 0 replies; 23+ messages in thread From: Rob Herring @ 2024-01-04 0:21 UTC (permalink / raw) To: baneric926 Cc: linux-hwmon, kwliu, conor+dt, jdelvare, corbet, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, devicetree, robh+dt, krzysztof.kozlowski+dt, openbmc, linux 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] 23+ messages in thread
* [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y 2023-12-22 1:33 ` baneric926 @ 2023-12-22 1:33 ` baneric926 -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y @ 2023-12-22 1:33 ` baneric926 0 siblings, 0 replies; 23+ 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, kcfeng0, kwliu, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, 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] 23+ messages in thread
* Re: [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y 2023-12-22 1:33 ` baneric926 @ 2024-02-03 15:06 ` Guenter Roeck -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y @ 2024-02-03 15:06 ` Guenter Roeck 0 siblings, 0 replies; 23+ messages in thread From: Guenter Roeck @ 2024-02-03 15:06 UTC (permalink / raw) To: baneric926 Cc: linux-hwmon, devicetree, conor+dt, jdelvare, corbet, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, robh+dt, krzysztof.kozlowski+dt, kwliu 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] 23+ 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 -1 siblings, 0 replies; 23+ 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] 23+ messages in thread
* Re: [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y @ 2024-02-21 1:26 ` Ban Feng 0 siblings, 0 replies; 23+ messages in thread From: Ban Feng @ 2024-02-21 1:26 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, devicetree, conor+dt, jdelvare, corbet, openbmc, linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai, kcfeng0, robh+dt, krzysztof.kozlowski+dt, kwliu 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] 23+ messages in thread
* [PATCH v3 0/3] Add devicetree support for max6639 @ 2022-10-12 20:57 Naresh Solanki 2022-10-12 20:57 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki 0 siblings, 1 reply; 23+ messages in thread From: Naresh Solanki @ 2022-10-12 20:57 UTC (permalink / raw) To: devicetree, Guenter Roeck, Jean Delvare Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Naresh Solanki These patches adds devicetree support for MAX6639. Changes V3: - correct fan dt property name - remove unrelevent changes Changes V2: - Fix dt schema error. Changes: - Add fan-common dt schema. - add dt-binding support for max6639 - add max6639 specific property Marcello Sylvester Bauer (1): dt-bindings: hwmon: Add binding for max6639 Naresh Solanki (2): dt-bindings: hwmon: fan: Add fan binding to schema hwmon: (max6639) Change from pdata to dt configuration .../devicetree/bindings/hwmon/fan-common.yaml | 48 ++++ .../bindings/hwmon/maxim,max6639.yaml | 86 ++++++++ drivers/hwmon/max6639.c | 206 +++++++++++++----- 3 files changed, 286 insertions(+), 54 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/fan-common.yaml create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml base-commit: 0cf46a653bdae56683fece68dc50340f7520e6c4 -- 2.37.3 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema 2022-10-12 20:57 [PATCH v3 0/3] Add devicetree support for max6639 Naresh Solanki @ 2022-10-12 20:57 ` Naresh Solanki 0 siblings, 0 replies; 23+ messages in thread From: Naresh Solanki @ 2022-10-12 20:57 UTC (permalink / raw) To: devicetree, Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski, Naresh Solanki Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Naresh Solanki 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# Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> --- .../devicetree/bindings/hwmon/fan-common.yaml | 48 +++++++++++++++++++ 1 file changed, 48 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..224f5013c93f --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml @@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-or-later 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> + +properties: + max-rpm: + description: + Max RPM supported by fan. + $ref: /schemas/types.yaml#/definitions/uint32 + + pulses-per-revolution: + description: + The number of pulse from fan sensor per revolution. + $ref: /schemas/types.yaml#/definitions/uint32 + + default-rpm: + description: + Target RPM the fan should be configured during driver probe. + $ref: /schemas/types.yaml#/definitions/uint32 + + pwm-frequency: + description: + PWM frequency for fan in Hertz(Hz). + $ref: /schemas/types.yaml#/definitions/uint32 + + pwm-polarity-inverse: + description: + Inverse PWM polarity for fan. + type: boolean + + label: + description: + Optional fan label + + fan-supply: + description: + Power supply for fan. + +additionalProperties: true + +... base-commit: 0cf46a653bdae56683fece68dc50340f7520e6c4 -- 2.37.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-02-23 2:38 UTC | newest] Thread overview: 23+ 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 ` 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:15 ` Rob Herring 2024-01-04 0:15 ` Rob Herring 2024-01-09 9:00 ` Ban Feng 2024-01-09 9:00 ` Ban Feng 2024-02-03 15:09 ` Guenter Roeck 2024-02-03 15:09 ` Guenter Roeck 2024-02-21 2:22 ` Ban Feng 2024-02-21 2:22 ` Ban Feng 2023-12-22 1:33 ` [PATCH v3 2/3] dt-bindings: hwmon: Add NCT7363Y documentation baneric926 2023-12-22 1:33 ` baneric926 2024-01-04 0:21 ` Rob Herring 2024-01-04 0:21 ` Rob Herring 2023-12-22 1:33 ` [PATCH v3 3/3] hwmon: Driver for Nuvoton NCT7363Y baneric926 2023-12-22 1:33 ` baneric926 2024-02-03 15:06 ` Guenter Roeck 2024-02-03 15:06 ` Guenter Roeck 2024-02-21 1:26 ` Ban Feng 2024-02-21 1:26 ` Ban Feng -- strict thread matches above, loose matches on Subject: below -- 2022-10-12 20:57 [PATCH v3 0/3] Add devicetree support for max6639 Naresh Solanki 2022-10-12 20:57 ` [PATCH v3 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.