Received-SPF: pass (google.com: domain of linux-kernel+bounces-168941-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <0dcc8788-604a-49c1-8c6b-fdbfa9192039@amperemail.onmicrosoft.com>
Date: Sun, 5 May 2024 17:08:15 +0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 3/3] dt-bindings: hwmon: max31790: Add
 maxim,pwmout-pin-as-tach-input property
To: Guenter Roeck <linux@roeck-us.net>, Conor Dooley <conor@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Chanh Nguyen <chanh@os.amperecomputing.com>, Jean Delvare
 <jdelvare@suse.com>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Justin Ledford
 <justinledford@google.com>, devicetree@vger.kernel.org,
 linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
 OpenBMC Maillist <openbmc@lists.ozlabs.org>,
 Open Source Submission <patches@amperecomputing.com>,
 Phong Vo <phong@os.amperecomputing.com>,
 Thang Nguyen <thang@os.amperecomputing.com>,
 Quan Nguyen <quan@os.amperecomputing.com>
References: <20240414042246.8681-1-chanh@os.amperecomputing.com>
 <20240414042246.8681-4-chanh@os.amperecomputing.com>
 <13b195e6-cbbd-4f74-a6fa-d874cb4aaa45@linaro.org>
 <065243cc-09cf-4087-8842-bd4394fb324f@amperemail.onmicrosoft.com>
 <d549cf2b-a7fa-4644-8fcb-3c420503ee01@amperemail.onmicrosoft.com>
 <20240423-gallantly-slurp-24adbfbd6f09@spud>
 <ab5cfd8c-0e88-4194-a77e-5ffbb6890319@amperemail.onmicrosoft.com>
 <396b47f5-9604-44ab-881f-94d0664bcab8@roeck-us.net>
