On Wed, Feb 14, 2024 at 10:48:52AM -0800, Guenter Roeck wrote: > 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. Is it unacceptable? If you're implying that I am saying it is, that's not what I was doing here - I'm just wondering why it was chosen. Numbering when there's only one seems odd, so I was just looking for the rationale. > 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.
Attachment:
signature.asc
Description: PGP signature
