[PATCH v3 1/3] dt-bindings: vendor-prefixes: Document Argon40

[PATCH v3 1/3] dt-bindings: vendor-prefixes: Document Argon40

[Marek Vasut]

Argon 40 Technologies Limited is a SBC expansion board vendor.
Document the prefix. For details see https://argon40.com .

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Uwe Kleine-König" <ukleinek@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-pwm@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Add AB from Conor
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 3f23f6f93b62..27e2ad136931 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -149,6 +149,8 @@ patternProperties:
     description: Arctic Sand
   "^arcx,.*":
     description: arcx Inc. / Archronix Inc.
+  "^argon40,.*":
+    description: Argon 40 Technologies Limited
   "^ariaboard,.*":
     description: Shanghai Novotech Co., Ltd. (Ariaboard)
   "^aries,.*":
-- 
2.47.2

[PATCH v3 2/3] dt-bindings: pwm: argon40,fan-hat: Document Argon40 Fan HAT

[Marek Vasut]

Document trivial PWM on Argon40 Fan HAT, which is a RaspberryPi
blower fan hat which can be controlled over I2C.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Uwe Kleine-König" <ukleinek@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-pwm@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Implement dedicated binding document
V3: Update the description and pwm-cells
---
 .../bindings/pwm/argon40,fan-hat.yaml         | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml

diff --git a/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
new file mode 100644
index 000000000000..a0010700ab12
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/argon40,fan-hat.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Argon40 Fan HAT PWM controller
+
+maintainers:
+  - Marek Vasut <marek.vasut+renesas@mailbox.org>
+
+description: |
+  The trivial PWM on Argon40 Fan HAT, which is a RaspberryPi blower fan
+  hat which can be controlled over I2C, generates a fixed 30 kHz period
+  PWM signal with configurable 0..100% duty cycle to control the fan
+  speed.
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: argon40,fan-hat
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pwm@1a {
+        compatible = "argon40,fan-hat";
+        reg = <0x1a>;
+        #pwm-cells = <2>;
+      };
+    };
-- 
2.47.2

Re: [PATCH v3 2/3] dt-bindings: pwm: argon40,fan-hat: Document Argon40 Fan HAT

[Rob Herring (Arm)]

On Tue, 17 Jun 2025 11:19:35 +0200, Marek Vasut wrote:
> Document trivial PWM on Argon40 Fan HAT, which is a RaspberryPi
> blower fan hat which can be controlled over I2C.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: "Uwe Kleine-König" <ukleinek@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-pwm@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: Implement dedicated binding document
> V3: Update the description and pwm-cells
> ---
>  .../bindings/pwm/argon40,fan-hat.yaml         | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pwm/argon40,fan-hat.example.dtb: pwm@1a (argon40,fan-hat): #pwm-cells: 3 was expected
	from schema $id: http://devicetree.org/schemas/pwm/argon40,fan-hat.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250617092037.37229-2-marek.vasut+renesas@mailbox.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 2/3] dt-bindings: pwm: argon40,fan-hat: Document Argon40 Fan HAT

[Rob Herring]

On Tue, Jun 17, 2025 at 11:19:35AM +0200, Marek Vasut wrote:
> Document trivial PWM on Argon40 Fan HAT, which is a RaspberryPi
> blower fan hat which can be controlled over I2C.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: "Uwe Kleine-König" <ukleinek@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-pwm@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: Implement dedicated binding document
> V3: Update the description and pwm-cells
> ---
>  .../bindings/pwm/argon40,fan-hat.yaml         | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
> new file mode 100644
> index 000000000000..a0010700ab12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/argon40,fan-hat.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Argon40 Fan HAT PWM controller
> +
> +maintainers:
> +  - Marek Vasut <marek.vasut+renesas@mailbox.org>
> +
> +description: |

Don't need '|'.

> +  The trivial PWM on Argon40 Fan HAT, which is a RaspberryPi blower fan
> +  hat which can be controlled over I2C, generates a fixed 30 kHz period
> +  PWM signal with configurable 0..100% duty cycle to control the fan
> +  speed.
> +
> +allOf:
> +  - $ref: pwm.yaml#
> +
> +properties:
> +  compatible:
> +    const: argon40,fan-hat
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#pwm-cells":
> +    const: 3
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      pwm@1a {
> +        compatible = "argon40,fan-hat";
> +        reg = <0x1a>;
> +        #pwm-cells = <2>;
> +      };
> +    };
> -- 
> 2.47.2
> 

