Re: [PATCH 2/3] hwmon: (pmbus/lm25066) Use PMBUS_REGULATOR_ONE to declare regulator

[Conor Dooley <conor@kernel.org>]

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

On Thu, Feb 15, 2024 at 02:14:37AM -0800, Zev Weiss wrote:
> On Wed, Feb 14, 2024 at 05:22:35PM PST, Guenter Roeck wrote:
> > On 2/14/24 17:04, Zev Weiss wrote:
> > > On Wed, Feb 14, 2024 at 11:43:41AM PST, Guenter Roeck wrote:
> > > > If a chip only provides a single regulator, it should be named 'vout'
> > > > and not 'vout0'. Declare regulator using PMBUS_REGULATOR_ONE() to make
> > > > that happen.
> > > > 
> > > 
> > > Hi Guenter,
> > > 
> > > This will necessitate a DTS update on at least one platform to maintain compatibility (Delta ahe50dc BMC, [1]).  I'm not sure offhand if there are process/policy rules about mixing code changes and device-tree changes in the same commit, but changing either one without the other would break things.
> > > 
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed/aspeed-bmc-delta-ahe50dc.dts?id=8d3dea210042f54b952b481838c1e7dfc4ec751d#n21
> > > 
> > > 
> > 
> > Sigh. Agreed, especially since changing the dts file in the kernel
> > won't change the dtb files on actual hardware.
> > 
> > I really have no good solution for this. We (Well, I) didn't realize that
> > there are regulator naming conventions/restrictions when we introduced
> > regulator support into PMBus drivers. My bad. Let's see what others say.
> > 
> > Guenter
> > 
> 
> Well, perhaps mitigating that slightly: I don't see any obvious cases of any
> other platforms' device-trees having any dependencies on the regulator
> naming that would be affected by this (judging by 'git grep vout0
> arch/*/boot/dts' anyway),

Which is a good thing, as none of these devices' bindings actually allow
regulator. Aspeed devicetrees are a mess of non-compliance with the
bindings, so I suppose that this is not surprising, but somewhere in the
wall of complaints there is a:
aspeed-bmc-delta-ahe50dc.dtb: /ahb/apb/bus@1e78a000/i2c-bus@40/pca9541@79/i2c-arb/efuse@12: failed to match any schema with compatible: ['lm25066']

I mentioned this on the other thread, but I guess the kernel is actually
using the i2c_device_ids to probe the driver and not the compatible,
since the documented compatible and the compatibles in the driver have a
vendor prefix?

> and at least with OpenBMC on the ahe50dc (the
> primary and AFAIK only user of that device-tree) the dtb would also be
> updated along with any kernel update.
> 
> So I wouldn't expect it to cause anyone any actual problems if we went ahead
> and changed it anyway; as long as the dts & driver do stay in sync with each
> other, maybe we could let it slide if it's otherwise a desirable change to
> make?

I think having "vout0" when there's nothing else looks a bit odd, but
I'm not gonna lose sleep over "vout0" being used if its used in device
documentation or is convention.

Whatever is done, please document the regulator child node in the
binding for this device and fix the dts to use a real compatible.

Cheers,
Conor.

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