AW: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8

[Sperling, Tobias]

> On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
> > >From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00
> 2001
> 
> Some junk ^^^ above. Please investigate how you send patches.

Yeah also saw this line, but of course tried to apply the patch again after sending it
as mail and that worked fine. But just checked again and seems like this line can be
dropped.
And yes, I sent the patches manually, as we likely have some restrictions for smtp,
but as I was able to apply them again it's fine I guess.

> > From: Tobias Sperling <tobias.sperling@xxxxxxxxxxx>
> > Date: Fri, 23 Aug 2024 12:08:33 +0200
> > Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
> 
> And all this suggests malformed patch.

Why? If I drop this I'm not able to apply the patch, so I think this should be fine.

> >
> > Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> > analog-to-digital converters. These ADCs have a wide operating range and
> > a wide feature set. Communication is based on an I2C interface.
> > The driver provides the functionality of manually reading single channels
> > or sequentially reading all channels automatically.
> >
> > Signed-off-by: Tobias Sperling <tobias.sperling@xxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
> >  Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
> >  Documentation/hwmon/index.rst                 |   1 +
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run  and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.

Had done this already before submitting the patches (at least without --strict),
but only reports a warning about splitting the patch (which I got wrong here)
and updating the maintainers.
I guess you were about suggesting a second script to run. Which one is that?

> > +$id:
> https://deu01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.
> org%2Fschemas%2Fhwmon%2Fti%2Cads71x8.yaml%23&data=05%7C02%7C%7C
> ff09fedbe2744394f78508dcc9881ee7%7Cfe3606fad3974238999768dcd7851f64
> %7C1%7C0%7C638606833686313557%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C
> %7C%7C&sdata=vZaCpdaNzELpNNnd6wp5P9MNLQTnAmWXYD%2BNKQYCJ78%
> 3D&reserved=0
> > +$schema:
> https://deu01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.
> org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C02%7C%7Cff09fedbe2744394f78508dcc
> 9881ee7%7Cfe3606fad3974238999768dcd7851f64%7C1%7C0%7C63860683368
> 6326954%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=EJflznuTZYGR
> BRjULwohiHk8gPF9iRusSbmF8CKkl5Q%3D&reserved=0
> > +
> > +title: Texas Instruments ADS7128/ADS7138 Analog to Digital Converter (ADC)
> > +
> > +maintainers:
> > +  - None
> 
> Fidn a person... otherwise why would we care?

I know it's not nice, but we are likely not implementing further features, but OK, will
add myself then.

> > +    default: 1
> > +
> > +  interrupts:
> > +    description: |
> > +      Only considered in mode 1!
> 
> What is "considered"? Driver considers? This does not matter. Describe
> the hardware and if this is hardware related, you should have
> allOf:if:then restricting this.

It's possible to define a mode, either manual sampling or autosampling. In the
latter mode, also the hardware capabilities change, e.g. the driver is able to
trigger an interrupt so defining the interrupt only makes sense in that mode.
Will have a look to allOf:if:then then.

> > +            compatible = "ti,ads7138";
> > +            reg = <0x10>;
> > +            avdd-supply = <&reg_stb_3v3>;
> > +            ti,mode = /bits/ 8 <1>;
> > +            ti,interval = /bits/ 16 <1000>;
> > +            interrupt-parent = <&gpio2>;
> > +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> > +            status = "okay";
> 
> Drop

I guess, I shall only drop the "status" not the whole section?

> Best regards,
> Krzysztof

Regards,
Tobias