linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Add devicetree support for max6639
@ 2022-10-13  9:48 Naresh Solanki
  2022-10-13  9:48 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Naresh Solanki @ 2022-10-13  9:48 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 V4:
- Fix dt error
- update comment
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, 287 insertions(+), 53 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] 21+ messages in thread

* [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-10-13  9:48 [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
@ 2022-10-13  9:48 ` Naresh Solanki
  2022-10-24 16:18   ` Rob Herring
  2022-10-13  9:48 ` [PATCH v4 2/3] dt-bindings: hwmon: Add binding for max6639 Naresh Solanki
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Naresh Solanki @ 2022-10-13  9:48 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] 21+ messages in thread

* [PATCH v4 2/3] dt-bindings: hwmon: Add binding for max6639
  2022-10-13  9:48 [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
  2022-10-13  9:48 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki
@ 2022-10-13  9:48 ` Naresh Solanki
  2022-10-13  9:48 ` [PATCH v4 3/3] hwmon: (max6639) Change from pdata to dt configuration Naresh Solanki
  2022-10-23  5:53 ` [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
  3 siblings, 0 replies; 21+ messages in thread
From: Naresh Solanki @ 2022-10-13  9:48 UTC (permalink / raw)
  To: devicetree, Guenter Roeck, Jean Delvare, Rob Herring,
	Krzysztof Kozlowski, Roland Stigge
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph,
	Marcello Sylvester Bauer, Naresh Solanki

From: Marcello Sylvester Bauer <sylv@sylv.io>

Add Devicetree binding documentation for Maxim MAX6639 temperature
monitor with PWM fan-speed controller.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 .../bindings/hwmon/maxim,max6639.yaml         | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
new file mode 100644
index 000000000000..22065ea56880
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim max6639
+
+maintainers:
+  - Roland Stigge <stigge@antcom.de>
+
+description: |
+  The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
+  fan-speed controller.  It monitors its own temperature and one external
+  diode-connected transistor or the temperatures of two external diode-connected
+  transistors, typically available in CPUs, FPGAs, or GPUs.
+
+  Datasheets:
+    https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf
+
+properties:
+  compatible:
+    enum:
+      - maxim,max6639
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^fan@[0-1]$":
+    type: object
+    description: |
+      Represents the two fans and their specific configuration.
+
+    $ref: fan-common.yaml#
+
+    properties:
+      reg:
+        description: |
+          The fan number.
+        items:
+          minimum: 0
+          maximum: 1
+
+    required:
+      - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      fan-controller@10 {
+        compatible = "maxim,max6639";
+        reg = <0x10>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        fan@0 {
+          reg = <0x0>;
+          pulses-per-revolution = <2>;
+          max-rpm = <4000>;
+        };
+
+        fan@1 {
+          reg = <0x1>;
+          pulses-per-revolution = <2>;
+          max-rpm = <8000>;
+        };
+      };
+    };
+...
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 3/3] hwmon: (max6639) Change from pdata to dt configuration
  2022-10-13  9:48 [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
  2022-10-13  9:48 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki
  2022-10-13  9:48 ` [PATCH v4 2/3] dt-bindings: hwmon: Add binding for max6639 Naresh Solanki
@ 2022-10-13  9:48 ` Naresh Solanki
  2022-10-23  5:53 ` [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
  3 siblings, 0 replies; 21+ messages in thread
From: Naresh Solanki @ 2022-10-13  9:48 UTC (permalink / raw)
  To: devicetree, Guenter Roeck, Jean Delvare
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Naresh Solanki

max6639_platform_data is not used by any in-kernel driver and does not
address the MAX6639 fans separately.
Move to device tree configuration with explicit properties to configure
each fan.

Non-DT platform can still use this module with its default
configuration.

Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/hwmon/max6639.c | 206 +++++++++++++++++++++++++++++-----------
 1 file changed, 153 insertions(+), 53 deletions(-)

diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
index 9b895402c80d..bb6bcd61f34d 100644
--- a/drivers/hwmon/max6639.c
+++ b/drivers/hwmon/max6639.c
@@ -19,7 +19,6 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
-#include <linux/platform_data/max6639.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
@@ -85,9 +84,9 @@ struct max6639_data {
 	u8 temp_ot[2];		/* OT Temperature, 0..255 C (->_emergency) */
 
 	/* Register values initialized only once */
-	u8 ppr;			/* Pulses per rotation 0..3 for 1..4 ppr */
-	u8 rpm_range;		/* Index in above rpm_ranges table */
-
+	u8 ppr[2];		/* Pulses per rotation 0..3 for 1..4 ppr */
+	u8 rpm_range[2];	/* Index in above rpm_ranges table */
+	u8 pwm_polarity[2];	/* Fans PWM polarity, 0..1 */
 	/* Optional regulator for FAN supply */
 	struct regulator *reg;
 };
@@ -319,7 +318,7 @@ static ssize_t fan_input_show(struct device *dev,
 		return PTR_ERR(data);
 
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index],
-		       data->rpm_range));
+		       data->rpm_range[attr->index]));
 }
 
 static ssize_t alarm_show(struct device *dev,
@@ -386,29 +385,40 @@ static struct attribute *max6639_attrs[] = {
 ATTRIBUTE_GROUPS(max6639);
 
 /*
- *  returns respective index in rpm_ranges table
- *  1 by default on invalid range
+ *  Get respective index in rpm_ranges table
  */
-static int rpm_range_to_reg(int range)
+static int rpm_range_to_index(struct device *dev, u8 *index, int rpm)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(rpm_ranges); i++) {
-		if (rpm_ranges[i] == range)
-			return i;
+	if (rpm < 0)
+		return -EINVAL;
+
+	/* Set index based on chip support */
+	switch (rpm) {
+	case 0 ... 2000:
+		*index = 0;
+		break;
+	case 2001 ... 4000:
+		*index = 1;
+		break;
+	case 4001 ... 8000:
+		*index = 2;
+		break;
+	case 8001 ... 16000:
+		*index = 3;
+		break;
+	default:
+		/* Use max range for higher RPM */
+		dev_warn(dev,
+		    "RPM higher than supported range. Default to 16000 RPM");
+		*index = 3;
 	}
-
-	return 1; /* default: 4000 RPM */
+	return 0;
 }
 
 static int max6639_init_client(struct i2c_client *client,
 			       struct max6639_data *data)
 {
-	struct max6639_platform_data *max6639_info =
-		dev_get_platdata(&client->dev);
-	int i;
-	int rpm_range = 1; /* default: 4000 RPM */
-	int err;
+	int i, err;
 
 	/* Reset chip to default values, see below for GCONFIG setup */
 	err = i2c_smbus_write_byte_data(client, MAX6639_REG_GCONFIG,
@@ -416,51 +426,32 @@ static int max6639_init_client(struct i2c_client *client,
 	if (err)
 		goto exit;
 
-	/* Fans pulse per revolution is 2 by default */
-	if (max6639_info && max6639_info->ppr > 0 &&
-			max6639_info->ppr < 5)
-		data->ppr = max6639_info->ppr;
-	else
-		data->ppr = 2;
-	data->ppr -= 1;
-
-	if (max6639_info)
-		rpm_range = rpm_range_to_reg(max6639_info->rpm_range);
-	data->rpm_range = rpm_range;
-
 	for (i = 0; i < 2; i++) {
 
 		/* Set Fan pulse per revolution */
 		err = i2c_smbus_write_byte_data(client,
 				MAX6639_REG_FAN_PPR(i),
-				data->ppr << 6);
+				data->ppr[i] << 6);
 		if (err)
 			goto exit;
 
 		/* Fans config PWM, RPM */
 		err = i2c_smbus_write_byte_data(client,
 			MAX6639_REG_FAN_CONFIG1(i),
-			MAX6639_FAN_CONFIG1_PWM | rpm_range);
+			MAX6639_FAN_CONFIG1_PWM | data->rpm_range[i]);
 		if (err)
 			goto exit;
 
-		/* Fans PWM polarity high by default */
-		if (max6639_info && max6639_info->pwm_polarity == 0)
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x00);
-		else
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x02);
-		if (err)
-			goto exit;
+		/* Fans PWM polarity */
+		err = i2c_smbus_write_byte_data(client,
+			MAX6639_REG_FAN_CONFIG2a(i), data->pwm_polarity[i] ? 0x02 : 0x00);
 
 		/*
-		 * /THERM full speed enable,
+		 * /THERM full speed disable,
 		 * PWM frequency 25kHz, see also GCONFIG below
 		 */
 		err = i2c_smbus_write_byte_data(client,
-			MAX6639_REG_FAN_CONFIG3(i),
-			MAX6639_FAN_CONFIG3_THERM_FULL_SPEED | 0x03);
+			MAX6639_REG_FAN_CONFIG3(i), 0x03);
 		if (err)
 			goto exit;
 
@@ -483,8 +474,6 @@ static int max6639_init_client(struct i2c_client *client,
 		if (err)
 			goto exit;
 
-		/* PWM 120/120 (i.e. 100%) */
-		data->pwm[i] = 120;
 		err = i2c_smbus_write_byte_data(client,
 				MAX6639_REG_TARGTDUTY(i), data->pwm[i]);
 		if (err)
@@ -524,12 +513,91 @@ static void max6639_regulator_disable(void *data)
 	regulator_disable(data);
 }
 
+static int max6639_probe_child_from_dt(struct i2c_client *client,
+				      struct device_node *child,
+				      struct max6639_data *data)
+
+{
+	struct device *dev = &client->dev;
+	u32 i, val, maxrpm;
+	int err;
+
+	err = of_property_read_u32(child, "reg", &i);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+
+	if (i >= 2) {
+		dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(child, "pulses-per-revolution", &val);
+	if (err) {
+		dev_err(dev, "missing pulses-per-revolution property of %pOFn\n", child);
+		return err;
+	}
+
+	if (val < 0 || val > 5) {
+		dev_err(dev, "invalid pulses-per-revolution %d of %pOFn\n", val, child);
+		return -EINVAL;
+	}
+	data->ppr[i] = val;
+
+	err = of_property_read_u32(child, "max-rpm", &maxrpm);
+	if (err) {
+		dev_err(dev, "missing max-rpm property of %pOFn\n", child);
+		return err;
+	}
+
+	err = rpm_range_to_index(dev, &data->rpm_range[i], maxrpm);
+	if (err) {
+		dev_err(dev, "invalid max-rpm %d of %pOFn\n", maxrpm, child);
+		return err;
+	}
+
+	err = of_property_read_u32(child, "target-rpm", &val);
+	/* Convert to PWM from provided target RPM */
+	if (!err && val != 0 && maxrpm > 0)
+		data->pwm[i] =
+			(u8)(val * 120 / maxrpm);
+
+	data->pwm_polarity[i] =  of_property_read_bool(child, "pwm-polarity-inverse");
+
+	return 0;
+}
+static int max6639_probe_from_dt(struct i2c_client *client, struct max6639_data *data)
+{
+	struct device *dev = &client->dev;
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	/* Compatible with non-DT platforms */
+	if (!np)
+		return 0;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "fan"))
+			continue;
+
+		err = max6639_probe_child_from_dt(client, child, data);
+		if (err) {
+			of_node_put(child);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
 static int max6639_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct max6639_data *data;
 	struct device *hwmon_dev;
-	int err;
+	int err, i;
 
 	data = devm_kzalloc(dev, sizeof(struct max6639_data), GFP_KERNEL);
 	if (!data)
@@ -539,9 +607,11 @@ static int max6639_probe(struct i2c_client *client)
 
 	data->reg = devm_regulator_get_optional(dev, "fan");
 	if (IS_ERR(data->reg)) {
-		if (PTR_ERR(data->reg) != -ENODEV)
-			return PTR_ERR(data->reg);
-
+		if (PTR_ERR(data->reg) != -ENODEV) {
+			err = (int)PTR_ERR(data->reg);
+			dev_warn(dev, "Failed looking up fan supply: %d\n", err);
+			return err;
+		}
 		data->reg = NULL;
 	} else {
 		/* Spin up fans */
@@ -560,6 +630,25 @@ static int max6639_probe(struct i2c_client *client)
 
 	mutex_init(&data->update_lock);
 
+	/* Below are defaults leter overridden by DT properties */
+	for (i = 0; i < 2; i++) {
+		/* 4000 RPM */
+		data->rpm_range[i] = 1;
+		data->ppr[i] = 2;
+		data->pwm_polarity[i] = 0;
+		/* Max. temp. 80C/90C/100C */
+		data->temp_therm[i] = 80;
+		data->temp_alert[i] = 90;
+		data->temp_ot[i] = 100;
+		/* PWM 120/120 (i.e. 100%) */
+		data->pwm[i] = 120;
+	}
+
+	/* Probe from DT to get configuration */
+	err = max6639_probe_from_dt(client, data);
+	if (err)
+		return err;
+
 	/* Initialize the max6639 chip */
 	err = max6639_init_client(client, data);
 	if (err < 0)
@@ -571,6 +660,7 @@ static int max6639_probe(struct i2c_client *client)
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
+#if IS_ENABLED(CONFIG_PM_SLEEP)
 static int max6639_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -608,6 +698,7 @@ static int max6639_resume(struct device *dev)
 	return i2c_smbus_write_byte_data(client,
 			MAX6639_REG_GCONFIG, ret & ~MAX6639_GCONFIG_STANDBY);
 }
+#endif
 
 static const struct i2c_device_id max6639_id[] = {
 	{"max6639", 0},
@@ -616,13 +707,22 @@ static const struct i2c_device_id max6639_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, max6639_id);
 
-static DEFINE_SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
+#ifdef CONFIG_OF
+static const struct of_device_id maxim_of_platform_match[] = {
+	{.compatible = "maxim,max6639"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, maxim_of_platform_match);
+#endif
+
+static SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
 
 static struct i2c_driver max6639_driver = {
 	.class = I2C_CLASS_HWMON,
 	.driver = {
 		   .name = "max6639",
 		   .pm = pm_sleep_ptr(&max6639_pm_ops),
+		   .of_match_table = of_match_ptr(maxim_of_platform_match),
 		   },
 	.probe_new = max6639_probe,
 	.id_table = max6639_id,
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/3] Add devicetree support for max6639
  2022-10-13  9:48 [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
                   ` (2 preceding siblings ...)
  2022-10-13  9:48 ` [PATCH v4 3/3] hwmon: (max6639) Change from pdata to dt configuration Naresh Solanki
@ 2022-10-23  5:53 ` Naresh Solanki
  2022-10-25 18:01   ` Guenter Roeck
  3 siblings, 1 reply; 21+ messages in thread
From: Naresh Solanki @ 2022-10-23  5:53 UTC (permalink / raw)
  To: devicetree, Guenter Roeck, Jean Delvare
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph

In this latest patch series V4 , I've addressed the comments of V3.
Since this is merely an upgrade of driver from platform driver to dt
support, can we get this in.

Thanks,
Naresh

Regards,
Naresh Solanki



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email:  naresh.solanki@9elements.com
Mobile:  +91 9538631477

Sitz der Gesellschaft: Bochum
Handelsregister: Amtsgericht Bochum, HRB 17519
Geschäftsführung: Sebastian Deutsch, Eray Basar

Datenschutzhinweise nach Art. 13 DSGVO


On Thu, 13 Oct 2022 at 15:18, Naresh Solanki
<naresh.solanki@9elements.com> wrote:
>
> These patches adds devicetree support for MAX6639.
>
> Changes V4:
> - Fix dt error
> - update comment
> 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, 287 insertions(+), 53 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] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-10-13  9:48 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki
@ 2022-10-24 16:18   ` Rob Herring
  2022-10-25  9:16     ` Naresh Solanki
  0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2022-10-24 16:18 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph

On Thu, Oct 13, 2022 at 11:48:36AM +0200, Naresh Solanki wrote:
> 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.

So if we unload and reload the driver module, it should go back to the 
default? 

I think it is really, 'target RPM if not already configured' which could 
be keep the setting from a register (e.g. what the bootloader set) or 
perhaps you already have temperature information to use...

> +    $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

As I said before, the PWM binding handles these 2 settings. Use it. Yes, 
it's a bit of an overkill when the child is the consumer of the parent. 
Until some 'clever' h/w engineer decides to use one of the PWMs for 
something else like a backlight.

Rob

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-10-24 16:18   ` Rob Herring
@ 2022-10-25  9:16     ` Naresh Solanki
  2022-10-26 13:37       ` Rob Herring
  0 siblings, 1 reply; 21+ messages in thread
From: Naresh Solanki @ 2022-10-25  9:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph



On 24-10-2022 09:48 pm, Rob Herring wrote:
> So if we unload and reload the driver module, it should go back to the
> default?
This is RPM to be set during probe if desired.
> 
> I think it is really, 'target RPM if not already configured' which could
> be keep the setting from a register (e.g. what the bootloader set) or
> perhaps you already have temperature information to use...
Yes. missed it. It should be target-rpm will correct this. in next version.
> 
>> +    $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
> As I said before, the PWM binding handles these 2 settings. Use it. Yes,
> it's a bit of an overkill when the child is the consumer of the parent.
> Until some 'clever' h/w engineer decides to use one of the PWMs for
> something else like a backlight.
I would like you to consider this as something recommended by fan 
datasheet for the given fan instance.
This info can be used by fan controller driver to configure PWM 
source/provider accordingly.

If you still feel that may not make sense then I'll remove this property.


Thanks,
Naresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/3] Add devicetree support for max6639
  2022-10-23  5:53 ` [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
@ 2022-10-25 18:01   ` Guenter Roeck
  2022-10-26  7:08     ` Naresh Solanki
  0 siblings, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2022-10-25 18:01 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: devicetree, Jean Delvare, linux-kernel, linux-hwmon, Patrick Rudolph

On Sun, Oct 23, 2022 at 11:23:42AM +0530, Naresh Solanki wrote:
> In this latest patch series V4 , I've addressed the comments of V3.
> Since this is merely an upgrade of driver from platform driver to dt
> support, can we get this in.
> 

This is not correct. You did not address many of my comments.

Guenter

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/3] Add devicetree support for max6639
  2022-10-25 18:01   ` Guenter Roeck
@ 2022-10-26  7:08     ` Naresh Solanki
  0 siblings, 0 replies; 21+ messages in thread
From: Naresh Solanki @ 2022-10-26  7:08 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: devicetree, Jean Delvare, linux-kernel, linux-hwmon, Patrick Rudolph

Hi Guenter,

On 25-10-2022 11:31 pm, Guenter Roeck wrote:
> You did not address many of my comments.
I've update comment to explain defaults. Thus retaining previous behaviour.
This patch series is about incremental change just to make it DT compatible.

Thanks,
Naresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-10-25  9:16     ` Naresh Solanki
@ 2022-10-26 13:37       ` Rob Herring
  2022-10-31  8:05         ` Naresh Solanki
  0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2022-10-26 13:37 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph

On Tue, Oct 25, 2022 at 4:16 AM Naresh Solanki
<naresh.solanki@9elements.com> wrote:
>
>
>
> On 24-10-2022 09:48 pm, Rob Herring wrote:
> > So if we unload and reload the driver module, it should go back to the
> > default?
> This is RPM to be set during probe if desired.
> >
> > I think it is really, 'target RPM if not already configured' which could
> > be keep the setting from a register (e.g. what the bootloader set) or
> > perhaps you already have temperature information to use...
> Yes. missed it. It should be target-rpm will correct this. in next version.
> >
> >> +    $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
> > As I said before, the PWM binding handles these 2 settings. Use it. Yes,
> > it's a bit of an overkill when the child is the consumer of the parent.
> > Until some 'clever' h/w engineer decides to use one of the PWMs for
> > something else like a backlight.
> I would like you to consider this as something recommended by fan
> datasheet for the given fan instance.
> This info can be used by fan controller driver to configure PWM
> source/provider accordingly.
>
> If you still feel that may not make sense then I'll remove this property.

You evidently don't understand my comments. My suggestion is to do this:

fanc: fan-controller {
  #pwm-cells = <3>;
  ...

  fan {
    pwms = <&fanc 0 500000  PWM_POLARITY_INVERTED>;
    ...
  };
};

0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per
consumer flags. See pwm.txt for more details.

Rob

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-10-26 13:37       ` Rob Herring
@ 2022-10-31  8:05         ` Naresh Solanki
  2022-11-01 18:44           ` Rob Herring
  0 siblings, 1 reply; 21+ messages in thread
From: Naresh Solanki @ 2022-10-31  8:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph

Hi Rob,

On 26-10-2022 07:07 pm, Rob Herring wrote:
> fanc: fan-controller {
>    #pwm-cells = <3>;
>    ...
> 
>    fan {
>      pwms = <&fanc 0 500000  PWM_POLARITY_INVERTED>;
>      ...
>    };
> };
> 
> 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per
> consumer flags. See pwm.txt for more details.

Did the implementation & while testing getting the below err:
[63.626505] max6639 166-002e: failed to create device link to 166-002e

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-10-31  8:05         ` Naresh Solanki
@ 2022-11-01 18:44           ` Rob Herring
  2022-11-01 19:22             ` Naresh Solanki
  2022-11-11 21:07             ` Naresh Solanki
  0 siblings, 2 replies; 21+ messages in thread
From: Rob Herring @ 2022-11-01 18:44 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph

On Mon, Oct 31, 2022 at 01:35:09PM +0530, Naresh Solanki wrote:
> Hi Rob,
> 
> On 26-10-2022 07:07 pm, Rob Herring wrote:
> > fanc: fan-controller {
> >    #pwm-cells = <3>;
> >    ...
> > 
> >    fan {
> >      pwms = <&fanc 0 500000  PWM_POLARITY_INVERTED>;
> >      ...
> >    };
> > };
> > 
> > 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per
> > consumer flags. See pwm.txt for more details.
> 
> Did the implementation & while testing getting the below err:
> [63.626505] max6639 166-002e: failed to create device link to 166-002e

Does turning off fw_devlink help (fw_devlink=off)?

Rob

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-11-01 18:44           ` Rob Herring
@ 2022-11-01 19:22             ` Naresh Solanki
  2022-11-11 21:07             ` Naresh Solanki
  1 sibling, 0 replies; 21+ messages in thread
From: Naresh Solanki @ 2022-11-01 19:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph

Hi Rob,

On 02-11-2022 12:14 am, Rob Herring wrote:
> Does turning off fw_devlink help (fw_devlink=off)?
This didn't bring any difference for the error.
Failing due to same consumer & supplier.
Returning from here:
https://github.com/torvalds/linux/blob/master/drivers/base/core.c#L702
Also this can cause return:
https://github.com/torvalds/linux/blob/master/drivers/base/core.c#L732

Regards,
Naresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2022-11-01 18:44           ` Rob Herring
  2022-11-01 19:22             ` Naresh Solanki
@ 2022-11-11 21:07             ` Naresh Solanki
  1 sibling, 0 replies; 21+ messages in thread
From: Naresh Solanki @ 2022-11-11 21:07 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Guenter Roeck, Jean Delvare, Krzysztof Kozlowski,
	linux-kernel, linux-hwmon, Patrick Rudolph

Hi Rob,

On 02-11-2022 12:14 am, Rob Herring wrote:
> On Mon, Oct 31, 2022 at 01:35:09PM +0530, Naresh Solanki wrote:
>> Hi Rob,
>>
>> On 26-10-2022 07:07 pm, Rob Herring wrote:
>>> fanc: fan-controller {
>>>     #pwm-cells = <3>;
>>>     ...
>>>
>>>     fan {
>>>       pwms = <&fanc 0 500000  PWM_POLARITY_INVERTED>;
>>>       ...
>>>     };
>>> };
>>>
>>> 0 is PWM number and 500000 is the PWM frequency. The 3rd cell are per
>>> consumer flags. See pwm.txt for more details.
>>
>> Did the implementation & while testing getting the below err:
>> [63.626505] max6639 166-002e: failed to create device link to 166-002e
> 
> Does turning off fw_devlink help (fw_devlink=off)?

Will supplier == consumer, device link creation fails.
Not sure what is best approach but not creating device link in this 
scenario help & for that below additional changes needed in pwm core.

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 4527f09a5c50..afea51c49138 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -730,6 +730,12 @@ static struct device_link 
*pwm_device_link_add(struct device *dev,
  		return NULL;
  	}

+	/*
+	 * Do not attempt to create link if consumer itself is supplier.
+	 */
+	if (dev == pwm->chip->dev)
+		return 0;
+
  	dl = device_link_add(dev, pwm->chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
  	if (!dl) {
  		dev_err(dev, "failed to create device link to %s\n",



Regards,
Naresh

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-03-07 18:53   ` Guenter Roeck
@ 2024-03-18  0:58     ` Ban Feng
  0 siblings, 0 replies; 21+ messages in thread
From: Ban Feng @ 2024-03-18  0:58 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,
	Rob Herring

Hi Guenter,

ok.

Thanks,
Ban

On Fri, Mar 8, 2024 at 2:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 2/26/24 16:56, 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
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > 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>
>
> This patch (through its submission with the aspeed-g6 fan driver) is now in hwmon-next.
>
> Please do not resend. Any updates should be submitted as follow-up patches.
>
> Guenter
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-02-27  0:56 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926
  2024-03-05  0:22   ` Zev Weiss
@ 2024-03-07 18:53   ` Guenter Roeck
  2024-03-18  0:58     ` Ban Feng
  1 sibling, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2024-03-07 18:53 UTC (permalink / raw)
  To: baneric926, jdelvare, 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,
	Rob Herring

On 2/26/24 16:56, 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
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 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>

This patch (through its submission with the aspeed-g6 fan driver) is now in hwmon-next.

Please do not resend. Any updates should be submitted as follow-up patches.

Guenter


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-03-05  0:22   ` Zev Weiss
  2024-03-05  0:41     ` Guenter Roeck
@ 2024-03-07  7:41     ` Ban Feng
  1 sibling, 0 replies; 21+ messages in thread
From: Ban Feng @ 2024-03-07  7:41 UTC (permalink / raw)
  To: Zev Weiss
  Cc: jdelvare, linux, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	corbet, linux-hwmon, devicetree, kcfeng0, kwliu, openbmc,
	linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki,
	billy_tsai, Rob Herring

Hi Zev,

This patch was suggested by reviewer and I cherry-pick from below link:
https://patchwork.kernel.org/project/linux-hwmon/patch/20240221104025.1306227-2-billy_tsai@aspeedtech.com/

Because I don't know the rule about the patch with cherry-pick, maybe
we should discuss it there?

Thanks,
Ban

On Tue, Mar 5, 2024 at 8:22 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> On Mon, Feb 26, 2024 at 04:56:04PM PST, 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
> >
> >Reviewed-by: Rob Herring <robh@kernel.org>
> >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 | 78 +++++++++++++++++++
> > 1 file changed, 78 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..15c591c74545
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml
> >@@ -0,0 +1,78 @@
> >+# 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
>
> I can't say with certainty that it's not, but are we sure 1000 is low
> enough?  Looking at just what I've got on hand, an 80mm fan I have will
> run steadily at about 1500RPM, and I'd assume larger ones (e.g. 120mm)
> could potentially go significantly lower...
>
> >+
> >+  pulses-per-revolution:
> >+    description:
> >+      The number of pulse from fan sensor per revolution.
> >+    $ref: /schemas/types.yaml#/definitions/uint32
> >+    maximum: 4
>
> Might we want 'default: 2' here?
>
> >+
> >+  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)
>
> Nit: could use a space before the parenthetical.
>
> >+    $ref: /schemas/types.yaml#/definitions/string
> >+    enum: [ dc, pwm ]
> >+
> >+  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
>
> Nit: s/channel/channels/ given that it's an array?
>
> >+
> >+  label:
> >+    description:
> >+      Optional fan label
> >+
> >+  fan-supply:
> >+    description:
> >+      Power supply for fan.
> >+
> >+  reg:
> >+    maxItems: 1
> >+
> >+additionalProperties: true
> >+
> >--
> >2.34.1
> >
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-03-05  0:41     ` Guenter Roeck
@ 2024-03-05  0:49       ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2024-03-05  0:49 UTC (permalink / raw)
  To: Zev Weiss, baneric926
  Cc: jdelvare, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet,
	linux-hwmon, devicetree, kcfeng0, kwliu, openbmc, linux-doc,
	linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai,
	Rob Herring

On 3/4/24 16:41, Guenter Roeck wrote:

>>> +
>>> +  min-rpm:
>>> +    description:
>>> +      Min RPM supported by fan.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    maximum: 1000
>>
>> I can't say with certainty that it's not, but are we sure 1000 is low enough?  Looking at just what I've got on hand, an 80mm fan I have will run steadily at about 1500RPM, and I'd assume larger ones (e.g. 120mm) could potentially go significantly lower...
>>
> 
> I have seen fans which run stable at < 400rpm.
> One of my systems right now has:
> 
> fan1:              732 RPM  (min =    0 RPM)
> fan2:                0 RPM  (min =    0 RPM)
> fan3:              586 RPM  (min =    0 RPM)
> fan4:              472 RPM  (min =    0 RPM)
> fan5:              480 RPM  (min =    0 RPM)
> 
> Those are 80mm fans. A quick check shows that various Noctua fans have a
> minimum speed of 300 rpm. So 1000 is indeed a bit high for the minimum speed.
> 

No, wait, that is the _maximum_ minimum speed. Got me there.

So, there is Noctua's NF-A4x20 PWM with a minimum rotational speed of 1,200 RPM.

If I interpret
https://www.mouser.com/datasheet/2/471/SanyoDenki_San_Ace_40LG28_E-3198440.pdf
correctly, it lists some fans with a minimum speed of 7,500 RPM.

Guenter


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-03-05  0:22   ` Zev Weiss
@ 2024-03-05  0:41     ` Guenter Roeck
  2024-03-05  0:49       ` Guenter Roeck
  2024-03-07  7:41     ` Ban Feng
  1 sibling, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2024-03-05  0:41 UTC (permalink / raw)
  To: Zev Weiss, baneric926
  Cc: jdelvare, robh+dt, krzysztof.kozlowski+dt, conor+dt, corbet,
	linux-hwmon, devicetree, kcfeng0, kwliu, openbmc, linux-doc,
	linux-kernel, DELPHINE_CHIU, naresh.solanki, billy_tsai,
	Rob Herring

On 3/4/24 16:22, Zev Weiss wrote:
> On Mon, Feb 26, 2024 at 04:56:04PM PST, 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
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> 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 | 78 +++++++++++++++++++
>> 1 file changed, 78 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..15c591c74545
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml
>> @@ -0,0 +1,78 @@
>> +# 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
> 
> I can't say with certainty that it's not, but are we sure 1000 is low enough?  Looking at just what I've got on hand, an 80mm fan I have will run steadily at about 1500RPM, and I'd assume larger ones (e.g. 120mm) could potentially go significantly lower...
> 

I have seen fans which run stable at < 400rpm.
One of my systems right now has:

fan1:              732 RPM  (min =    0 RPM)
fan2:                0 RPM  (min =    0 RPM)
fan3:              586 RPM  (min =    0 RPM)
fan4:              472 RPM  (min =    0 RPM)
fan5:              480 RPM  (min =    0 RPM)

Those are 80mm fans. A quick check shows that various Noctua fans have a
minimum speed of 300 rpm. So 1000 is indeed a bit high for the minimum speed.

Guenter


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-02-27  0:56 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926
@ 2024-03-05  0:22   ` Zev Weiss
  2024-03-05  0:41     ` Guenter Roeck
  2024-03-07  7:41     ` Ban Feng
  2024-03-07 18:53   ` Guenter Roeck
  1 sibling, 2 replies; 21+ messages in thread
From: Zev Weiss @ 2024-03-05  0:22 UTC (permalink / raw)
  To: baneric926
  Cc: jdelvare, linux, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	corbet, linux-hwmon, devicetree, kcfeng0, kwliu, openbmc,
	linux-doc, linux-kernel, DELPHINE_CHIU, naresh.solanki,
	billy_tsai, Rob Herring

On Mon, Feb 26, 2024 at 04:56:04PM PST, 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
>
>Reviewed-by: Rob Herring <robh@kernel.org>
>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 | 78 +++++++++++++++++++
> 1 file changed, 78 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..15c591c74545
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml
>@@ -0,0 +1,78 @@
>+# 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

I can't say with certainty that it's not, but are we sure 1000 is low 
enough?  Looking at just what I've got on hand, an 80mm fan I have will 
run steadily at about 1500RPM, and I'd assume larger ones (e.g. 120mm) 
could potentially go significantly lower...

>+
>+  pulses-per-revolution:
>+    description:
>+      The number of pulse from fan sensor per revolution.
>+    $ref: /schemas/types.yaml#/definitions/uint32
>+    maximum: 4

Might we want 'default: 2' here?

>+
>+  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)