Re: [PATCH v3 2/3] dt-bindings: pwm: argon40,fan-hat: Document Argon40 Fan HAT

[Marek Vasut]

On 6/17/25 3:37 PM, Rob Herring wrote:

[...]

>> +++ b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
>> @@ -0,0 +1,48 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pwm/argon40,fan-hat.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Argon40 Fan HAT PWM controller
>> +
>> +maintainers:
>> +  - Marek Vasut <marek.vasut+renesas@mailbox.org>
>> +
>> +description: |
> 
> Don't need '|'.

Fixed in V4 ...

>> +      pwm@1a {
>> +        compatible = "argon40,fan-hat";
>> +        reg = <0x1a>;
>> +        #pwm-cells = <2>;
... and also this one, detected by the bot, thanks.

Re: [PATCH v3 2/3] dt-bindings: pwm: argon40,fan-hat: Document Argon40 Fan HAT

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

Hello Marek,

On Tue, Jun 17, 2025 at 03:48:13PM +0200, Marek Vasut wrote:
> On 6/17/25 3:37 PM, Rob Herring wrote:
> 
> [...]
> 
> > > +++ b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
> > > @@ -0,0 +1,48 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pwm/argon40,fan-hat.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Argon40 Fan HAT PWM controller
> > > +
> > > +maintainers:
> > > +  - Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > +
> > > +description: |
> > 
> > Don't need '|'.
> 
> Fixed in V4 ...

That sounds as if there is already a v4 on the lists. I don't find such
a patch series though. Am I missing something or just misinterpreting
your words?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 2/3] dt-bindings: pwm: argon40,fan-hat: Document Argon40 Fan HAT

[Marek Vasut]

On 6/20/25 12:27 PM, Uwe Kleine-König wrote:
> Hello Marek,

Hi,

> On Tue, Jun 17, 2025 at 03:48:13PM +0200, Marek Vasut wrote:
>> On 6/17/25 3:37 PM, Rob Herring wrote:
>>
>> [...]
>>
>>>> +++ b/Documentation/devicetree/bindings/pwm/argon40,fan-hat.yaml
>>>> @@ -0,0 +1,48 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/pwm/argon40,fan-hat.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Argon40 Fan HAT PWM controller
>>>> +
>>>> +maintainers:
>>>> +  - Marek Vasut <marek.vasut+renesas@mailbox.org>
>>>> +
>>>> +description: |
>>>
>>> Don't need '|'.
>>
>> Fixed in V4 ...
> 
> That sounds as if there is already a v4 on the lists. I don't find such
> a patch series though. Am I missing something or just misinterpreting
> your words?

Its coming once I sort out all the feedback, I did not post V4 yet.

-- 
Best regards,
Marek Vasut

[PATCH v3 3/3] pwm: argon-fan-hat: Add Argon40 Fan HAT support

[Marek Vasut]

Add trivial PWM driver for Argon40 Fan HAT, which is a RaspberryPi
blower fan hat which can be controlled over I2C. Model this device
as a PWM, so the pwm-fan can be attached to it and handle thermal
zones and RPM management in a generic manner.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Uwe Kleine-König" <ukleinek@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-pwm@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Switch to waveform ops
    - Add shutdown hook to force the fan to maximum RPM on shutdown
      instead of stopping it, to be on the safe side
V3: - Find the 30 kHz fixed period PWM, use that
    - Add comments
    - Consolidate argon_fan_hat_write()
---
 drivers/pwm/Kconfig             |   9 +++
 drivers/pwm/Makefile            |   1 +
 drivers/pwm/pwm-argon-fan-hat.c | 122 ++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+)
 create mode 100644 drivers/pwm/pwm-argon-fan-hat.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 6e113f8b4baf..3ef1757502eb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -66,6 +66,15 @@ config PWM_APPLE
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-apple.
 
+config PWM_ARGON_FAN_HAT
+	tristate "Argon40 Fan HAT support"
+	depends on I2C && OF
+	help
+	  Generic PWM framework driver for Argon40 Fan HAT.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-argon-fan-hat.
+
 config PWM_ATMEL
 	tristate "Atmel PWM support"
 	depends on ARCH_AT91 || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 96160f4257fc..ff4f47e5fb7a 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PWM)		+= core.o
 obj-$(CONFIG_PWM_AB8500)	+= pwm-ab8500.o
 obj-$(CONFIG_PWM_ADP5585)	+= pwm-adp5585.o
 obj-$(CONFIG_PWM_APPLE)		+= pwm-apple.o
