* [PATCH 0/2] Nuvoton NCT7362 driver & dt-binding @ 2023-06-07 10:18 Zev Weiss 2023-06-07 10:18 ` [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding Zev Weiss 2023-06-07 10:18 ` [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver Zev Weiss 0 siblings, 2 replies; 7+ messages in thread From: Zev Weiss @ 2023-06-07 10:18 UTC (permalink / raw) To: Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Zev Weiss, devicetree, linux-hwmon, linux-kernel, openbmc Hello, This patch series adds support for the Nuvoton NCT7362 fan/GPIO controller. The chip provides 16 pins that can be individually configured for PWM, tach, or GPIO functionality, and is controlled via an I2C interface. There is a sibling NCT7360 chip covered by the same datasheet that's very similar but only provides 8 fan/tach/gpio pins instead of 16; it could be supported fairly easily with minor adjustments to the same driver, but since I don't have an instance of it to test against I've only implemented support for the NCT7362. The first patch adds a DT binding, and the second adds a hwmon driver with optional GPIO support enabled via CONFIG_SENSORS_NCT7362_GPIO. Thanks, Zev Zev Weiss (2): dt-bindings: hwmon: Add Nuvoton NCT7362Y binding hwmon: (nct7362) Add nct7362 driver .../bindings/hwmon/nuvoton,nct7362.yaml | 123 ++++ MAINTAINERS | 7 + drivers/hwmon/Kconfig | 18 + drivers/hwmon/Makefile | 1 + drivers/hwmon/nct7362.c | 697 ++++++++++++++++++ 5 files changed, 846 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml create mode 100644 drivers/hwmon/nct7362.c -- 2.40.0.5.gf6e3b97ba6d2.dirty ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding 2023-06-07 10:18 [PATCH 0/2] Nuvoton NCT7362 driver & dt-binding Zev Weiss @ 2023-06-07 10:18 ` Zev Weiss 2023-06-07 18:24 ` Krzysztof Kozlowski 2023-06-14 22:57 ` Rob Herring 2023-06-07 10:18 ` [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver Zev Weiss 1 sibling, 2 replies; 7+ messages in thread From: Zev Weiss @ 2023-06-07 10:18 UTC (permalink / raw) To: Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree Cc: Zev Weiss, Guenter Roeck, Jean Delvare, linux-hwmon, linux-kernel, openbmc This binding describes the NCT7362Y, a 16-channel fan/GPIO controller. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- .../bindings/hwmon/nuvoton,nct7362.yaml | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml new file mode 100644 index 000000000000..630dcce7a14c --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7362.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NCT7362Y fan controller + +maintainers: + - Zev Weiss <zev@bewilderbeest.net> + +description: | + The Nuvoton NCT7362Y is an I2C fan controller with 16 pins that can + be independently configured for PWM, fan tach, or GPIO + functionality. Each pin's functionality is represented by a child + node. + + The datasheet is not publicly available but can be requested from + Nuvoton via their web site. + +properties: + compatible: + enum: + - nuvoton,nct7362 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + gpio-controller: true + + "#gpio-cells": + const: 2 + + gpio-line-names: + minItems: 1 + maxItems: 16 + +patternProperties: + "^tach@([1-8]|1[0-7])$": + type: object + properties: + reg: + maxItems: 1 + description: The pin number. + + nuvoton,pulses-per-revolution: + description: | + The number of tach pulses per revolution of the fan. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + default: 2 + + required: + - reg + + "^pwm@([1-8]|1[0-7])$": + type: object + properties: + reg: + maxItems: 1 + description: The pin number. + required: + - reg + + "^gpio@([1-8]|1[0-7])$": + type: object + properties: + reg: + maxItems: 1 + description: The pin number. + required: + - reg + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + fanctl@22 { + compatible = "nuvoton,nct7362"; + gpio-controller; + #gpio-cells = <2>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x22>; + gpio-line-names = "", "", "", "", "", "", "", "", + "", "", "", "", "", "", "led1", "led2"; + + tach@1 { + reg = <1>; + }; + tach@2 { + reg = <2>; + nuvoton,pulses-per-revolution = <1>; + }; + pwm@10 { + reg = <10>; + }; + pwm@11 { + reg = <11>; + }; + gpio@16 { + reg = <16>; + }; + gpio@17 { + reg = <17>; + }; + }; + }; -- 2.40.0.5.gf6e3b97ba6d2.dirty ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding 2023-06-07 10:18 ` [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding Zev Weiss @ 2023-06-07 18:24 ` Krzysztof Kozlowski 2023-06-14 22:57 ` Rob Herring 1 sibling, 0 replies; 7+ messages in thread From: Krzysztof Kozlowski @ 2023-06-07 18:24 UTC (permalink / raw) To: Zev Weiss, Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree Cc: Guenter Roeck, Jean Delvare, linux-hwmon, linux-kernel, openbmc On 07/06/2023 12:18, Zev Weiss wrote: > This binding describes the NCT7362Y, a 16-channel fan/GPIO controller. A nit, subject: drop second/last, redundant "binding". The "dt-bindings" prefix is already stating that these are bindings. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > .../bindings/hwmon/nuvoton,nct7362.yaml | 123 ++++++++++++++++++ > 1 file changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > new file mode 100644 > index 000000000000..630dcce7a14c > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > @@ -0,0 +1,123 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7362.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton NCT7362Y fan controller > + > +maintainers: > + - Zev Weiss <zev@bewilderbeest.net> > + > +description: | > + The Nuvoton NCT7362Y is an I2C fan controller with 16 pins that can > + be independently configured for PWM, fan tach, or GPIO > + functionality. Each pin's functionality is represented by a child > + node. > + > + The datasheet is not publicly available but can be requested from > + Nuvoton via their web site. > + > +properties: > + compatible: > + enum: > + - nuvoton,nct7362 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + > + gpio-line-names: > + minItems: 1 > + maxItems: 16 > + > +patternProperties: > + "^tach@([1-8]|1[0-7])$": @9 is not valid? Aren't you using some non-numerical values? > + type: object > + properties: > + reg: > + maxItems: 1 > + description: The pin number. > + > + nuvoton,pulses-per-revolution: Just "pulses-per-revolution" https://lore.kernel.org/linux-devicetree/20221116213615.1256297-2-Naresh.Solanki@9elements.com/ > + description: | > + The number of tach pulses per revolution of the fan. > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 1 > + default: 2 > + > + required: > + - reg > + > + "^pwm@([1-8]|1[0-7])$": > + type: object > + properties: > + reg: > + maxItems: 1 > + description: The pin number. This is some plague of single-property-nodes... Aspeed also sends something similar. Why the heck do you need empty nodes? Drop entire node pwm. > + required: > + - reg > + > + "^gpio@([1-8]|1[0-7])$": > + type: object > + properties: > + reg: > + maxItems: 1 > + description: The pin number. Drop entire node gpio. If this is pinctrl, then make it a pinctrl. > + required: > + - reg > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" Best regards, Krzysztof ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding 2023-06-07 10:18 ` [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding Zev Weiss 2023-06-07 18:24 ` Krzysztof Kozlowski @ 2023-06-14 22:57 ` Rob Herring 1 sibling, 0 replies; 7+ messages in thread From: Rob Herring @ 2023-06-14 22:57 UTC (permalink / raw) To: Zev Weiss Cc: Conor Dooley, Krzysztof Kozlowski, devicetree, Guenter Roeck, Jean Delvare, linux-hwmon, linux-kernel, openbmc On Wed, Jun 07, 2023 at 03:18:29AM -0700, Zev Weiss wrote: > This binding describes the NCT7362Y, a 16-channel fan/GPIO controller. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > .../bindings/hwmon/nuvoton,nct7362.yaml | 123 ++++++++++++++++++ > 1 file changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > new file mode 100644 > index 000000000000..630dcce7a14c > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > @@ -0,0 +1,123 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7362.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton NCT7362Y fan controller > + > +maintainers: > + - Zev Weiss <zev@bewilderbeest.net> > + > +description: | > + The Nuvoton NCT7362Y is an I2C fan controller with 16 pins that can > + be independently configured for PWM, fan tach, or GPIO > + functionality. Each pin's functionality is represented by a child > + node. > + > + The datasheet is not publicly available but can be requested from > + Nuvoton via their web site. > + > +properties: > + compatible: > + enum: > + - nuvoton,nct7362 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + > + gpio-line-names: > + minItems: 1 > + maxItems: 16 > + > +patternProperties: > + "^tach@([1-8]|1[0-7])$": Unit-addresses are hex typically. Why do you need a child node for tach. Is that a separate h/w block. > + type: object > + properties: > + reg: > + maxItems: 1 > + description: The pin number. > + > + nuvoton,pulses-per-revolution: This is a property of the fan attached and belongs in a fan node describing the fan(s) properties. Until a common binding exists, further fan controller bindings are going to be rejected. The furthest attempt was here[1]. And there's the Aspeed effort[2] which keeps ignoring our feedback. Please work together on these. Rob [1] https://lore.kernel.org/all/20221121122932.2493174-2-Naresh.Solanki@9elements.com/ [2] https://lore.kernel.org/all/20230608021839.12769-1-billy_tsai@aspeedtech.com/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver 2023-06-07 10:18 [PATCH 0/2] Nuvoton NCT7362 driver & dt-binding Zev Weiss 2023-06-07 10:18 ` [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding Zev Weiss @ 2023-06-07 10:18 ` Zev Weiss 2023-06-07 18:00 ` Guenter Roeck 1 sibling, 1 reply; 7+ messages in thread From: Zev Weiss @ 2023-06-07 10:18 UTC (permalink / raw) To: Guenter Roeck, Jean Delvare, linux-hwmon; +Cc: Zev Weiss, linux-kernel, openbmc This driver supports the Nuvoton NCT7362Y, an I2C fan controller with 16 channels independently configurable for operation as PWM outputs, fan tachometer inputs, or GPIOs. Most aspects of the chip's functionality are supported, including PWM duty cycle, frequency, enable/disable, and DC-mode operation. To support its GPIO functionality, the driver also exposes a gpiochip interface if CONFIG_SENSORS_NCT7362_GPIO is enabled. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- MAINTAINERS | 7 + drivers/hwmon/Kconfig | 18 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/nct7362.c | 697 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 723 insertions(+) create mode 100644 drivers/hwmon/nct7362.c diff --git a/MAINTAINERS b/MAINTAINERS index e0ad886d3163..7b2abff6d049 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14468,6 +14468,13 @@ S: Maintained F: Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml F: drivers/hwmon/nct6775-i2c.c +NCT7362 HARDWARE MONITOR DRIVER +M: Zev Weiss <zev@bewilderbeest.net> +L: linux-hwmon@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml +F: drivers/hwmon/nct7362.c + NETDEVSIM M: Jakub Kicinski <kuba@kernel.org> S: Maintained diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index fc640201a2de..5ca51b4b52b6 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1562,6 +1562,24 @@ config SENSORS_NCT6775_I2C This driver can also be built as a module. If so, the module will be called nct6775-i2c. +config SENSORS_NCT7362 + tristate "Nuvoton NCT7362Y fan controller" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get support for the Nuvoton NCT7362Y + fan controller. + + This driver can also be built as a module. If so, the module + will be called nct7362. + +config SENSORS_NCT7362_GPIO + bool "Enable NCT7362Y GPIO support" + depends on SENSORS_NCT7362 && GPIOLIB + help + Allow nct7362 pins not used as PWM outputs or tach inputs to + be used as GPIOs. + config SENSORS_NCT7802 tristate "Nuvoton NCT7802Y" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index cd8c568c80a9..29c674979280 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -162,6 +162,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_NCT7362) += nct7362.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/nct7362.c b/drivers/hwmon/nct7362.c new file mode 100644 index 000000000000..8f8e49097710 --- /dev/null +++ b/drivers/hwmon/nct7362.c @@ -0,0 +1,697 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * nct7362 - Driver for Nuvoton NCT7362Y fan controller + * + * Copyright (C) 2023 Zev Weiss <zev@bewilderbeest.net> + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/of.h> +#include <linux/i2c.h> +#include <linux/regmap.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/gpio/driver.h> + +#define DRVNAME "nct7362" + +#define REG_GPIO0_INPUT 0x00 +#define REG_GPIO1_INPUT 0x10 + +#define REG_GPIO0_OUTPUT 0x01 +#define REG_GPIO1_OUTPUT 0x11 + +#define REG_GPIO0_IOCFG 0x03 +#define REG_GPIO1_IOCFG 0x13 + +#define REG_GPIO0_OUTMODE 0x05 +#define REG_GPIO1_OUTMODE 0x15 + +#define REG_PINCTRL_00 0x20 +#define REG_PINCTRL_04 0x21 +#define REG_PINCTRL_10 0x22 +#define REG_PINCTRL_14 0x23 + +#define REG_PWM_0_7_EN 0x38 +#define REG_PWM_8_15_EN 0x39 + +#define REG_TACH_GLOBAL_CTL 0x40 +#define REG_TACH_0_7_EN 0x41 +#define REG_TACH_8_15_EN 0x42 + +#define REG_TACH_HVAL(n) (0x48 + (2 * (n))) +#define REG_TACH_LVAL(n) (0x49 + (2 * (n))) + +#define REG_FSCP_DUTY(n) (0x90 + (2 * (n))) +#define REG_FSCP_DIV(n) (0x91 + (2 * (n))) +#define REG_FSCP_CFG_BASE 0xb0 + +/* + * For use in REG_PINCTRL_xx sub-fields. ALERT (3) also exists for some pins, + * but is currently unsupported. + */ +#define PIN_MODE_GPIO 0 +#define PIN_MODE_PWM 1 +#define PIN_MODE_TACH 2 + +#define NUM_CHANNELS 16 + +/* Default all PWM channels to 100% */ +#define DEFAULT_PWM_DUTY 0xff + +/* Default all PWMs to 1/256 resolution, non-inverted, PWM mode (non-DC) */ +#define DEFAULT_FSCP_CFG 0x44 + +struct nct7362_data { + struct i2c_client *client; + struct regmap *regmap; + + /* + * Bitmasks of which channels of which functions are enabled (offset by + * one so channel 1 is at bit 0). Because the correspondence between + * pin numbers and tach channels differs from that of GPIO/PWM channels, + * some bit positions may be set in more than one of these (and some may + * be set in none). + */ + struct { + u16 gpio; + u16 pwm; + u16 tach; + } functions; + + /* Pulses-per-revolution for each tach channel */ + u32 tach_ppr[NUM_CHANNELS]; + +#ifdef CONFIG_SENSORS_NCT7362_GPIO + /* In-use pins, either as PWM/tach or an actively-used GPIO */ + unsigned long active_pins; + + struct gpio_chip gpio; +#endif +}; + +#ifdef CONFIG_SENSORS_NCT7362_GPIO +static int nct7362_gpio_direction_input(struct gpio_chip *gpio, unsigned int offset) +{ + struct nct7362_data *chip = gpiochip_get_data(gpio); + u8 iocfg = offset < 8 ? REG_GPIO0_IOCFG : REG_GPIO1_IOCFG; + u8 bit = BIT(offset % 8); + + return regmap_update_bits(chip->regmap, iocfg, bit, bit); +} + +static int nct7362_gpio_direction_output(struct gpio_chip *gpio, unsigned int offset, int val) +{ + int status; + struct nct7362_data *chip = gpiochip_get_data(gpio); + u8 outreg = offset < 8 ? REG_GPIO0_OUTPUT : REG_GPIO1_OUTPUT; + u8 iocfg = offset < 8 ? REG_GPIO0_IOCFG : REG_GPIO1_IOCFG; + u8 bit = BIT(offset % 8); + + status = regmap_update_bits(chip->regmap, iocfg, bit, 0); + if (status) + return status; + + return regmap_update_bits(chip->regmap, outreg, bit, val ? bit : 0); +} + +static int nct7362_gpio_get_value(struct gpio_chip *gpio, unsigned int offset) +{ + unsigned int inbits; + struct nct7362_data *chip = gpiochip_get_data(gpio); + u8 inreg = offset < 8 ? REG_GPIO0_INPUT : REG_GPIO1_INPUT; + int status = regmap_read(chip->regmap, inreg, &inbits); + + return status ? : !!(inbits & BIT(offset % 8)); +} + +static void nct7362_gpio_set_value(struct gpio_chip *gpio, unsigned int offset, int value) +{ + struct nct7362_data *chip = gpiochip_get_data(gpio); + u8 outreg = offset < 8 ? REG_GPIO0_OUTPUT : REG_GPIO1_OUTPUT; + u8 bit = BIT(offset % 8); + int status = regmap_update_bits(chip->regmap, outreg, bit, value ? bit : 0); + + /* No other way to report failure */ + if (status) + dev_dbg(&chip->client->dev, "regmap_update_bits() failed: %d\n", status); +} + +static int nct7362_gpio_request_pin(struct gpio_chip *gpio, unsigned int offset) +{ + struct nct7362_data *chip = gpiochip_get_data(gpio); + + return test_and_set_bit(offset, &chip->active_pins) ? -EBUSY : 0; +} + +static void nct7362_gpio_free_pin(struct gpio_chip *gpio, unsigned int offset) +{ + struct nct7362_data *chip = gpiochip_get_data(gpio); + + clear_bit(offset, &chip->active_pins); +} + +static int nct7362_gpio_set_config(struct gpio_chip *gpio, unsigned int offset, + unsigned long config) +{ + struct nct7362_data *chip = gpiochip_get_data(gpio); + u8 modereg = offset < 8 ? REG_GPIO0_OUTMODE : REG_GPIO1_OUTMODE; + u8 bit = BIT(offset % 8); + int setting; + enum pin_config_param param = pinconf_to_config_param(config); + + switch (param) { + case PIN_CONFIG_DRIVE_OPEN_DRAIN: + setting = 0; + break; + case PIN_CONFIG_DRIVE_PUSH_PULL: + setting = 1; + break; + default: + dev_dbg(&chip->client->dev, "unsupported config param %d\n", param); + return -ENOTSUPP; + } + + return regmap_update_bits(chip->regmap, modereg, bit, setting ? bit : 0); +} +#endif + +/* Convert a pin number to a PWM channel number */ +static inline int pin_to_pwm(int pin) +{ + return pin < 9 ? (pin - 1) : (pin - 2); +} + +#ifdef CONFIG_SENSORS_NCT7362_GPIO +/* + * Convert a pin number to a GPIO number -- interpreting the numbers in the + * datasheet as octal, these are the same as the (decimal) PWM numbers. + */ +static inline int pin_to_gpio(int pin) +{ + return pin_to_pwm(pin); +} + +/* GPIO that shares the same pin as the given PWM */ +static inline int pwm_to_gpio(int pwm) +{ + /* GPIO & PWM numbering are the same */ + return pwm; +} + +/* GPIO that shares the same pin as the given tach */ +static inline int tach_to_gpio(int tach) +{ + int pin = tach < 8 ? (tach + 10) : (tach - 7); + + return pin_to_gpio(pin); +} +#endif + +/* Convert a pin number to a tach (FANIN) channel number */ +static inline int pin_to_tach(int pin) +{ + return pin < 9 ? (pin + 7) : (pin - 10); +} + +static int nct7362_init_pin_functions(struct nct7362_data *chip) +{ + int channel, reg, status; + u16 *mask; + u8 function; + struct device_node *child; + u8 pinctrl[] = { 0, 0, 0, 0, }; + struct device *dev = &chip->client->dev; + + for_each_available_child_of_node(dev->of_node, child) { + int pwmnum, pinctrl_idx, pinctrl_shift; + u32 pin; + + if (of_property_read_u32(child, "reg", &pin)) { + dev_warn(dev, "skipping %pOF due to missing 'reg' property\n", child); + continue; + } + + if (pin == 0 || pin == 9 || pin > 17) { + dev_warn(dev, "skipping %pOF with invalid pin number %u\n", child, pin); + continue; + } + + pwmnum = pin_to_pwm(pin); + pinctrl_idx = pwmnum / 4; + pinctrl_shift = 2 * (pwmnum % 4); + + if (of_node_name_prefix(child, "pwm")) { + function = PIN_MODE_PWM; + channel = pin_to_pwm(pin); + mask = &chip->functions.pwm; + } else if (of_node_name_prefix(child, "tach")) { + function = PIN_MODE_TACH; + channel = pin_to_tach(pin); + mask = &chip->functions.tach; + if (of_property_read_u32(child, "nuvoton,pulses-per-revolution", + &chip->tach_ppr[channel])) + chip->tach_ppr[channel] = 2; +#ifdef CONFIG_SENSORS_NCT7362_GPIO + } else if (of_node_name_prefix(child, "gpio")) { + function = PIN_MODE_GPIO; + channel = pin_to_gpio(pin); + mask = &chip->functions.gpio; +#endif + } else { + dev_warn(dev, "skipping %pOF of unknown function\n", child); + continue; + } + + *mask |= BIT(channel); + + pinctrl[pinctrl_idx] |= function << pinctrl_shift; + } + + for (reg = 0; reg < 4; reg++) { + status = regmap_write(chip->regmap, REG_PINCTRL_00 + reg, pinctrl[reg]); + if (status) + return status; + } + + return 0; +} + +static int nct7362_init_pwm(struct nct7362_data *chip) +{ + int ret; + + for (int i = 0; i < 8; i++) { + ret = regmap_write(chip->regmap, REG_FSCP_CFG_BASE + i, DEFAULT_FSCP_CFG); + if (ret) + return ret; + } + + for (int i = 0; i < NUM_CHANNELS; i++) { + ret = regmap_write(chip->regmap, REG_FSCP_DUTY(i), DEFAULT_PWM_DUTY); + if (ret) + return ret; + } + + for (int i = 0; i < 2; i++) { + ret = regmap_write(chip->regmap, REG_PWM_0_7_EN + i, 0xff); + if (ret) + return ret; + } + + return 0; +} + +static int nct7362_init_fan(struct nct7362_data *chip) +{ + int ret; + + /* + * tach_ppr *should* be set for all configured tach channels by this + * point, but to ensure we don't end up dividing by zero let's make sure. + */ + for (int i = 0; i < 16; i++) { + if (!chip->tach_ppr[i]) + chip->tach_ppr[i] = 2; + } + + /* Enable individual fan tachs */ + for (int i = 0; i < 2; i++) { + ret = regmap_write(chip->regmap, REG_TACH_0_7_EN + i, 0xff); + if (ret) + return ret; + } + + /* Ensure global fan tach enable is on */ + return regmap_update_bits(chip->regmap, REG_TACH_GLOBAL_CTL, 0x80, 0x80); +} + +static int nct7362_init_gpio(struct nct7362_data *chip) +{ +#ifdef CONFIG_SENSORS_NCT7362_GPIO + struct gpio_chip *gpio = &chip->gpio; + + for (int i = 0; i < 16; i++) { + /* Pins in use as PWM or tach are permanently active */ + if (chip->functions.pwm & BIT(i)) + set_bit(pwm_to_gpio(i), &chip->active_pins); + if (chip->functions.tach & BIT(i)) + set_bit(tach_to_gpio(i), &chip->active_pins); + } + + gpio->label = dev_name(&chip->client->dev); + gpio->direction_input = nct7362_gpio_direction_input; + gpio->direction_output = nct7362_gpio_direction_output; + gpio->get = nct7362_gpio_get_value; + gpio->set = nct7362_gpio_set_value; + gpio->request = nct7362_gpio_request_pin; + gpio->free = nct7362_gpio_free_pin; + gpio->set_config = nct7362_gpio_set_config; + gpio->can_sleep = 1; + gpio->base = -1; + gpio->ngpio = NUM_CHANNELS; + gpio->parent = &chip->client->dev; + gpio->owner = THIS_MODULE; + + return devm_gpiochip_add_data(&chip->client->dev, gpio, chip); +#else + return 0; +#endif +} + +static umode_t nct7362_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + umode_t mode = 0; + const struct nct7362_data *chip = data; + + switch (type) { + case hwmon_pwm: + if (!(chip->functions.pwm & BIT(channel))) + break; + switch (attr) { + case hwmon_pwm_enable: + case hwmon_pwm_input: + case hwmon_pwm_freq: + case hwmon_pwm_mode: + mode = 0644; + break; + default: + break; + } + break; + case hwmon_fan: + if (!(chip->functions.tach & BIT(channel))) + break; + switch (attr) { + case hwmon_fan_input: + mode = 0444; + break; + default: + break; + } + break; + default: + break; + } + + return mode; +} + +/* + * We set STEP to 1 on all PWM channels during initialization (for + * higher-resolution duty cycle control) and never change it, so our frequency + * calculations all assume it's set to 1 rather than re-reading the register all + * the time. + * + * Supported frequency ranges: + * DIVSEL=0: 488Hz - 62.5KHz + * DIVSEL=1: 2Hz - 244Hz + */ +#define PWM_STEP 1 +#define BASE_FANCLK 16000000 +#define PWM_FREQ_MIN_DIVSEL0 (BASE_FANCLK / 32768) /* 488 */ +#define PWM_FREQ_MAX_DIVSEL1 (BASE_FANCLK / 65536) /* 244 */ + +/* Frequency threshold below which we switch to DIVSEL=1 */ +#define PWM_DIVSEL_CUTOFF ((PWM_FREQ_MIN_DIVSEL0 + PWM_FREQ_MAX_DIVSEL1) / 2) + +static long nct7362_pwm_freq(long divisor, bool divsel) +{ + long denom = (divisor + 1) << (7 + PWM_STEP + (divsel ? 8 : 0)); + + return BASE_FANCLK / denom; +} + +static int nct7362_read_pwm(struct nct7362_data *chip, u32 attr, int channel, long *val) +{ + u8 reg; + int status; + unsigned int tmp; + + if (!(chip->functions.pwm & BIT(channel))) + return -EBUSY; + + switch (attr) { + case hwmon_pwm_enable: + reg = channel < 8 ? REG_PWM_0_7_EN : REG_PWM_8_15_EN; + status = regmap_read(chip->regmap, reg, &tmp); + if (!status) + *val = !!(tmp & BIT(channel % 8)); + break; + + case hwmon_pwm_freq: + reg = REG_FSCP_DIV(channel); + status = regmap_read(chip->regmap, reg, &tmp); + if (status) + return status; + + *val = nct7362_pwm_freq(tmp & 0x7f, !!(tmp & 0x80)); + break; + + case hwmon_pwm_input: + reg = REG_FSCP_DUTY(channel); + status = regmap_read(chip->regmap, reg, &tmp); + if (!status) + *val = tmp; + break; + + case hwmon_pwm_mode: + reg = REG_FSCP_CFG_BASE + (channel / 2); + status = regmap_read(chip->regmap, reg, &tmp); + if (!status) + *val = !(tmp & BIT(4 * (channel % 2))); + break; + + default: + return -EOPNOTSUPP; + } + + return status; +} + +static int nct7362_write_pwm(struct nct7362_data *chip, u32 attr, int channel, long val) +{ + u8 reg, bit, divsel; + long divisor; + int status; + + if (!(chip->functions.pwm & BIT(channel))) + return -EBUSY; + + switch (attr) { + case hwmon_pwm_input: + reg = REG_FSCP_DUTY(channel); + status = regmap_write(chip->regmap, reg, val); + break; + + case hwmon_pwm_enable: + reg = channel < 8 ? REG_PWM_0_7_EN : REG_PWM_8_15_EN; + bit = BIT(channel % 8); + status = regmap_update_bits(chip->regmap, reg, bit, val ? bit : 0); + break; + + case hwmon_pwm_mode: + reg = REG_FSCP_CFG_BASE + (channel / 2); + bit = BIT(4 * (channel % 2)); + status = regmap_update_bits(chip->regmap, reg, bit, val ? 0 : bit); + break; + + case hwmon_pwm_freq: + divsel = val < PWM_DIVSEL_CUTOFF; + + divisor = BASE_FANCLK / (val << (7 + PWM_STEP + (divsel ? 8 : 0))) - 1; + divisor = clamp(divisor, 0L, 0x7fL); + divisor |= divsel << 7; + + reg = REG_FSCP_DIV(channel); + status = regmap_write(chip->regmap, reg, divisor); + break; + + default: + return -EOPNOTSUPP; + } + + return status; +} + +static int nct7362_read_fan_speed(struct nct7362_data *chip, int channel, long *val) +{ + unsigned int hi, lo, count; + int status; + + status = regmap_read(chip->regmap, REG_TACH_HVAL(channel), &hi); + if (status) + return status; + + status = regmap_read(chip->regmap, REG_TACH_LVAL(channel), &lo); + if (status) + return status; + + count = (hi << 5) | (lo & 0x1f); + + *val = 1350000 / (count * chip->tach_ppr[channel] / 2); + + return 0; +} + +static int nct7362_read_tach(struct nct7362_data *chip, u32 attr, int channel, long *val) +{ + if (!(chip->functions.tach & BIT(channel))) + return -EBUSY; + + switch (attr) { + case hwmon_fan_input: + return nct7362_read_fan_speed(chip, channel, val); + default: + return -EOPNOTSUPP; + } +} + +static int nct7362_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct nct7362_data *chip = dev_get_drvdata(dev); + + switch (type) { + case hwmon_pwm: + return nct7362_read_pwm(chip, attr, channel, val); + case hwmon_fan: + return nct7362_read_tach(chip, attr, channel, val); + default: + return -EOPNOTSUPP; + } +} + +static int nct7362_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + struct nct7362_data *chip = dev_get_drvdata(dev); + + switch (type) { + case hwmon_pwm: + return nct7362_write_pwm(chip, attr, channel, val); + default: + return -EOPNOTSUPP; + } +} + +static const struct hwmon_ops nct7362_hwmon_ops = { + .is_visible = nct7362_is_visible, + .read = nct7362_read, + .write = nct7362_write, +}; + +static const struct hwmon_channel_info *nct7362_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_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE), + NULL, +}; + +static const struct hwmon_chip_info nct7362_chip_info = { + .ops = &nct7362_hwmon_ops, + .info = nct7362_info, +}; + +static const struct regmap_config nct7362_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int nct7362_probe(struct i2c_client *client) +{ + struct device *hwmon_dev; + struct device *dev = &client->dev; + struct nct7362_data *chip; + int ret; + + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL); + if (!chip) + return -ENOMEM; + + chip->client = client; + chip->regmap = devm_regmap_init_i2c(client, &nct7362_regmap_config); + if (IS_ERR(chip->regmap)) + return PTR_ERR(chip->regmap); + + ret = nct7362_init_pin_functions(chip); + if (ret) + return ret; + + ret = nct7362_init_pwm(chip); + if (ret) + return ret; + + ret = nct7362_init_fan(chip); + if (ret) + return ret; + + ret = nct7362_init_gpio(chip); + if (ret) + return ret; + + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, chip, + &nct7362_chip_info, NULL); + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct i2c_device_id nct7362_idtable[] = { + { "nct7362", }, + { } +}; +MODULE_DEVICE_TABLE(i2c, nct7362_idtable); + +static const struct of_device_id __maybe_unused nct7362_of_match[] = { + { .compatible = "nuvoton,nct7362", }, + { }, +}; +MODULE_DEVICE_TABLE(of, nct7362_of_match); + +static struct i2c_driver nct7362_driver = { + .class = I2C_CLASS_HWMON, + .driver = { + .name = DRVNAME, + .of_match_table = of_match_ptr(nct7362_of_match), + }, + .probe_new = nct7362_probe, + .id_table = nct7362_idtable, +}; + +module_i2c_driver(nct7362_driver); + +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); +MODULE_DESCRIPTION("NCT7362Y fan controller driver"); +MODULE_LICENSE("GPL"); -- 2.40.0.5.gf6e3b97ba6d2.dirty ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver 2023-06-07 10:18 ` [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver Zev Weiss @ 2023-06-07 18:00 ` Guenter Roeck 2023-06-07 19:33 ` Zev Weiss 0 siblings, 1 reply; 7+ messages in thread From: Guenter Roeck @ 2023-06-07 18:00 UTC (permalink / raw) To: Zev Weiss; +Cc: Jean Delvare, linux-hwmon, linux-kernel, openbmc On Wed, Jun 07, 2023 at 03:18:30AM -0700, Zev Weiss wrote: > This driver supports the Nuvoton NCT7362Y, an I2C fan controller with > 16 channels independently configurable for operation as PWM outputs, > fan tachometer inputs, or GPIOs. Most aspects of the chip's Is this a secret chip ? > functionality are supported, including PWM duty cycle, frequency, > enable/disable, and DC-mode operation. To support its GPIO > functionality, the driver also exposes a gpiochip interface if > CONFIG_SENSORS_NCT7362_GPIO is enabled. I do not see the point of this configuration flag. Why not just always enable it ? > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > MAINTAINERS | 7 + > drivers/hwmon/Kconfig | 18 ++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/nct7362.c | 697 ++++++++++++++++++++++++++++++++++++++++ Driver documentation is missing. > 4 files changed, 723 insertions(+) > create mode 100644 drivers/hwmon/nct7362.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index e0ad886d3163..7b2abff6d049 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14468,6 +14468,13 @@ S: Maintained > F: Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml > F: drivers/hwmon/nct6775-i2c.c > > +NCT7362 HARDWARE MONITOR DRIVER > +M: Zev Weiss <zev@bewilderbeest.net> > +L: linux-hwmon@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml > +F: drivers/hwmon/nct7362.c > + > NETDEVSIM > M: Jakub Kicinski <kuba@kernel.org> > S: Maintained > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index fc640201a2de..5ca51b4b52b6 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1562,6 +1562,24 @@ config SENSORS_NCT6775_I2C > This driver can also be built as a module. If so, the module > will be called nct6775-i2c. > > +config SENSORS_NCT7362 > + tristate "Nuvoton NCT7362Y fan controller" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for the Nuvoton NCT7362Y > + fan controller. > + > + This driver can also be built as a module. If so, the module > + will be called nct7362. > + > +config SENSORS_NCT7362_GPIO > + bool "Enable NCT7362Y GPIO support" > + depends on SENSORS_NCT7362 && GPIOLIB Why depends on GPIOLIB and not "select GPIOLIB" like everywhere else ? > + help > + Allow nct7362 pins not used as PWM outputs or tach inputs to > + be used as GPIOs. > + > config SENSORS_NCT7802 > tristate "Nuvoton NCT7802Y" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index cd8c568c80a9..29c674979280 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -162,6 +162,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_NCT7362) += nct7362.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/nct7362.c b/drivers/hwmon/nct7362.c > new file mode 100644 > index 000000000000..8f8e49097710 > --- /dev/null > +++ b/drivers/hwmon/nct7362.c > @@ -0,0 +1,697 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * nct7362 - Driver for Nuvoton NCT7362Y fan controller > + * > + * Copyright (C) 2023 Zev Weiss <zev@bewilderbeest.net> > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/of.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/hwmon.h> > +#include <linux/module.h> > +#include <linux/gpio/driver.h> Alphabetic order, please. > + > +#define DRVNAME "nct7362" > + > +#define REG_GPIO0_INPUT 0x00 > +#define REG_GPIO1_INPUT 0x10 Please always use #define<space>DEFINE<tab>value > + > +#define REG_GPIO0_OUTPUT 0x01 > +#define REG_GPIO1_OUTPUT 0x11 > + > +#define REG_GPIO0_IOCFG 0x03 > +#define REG_GPIO1_IOCFG 0x13 > + > +#define REG_GPIO0_OUTMODE 0x05 > +#define REG_GPIO1_OUTMODE 0x15 > + > +#define REG_PINCTRL_00 0x20 > +#define REG_PINCTRL_04 0x21 > +#define REG_PINCTRL_10 0x22 > +#define REG_PINCTRL_14 0x23 > + > +#define REG_PWM_0_7_EN 0x38 > +#define REG_PWM_8_15_EN 0x39 > + > +#define REG_TACH_GLOBAL_CTL 0x40 > +#define REG_TACH_0_7_EN 0x41 > +#define REG_TACH_8_15_EN 0x42 > + > +#define REG_TACH_HVAL(n) (0x48 + (2 * (n))) > +#define REG_TACH_LVAL(n) (0x49 + (2 * (n))) > + > +#define REG_FSCP_DUTY(n) (0x90 + (2 * (n))) > +#define REG_FSCP_DIV(n) (0x91 + (2 * (n))) > +#define REG_FSCP_CFG_BASE 0xb0 > + > +/* > + * For use in REG_PINCTRL_xx sub-fields. ALERT (3) also exists for some pins, > + * but is currently unsupported. > + */ > +#define PIN_MODE_GPIO 0 > +#define PIN_MODE_PWM 1 > +#define PIN_MODE_TACH 2 > + > +#define NUM_CHANNELS 16 > + > +/* Default all PWM channels to 100% */ > +#define DEFAULT_PWM_DUTY 0xff > + > +/* Default all PWMs to 1/256 resolution, non-inverted, PWM mode (non-DC) */ > +#define DEFAULT_FSCP_CFG 0x44 > + > +struct nct7362_data { > + struct i2c_client *client; > + struct regmap *regmap; > + > + /* > + * Bitmasks of which channels of which functions are enabled (offset by > + * one so channel 1 is at bit 0). Because the correspondence between > + * pin numbers and tach channels differs from that of GPIO/PWM channels, > + * some bit positions may be set in more than one of these (and some may > + * be set in none). > + */ > + struct { > + u16 gpio; > + u16 pwm; > + u16 tach; > + } functions; > + > + /* Pulses-per-revolution for each tach channel */ > + u32 tach_ppr[NUM_CHANNELS]; > + > +#ifdef CONFIG_SENSORS_NCT7362_GPIO > + /* In-use pins, either as PWM/tach or an actively-used GPIO */ > + unsigned long active_pins; > + > + struct gpio_chip gpio; > +#endif > +}; > + > +#ifdef CONFIG_SENSORS_NCT7362_GPIO > +static int nct7362_gpio_direction_input(struct gpio_chip *gpio, unsigned int offset) > +{ > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + u8 iocfg = offset < 8 ? REG_GPIO0_IOCFG : REG_GPIO1_IOCFG; > + u8 bit = BIT(offset % 8); > + > + return regmap_update_bits(chip->regmap, iocfg, bit, bit); > +} > + > +static int nct7362_gpio_direction_output(struct gpio_chip *gpio, unsigned int offset, int val) > +{ > + int status; > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + u8 outreg = offset < 8 ? REG_GPIO0_OUTPUT : REG_GPIO1_OUTPUT; > + u8 iocfg = offset < 8 ? REG_GPIO0_IOCFG : REG_GPIO1_IOCFG; > + u8 bit = BIT(offset % 8); > + > + status = regmap_update_bits(chip->regmap, iocfg, bit, 0); > + if (status) > + return status; > + > + return regmap_update_bits(chip->regmap, outreg, bit, val ? bit : 0); > +} > + > +static int nct7362_gpio_get_value(struct gpio_chip *gpio, unsigned int offset) > +{ > + unsigned int inbits; > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + u8 inreg = offset < 8 ? REG_GPIO0_INPUT : REG_GPIO1_INPUT; > + int status = regmap_read(chip->regmap, inreg, &inbits); > + > + return status ? : !!(inbits & BIT(offset % 8)); > +} > + > +static void nct7362_gpio_set_value(struct gpio_chip *gpio, unsigned int offset, int value) > +{ > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + u8 outreg = offset < 8 ? REG_GPIO0_OUTPUT : REG_GPIO1_OUTPUT; > + u8 bit = BIT(offset % 8); > + int status = regmap_update_bits(chip->regmap, outreg, bit, value ? bit : 0); > + > + /* No other way to report failure */ > + if (status) > + dev_dbg(&chip->client->dev, "regmap_update_bits() failed: %d\n", status); > +} > + > +static int nct7362_gpio_request_pin(struct gpio_chip *gpio, unsigned int offset) > +{ > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + > + return test_and_set_bit(offset, &chip->active_pins) ? -EBUSY : 0; > +} > + > +static void nct7362_gpio_free_pin(struct gpio_chip *gpio, unsigned int offset) > +{ > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + > + clear_bit(offset, &chip->active_pins); > +} > + > +static int nct7362_gpio_set_config(struct gpio_chip *gpio, unsigned int offset, > + unsigned long config) > +{ > + struct nct7362_data *chip = gpiochip_get_data(gpio); > + u8 modereg = offset < 8 ? REG_GPIO0_OUTMODE : REG_GPIO1_OUTMODE; > + u8 bit = BIT(offset % 8); > + int setting; > + enum pin_config_param param = pinconf_to_config_param(config); > + > + switch (param) { > + case PIN_CONFIG_DRIVE_OPEN_DRAIN: > + setting = 0; > + break; > + case PIN_CONFIG_DRIVE_PUSH_PULL: > + setting = 1; > + break; > + default: > + dev_dbg(&chip->client->dev, "unsupported config param %d\n", param); > + return -ENOTSUPP; checkpatch: WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP #268: FILE: drivers/hwmon/nct7362.c:173: + return -ENOTSUPP; > + } > + > + return regmap_update_bits(chip->regmap, modereg, bit, setting ? bit : 0); Why not just set "setting" to "bit" directly and avoid the conditinal here ? > +} > +#endif > + > +/* Convert a pin number to a PWM channel number */ > +static inline int pin_to_pwm(int pin) > +{ > + return pin < 9 ? (pin - 1) : (pin - 2); > +} > + > +#ifdef CONFIG_SENSORS_NCT7362_GPIO > +/* > + * Convert a pin number to a GPIO number -- interpreting the numbers in the > + * datasheet as octal, these are the same as the (decimal) PWM numbers. > + */ > +static inline int pin_to_gpio(int pin) > +{ > + return pin_to_pwm(pin); > +} > + > +/* GPIO that shares the same pin as the given PWM */ > +static inline int pwm_to_gpio(int pwm) > +{ > + /* GPIO & PWM numbering are the same */ > + return pwm; > +} > + > +/* GPIO that shares the same pin as the given tach */ > +static inline int tach_to_gpio(int tach) > +{ > + int pin = tach < 8 ? (tach + 10) : (tach - 7); > + > + return pin_to_gpio(pin); > +} > +#endif > + > +/* Convert a pin number to a tach (FANIN) channel number */ > +static inline int pin_to_tach(int pin) > +{ > + return pin < 9 ? (pin + 7) : (pin - 10); > +} > + > +static int nct7362_init_pin_functions(struct nct7362_data *chip) > +{ > + int channel, reg, status; > + u16 *mask; > + u8 function; > + struct device_node *child; > + u8 pinctrl[] = { 0, 0, 0, 0, }; > + struct device *dev = &chip->client->dev; > + > + for_each_available_child_of_node(dev->of_node, child) { > + int pwmnum, pinctrl_idx, pinctrl_shift; > + u32 pin; > + > + if (of_property_read_u32(child, "reg", &pin)) { > + dev_warn(dev, "skipping %pOF due to missing 'reg' property\n", child); > + continue; This should abort with an error. > + } > + > + if (pin == 0 || pin == 9 || pin > 17) { > + dev_warn(dev, "skipping %pOF with invalid pin number %u\n", child, pin); > + continue; Same here. > + } > + > + pwmnum = pin_to_pwm(pin); > + pinctrl_idx = pwmnum / 4; > + pinctrl_shift = 2 * (pwmnum % 4); > + > + if (of_node_name_prefix(child, "pwm")) { > + function = PIN_MODE_PWM; > + channel = pin_to_pwm(pin); > + mask = &chip->functions.pwm; > + } else if (of_node_name_prefix(child, "tach")) { > + function = PIN_MODE_TACH; > + channel = pin_to_tach(pin); > + mask = &chip->functions.tach; > + if (of_property_read_u32(child, "nuvoton,pulses-per-revolution", > + &chip->tach_ppr[channel])) > + chip->tach_ppr[channel] = 2; > +#ifdef CONFIG_SENSORS_NCT7362_GPIO > + } else if (of_node_name_prefix(child, "gpio")) { > + function = PIN_MODE_GPIO; > + channel = pin_to_gpio(pin); > + mask = &chip->functions.gpio; > +#endif > + } else { > + dev_warn(dev, "skipping %pOF of unknown function\n", child); and here. > + continue; > + } > + > + *mask |= BIT(channel); > + > + pinctrl[pinctrl_idx] |= function << pinctrl_shift; > + } > + > + for (reg = 0; reg < 4; reg++) { > + status = regmap_write(chip->regmap, REG_PINCTRL_00 + reg, pinctrl[reg]); > + if (status) > + return status; > + } > + > + return 0; > +} > + > +static int nct7362_init_pwm(struct nct7362_data *chip) > +{ > + int ret; > + > + for (int i = 0; i < 8; i++) { > + ret = regmap_write(chip->regmap, REG_FSCP_CFG_BASE + i, DEFAULT_FSCP_CFG); > + if (ret) > + return ret; > + } > + > + for (int i = 0; i < NUM_CHANNELS; i++) { > + ret = regmap_write(chip->regmap, REG_FSCP_DUTY(i), DEFAULT_PWM_DUTY); > + if (ret) > + return ret; > + } > + > + for (int i = 0; i < 2; i++) { > + ret = regmap_write(chip->regmap, REG_PWM_0_7_EN + i, 0xff); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static int nct7362_init_fan(struct nct7362_data *chip) > +{ > + int ret; > + > + /* > + * tach_ppr *should* be set for all configured tach channels by this > + * point, but to ensure we don't end up dividing by zero let's make sure. > + */ > + for (int i = 0; i < 16; i++) { > + if (!chip->tach_ppr[i]) > + chip->tach_ppr[i] = 2; > + } > + > + /* Enable individual fan tachs */ > + for (int i = 0; i < 2; i++) { > + ret = regmap_write(chip->regmap, REG_TACH_0_7_EN + i, 0xff); > + if (ret) > + return ret; > + } > + > + /* Ensure global fan tach enable is on */ > + return regmap_update_bits(chip->regmap, REG_TACH_GLOBAL_CTL, 0x80, 0x80); > +} > + > +static int nct7362_init_gpio(struct nct7362_data *chip) > +{ > +#ifdef CONFIG_SENSORS_NCT7362_GPIO > + struct gpio_chip *gpio = &chip->gpio; > + > + for (int i = 0; i < 16; i++) { > + /* Pins in use as PWM or tach are permanently active */ > + if (chip->functions.pwm & BIT(i)) > + set_bit(pwm_to_gpio(i), &chip->active_pins); > + if (chip->functions.tach & BIT(i)) > + set_bit(tach_to_gpio(i), &chip->active_pins); > + } > + > + gpio->label = dev_name(&chip->client->dev); > + gpio->direction_input = nct7362_gpio_direction_input; > + gpio->direction_output = nct7362_gpio_direction_output; > + gpio->get = nct7362_gpio_get_value; > + gpio->set = nct7362_gpio_set_value; > + gpio->request = nct7362_gpio_request_pin; > + gpio->free = nct7362_gpio_free_pin; > + gpio->set_config = nct7362_gpio_set_config; > + gpio->can_sleep = 1; > + gpio->base = -1; > + gpio->ngpio = NUM_CHANNELS; > + gpio->parent = &chip->client->dev; > + gpio->owner = THIS_MODULE; > + > + return devm_gpiochip_add_data(&chip->client->dev, gpio, chip); > +#else > + return 0; > +#endif > +} > + > +static umode_t nct7362_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + umode_t mode = 0; > + const struct nct7362_data *chip = data; > + Please try to declare variables in reverse christmas tree order where possible to improve readability. > + switch (type) { > + case hwmon_pwm: > + if (!(chip->functions.pwm & BIT(channel))) > + break; > + switch (attr) { > + case hwmon_pwm_enable: > + case hwmon_pwm_input: > + case hwmon_pwm_freq: > + case hwmon_pwm_mode: > + mode = 0644; > + break; > + default: > + break; > + } > + break; > + case hwmon_fan: > + if (!(chip->functions.tach & BIT(channel))) > + break; > + switch (attr) { > + case hwmon_fan_input: > + mode = 0444; > + break; > + default: > + break; > + } > + break; > + default: > + break; > + } > + > + return mode; > +} > + > +/* > + * We set STEP to 1 on all PWM channels during initialization (for > + * higher-resolution duty cycle control) and never change it, so our frequency > + * calculations all assume it's set to 1 rather than re-reading the register all > + * the time. > + * > + * Supported frequency ranges: > + * DIVSEL=0: 488Hz - 62.5KHz > + * DIVSEL=1: 2Hz - 244Hz > + */ > +#define PWM_STEP 1 > +#define BASE_FANCLK 16000000 > +#define PWM_FREQ_MIN_DIVSEL0 (BASE_FANCLK / 32768) /* 488 */ > +#define PWM_FREQ_MAX_DIVSEL1 (BASE_FANCLK / 65536) /* 244 */ > + > +/* Frequency threshold below which we switch to DIVSEL=1 */ > +#define PWM_DIVSEL_CUTOFF ((PWM_FREQ_MIN_DIVSEL0 + PWM_FREQ_MAX_DIVSEL1) / 2) > + > +static long nct7362_pwm_freq(long divisor, bool divsel) > +{ > + long denom = (divisor + 1) << (7 + PWM_STEP + (divsel ? 8 : 0)); > + > + return BASE_FANCLK / denom; > +} > + > +static int nct7362_read_pwm(struct nct7362_data *chip, u32 attr, int channel, long *val) > +{ > + u8 reg; > + int status; > + unsigned int tmp; > + > + if (!(chip->functions.pwm & BIT(channel))) > + return -EBUSY; Why is this needed ? If the bit is configured as gpio the pwm channel should not be available. > + > + switch (attr) { > + case hwmon_pwm_enable: > + reg = channel < 8 ? REG_PWM_0_7_EN : REG_PWM_8_15_EN; > + status = regmap_read(chip->regmap, reg, &tmp); > + if (!status) > + *val = !!(tmp & BIT(channel % 8)); Please consistently use if (status) return status; *val = ... > + break; > + > + case hwmon_pwm_freq: > + reg = REG_FSCP_DIV(channel); > + status = regmap_read(chip->regmap, reg, &tmp); > + if (status) > + return status; > + > + *val = nct7362_pwm_freq(tmp & 0x7f, !!(tmp & 0x80)); > + break; > + > + case hwmon_pwm_input: > + reg = REG_FSCP_DUTY(channel); > + status = regmap_read(chip->regmap, reg, &tmp); > + if (!status) > + *val = tmp; > + break; > + > + case hwmon_pwm_mode: > + reg = REG_FSCP_CFG_BASE + (channel / 2); > + status = regmap_read(chip->regmap, reg, &tmp); > + if (!status) > + *val = !(tmp & BIT(4 * (channel % 2))); > + break; > + > + default: > + return -EOPNOTSUPP; > + } > + > + return status; > +} > + > +static int nct7362_write_pwm(struct nct7362_data *chip, u32 attr, int channel, long val) > +{ > + u8 reg, bit, divsel; > + long divisor; > + int status; > + > + if (!(chip->functions.pwm & BIT(channel))) > + return -EBUSY; Same question as above. > + > + switch (attr) { > + case hwmon_pwm_input: > + reg = REG_FSCP_DUTY(channel); > + status = regmap_write(chip->regmap, reg, val); > + break; > + > + case hwmon_pwm_enable: > + reg = channel < 8 ? REG_PWM_0_7_EN : REG_PWM_8_15_EN; > + bit = BIT(channel % 8); > + status = regmap_update_bits(chip->regmap, reg, bit, val ? bit : 0); This (and other functions below) should not accept random values. > + break; > + > + case hwmon_pwm_mode: > + reg = REG_FSCP_CFG_BASE + (channel / 2); > + bit = BIT(4 * (channel % 2)); > + status = regmap_update_bits(chip->regmap, reg, bit, val ? 0 : bit); > + break; > + > + case hwmon_pwm_freq: > + divsel = val < PWM_DIVSEL_CUTOFF; > + > + divisor = BASE_FANCLK / (val << (7 + PWM_STEP + (divsel ? 8 : 0))) - 1; > + divisor = clamp(divisor, 0L, 0x7fL); > + divisor |= divsel << 7; > + > + reg = REG_FSCP_DIV(channel); > + status = regmap_write(chip->regmap, reg, divisor); > + break; > + > + default: > + return -EOPNOTSUPP; > + } > + > + return status; > +} > + > +static int nct7362_read_fan_speed(struct nct7362_data *chip, int channel, long *val) > +{ > + unsigned int hi, lo, count; > + int status; > + > + status = regmap_read(chip->regmap, REG_TACH_HVAL(channel), &hi); > + if (status) > + return status; > + > + status = regmap_read(chip->regmap, REG_TACH_LVAL(channel), &lo); > + if (status) > + return status; > + > + count = (hi << 5) | (lo & 0x1f); > + The problem with this approach is that hval could change after it was read. The datasheet is not public, so there is no means for me to verify if the chip prevents this from happening. Please add a comment explaining why this can not result in bogus readings. > + *val = 1350000 / (count * chip->tach_ppr[channel] / 2); > + > + return 0; > +} > + > +static int nct7362_read_tach(struct nct7362_data *chip, u32 attr, int channel, long *val) > +{ > + if (!(chip->functions.tach & BIT(channel))) > + return -EBUSY; > + Same question as above. > + switch (attr) { > + case hwmon_fan_input: > + return nct7362_read_fan_speed(chip, channel, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int nct7362_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct nct7362_data *chip = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_pwm: > + return nct7362_read_pwm(chip, attr, channel, val); > + case hwmon_fan: > + return nct7362_read_tach(chip, attr, channel, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int nct7362_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + struct nct7362_data *chip = dev_get_drvdata(dev); > + > + switch (type) { > + case hwmon_pwm: > + return nct7362_write_pwm(chip, attr, channel, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static const struct hwmon_ops nct7362_hwmon_ops = { > + .is_visible = nct7362_is_visible, > + .read = nct7362_read, > + .write = nct7362_write, > +}; > + > +static const struct hwmon_channel_info *nct7362_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_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, > + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE), > + NULL, > +}; > + > +static const struct hwmon_chip_info nct7362_chip_info = { > + .ops = &nct7362_hwmon_ops, > + .info = nct7362_info, > +}; > + > +static const struct regmap_config nct7362_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > +static int nct7362_probe(struct i2c_client *client) > +{ > + struct device *hwmon_dev; > + struct device *dev = &client->dev; > + struct nct7362_data *chip; > + int ret; > + > + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL); > + if (!chip) > + return -ENOMEM; > + > + chip->client = client; > + chip->regmap = devm_regmap_init_i2c(client, &nct7362_regmap_config); > + if (IS_ERR(chip->regmap)) > + return PTR_ERR(chip->regmap); > + > + ret = nct7362_init_pin_functions(chip); > + if (ret) > + return ret; > + > + ret = nct7362_init_pwm(chip); > + if (ret) > + return ret; > + > + ret = nct7362_init_fan(chip); > + if (ret) > + return ret; > + > + ret = nct7362_init_gpio(chip); > + if (ret) > + return ret; > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, chip, > + &nct7362_chip_info, NULL); > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct i2c_device_id nct7362_idtable[] = { > + { "nct7362", }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, nct7362_idtable); > + > +static const struct of_device_id __maybe_unused nct7362_of_match[] = { > + { .compatible = "nuvoton,nct7362", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, nct7362_of_match); > + > +static struct i2c_driver nct7362_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = DRVNAME, > + .of_match_table = of_match_ptr(nct7362_of_match), > + }, > + .probe_new = nct7362_probe, > + .id_table = nct7362_idtable, > +}; > + > +module_i2c_driver(nct7362_driver); > + > +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); > +MODULE_DESCRIPTION("NCT7362Y fan controller driver"); > +MODULE_LICENSE("GPL"); > -- > 2.40.0.5.gf6e3b97ba6d2.dirty > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver 2023-06-07 18:00 ` Guenter Roeck @ 2023-06-07 19:33 ` Zev Weiss 0 siblings, 0 replies; 7+ messages in thread From: Zev Weiss @ 2023-06-07 19:33 UTC (permalink / raw) To: Guenter Roeck; +Cc: Jean Delvare, linux-hwmon, linux-kernel, openbmc On Wed, Jun 07, 2023 at 11:00:44AM PDT, Guenter Roeck wrote: >On Wed, Jun 07, 2023 at 03:18:30AM -0700, Zev Weiss wrote: >> This driver supports the Nuvoton NCT7362Y, an I2C fan controller with >> 16 channels independently configurable for operation as PWM outputs, >> fan tachometer inputs, or GPIOs. Most aspects of the chip's > >Is this a secret chip ? > Maybe so; I couldn't find any public information about it, but Nuvoton provided me the datasheet when I requested it from them. >> functionality are supported, including PWM duty cycle, frequency, >> enable/disable, and DC-mode operation. To support its GPIO >> functionality, the driver also exposes a gpiochip interface if >> CONFIG_SENSORS_NCT7362_GPIO is enabled. > >I do not see the point of this configuration flag. Why not just >always enable it ? > I don't have any particular need for it myself, but was following the pattern of LEDS_PCA95(5X|32)_GPIO and the various pmbus drivers' SENSORS_*_REGULATOR options, which I assume were intended to allow using the base driver without the CONFIG_(GPIOLIB|REGULATOR) dependency. Would you prefer I get rid of it or leave it as is? >> >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >> --- >> MAINTAINERS | 7 + >> drivers/hwmon/Kconfig | 18 ++ >> drivers/hwmon/Makefile | 1 + >> drivers/hwmon/nct7362.c | 697 ++++++++++++++++++++++++++++++++++++++++ > >Driver documentation is missing. > Ack, will add in v2. >> 4 files changed, 723 insertions(+) >> create mode 100644 drivers/hwmon/nct7362.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index e0ad886d3163..7b2abff6d049 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -14468,6 +14468,13 @@ S: Maintained >> F: Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml >> F: drivers/hwmon/nct6775-i2c.c >> >> +NCT7362 HARDWARE MONITOR DRIVER >> +M: Zev Weiss <zev@bewilderbeest.net> >> +L: linux-hwmon@vger.kernel.org >> +S: Maintained >> +F: Documentation/devicetree/bindings/hwmon/nuvoton,nct7362.yaml >> +F: drivers/hwmon/nct7362.c >> + >> NETDEVSIM >> M: Jakub Kicinski <kuba@kernel.org> >> S: Maintained >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >> index fc640201a2de..5ca51b4b52b6 100644 >> --- a/drivers/hwmon/Kconfig >> +++ b/drivers/hwmon/Kconfig >> @@ -1562,6 +1562,24 @@ config SENSORS_NCT6775_I2C >> This driver can also be built as a module. If so, the module >> will be called nct6775-i2c. >> >> +config SENSORS_NCT7362 >> + tristate "Nuvoton NCT7362Y fan controller" >> + depends on I2C >> + select REGMAP_I2C >> + help >> + If you say yes here you get support for the Nuvoton NCT7362Y >> + fan controller. >> + >> + This driver can also be built as a module. If so, the module >> + will be called nct7362. >> + >> +config SENSORS_NCT7362_GPIO >> + bool "Enable NCT7362Y GPIO support" >> + depends on SENSORS_NCT7362 && GPIOLIB > >Why depends on GPIOLIB and not "select GPIOLIB" like everywhere else ? > As above, just following the pattern of existing led/gpio and pmbus/regulator drivers, but can change it if you prefer. >> + help >> + Allow nct7362 pins not used as PWM outputs or tach inputs to >> + be used as GPIOs. >> + >> config SENSORS_NCT7802 >> tristate "Nuvoton NCT7802Y" >> depends on I2C >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >> index cd8c568c80a9..29c674979280 100644 >> --- a/drivers/hwmon/Makefile >> +++ b/drivers/hwmon/Makefile >> @@ -162,6 +162,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_NCT7362) += nct7362.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/nct7362.c b/drivers/hwmon/nct7362.c >> new file mode 100644 >> index 000000000000..8f8e49097710 >> --- /dev/null >> +++ b/drivers/hwmon/nct7362.c >> @@ -0,0 +1,697 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * nct7362 - Driver for Nuvoton NCT7362Y fan controller >> + * >> + * Copyright (C) 2023 Zev Weiss <zev@bewilderbeest.net> >> + */ >> + >> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> + >> +#include <linux/of.h> >> +#include <linux/i2c.h> >> +#include <linux/regmap.h> >> +#include <linux/hwmon.h> >> +#include <linux/module.h> >> +#include <linux/gpio/driver.h> > >Alphabetic order, please. > Ack. >> + >> +#define DRVNAME "nct7362" >> + >> +#define REG_GPIO0_INPUT 0x00 >> +#define REG_GPIO1_INPUT 0x10 > >Please always use > >#define<space>DEFINE<tab>value > Ack. >> + >> +#define REG_GPIO0_OUTPUT 0x01 >> +#define REG_GPIO1_OUTPUT 0x11 >> + >> +#define REG_GPIO0_IOCFG 0x03 >> +#define REG_GPIO1_IOCFG 0x13 >> + >> +#define REG_GPIO0_OUTMODE 0x05 >> +#define REG_GPIO1_OUTMODE 0x15 >> + >> +#define REG_PINCTRL_00 0x20 >> +#define REG_PINCTRL_04 0x21 >> +#define REG_PINCTRL_10 0x22 >> +#define REG_PINCTRL_14 0x23 >> + >> +#define REG_PWM_0_7_EN 0x38 >> +#define REG_PWM_8_15_EN 0x39 >> + >> +#define REG_TACH_GLOBAL_CTL 0x40 >> +#define REG_TACH_0_7_EN 0x41 >> +#define REG_TACH_8_15_EN 0x42 >> + >> +#define REG_TACH_HVAL(n) (0x48 + (2 * (n))) >> +#define REG_TACH_LVAL(n) (0x49 + (2 * (n))) >> + >> +#define REG_FSCP_DUTY(n) (0x90 + (2 * (n))) >> +#define REG_FSCP_DIV(n) (0x91 + (2 * (n))) >> +#define REG_FSCP_CFG_BASE 0xb0 >> + >> +/* >> + * For use in REG_PINCTRL_xx sub-fields. ALERT (3) also exists for some pins, >> + * but is currently unsupported. >> + */ >> +#define PIN_MODE_GPIO 0 >> +#define PIN_MODE_PWM 1 >> +#define PIN_MODE_TACH 2 >> + >> +#define NUM_CHANNELS 16 >> + >> +/* Default all PWM channels to 100% */ >> +#define DEFAULT_PWM_DUTY 0xff >> + >> +/* Default all PWMs to 1/256 resolution, non-inverted, PWM mode (non-DC) */ >> +#define DEFAULT_FSCP_CFG 0x44 >> + >> +struct nct7362_data { >> + struct i2c_client *client; >> + struct regmap *regmap; >> + >> + /* >> + * Bitmasks of which channels of which functions are enabled (offset by >> + * one so channel 1 is at bit 0). Because the correspondence between >> + * pin numbers and tach channels differs from that of GPIO/PWM channels, >> + * some bit positions may be set in more than one of these (and some may >> + * be set in none). >> + */ >> + struct { >> + u16 gpio; >> + u16 pwm; >> + u16 tach; >> + } functions; >> + >> + /* Pulses-per-revolution for each tach channel */ >> + u32 tach_ppr[NUM_CHANNELS]; >> + >> +#ifdef CONFIG_SENSORS_NCT7362_GPIO >> + /* In-use pins, either as PWM/tach or an actively-used GPIO */ >> + unsigned long active_pins; >> + >> + struct gpio_chip gpio; >> +#endif >> +}; >> + >> +#ifdef CONFIG_SENSORS_NCT7362_GPIO >> +static int nct7362_gpio_direction_input(struct gpio_chip *gpio, unsigned int offset) >> +{ >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + u8 iocfg = offset < 8 ? REG_GPIO0_IOCFG : REG_GPIO1_IOCFG; >> + u8 bit = BIT(offset % 8); >> + >> + return regmap_update_bits(chip->regmap, iocfg, bit, bit); >> +} >> + >> +static int nct7362_gpio_direction_output(struct gpio_chip *gpio, unsigned int offset, int val) >> +{ >> + int status; >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + u8 outreg = offset < 8 ? REG_GPIO0_OUTPUT : REG_GPIO1_OUTPUT; >> + u8 iocfg = offset < 8 ? REG_GPIO0_IOCFG : REG_GPIO1_IOCFG; >> + u8 bit = BIT(offset % 8); >> + >> + status = regmap_update_bits(chip->regmap, iocfg, bit, 0); >> + if (status) >> + return status; >> + >> + return regmap_update_bits(chip->regmap, outreg, bit, val ? bit : 0); >> +} >> + >> +static int nct7362_gpio_get_value(struct gpio_chip *gpio, unsigned int offset) >> +{ >> + unsigned int inbits; >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + u8 inreg = offset < 8 ? REG_GPIO0_INPUT : REG_GPIO1_INPUT; >> + int status = regmap_read(chip->regmap, inreg, &inbits); >> + >> + return status ? : !!(inbits & BIT(offset % 8)); >> +} >> + >> +static void nct7362_gpio_set_value(struct gpio_chip *gpio, unsigned int offset, int value) >> +{ >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + u8 outreg = offset < 8 ? REG_GPIO0_OUTPUT : REG_GPIO1_OUTPUT; >> + u8 bit = BIT(offset % 8); >> + int status = regmap_update_bits(chip->regmap, outreg, bit, value ? bit : 0); >> + >> + /* No other way to report failure */ >> + if (status) >> + dev_dbg(&chip->client->dev, "regmap_update_bits() failed: %d\n", status); >> +} >> + >> +static int nct7362_gpio_request_pin(struct gpio_chip *gpio, unsigned int offset) >> +{ >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + >> + return test_and_set_bit(offset, &chip->active_pins) ? -EBUSY : 0; >> +} >> + >> +static void nct7362_gpio_free_pin(struct gpio_chip *gpio, unsigned int offset) >> +{ >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + >> + clear_bit(offset, &chip->active_pins); >> +} >> + >> +static int nct7362_gpio_set_config(struct gpio_chip *gpio, unsigned int offset, >> + unsigned long config) >> +{ >> + struct nct7362_data *chip = gpiochip_get_data(gpio); >> + u8 modereg = offset < 8 ? REG_GPIO0_OUTMODE : REG_GPIO1_OUTMODE; >> + u8 bit = BIT(offset % 8); >> + int setting; >> + enum pin_config_param param = pinconf_to_config_param(config); >> + >> + switch (param) { >> + case PIN_CONFIG_DRIVE_OPEN_DRAIN: >> + setting = 0; >> + break; >> + case PIN_CONFIG_DRIVE_PUSH_PULL: >> + setting = 1; >> + break; >> + default: >> + dev_dbg(&chip->client->dev, "unsupported config param %d\n", param); >> + return -ENOTSUPP; > >checkpatch: > >WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP >#268: FILE: drivers/hwmon/nct7362.c:173: >+ return -ENOTSUPP; > Unfortunately ENOTSUPP is specifically required for correct functionality with gpiolib (see gpio_set_config_with_argument_optional() in drivers/gpio/gpiolib.c), so I opted to ignore the checkpatch warning. >> + } >> + >> + return regmap_update_bits(chip->regmap, modereg, bit, setting ? bit : 0); > >Why not just set "setting" to "bit" directly and avoid the conditinal here ? > True, that would be a bit simpler I suppose -- will do. >> +} >> +#endif >> + >> +/* Convert a pin number to a PWM channel number */ >> +static inline int pin_to_pwm(int pin) >> +{ >> + return pin < 9 ? (pin - 1) : (pin - 2); >> +} >> + >> +#ifdef CONFIG_SENSORS_NCT7362_GPIO >> +/* >> + * Convert a pin number to a GPIO number -- interpreting the numbers in the >> + * datasheet as octal, these are the same as the (decimal) PWM numbers. >> + */ >> +static inline int pin_to_gpio(int pin) >> +{ >> + return pin_to_pwm(pin); >> +} >> + >> +/* GPIO that shares the same pin as the given PWM */ >> +static inline int pwm_to_gpio(int pwm) >> +{ >> + /* GPIO & PWM numbering are the same */ >> + return pwm; >> +} >> + >> +/* GPIO that shares the same pin as the given tach */ >> +static inline int tach_to_gpio(int tach) >> +{ >> + int pin = tach < 8 ? (tach + 10) : (tach - 7); >> + >> + return pin_to_gpio(pin); >> +} >> +#endif >> + >> +/* Convert a pin number to a tach (FANIN) channel number */ >> +static inline int pin_to_tach(int pin) >> +{ >> + return pin < 9 ? (pin + 7) : (pin - 10); >> +} >> + >> +static int nct7362_init_pin_functions(struct nct7362_data *chip) >> +{ >> + int channel, reg, status; >> + u16 *mask; >> + u8 function; >> + struct device_node *child; >> + u8 pinctrl[] = { 0, 0, 0, 0, }; >> + struct device *dev = &chip->client->dev; >> + >> + for_each_available_child_of_node(dev->of_node, child) { >> + int pwmnum, pinctrl_idx, pinctrl_shift; >> + u32 pin; >> + >> + if (of_property_read_u32(child, "reg", &pin)) { >> + dev_warn(dev, "skipping %pOF due to missing 'reg' property\n", child); >> + continue; > >This should abort with an error. > Ack. >> + } >> + >> + if (pin == 0 || pin == 9 || pin > 17) { >> + dev_warn(dev, "skipping %pOF with invalid pin number %u\n", child, pin); >> + continue; > >Same here. > Ack. >> + } >> + >> + pwmnum = pin_to_pwm(pin); >> + pinctrl_idx = pwmnum / 4; >> + pinctrl_shift = 2 * (pwmnum % 4); >> + >> + if (of_node_name_prefix(child, "pwm")) { >> + function = PIN_MODE_PWM; >> + channel = pin_to_pwm(pin); >> + mask = &chip->functions.pwm; >> + } else if (of_node_name_prefix(child, "tach")) { >> + function = PIN_MODE_TACH; >> + channel = pin_to_tach(pin); >> + mask = &chip->functions.tach; >> + if (of_property_read_u32(child, "nuvoton,pulses-per-revolution", >> + &chip->tach_ppr[channel])) >> + chip->tach_ppr[channel] = 2; >> +#ifdef CONFIG_SENSORS_NCT7362_GPIO >> + } else if (of_node_name_prefix(child, "gpio")) { >> + function = PIN_MODE_GPIO; >> + channel = pin_to_gpio(pin); >> + mask = &chip->functions.gpio; >> +#endif >> + } else { >> + dev_warn(dev, "skipping %pOF of unknown function\n", child); > >and here. > Ack. >> + continue; >> + } >> + >> + *mask |= BIT(channel); >> + >> + pinctrl[pinctrl_idx] |= function << pinctrl_shift; >> + } >> + >> + for (reg = 0; reg < 4; reg++) { >> + status = regmap_write(chip->regmap, REG_PINCTRL_00 + reg, pinctrl[reg]); >> + if (status) >> + return status; >> + } >> + >> + return 0; >> +} >> + >> +static int nct7362_init_pwm(struct nct7362_data *chip) >> +{ >> + int ret; >> + >> + for (int i = 0; i < 8; i++) { >> + ret = regmap_write(chip->regmap, REG_FSCP_CFG_BASE + i, DEFAULT_FSCP_CFG); >> + if (ret) >> + return ret; >> + } >> + >> + for (int i = 0; i < NUM_CHANNELS; i++) { >> + ret = regmap_write(chip->regmap, REG_FSCP_DUTY(i), DEFAULT_PWM_DUTY); >> + if (ret) >> + return ret; >> + } >> + >> + for (int i = 0; i < 2; i++) { >> + ret = regmap_write(chip->regmap, REG_PWM_0_7_EN + i, 0xff); >> + if (ret) >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int nct7362_init_fan(struct nct7362_data *chip) >> +{ >> + int ret; >> + >> + /* >> + * tach_ppr *should* be set for all configured tach channels by this >> + * point, but to ensure we don't end up dividing by zero let's make sure. >> + */ >> + for (int i = 0; i < 16; i++) { >> + if (!chip->tach_ppr[i]) >> + chip->tach_ppr[i] = 2; >> + } >> + >> + /* Enable individual fan tachs */ >> + for (int i = 0; i < 2; i++) { >> + ret = regmap_write(chip->regmap, REG_TACH_0_7_EN + i, 0xff); >> + if (ret) >> + return ret; >> + } >> + >> + /* Ensure global fan tach enable is on */ >> + return regmap_update_bits(chip->regmap, REG_TACH_GLOBAL_CTL, 0x80, 0x80); >> +} >> + >> +static int nct7362_init_gpio(struct nct7362_data *chip) >> +{ >> +#ifdef CONFIG_SENSORS_NCT7362_GPIO >> + struct gpio_chip *gpio = &chip->gpio; >> + >> + for (int i = 0; i < 16; i++) { >> + /* Pins in use as PWM or tach are permanently active */ >> + if (chip->functions.pwm & BIT(i)) >> + set_bit(pwm_to_gpio(i), &chip->active_pins); >> + if (chip->functions.tach & BIT(i)) >> + set_bit(tach_to_gpio(i), &chip->active_pins); >> + } >> + >> + gpio->label = dev_name(&chip->client->dev); >> + gpio->direction_input = nct7362_gpio_direction_input; >> + gpio->direction_output = nct7362_gpio_direction_output; >> + gpio->get = nct7362_gpio_get_value; >> + gpio->set = nct7362_gpio_set_value; >> + gpio->request = nct7362_gpio_request_pin; >> + gpio->free = nct7362_gpio_free_pin; >> + gpio->set_config = nct7362_gpio_set_config; >> + gpio->can_sleep = 1; >> + gpio->base = -1; >> + gpio->ngpio = NUM_CHANNELS; >> + gpio->parent = &chip->client->dev; >> + gpio->owner = THIS_MODULE; >> + >> + return devm_gpiochip_add_data(&chip->client->dev, gpio, chip); >> +#else >> + return 0; >> +#endif >> +} >> + >> +static umode_t nct7362_is_visible(const void *data, enum hwmon_sensor_types type, >> + u32 attr, int channel) >> +{ >> + umode_t mode = 0; >> + const struct nct7362_data *chip = data; >> + > >Please try to declare variables in reverse christmas tree order where >possible to improve readability. > Ack. >> + switch (type) { >> + case hwmon_pwm: >> + if (!(chip->functions.pwm & BIT(channel))) >> + break; >> + switch (attr) { >> + case hwmon_pwm_enable: >> + case hwmon_pwm_input: >> + case hwmon_pwm_freq: >> + case hwmon_pwm_mode: >> + mode = 0644; >> + break; >> + default: >> + break; >> + } >> + break; >> + case hwmon_fan: >> + if (!(chip->functions.tach & BIT(channel))) >> + break; >> + switch (attr) { >> + case hwmon_fan_input: >> + mode = 0444; >> + break; >> + default: >> + break; >> + } >> + break; >> + default: >> + break; >> + } >> + >> + return mode; >> +} >> + >> +/* >> + * We set STEP to 1 on all PWM channels during initialization (for >> + * higher-resolution duty cycle control) and never change it, so our frequency >> + * calculations all assume it's set to 1 rather than re-reading the register all >> + * the time. >> + * >> + * Supported frequency ranges: >> + * DIVSEL=0: 488Hz - 62.5KHz >> + * DIVSEL=1: 2Hz - 244Hz >> + */ >> +#define PWM_STEP 1 >> +#define BASE_FANCLK 16000000 >> +#define PWM_FREQ_MIN_DIVSEL0 (BASE_FANCLK / 32768) /* 488 */ >> +#define PWM_FREQ_MAX_DIVSEL1 (BASE_FANCLK / 65536) /* 244 */ >> + >> +/* Frequency threshold below which we switch to DIVSEL=1 */ >> +#define PWM_DIVSEL_CUTOFF ((PWM_FREQ_MIN_DIVSEL0 + PWM_FREQ_MAX_DIVSEL1) / 2) >> + >> +static long nct7362_pwm_freq(long divisor, bool divsel) >> +{ >> + long denom = (divisor + 1) << (7 + PWM_STEP + (divsel ? 8 : 0)); >> + >> + return BASE_FANCLK / denom; >> +} >> + >> +static int nct7362_read_pwm(struct nct7362_data *chip, u32 attr, int channel, long *val) >> +{ >> + u8 reg; >> + int status; >> + unsigned int tmp; >> + >> + if (!(chip->functions.pwm & BIT(channel))) >> + return -EBUSY; > >Why is this needed ? If the bit is configured as gpio the pwm channel >should not be available. > I added it as a "belt and suspenders" sort of check, but perhaps it's excessive -- I'll take it out. >> + >> + switch (attr) { >> + case hwmon_pwm_enable: >> + reg = channel < 8 ? REG_PWM_0_7_EN : REG_PWM_8_15_EN; >> + status = regmap_read(chip->regmap, reg, &tmp); >> + if (!status) >> + *val = !!(tmp & BIT(channel % 8)); > >Please consistently use > if (status) > return status; > *val = ... > Ack. >> + break; >> + >> + case hwmon_pwm_freq: >> + reg = REG_FSCP_DIV(channel); >> + status = regmap_read(chip->regmap, reg, &tmp); >> + if (status) >> + return status; >> + >> + *val = nct7362_pwm_freq(tmp & 0x7f, !!(tmp & 0x80)); >> + break; >> + >> + case hwmon_pwm_input: >> + reg = REG_FSCP_DUTY(channel); >> + status = regmap_read(chip->regmap, reg, &tmp); >> + if (!status) >> + *val = tmp; >> + break; >> + >> + case hwmon_pwm_mode: >> + reg = REG_FSCP_CFG_BASE + (channel / 2); >> + status = regmap_read(chip->regmap, reg, &tmp); >> + if (!status) >> + *val = !(tmp & BIT(4 * (channel % 2))); >> + break; >> + >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + return status; >> +} >> + >> +static int nct7362_write_pwm(struct nct7362_data *chip, u32 attr, int channel, long val) >> +{ >> + u8 reg, bit, divsel; >> + long divisor; >> + int status; >> + >> + if (!(chip->functions.pwm & BIT(channel))) >> + return -EBUSY; > >Same question as above. > Ack. >> + >> + switch (attr) { >> + case hwmon_pwm_input: >> + reg = REG_FSCP_DUTY(channel); >> + status = regmap_write(chip->regmap, reg, val); >> + break; >> + >> + case hwmon_pwm_enable: >> + reg = channel < 8 ? REG_PWM_0_7_EN : REG_PWM_8_15_EN; >> + bit = BIT(channel % 8); >> + status = regmap_update_bits(chip->regmap, reg, bit, val ? bit : 0); > >This (and other functions below) should not accept random values. > Oh, I hadn't realized that pwm_enable wasn't just a boolean -- will fix to reject val >= 2. >> + break; >> + >> + case hwmon_pwm_mode: >> + reg = REG_FSCP_CFG_BASE + (channel / 2); >> + bit = BIT(4 * (channel % 2)); >> + status = regmap_update_bits(chip->regmap, reg, bit, val ? 0 : bit); >> + break; >> + >> + case hwmon_pwm_freq: >> + divsel = val < PWM_DIVSEL_CUTOFF; >> + >> + divisor = BASE_FANCLK / (val << (7 + PWM_STEP + (divsel ? 8 : 0))) - 1; >> + divisor = clamp(divisor, 0L, 0x7fL); >> + divisor |= divsel << 7; >> + >> + reg = REG_FSCP_DIV(channel); >> + status = regmap_write(chip->regmap, reg, divisor); >> + break; >> + >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + return status; >> +} >> + >> +static int nct7362_read_fan_speed(struct nct7362_data *chip, int channel, long *val) >> +{ >> + unsigned int hi, lo, count; >> + int status; >> + >> + status = regmap_read(chip->regmap, REG_TACH_HVAL(channel), &hi); >> + if (status) >> + return status; >> + >> + status = regmap_read(chip->regmap, REG_TACH_LVAL(channel), &lo); >> + if (status) >> + return status; >> + >> + count = (hi << 5) | (lo & 0x1f); >> + > >The problem with this approach is that hval could change after it was read. >The datasheet is not public, so there is no means for me to verify if the >chip prevents this from happening. Please add a comment explaining why this >can not result in bogus readings. > The datasheet has a note explaining that reading the HVAL register latches the value in the LVAL register -- I'll add a corresponding comment. >> + *val = 1350000 / (count * chip->tach_ppr[channel] / 2); >> + >> + return 0; >> +} >> + >> +static int nct7362_read_tach(struct nct7362_data *chip, u32 attr, int channel, long *val) >> +{ >> + if (!(chip->functions.tach & BIT(channel))) >> + return -EBUSY; >> + > >Same question as above. > Ack. >> + switch (attr) { >> + case hwmon_fan_input: >> + return nct7362_read_fan_speed(chip, channel, val); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int nct7362_read(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long *val) >> +{ >> + struct nct7362_data *chip = dev_get_drvdata(dev); >> + >> + switch (type) { >> + case hwmon_pwm: >> + return nct7362_read_pwm(chip, attr, channel, val); >> + case hwmon_fan: >> + return nct7362_read_tach(chip, attr, channel, val); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int nct7362_write(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long val) >> +{ >> + struct nct7362_data *chip = dev_get_drvdata(dev); >> + >> + switch (type) { >> + case hwmon_pwm: >> + return nct7362_write_pwm(chip, attr, channel, val); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static const struct hwmon_ops nct7362_hwmon_ops = { >> + .is_visible = nct7362_is_visible, >> + .read = nct7362_read, >> + .write = nct7362_write, >> +}; >> + >> +static const struct hwmon_channel_info *nct7362_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_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE, >> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE | HWMON_PWM_FREQ | HWMON_PWM_MODE), >> + NULL, >> +}; >> + >> +static const struct hwmon_chip_info nct7362_chip_info = { >> + .ops = &nct7362_hwmon_ops, >> + .info = nct7362_info, >> +}; >> + >> +static const struct regmap_config nct7362_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> +}; >> + >> +static int nct7362_probe(struct i2c_client *client) >> +{ >> + struct device *hwmon_dev; >> + struct device *dev = &client->dev; >> + struct nct7362_data *chip; >> + int ret; >> + >> + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL); >> + if (!chip) >> + return -ENOMEM; >> + >> + chip->client = client; >> + chip->regmap = devm_regmap_init_i2c(client, &nct7362_regmap_config); >> + if (IS_ERR(chip->regmap)) >> + return PTR_ERR(chip->regmap); >> + >> + ret = nct7362_init_pin_functions(chip); >> + if (ret) >> + return ret; >> + >> + ret = nct7362_init_pwm(chip); >> + if (ret) >> + return ret; >> + >> + ret = nct7362_init_fan(chip); >> + if (ret) >> + return ret; >> + >> + ret = nct7362_init_gpio(chip); >> + if (ret) >> + return ret; >> + >> + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, chip, >> + &nct7362_chip_info, NULL); >> + return PTR_ERR_OR_ZERO(hwmon_dev); >> +} >> + >> +static const struct i2c_device_id nct7362_idtable[] = { >> + { "nct7362", }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(i2c, nct7362_idtable); >> + >> +static const struct of_device_id __maybe_unused nct7362_of_match[] = { >> + { .compatible = "nuvoton,nct7362", }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, nct7362_of_match); >> + >> +static struct i2c_driver nct7362_driver = { >> + .class = I2C_CLASS_HWMON, >> + .driver = { >> + .name = DRVNAME, >> + .of_match_table = of_match_ptr(nct7362_of_match), >> + }, >> + .probe_new = nct7362_probe, >> + .id_table = nct7362_idtable, >> +}; >> + >> +module_i2c_driver(nct7362_driver); >> + >> +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); >> +MODULE_DESCRIPTION("NCT7362Y fan controller driver"); >> +MODULE_LICENSE("GPL"); >> -- >> 2.40.0.5.gf6e3b97ba6d2.dirty >> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-14 22:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-07 10:18 [PATCH 0/2] Nuvoton NCT7362 driver & dt-binding Zev Weiss 2023-06-07 10:18 ` [PATCH 1/2] dt-bindings: hwmon: Add Nuvoton NCT7362Y binding Zev Weiss 2023-06-07 18:24 ` Krzysztof Kozlowski 2023-06-14 22:57 ` Rob Herring 2023-06-07 10:18 ` [PATCH 2/2] hwmon: (nct7362) Add nct7362 driver Zev Weiss 2023-06-07 18:00 ` Guenter Roeck 2023-06-07 19:33 ` Zev Weiss
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox