Received-SPF: pass (google.com: domain of linux-kernel+bounces-146231-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <15faebc0-9b20-4d0f-a946-c047bc6ae564@amperemail.onmicrosoft.com>
Date: Tue, 16 Apr 2024 11:38:53 +0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 1/3] dt-bindings: hwmon: Add maxim max31790 bindings
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Chanh Nguyen <chanh@os.amperecomputing.com>, Jean Delvare
 <jdelvare@suse.com>, Guenter Roeck <linux@roeck-us.net>,
 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>
Cc: 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-2-chanh@os.amperecomputing.com>
 <80effcab-74b8-4e15-a4db-9982b000b6b1@linaro.org>
Content-Language: en-US
From: Chanh Nguyen <chanh@amperemail.onmicrosoft.com>
In-Reply-To: <80effcab-74b8-4e15-a4db-9982b000b6b1@linaro.org>
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?a2NraGJSaTBlY2VRYm5ydzB4aENFVUplUG0rbW5qa296K2VtYkozSWpwNkZa?=
 =?utf-8?B?Y1VrNmlSaW5LeEpWWG5yVnJkdVF3dU9iZXlEZGlGK2ZHSXZJNjZ2K1pnaHJw?=
 =?utf-8?B?UWlMVW45UHNoV25hb29oRGlveEJYMGhCK0M1WXNlczkxTFNwQUsrUnRNWS8z?=
 =?utf-8?B?VGVpVTM5UlVTczU1aUo5ZzRDanYxV1VlYS9yUXpGa08xaUZDdCtrRmRCSW5I?=
 =?utf-8?B?TUcvNjN5MGVCUFM4YkM4bG1MdTJCVklCdTlGby9idksxVDhITU9Ob1FzSUY5?=
 =?utf-8?B?ZkxFZkl5UEl3TkxNbFVVQ3QwVXVjSVFHTFdxdUpXQU1jZnFkbEpPTTNSWVlN?=
 =?utf-8?B?Y3hvcURES3RnVnpCY2FlM0p4bldPWHByU1hwMnd1RnNqL1RER0RmQ3J1dFp6?=
 =?utf-8?B?Q3ZhbWk0aEpWUXh2U0lPSWM2TER2U1VLM2RuYUx2Um03NEEzVUtlN3FIcld6?=
 =?utf-8?B?aUxvRkJuS1ZKZWZnOERlUk5PZCtjdStnaGVyNjU5Qkl6ekRhRXZOK0w2dkhz?=
 =?utf-8?B?VDRHTkY2bmhrRVprL1p6RGhNamhuQzNTV0Z2VGVVSWozeXdOYzdqemNKQVRx?=
 =?utf-8?B?QlpwZzM0M3NtZmhOYnlkSmtCd0IxWSthL05BRnd2LzJLQklFNml3eHdYajR6?=
 =?utf-8?B?M2M3SE5iS0Q0L3JNaEczSENUUnhGNnFRN3NXS2puajVMSkt6NHlLY0ZxeHda?=
 =?utf-8?B?V3dxd1F4YzNmS21QV0ZBMG8zMUZLSFIveW5iZnZUajRkVVNaeE5pUWdNdGFn?=
 =?utf-8?B?WVZlMzVZV2Irbm5RaDhaN2lyblRlRlgwOEhEcFo0L1A0N3FtQksvaEpEU0dB?=
 =?utf-8?B?eGJtaVlsUFIrSHUrVlh2b2hheEkxZ25vam4xWlN3ait0VWVUYVE1Qkt1ZE9L?=
 =?utf-8?B?emlldjQ3a3FGeDUvZmNOMGNyNkd3Zk5LR01ib0ExS0d5V3prMThhSE5RK05G?=
 =?utf-8?B?NGdrMXoyNEltLyt6TnVOaEZwS1NFUWcyY1MvL1NETHIrRHRHQy8xRW5ya3lF?=
 =?utf-8?B?cEZoU21tUUxYVUl3clFNTnEvaUZ5NHJUZ05wRXhPNUZDSDZFTmg4R1g2Qmtv?=
 =?utf-8?B?V0JEbU5URkI5REV4Rlc1TGZCaTlkL2ExdVBka3FNN3diMWYvSDRLZTNpNTBH?=
 =?utf-8?B?RkYveVpnUFhkZjNSdGg0MmpOZlpGUVZDS2RXNC8zTkU3V01UUWVGU1RuMDd5?=
 =?utf-8?B?dnFWR3QxbHk0T2dKMVp5OS9BcE5kZkR4bnFhTGcwb21GUlo2MzVLcVEvZnpG?=
 =?utf-8?B?VU5rQzUwa3pZZEFZOTA5ekVCdzQ2N2h4WHJmREZBTlNjQlBzR2Vid25WOWcr?=
 =?utf-8?B?S0dmcTFSR1FHbnV4UmVmdDVtV3F5TVJVcU90S0VxZ2Zmd2xITjQ5R2Y4Rlhz?=
 =?utf-8?B?SVFoTGJpOXZZenc5Mi9zaWZTM3JmdVJ5eGVXVUQ2bnB2NzNhUEZWMWRVKzhC?=
 =?utf-8?B?ZVlZWlljT0tkcFhWSjgwUWt5SG1McllaMk83cTZ6ZXVUMDhXRG9WbXhqMlpn?=
 =?utf-8?B?bU5qM2hrOWg2R1FYbm9DYU1BT3JhcEl3dkFnMko4R2EwTDRhTk14QjArdy9V?=
 =?utf-8?B?TVplREpXcU1PODcrdmpuTTNBU2hTY0pTcEtnR2RiS2tqS0VKZXRybGY3ZWpi?=
 =?utf-8?B?QmdHdWFvU2tDV3ovbTBqMWJFTllPK25MRUFZTjI1Y0s1V2Jpam9wWXR1QndL?=
 =?utf-8?B?d3plQ2t4MjlwL2xKajZiYkIwNVZjLzdyV2F4R042cEV6V0d6REJLSnZjK01x?=
 =?utf-8?B?a3BuVmRjengxaUtwVTFUc3VYSkJHemdyR0lYMzVXWG9xSFo3bzIwdFpqSHkv?=
 =?utf-8?B?MFBWR1BEUG1Ja0Y5NUc2YnhNL2Q5ZmxxSjdKTjVlWVVESGw2YVMvN09VZHEr?=
 =?utf-8?B?Rmo3TWNqRGdJbUJGK2g4Y291aXpRNWdXZVltSHRHU0RMRS9XUVlDemgxNHRa?=
 =?utf-8?B?WTFtd3cyc0IwV1BYZmM0Z2xBS1dockdXd1RMQzdNQXZGNDhVR2FJc096RWY3?=
 =?utf-8?B?YnZoNFFFUWxWTDIvM2pnN3dhazdhRFlka2lIRmVrbGE2OXVRQ0poZDZvcVpp?=
 =?utf-8?B?d295Z1NUMHBDM1AwTzhPenJBVS8yd29DSE4vWFlGc2NSV2VjOWpJZ0IxN0J6?=
 =?utf-8?B?YnlaUXMvaFRXa1dPM1NGVTdnMHFSZ2IvOXhOWTJuZnBMUkUxNElNNVl0a3k5?=
 =?utf-8?Q?4TUvcXpB0f8oIptI1vakWuw=3D?=