+obj-$(CONFIG_PWM_ARGON_FAN_HAT)	+= pwm-argon-fan-hat.o
 obj-$(CONFIG_PWM_ATMEL)		+= pwm-atmel.o
 obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+= pwm-atmel-hlcdc.o
 obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
diff --git a/drivers/pwm/pwm-argon-fan-hat.c b/drivers/pwm/pwm-argon-fan-hat.c
new file mode 100644
index 000000000000..58b94ad4d9bc
--- /dev/null
+++ b/drivers/pwm/pwm-argon-fan-hat.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Marek Vasut
+ *
+ * Limitations:
+ * - no support for offset/polarity
+ * - fixed 30 kHz period
+ *
+ * Argon Fan HAT https://argon40.com/products/argon-fan-hat
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pwm.h>
+
+#define ARGON40_FAN_HAT_PERIOD_NS	33333	/* ~30 kHz */
+
+static int argon_fan_hat_round_waveform_tohw(struct pwm_chip *chip,
+					     struct pwm_device *pwm,
+					     const struct pwm_waveform *wf,
+					     void *_wfhw)
+{
+	u8 *wfhw = _wfhw;
+
+	*wfhw = DIV_ROUND_CLOSEST_ULL(wf->duty_length_ns * 100, wf->period_length_ns);
+
+	return 0;
+}
+
+static int argon_fan_hat_round_waveform_fromhw(struct pwm_chip *chip,
+					       struct pwm_device *pwm,
+					       const void *_wfhw,
+					       struct pwm_waveform *wf)
+{
+	const u8 *wfhw = _wfhw;
+
+	wf->period_length_ns = ARGON40_FAN_HAT_PERIOD_NS;
+	wf->duty_length_ns = DIV64_U64_ROUND_UP(wf->period_length_ns * *wfhw, 100);
+	wf->duty_offset_ns = 0;
+
+	return 0;
+}
+
+static int argon_fan_hat_write(struct i2c_client *i2c, const u8 wfhw)
+{
+	u8 tx[2] = { 0x80, wfhw };
+	struct i2c_msg msg = {
+		.addr = i2c->addr,
+		.len = 2,
+		.buf = tx,
+	};
+
+	return (i2c_transfer(i2c->adapter, &msg, 1) == 1) ? 0 : -EINVAL;
+}
+
+static int argon_fan_hat_write_waveform(struct pwm_chip *chip,
+					struct pwm_device *pwm,
+					const void *_wfhw)
+{
+	struct i2c_client *i2c = pwmchip_get_drvdata(chip);
+	const u8 *wfhw = _wfhw;
+
+	return argon_fan_hat_write(i2c, *wfhw);
+}
+
+static const struct pwm_ops argon_fan_hat_pwm_ops = {
+	.sizeof_wfhw		= sizeof(u8),
+	.round_waveform_fromhw	= argon_fan_hat_round_waveform_fromhw,
+	.round_waveform_tohw	= argon_fan_hat_round_waveform_tohw,
+	.write_waveform		= argon_fan_hat_write_waveform,
+	/*
+	 * The controller does not provide any way to read info back,
+	 * reading from the controller stops the fan, therefore there
+	 * is no .read_waveform here.
+	 */
+};
+
+static int argon_fan_hat_i2c_probe(struct i2c_client *i2c)
+{
+	struct pwm_chip *pc = devm_pwmchip_alloc(&i2c->dev, 1, 0);
+	int ret;
+
+	if (IS_ERR(pc))
+		return PTR_ERR(pc);
+
+	pc->ops = &argon_fan_hat_pwm_ops;
+	pwmchip_set_drvdata(pc, i2c);
+
+	ret = devm_pwmchip_add(&i2c->dev, pc);
+	if (ret)
+		return dev_err_probe(&i2c->dev, ret, "Could not add PWM chip\n");
+
+	return 0;
+}
+
+static void argon_fan_hat_i2c_shutdown(struct i2c_client *i2c)
+{
+	argon_fan_hat_write(i2c, 100);
+}
+
+static const struct of_device_id argon_fan_hat_dt_ids[] = {
+	{ .compatible = "argon40,fan-hat" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, argon_fan_hat_dt_ids);
+
+static struct i2c_driver argon_fan_hat_driver = {
+	.driver = {
+		.name = "argon-fan-hat",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = argon_fan_hat_dt_ids,
+	},
+	.probe = argon_fan_hat_i2c_probe,
+	.shutdown = argon_fan_hat_i2c_shutdown,
+};
+
+module_i2c_driver(argon_fan_hat_driver);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut+renesas@mailbox.org>");
+MODULE_DESCRIPTION("Argon40 Fan HAT");
+MODULE_LICENSE("GPL");
-- 
2.47.2

Re: [PATCH v3 3/3] pwm: argon-fan-hat: Add Argon40 Fan HAT support

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 4507 bytes --]

