Re: [PATCH 2/4] ARM: dts: aspeed: mtmitchell: Add I2C Fan

[Chanh Nguyen <chanh@amperemail.onmicrosoft.com>]

On 07/05/2023 15:23, Krzysztof Kozlowski wrote:
> On 07/05/2023 10:20, Chanh Nguyen wrote:
>>
>> On 25/04/2023 20:15, Krzysztof Kozlowski wrote:
>>> On 25/04/2023 08:57, Chanh Nguyen wrote:
>>>> Add the MAX31790 node as a Fan I2C controller. It controls the
>>>> TACH and PWM for Fan Mt.Mitchell system.
>>>>
>>>> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
>>>> ---
>>>>    arch/arm/boot/dts/aspeed-bmc-ampere-mtmitchell.dts | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-ampere-mtmitchell.dts b/arch/arm/boot/dts/aspeed-bmc-ampere-mtmitchell.dts
>>>> index e79f56208b89..6455cf80da0e 100644
>>>> --- a/arch/arm/boot/dts/aspeed-bmc-ampere-mtmitchell.dts
>>>> +++ b/arch/arm/boot/dts/aspeed-bmc-ampere-mtmitchell.dts
>>>> @@ -477,6 +477,18 @@
>>>>    			line-name = "bmc-ocp0-en-n";
>>>>    		};
>>>>    	};
>>>> +
>>>> +	max31790@20 {
>>>
>>> Node names should be generic.
>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>>
>> Thank Krzysztof,
>>
>> I think these node names should be "fan-i2c-0" and "fan-i2c-1". Do you
>> have any other idea ?
> 
> i2c-0 is not generic. This should be either fan or fan-controller,
> depending what this is.

Thank Krzysztof! I will update node name is "fan-controller" in PATCH v2.

>>
>>>
>>>> +		compatible = "maxim,max31790";
>>>
>>> Unfortunately the compatible is undocumented.
>>>
>>> Please run scripts/checkpatch.pl and fix reported warnings.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Yes Krzysztof,
>>
>> This compatible has not yes documented.
>>
>> Should I push a document for max31790 to
>> ./Documentation/devicetree/bindings/ or ask to maintainer (Guenter Roeck
>> <linux@roeck-us.net> or Jean Delvare <jdelvare@suse.com>) ?
> 
> Check on lore.kernel.org if there is ongoing work. If there is no, then
> please submit new the bindings (in DT schema). Maintainers are not for
> writing your code, so it's you or some other developer who should do it.
> 
> Best regards,
> Krzysztof
> 

Thank Krzysztof!

I checked on lore.kernel.org but no submit is in-progress for that. I'll 
submit new the binding document for max31790.

Now, Do I need to remove the "[PATCH 2/4] ARM: dts: aspeed: mtmitchell: 
Add I2C Fan" from PATCH v2?

And will push again "ARM: dts: aspeed: mtmitchell: Add I2C Fan" commit 
once max31790 binding document was available upstream.