X-OriginatorOrg: amperemail.onmicrosoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5503807a-474d-49cb-1d23-08dc5dcf251d
X-MS-Exchange-CrossTenant-AuthSource: DM6PR01MB5947.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2024 04:39:04.6123
 (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: Nyr2HFYBFo1t5hFZjAEoPF/QVj57c1u/LsW+SNry+kXa3tgKMWmfniz/LePE/c860sD15Wu8UFWn/t0a0lEAGMU8l/C+P6nopyrsRs8lB/b3coBigzta0rhQ/2XiWSYj
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR01MB6435


On 14/04/2024 13:03, Krzysztof Kozlowski wrote:
> On 14/04/2024 06:22, Chanh Nguyen wrote:
>> Add a device tree bindings for max31790 device.
>>
> 
> Nothing improved in commit msg, where is the rationale of split of bindings?
> 

Yes Krzysztof, I will use "tach-ch" property from the fan-common.yaml to 
solve my case. So I will not need a split of bindings.

Finally, my patch series will only be two patches. They are 
"dt-bindings: hwmon: Add maxim max31790" and "hwmon: (max31790): Support 
config PWM output becomes TACH".

I'm going to update that in the patch v3 series.

> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 

Thank Krzysztof, I'll drop "bindings" in commit msg.

>> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
>> ---
>> Changes in v2:
>>   - Update filename of the maxim,max31790.yaml                            [Krzysztof]
>>   - Add the common fan schema to $ref                                     [Krzysztof]
>>   - Update the node name to "fan-controller" in maxim,max31790.yaml       [Krzysztof]
>>   - Drop "driver" in commit title                                         [Krzysztof]
>> ---
>>   .../bindings/hwmon/maxim,max31790.yaml        | 52 +++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml
>> new file mode 100644
>> index 000000000000..a561e5a3e9e4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml
>> @@ -0,0 +1,52 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/maxim,max31790.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: The Maxim MAX31790 Fan Controller
>> +
>> +maintainers:
>> +  - Guenter Roeck <linux@roeck-us.net>
>> +
>> +description: >
>> +  The MAX31790 controls the speeds of up to six fans using six
>> +  independent PWM outputs. The desired fan speeds (or PWM duty cycles)
>> +  are written through the I2C interface.
>> +
>> +  Datasheets:
>> +    https://datasheets.maximintegrated.com/en/ds/MAX31790.pdf
>> +
>> +properties:
>> +  compatible:
>> +    const: maxim,max31790
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +allOf:
>> +  - $ref: fan-common.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      fan-controller@20 {
>> +        compatible = "maxim,max31790";
>> +        reg = <0x20>;
> 
> Make the example complete - add all properties, like clocks, resets and
> any others which are applicable.
> 

Ok Krzysztof, I'll update that in the patch v3

> Best regards,
> Krzysztof
>