Content-Language: en-US
From: Chanh Nguyen <chanh@amperemail.onmicrosoft.com>
In-Reply-To: <396b47f5-9604-44ab-881f-94d0664bcab8@roeck-us.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?TmMzanhzVXU1TWpxcDJVZWJ6eTJETitSdXFFeUpoM1YwMFBTTEZMWit4SkhB?=
 =?utf-8?B?TnR4c1FrSjFBaWl0Y1FlZndsdjloTDVsOW1wZEdzS2VIVHFtTEp3ZTB0L2FK?=
 =?utf-8?B?bERxOUZGclhxQ0k3Ni9ZY1UydktUYkVqeFNEZG0yTGE5L3lmV2oya29ObmNw?=
 =?utf-8?B?alRQalFxWVpjZytqRGRwUk9yQWtwM2h3S2ZSSlVnM25qK0ZDQnN6WHdOY005?=
 =?utf-8?B?R0xRV1E4M2tmSnZsOTdhcE4yN1JwbTNjR21oMHFtY1VXMGJQMzErb1FUdHRu?=
 =?utf-8?B?N1VqeFpUejlqNzZ5TWJPd3ZQbVhxT2xvRnpvbUF6bSt3dktaazZtOWJ4OW5y?=
 =?utf-8?B?UENxclcwZEUrN3ZHdHc2TkZlT2RNWllDanROdWErSDJWOGxzY1VSaDY4anA5?=
 =?utf-8?B?RmduSmMzbkRxNmZQbzlNbExEMmZQdEFhYVR1UXVjQ0dOUWI5WHQ4Q1NkOHJQ?=
 =?utf-8?B?ZTBIajBMVTM3NlBrV1V6eTdrMTBPL2FmZ25WYk1CeEd0SnQ4T0pCaCtrOUg2?=
 =?utf-8?B?L1dVRjhzZnJjbVp2ZVB0QU5SempRZzJIdFJjem5oaklLc05Eem9QS2JMSUt4?=
 =?utf-8?B?U3UzV3RES09vdloycjVOcVM5Y1JNT211cTBlVmdHWHJtWmZ4Rzl3TjZvaElz?=
 =?utf-8?B?MW5pWFNnL2NFek8vT3hEbytNYmFTTjg0YVJ1UkNvR0JpVSs5bkJRTEUrVzRU?=
 =?utf-8?B?YUpLWTRGbW5NU3REZ2dGZGRVdUFLdEhreS83M25qRTQ1REpkaVZBZWMzelhB?=
 =?utf-8?B?OHlBWXVOWVBqY1VaeW53TnJuak5ZM2hDQTFjR0hkQXROeXVpQi8wdDFHb1pr?=
 =?utf-8?B?TDhRcHpXR3dZaUExdjliK0o1aS8rbGRHYVJwM1UxSnNyalcxeHQ0MEdoZVc3?=
 =?utf-8?B?dm1uMnV4Ym4vS3FqMnNWOXdnV2VSbk5sNWtwQVE5QURaQ0hSVzlTTXBDMGky?=
 =?utf-8?B?TGR5S3lmVGtRd0FPWTFvcWlNSXZzZUw1N2dKYXZUZytxUHg5amZqR2htVFdO?=
 =?utf-8?B?MUJIeTc3eFhjVWpOTWhEVjZCaFUxT0J5UGkrOW9ENmtxenBmZ1REQUozRnBN?=
 =?utf-8?B?WHhUSndZRUxVTWxpU1Y5Vk1PWWgyUFBHejhWKzlhYzJVcjBIQ1ZibkQwVXZv?=
 =?utf-8?B?dHM1YkllUndFZWdIdzFsR1AwYVpjRkJpQnZJQVZJSVJUb1NyMk5wWExYVlhE?=
 =?utf-8?B?ZVM3SUJJekJrbmRWd01KMjNyWDFrekYrYlNJNnk0UkFvV2Jxb24rKytHY25n?=
 =?utf-8?B?THVnVnhJN0ZlcVhOdGlLVVZQYVluSXBBL1F6eFZEemk4T3RGSFRjUGVpcXBz?=
 =?utf-8?B?QmtrZzdwUVFabWk0dGd5VU9PcStsMjFMek5qRmZlUUZSWVY1V09XbWc3NlV6?=
 =?utf-8?B?aE9rN3FmRUYxSVE5REk4ZVpIZ3E0MHpOR1pIejYvTmhZNFExN1YwOFozMUVR?=
 =?utf-8?B?V3lSZkN5SGpXdzV5U2IwenovMElaY0VPNUIwVGsxNzF1MUYyTStFcWVpZmY2?=
 =?utf-8?B?WkpjYVJRU3hPVVd1NGo2dkMzcTRrZzZUaERuM2tTTXJUemVDTnpxVzFTOEJl?=
 =?utf-8?B?SzJ2anM1anp1cTNJVEg1eEtUcEVOQUFDejlCZzF4QUhmYkoydEhYWGVmV1hk?=
 =?utf-8?B?NytQZThXbkt0MjJqM2U0d2c2U25PRk5rb3B4dk1SNUsxMktub3pJN0hYNUZu?=
 =?utf-8?B?bVhpaEo5TnFKdkZ2VGszWXZtYUVDM1FtaFQ1bE14MkxERWpiQzJ5RmE3dkZ5?=
 =?utf-8?B?dm9RUGF4L0pVY292d3pSTEMwNWV5WmFYRnA0REhmblh0bFFxZDB3ZUhyVmlT?=
 =?utf-8?B?STNFWnZGZ3lMNkRvdW0vdm1uTnFTUWRqWCtrdzNwS1VsNysvLzZzTlpCZVF6?=
 =?utf-8?B?b0hlaTRWSVdrekw0eHgvcHJqZER0bGdoRDVCeGtZOCtNVlBTT2VhOTh3TW9m?=
 =?utf-8?B?L0wrK3p3cVMyVG9aWS8xM2hRZkZ4L2QxZURMejdYNGJXS00yMXZaTjR5NWZO?=
 =?utf-8?B?d1l3QnhpVm1uYmFtSG9QN2VlV3dLckZtK1BDU1k0cU1tMlljSXJMRUhmY0xz?=
 =?utf-8?B?Ny8rY3lZeUw2c2w2VE5EcTdicUgveVFqcktOTVQ2V2ZVM3B6Y29VcWFmZjVL?=
 =?utf-8?B?SEtMNGMzcEZlb3RYM3BPejlqVzhsMHFNdWJDZ3duL012Zm93ZEF0QktMZS9l?=
 =?utf-8?Q?gH9gwa8YrnWFTvnMMGOOOR4=3D?=
X-OriginatorOrg: amperemail.onmicrosoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dc41bc7d-d0bf-4389-a396-08dc6ceb4e8c
X-MS-Exchange-CrossTenant-AuthSource: DM6PR01MB5947.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 May 2024 10:08:27.9796
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cUwV3jVBx+UIybofd8tnez0e5asdcVLxG4/s11EQOWu8hgdD5IN/KzvM+bBnWQpPVlSY2NKq0cfTVFSMNoU/SWyqSUYsyB9gwIZjx7y9UqqcGQIvMakBR0FvsykJwFCD
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR01MB8277


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
>