Hello Marek,

On Tue, Jun 17, 2025 at 11:19:36AM +0200, Marek Vasut wrote:
> diff --git a/drivers/pwm/pwm-argon-fan-hat.c b/drivers/pwm/pwm-argon-fan-hat.c
> new file mode 100644
> index 000000000000..58b94ad4d9bc
> --- /dev/null
> +++ b/drivers/pwm/pwm-argon-fan-hat.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Marek Vasut
> + *
> + * Limitations:
> + * - no support for offset/polarity
> + * - fixed 30 kHz period
> + *
> + * Argon Fan HAT https://argon40.com/products/argon-fan-hat
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pwm.h>
> +
> +#define ARGON40_FAN_HAT_PERIOD_NS	33333	/* ~30 kHz */
> +
> +static int argon_fan_hat_round_waveform_tohw(struct pwm_chip *chip,
> +					     struct pwm_device *pwm,
> +					     const struct pwm_waveform *wf,
> +					     void *_wfhw)
> +{
> +	u8 *wfhw = _wfhw;
> +
> +	*wfhw = DIV_ROUND_CLOSEST_ULL(wf->duty_length_ns * 100, wf->period_length_ns);

Oh, I thought I already pointed this out to be wrong in reply to v2, but
it seems I didn't :-\ Ah, I did that for your other PWM patch
https://lore.kernel.org/all/jgafc7te2zotqwvetudo6c4o4nit6zi3yp4ecg4gka53hrypk5@7ypb5asrc2ct/.

I see you have concerns there, I'll address these in that thread.

> +	return 0;
> +}
> +
> +static int argon_fan_hat_round_waveform_fromhw(struct pwm_chip *chip,
> +					       struct pwm_device *pwm,
> +					       const void *_wfhw,
> +					       struct pwm_waveform *wf)
> +{
> +	const u8 *wfhw = _wfhw;
> +
> +	wf->period_length_ns = ARGON40_FAN_HAT_PERIOD_NS;
> +	wf->duty_length_ns = DIV64_U64_ROUND_UP(wf->period_length_ns * *wfhw, 100);
> +	wf->duty_offset_ns = 0;
> +
> +	return 0;
> +}
> +
> +static int argon_fan_hat_write(struct i2c_client *i2c, const u8 wfhw)
> +{
> +	u8 tx[2] = { 0x80, wfhw };

The vendor "driver" has `ADDR_ARGONONEREG_DUTYCYCLE=0x80`, maybe put that
into a define, too?

> +	struct i2c_msg msg = {
> +		.addr = i2c->addr,
> +		.len = 2,
> +		.buf = tx,
> +	};
> +
> +	return (i2c_transfer(i2c->adapter, &msg, 1) == 1) ? 0 : -EINVAL;

The vendor driver uses smbus stuff. I suggest to use
i2c_smbus_write_byte_data() here, too.

I also found that the vendor driver reads the duty-cycle register (using
i2c_smbus_read_byte_data()). Strange that this stops the fan.

> +}
> +
> +static int argon_fan_hat_write_waveform(struct pwm_chip *chip,
> +					struct pwm_device *pwm,
> +					const void *_wfhw)
> +{
> +	struct i2c_client *i2c = pwmchip_get_drvdata(chip);
> +	const u8 *wfhw = _wfhw;
> +
> +	return argon_fan_hat_write(i2c, *wfhw);
> +}
> +
> +static const struct pwm_ops argon_fan_hat_pwm_ops = {
> +	.sizeof_wfhw		= sizeof(u8),
> +	.round_waveform_fromhw	= argon_fan_hat_round_waveform_fromhw,
> +	.round_waveform_tohw	= argon_fan_hat_round_waveform_tohw,
> +	.write_waveform		= argon_fan_hat_write_waveform,

Please don't align the =, use a single space only.

> +	/*
> +	 * The controller does not provide any way to read info back,
> +	 * reading from the controller stops the fan, therefore there
> +	 * is no .read_waveform here.
> +	 */
> +};
> +
> +static int argon_fan_hat_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct pwm_chip *pc = devm_pwmchip_alloc(&i2c->dev, 1, 0);

