On 2/14/24 09:51, Conor Dooley wrote:
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?
Primarily because that is what the PMBus core generates for the driver
because no one including me was aware that this is unacceptable
for single-output drivers. We now have commit 88b5970e92d0 ("hwmon:
(pmbus/core) Add helper macro to define single pmbus regulator").
I guess we can update the tda38640 driver to use the new macro
to register vout instead of vout0.
Guenter