Nit: could use a space before the parenthetical.

>+    $ref: /schemas/types.yaml#/definitions/string
>+    enum: [ dc, pwm ]
>+
>+  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

Nit: s/channel/channels/ given that it's an array?

>+
>+  label:
>+    description:
>+      Optional fan label
>+
>+  fan-supply:
>+    description:
>+      Power supply for fan.
>+
>+  reg:
>+    maxItems: 1
>+
>+additionalProperties: true
>+
>-- 
>2.34.1
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema
  2024-02-27  0:56 [PATCH v4 0/3] hwmon: Driver for Nuvoton NCT7363Y baneric926
@ 2024-02-27  0:56 ` baneric926
  2024-03-05  0:22   ` Zev Weiss
  2024-03-07 18:53   ` Guenter Roeck
  0 siblings, 2 replies; 21+ messages in thread
From: baneric926 @ 2024-02-27  0:56 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,
	Rob Herring

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

Reviewed-by: Rob Herring <robh@kernel.org>
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 | 78 +++++++++++++++++++
 1 file changed, 78 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..15c591c74545
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/fan-common.yaml
@@ -0,0 +1,78 @@
+# 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
+    enum: [ dc, pwm ]
+
+  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] 21+ messages in thread

end of thread, other threads:[~2024-03-18  0:58 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13  9:48 [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
2022-10-13  9:48 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema Naresh Solanki
2022-10-24 16:18   ` Rob Herring
2022-10-25  9:16     ` Naresh Solanki
2022-10-26 13:37       ` Rob Herring
2022-10-31  8:05         ` Naresh Solanki
2022-11-01 18:44           ` Rob Herring
2022-11-01 19:22             ` Naresh Solanki
2022-11-11 21:07             ` Naresh Solanki
2022-10-13  9:48 ` [PATCH v4 2/3] dt-bindings: hwmon: Add binding for max6639 Naresh Solanki
2022-10-13  9:48 ` [PATCH v4 3/3] hwmon: (max6639) Change from pdata to dt configuration Naresh Solanki
2022-10-23  5:53 ` [PATCH v4 0/3] Add devicetree support for max6639 Naresh Solanki
2022-10-25 18:01   ` Guenter Roeck
2022-10-26  7:08     ` Naresh Solanki
2024-02-27  0:56 [PATCH v4 0/3] hwmon: Driver for Nuvoton NCT7363Y baneric926
2024-02-27  0:56 ` [PATCH v4 1/3] dt-bindings: hwmon: fan: Add fan binding to schema baneric926
2024-03-05  0:22   ` Zev Weiss
2024-03-05  0:41     ` Guenter Roeck
2024-03-05  0:49       ` Guenter Roeck
2024-03-07  7:41     ` Ban Feng
2024-03-07 18:53   ` Guenter Roeck
2024-03-18  0:58     ` Ban Feng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).