On Wed, Feb 14, 2024 at 02:55:03PM +0530, Naresh Solanki wrote: > Add properties for interrupt & regulator. > Also update example. I feel like a broken record. Your patches need to explain _why_ you're doing what you're doing. I can read the diff and see this, but I do not know what the justification for it is. /30 seconds later I really am a broken record, to quote from v1: | Feeling like a broken record, given I am leaving the same comments on | multiple patches. The commit message needs to explain why you're doing | something. I can read the diff and see what you did! https://lore.kernel.org/all/20240126-fleshed-subdued-36bae813e2ba@spud/ The patch itself does look better than the v1, with one minor comment below. Thanks, Conor. > Signed-off-by: Naresh Solanki <naresh.solanki@xxxxxxxxxxxxx> > > --- > Changes in v2: > 1. Remove TEST=.. > 2. Update regulator subnode property as vout0 > 3. Restore commented line in example > 4. blank line after interrupts property in example. > --- > .../hwmon/pmbus/infineon,tda38640.yaml | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > index ded1c115764b..a93b3f86ee87 100644 > --- a/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/infineon,tda38640.yaml > @@ -30,6 +30,23 @@ properties: > unconnected(has internal pull-down). > type: boolean > > + interrupts: > + maxItems: 1 > + > + regulators: > + type: object > + description: > + list of regulators provided by this controller. > + > + properties: > + vout0: Why "vout0" if there's only one output? Is it called that in the documentation? I had a quick check but only saw it called "vout". Are there other related devices that would have multiple regulators that might end up sharing the binding? Thanks, Conor. > + $ref: /schemas/regulator/regulator.yaml# > + type: object > + > + unevaluatedProperties: false > + > + additionalProperties: false > + > required: > - compatible > - reg > @@ -38,6 +55,7 @@ additionalProperties: false > > examples: > - | > + #include <dt-bindings/interrupt-controller/irq.h> > i2c { > #address-cells = <1>; > #size-cells = <0>; > @@ -45,5 +63,15 @@ examples: > tda38640@40 { > compatible = "infineon,tda38640"; > reg = <0x40>; > + > + interrupt-parent = <&smb_pex_cpu0_event>; > + interrupts = <10 IRQ_TYPE_LEVEL_LOW>; > + > + regulators { > + pvnn_main_cpu0: vout0 { > + regulator-name = "pvnn_main_cpu0"; > + regulator-enable-ramp-delay = <200>; > + }; > + }; > }; > }; > > base-commit: 7e90b5c295ec1e47c8ad865429f046970c549a66 > -- > 2.42.0 >
Attachment:
signature.asc
Description: PGP signature
