Re: [PATCH 3/3] dt-bindings: hwmon: max31790: Add pwmout-pin-as-tach-input property

From: Chanh Nguyen
Date: Mon Mar 18 2024 - 12:51:53 EST




On 18/03/2024 17:02, Krzysztof Kozlowski wrote:
On 18/03/2024 10:48, Chanh Nguyen wrote:


On 11/03/2024 23:56, Krzysztof Kozlowski wrote:
On 11/03/2024 12:13, Chanh Nguyen wrote:
Add pwmout-pin-as-tach-input property.

Why is this split from original binding? This does not make much
sense... Add complete hardware description.


Ok Krzysztof, I will merg the "[PATCH 1/3] dt-bindings: hwmon: Add maxim
max31790 driver bindings" commit and "[PATCH 3/3] dt-bindings: hwmon:
max31790: Add pwmout-pin-as-tach-input property" commit.

Later I checked your driver code and this explains a bit. However first
patch should explain that instead. The split is fine, but just lack of
rationale is confusing.


Thank Krzysztof. I'll try to explain in detail each patch in v2.




Signed-off-by: Chanh Nguyen <chanh@xxxxxxxxxxxxxxxxxxxxxx>
---
Documentation/devicetree/bindings/hwmon/max31790.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/max31790.yaml b/Documentation/devicetree/bindings/hwmon/max31790.yaml
index 5a93e6bdebda..447cac17053a 100644
--- a/Documentation/devicetree/bindings/hwmon/max31790.yaml
+++ b/Documentation/devicetree/bindings/hwmon/max31790.yaml
@@ -25,6 +25,16 @@ properties:
reg:
maxItems: 1
+ pwmout-pin-as-tach-input:
+ description: |
+ An array of six integers responds to six PWM channels for
+ configuring the pwm to tach mode.
+ When set to 0, the associated PWMOUT produces a PWM waveform for
+ control of fan speed. When set to 1, PWMOUT becomes a TACH input

No vendor prefix, so generic property... but where is it defined?


Thank Krzysztof! It is not generic property, I'll add the vendor prefix.

I'll update the "pwmout-pin-as-tach-input" to
"maxim,pwmout-pin-as-tach-input" at v2.

Except that you should really look into common properties and use them.
Or explain why do you need new property?

Best regards,
Krzysztof


I'm also discussing with Rob Herring that on patch 3/3, I checked in the Documentation/devicetree/bindings/hwmon/fan-common.yaml. I found the tach-ch property, but it seems to define the tach channel used for the fan. It does not match my purpose. I want to configure the PWM-OUT pin to become a TACH-IN pin. I wonder if I can use the tach-ch property for my purpose.