Date: Sun, 5 May 2024 17:08:15 +0700
From: Chanh Nguyen <chanh@...eremail.onmicrosoft.com>
To: Guenter Roeck <linux@...ck-us.net>, Conor Dooley <conor@...nel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 Chanh Nguyen <chanh@...amperecomputing.com>, Jean Delvare
 <jdelvare@...e.com>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Justin Ledford
 <justinledford@...gle.com>, devicetree@...r.kernel.org,
 linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
 OpenBMC Maillist <openbmc@...ts.ozlabs.org>,
 Open Source Submission <patches@...erecomputing.com>,
 Phong Vo <phong@...amperecomputing.com>,
 Thang Nguyen <thang@...amperecomputing.com>,
 Quan Nguyen <quan@...amperecomputing.com>
Subject: Re: [PATCH v2 3/3] dt-bindings: hwmon: max31790: Add
 maxim,pwmout-pin-as-tach-input property



On 25/04/2024 21:05, Guenter Roeck wrote:
> On 4/25/24 03:33, Chanh Nguyen wrote:
>>
>>
>> On 24/04/2024 00:02, Conor Dooley wrote:
>>> [EXTERNAL EMAIL NOTICE: This email originated from an external 
>>> sender. Please be mindful of safe email handling and proprietary 
>>> information protection practices.]
>>>
>>
> 
> The quote doesn't make much sense.
> 
>> Sorry Conor, there may be confusion here. I mean the mapping of the 
>> PWM output to the TACH input, which is on the MAX31790, and it is not 
>> sure a common feature on all fan controllers.
>>
> 
> I think the term "mapping" is a bit confusing here.
> 
> tach-ch, as I understand it, is supposed to associate a tachometer input
> with a pwm output, meaning the fan speed measured with the tachometer input
> is expected to change if the pwm output changes.
> 
> On MAX31790, it is possible to configure a pwm output pin as tachometer 
> input pin.
> That is something completely different. Also, the association is fixed.
> If the first pwm channel is used as tachometer channel, it would show up 
> as 7th
> tachometer channel. If the 6th pwm channel is configured to be used as 
> tachometer
> input, it would show up as 12th tachometer channel.
> 
> Overall, the total number of channels on MAX31790 is always 12. 6 of them
> are always tachometer inputs, the others can be configured to either be a
> pwm output or a tachometer input.

Thank you, Guenter, for your explanation. That is also my understanding 
of the MAX31790 feature.

So, I think we should introduce a vendor property to configure the pwm 
output pins to become tachometer input pins. We shouldn't use the 
tach-ch property. Because they are completely different, I think.

What's your idea ? Please help share me, Guenter


> 
> pwm outputs on MAX31790 are always tied to the matching tachometer inputs
> (pwm1 <--> tach1 etc) and can not be reconfigured, meaning tach-ch for
> channel X would always be X.
> 
>> I would like to open a discussion about whether we should use the 
>> tach-ch property on the fan-common.yaml
>>
>> I'm looking forward to hearing comments from everyone. For me, both 
>> tach-ch and vendor property are good.
>>
> 
> I am not even sure how to define tach-ch to mean "use the pwm output pin
> associated with this tachometer input channel not as pwm output
> but as tachometer input". That would be a boolean, not a number.
> 

Thank Guenter,

I reviewed again the "tach-ch" property, which is used in the 
https://elixir.bootlin.com/linux/v6.9-rc6/source/Documentation/devicetree/bindings/hwmon/aspeed,g6-pwm-tach.yaml#L68 
and 
https://elixir.bootlin.com/linux/v6.9-rc6/source/drivers/hwmon/aspeed-g6-pwm-tach.c#L434 


That is something completely different from my purpose.

> Guenter
> 

Powered by blists - more mailing lists