[PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document target-pwm property

[PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document target-pwm property

[Peter Korsgaard]

Similar to target-rpm from fan-common.yaml but for the PWM setting
(0..255).

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 Documentation/devicetree/bindings/hwmon/pwm-fan.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
index 4e5abf7580cc..58513ff732af 100644
--- a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
@@ -46,6 +46,14 @@ properties:
 
   "#cooling-cells": true
 
+  target-pwm:
+    description:
+      The default desired fan PWM.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 255
+    default: 255
+
 required:
   - compatible
   - pwms
-- 
2.39.2

[PATCH 2/2] hwmon: (pwm-fan): support target-pwm property to set default PWM value

[Peter Korsgaard]

For some use cases defaulting the PWM to full fan speed is not ideal
(noise, power consumption, ..), so support an optional target-pwm
property (0..255) to override the default PWM value.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 drivers/hwmon/pwm-fan.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index b67bc9e833c0..ebdefbd5789c 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -482,6 +482,7 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	const struct hwmon_channel_info **channels;
 	u32 *fan_channel_config;
 	int channel_count = 1;	/* We always have a PWM channel. */
+	u32 target_pwm = MAX_PWM;
 	int i;
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
@@ -527,11 +528,17 @@ static int pwm_fan_probe(struct platform_device *pdev)
 
 	ctx->enable_mode = pwm_disable_reg_enable;
 
+	of_property_read_u32(dev->of_node, "target-pwm", &target_pwm);
+	if (target_pwm > (u32)MAX_PWM) {
+		dev_err(dev, "Invalid target-pwm: %u > %d\n", target_pwm, MAX_PWM);
+		return -EINVAL;
+	}
+
 	/*
-	 * Set duty cycle to maximum allowed and enable PWM output as well as
+	 * Set duty cycle to target and enable PWM output as well as
 	 * the regulator. In case of error nothing is changed
 	 */
-	ret = set_pwm(ctx, MAX_PWM);
+	ret = set_pwm(ctx, target_pwm);
 	if (ret) {
 		dev_err(dev, "Failed to configure PWM: %d\n", ret);
 		return ret;
-- 
2.39.2

Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document target-pwm property

[Guenter Roeck]

On 4/24/24 02:04, Peter Korsgaard wrote:
> Similar to target-rpm from fan-common.yaml but for the PWM setting
> (0..255).
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>   Documentation/devicetree/bindings/hwmon/pwm-fan.yaml | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> index 4e5abf7580cc..58513ff732af 100644
> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> @@ -46,6 +46,14 @@ properties:
>   
>     "#cooling-cells": true
>   
> +  target-pwm:
> +    description:
> +      The default desired fan PWM.

Unlike target-rpm, there is no "desired" here. If this is the default pwm,
to be set, name and describe it accordingly.

Guenter

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +    default: 255
> +
>   required:
>     - compatible
>     - pwms

Re: [PATCH 2/2] hwmon: (pwm-fan): support target-pwm property to set default PWM value

[Guenter Roeck]

On 4/24/24 02:04, Peter Korsgaard wrote:
> For some use cases defaulting the PWM to full fan speed is not ideal
> (noise, power consumption, ..), so support an optional target-pwm
> property (0..255) to override the default PWM value.
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>   drivers/hwmon/pwm-fan.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index b67bc9e833c0..ebdefbd5789c 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -482,6 +482,7 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   	const struct hwmon_channel_info **channels;
>   	u32 *fan_channel_config;
>   	int channel_count = 1;	/* We always have a PWM channel. */
> +	u32 target_pwm = MAX_PWM;
>   	int i;
>   
>   	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> @@ -527,11 +528,17 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   
>   	ctx->enable_mode = pwm_disable_reg_enable;
>   
> +	of_property_read_u32(dev->of_node, "target-pwm", &target_pwm);
> +	if (target_pwm > (u32)MAX_PWM) {
> +		dev_err(dev, "Invalid target-pwm: %u > %d\n", target_pwm, MAX_PWM);
> +		return -EINVAL;
> +	}
> +
>   	/*
> -	 * Set duty cycle to maximum allowed and enable PWM output as well as
> +	 * Set duty cycle to target and enable PWM output as well as
>   	 * the regulator. In case of error nothing is changed
>   	 */
> -	ret = set_pwm(ctx, MAX_PWM);
> +	ret = set_pwm(ctx, target_pwm);
>   	if (ret) {
>   		dev_err(dev, "Failed to configure PWM: %d\n", ret);
>   		return ret;

A much better name would be default-pwm for the property name.
target-pwm is misleading and doesn't really make sense because
there is no "target", just a value that is being set.

Guenter

Re: [PATCH 2/2] hwmon: (pwm-fan): support target-pwm property to set default PWM value

[Guenter Roeck]

On 4/24/24 02:04, Peter Korsgaard wrote:
> For some use cases defaulting the PWM to full fan speed is not ideal
> (noise, power consumption, ..), so support an optional target-pwm
> property (0..255) to override the default PWM value.
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>   drivers/hwmon/pwm-fan.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index b67bc9e833c0..ebdefbd5789c 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -482,6 +482,7 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   	const struct hwmon_channel_info **channels;
>   	u32 *fan_channel_config;
>   	int channel_count = 1;	/* We always have a PWM channel. */
> +	u32 target_pwm = MAX_PWM;
>   	int i;
>   
>   	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> @@ -527,11 +528,17 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   
>   	ctx->enable_mode = pwm_disable_reg_enable;
>   
> +	of_property_read_u32(dev->of_node, "target-pwm", &target_pwm);
> +	if (target_pwm > (u32)MAX_PWM) {

Unnecessary type cast.

Guenter

> +		dev_err(dev, "Invalid target-pwm: %u > %d\n", target_pwm, MAX_PWM);
> +		return -EINVAL;
> +	}
> +
>   	/*
> -	 * Set duty cycle to maximum allowed and enable PWM output as well as
> +	 * Set duty cycle to target and enable PWM output as well as
>   	 * the regulator. In case of error nothing is changed
>   	 */
> -	ret = set_pwm(ctx, MAX_PWM);
> +	ret = set_pwm(ctx, target_pwm);
>   	if (ret) {
>   		dev_err(dev, "Failed to configure PWM: %d\n", ret);
>   		return ret;

Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document target-pwm property

[Peter Korsgaard]

On 24/04/2024 16.10, Guenter Roeck wrote:
> On 4/24/24 02:04, Peter Korsgaard wrote:
>> Similar to target-rpm from fan-common.yaml but for the PWM setting
>> (0..255).
>>
>> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
>> ---
>>   Documentation/devicetree/bindings/hwmon/pwm-fan.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml 
>> b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> index 4e5abf7580cc..58513ff732af 100644
>> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> @@ -46,6 +46,14 @@ properties:
>>     "#cooling-cells": true
>> +  target-pwm:
>> +    description:
>> +      The default desired fan PWM.
> 
> Unlike target-rpm, there is no "desired" here. If this is the default pwm,
> to be set, name and describe it accordingly.

OK, I will rename to default-pwm and send a v2 - Thanks.

-- 
Bye, Peter Korsgaard

Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document target-pwm property

[Rob Herring]

On Wed, Apr 24, 2024 at 11:04:52AM +0200, Peter Korsgaard wrote:
> Similar to target-rpm from fan-common.yaml but for the PWM setting
> (0..255).

IIRC, we have a map of RPMs to PWM duty cycle, so why can't you 
use that plus target-rpm?

Anything new for existing fan bindings should ideally use what 
fan-common.yaml defined or be added to it.

> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>  Documentation/devicetree/bindings/hwmon/pwm-fan.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> index 4e5abf7580cc..58513ff732af 100644
> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> @@ -46,6 +46,14 @@ properties:
>  
>    "#cooling-cells": true
>  
> +  target-pwm:
> +    description:
> +      The default desired fan PWM.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +    default: 255
> +
>  required:
>    - compatible
>    - pwms
> -- 
> 2.39.2
> 

Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document target-pwm property

[Guenter Roeck]

On 4/24/24 15:04, Rob Herring wrote:
> On Wed, Apr 24, 2024 at 11:04:52AM +0200, Peter Korsgaard wrote:
>> Similar to target-rpm from fan-common.yaml but for the PWM setting
>> (0..255).
> 
> IIRC, we have a map of RPMs to PWM duty cycle, so why can't you
> use that plus target-rpm?
> 

target-rpm is the target fan speed. The property defined here
is the default pwm to set when the device is instantiated.

The two values are also orthogonal. The fan rpm is fan dependent.
Each fan will require a different pwm value to reach the target speed.
Trying to use target-rpm to set a default pwm value would really
not make much if any sense.

Guenter

> Anything new for existing fan bindings should ideally use what
> fan-common.yaml defined or be added to it.
> 
>>
>> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
>> ---
>>   Documentation/devicetree/bindings/hwmon/pwm-fan.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> index 4e5abf7580cc..58513ff732af 100644
>> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> @@ -46,6 +46,14 @@ properties:
>>   
>>     "#cooling-cells": true
>>   
>> +  target-pwm:
>> +    description:
>> +      The default desired fan PWM.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 255
>> +    default: 255
>> +
>>   required:
>>     - compatible
>>     - pwms
>> -- 
>> 2.39.2
>>