Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On Tue, Jun 23, 2026 at 09:01:32PM +0000, Selvamani Rajagopal wrote:
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
> > 
> >
> > > +
> > > +title: onsemi's multi-phase digital controllers
> > 
> > Can someone explain to me what a "digital controller" actually is?
> > Seems very generi and that a word may have been left out, were it not
> > for the fact that this wording is used several times in the patch.
> > 
> 
> Thanks for reviewing.
> 
> According to me, "digital controller" means the controller uses digital circuits to implement 
> the features and functionality. We can remove "digital" and keep only controller. It won't make any
> difference for Linux documentation.

My point is that what's actually being controlled is missing. Maybe it
is obvious to you, but it is not to me. Your nodename in your example is
> +      fd5121@50 {
which doesn't comply with node naming requirements and I wanted to come
up with a suggestion for what it should be.
I am assuming that its power or voltage that you're controlling so
either it should be hwmon@ or regulator@. 

> 
> > > +
> > > + enum:
> > > + - onnn,fd5121
> > > + - onnn,fd5123
> > > + - onnn,fd5125
> > 
> > Your /OF/ match data in your driver suggests that you intended to permit
> > fallback compatibles here?
> 
> Agree. Sorry about the discrepancy. Will fix it.
> 
> > 
> > |+#ifdef CONFIG_OF
> > |+static const struct of_device_id fd5121_of_match[] = {
> > |+ { .compatible = "onnn,fd5121" },
> > |+ { }
> > |+};
> > |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> > |+#endif
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]