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

[Chanh Nguyen]

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@xxxxxxxxxxxxxxxxxxxxxx>
> > > > ---
> > > >    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@xxxxxxxxxxxx> or Jean Delvare <jdelvare@xxxxxxxx>) ?
>
> 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.