Can you please rename "pc" to "chip"? "pc" is usually used for the
driver data struct.

> +	int ret;
> +
> +	if (IS_ERR(pc))
> +		return PTR_ERR(pc);
> +
> +	pc->ops = &argon_fan_hat_pwm_ops;
> +	pwmchip_set_drvdata(pc, i2c);
> +
> +	ret = devm_pwmchip_add(&i2c->dev, pc);
> +	if (ret)
> +		return dev_err_probe(&i2c->dev, ret, "Could not add PWM chip\n");
> +
> +	return 0;
> +}
> +
> +static void argon_fan_hat_i2c_shutdown(struct i2c_client *i2c)
> +{
> +	argon_fan_hat_write(i2c, 100);
> +}

Isn't that something that the fan driver should cope for? PWM drivers
usually do nothing on shutdown.

> +static const struct of_device_id argon_fan_hat_dt_ids[] = {
> +	{ .compatible = "argon40,fan-hat" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, argon_fan_hat_dt_ids);
> +
> +static struct i2c_driver argon_fan_hat_driver = {
> +	.driver = {
> +		.name = "argon-fan-hat",
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +		.of_match_table = argon_fan_hat_dt_ids,
> +	},
> +	.probe = argon_fan_hat_i2c_probe,
> +	.shutdown = argon_fan_hat_i2c_shutdown,
> +};
> +
> +module_i2c_driver(argon_fan_hat_driver);
> +
> +MODULE_AUTHOR("Marek Vasut <marek.vasut+renesas@mailbox.org>");
> +MODULE_DESCRIPTION("Argon40 Fan HAT");
> +MODULE_LICENSE("GPL");

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 3/3] pwm: argon-fan-hat: Add Argon40 Fan HAT support

[Marek Vasut]

On 6/17/25 4:02 PM, Uwe Kleine-König wrote:
> Hello Marek,

Hi,

>> +static int argon_fan_hat_write(struct i2c_client *i2c, const u8 wfhw)
>> +{
>> +	u8 tx[2] = { 0x80, wfhw };
> 
> The vendor "driver" has `ADDR_ARGONONEREG_DUTYCYCLE=0x80`, maybe put that
> into a define, too?
> 
>> +	struct i2c_msg msg = {
>> +		.addr = i2c->addr,
>> +		.len = 2,
>> +		.buf = tx,
>> +	};
>> +
>> +	return (i2c_transfer(i2c->adapter, &msg, 1) == 1) ? 0 : -EINVAL;
> 
> The vendor driver uses smbus stuff. I suggest to use
> i2c_smbus_write_byte_data() here, too.
> 
> I also found that the vendor driver reads the duty-cycle register (using
> i2c_smbus_read_byte_data()). Strange that this stops the fan.

So it seems.

[...]

>> +static void argon_fan_hat_i2c_shutdown(struct i2c_client *i2c)
>> +{
>> +	argon_fan_hat_write(i2c, 100);
>> +}
> 
> Isn't that something that the fan driver should cope for? PWM drivers
> usually do nothing on shutdown.
It's probably best to force the blower fan to maximum before 
reboot/shutdown. The firmware might not handle the fan and the SoC might 
get very hot before Linux boots again.

The rest is addressed in V4.

Re: [PATCH v3 3/3] pwm: argon-fan-hat: Add Argon40 Fan HAT support

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

Hello Marek,

On Sat, Jun 21, 2025 at 07:19:51PM +0200, Marek Vasut wrote:
> On 6/17/25 4:02 PM, Uwe Kleine-König wrote:
> > > +static void argon_fan_hat_i2c_shutdown(struct i2c_client *i2c)
> > > +{
> > > +	argon_fan_hat_write(i2c, 100);
> > > +}
> > 
> > Isn't that something that the fan driver should cope for? PWM drivers
> > usually do nothing on shutdown.
> 
> It's probably best to force the blower fan to maximum before
> reboot/shutdown. The firmware might not handle the fan and the SoC might get
> very hot before Linux boots again.

This is a IMHO a layer violation though. Yes, this PWM only comes in
combination with a fan. But still, the reason to want to emit a 100%
relative duty is because there is a fan, not because this is a PWM. So
this belongs into the fan part. And "funny" enough pwm-fan also has a
.shutdown callback that disables the fan.

So I'd ask you to address this part in the pwm-fan driver and drop the
shutdown callback here.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]