linux-kernel - RE: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <PH0PR03MB69385BEFFD04ECF850311E988EDE2@PH0PR03MB6938.namprd03.prod.outlook.com>
Date: Tue, 18 Mar 2025 10:03:42 +0000
From: "Encarnacion, Cedric justine" <Cedricjustine.Encarnacion@...log.com>
To: Guenter Roeck <linux@...ck-us.net>, Krzysztof Kozlowski <krzk@...nel.org>
CC: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, Jean Delvare <jdelvare@...e.com>,
        Jonathan Corbet <corbet@....net>,
        Delphine CC Chiu
	<Delphine_CC_Chiu@...ynn.com>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "linux-hwmon@...r.kernel.org"
	<linux-hwmon@...r.kernel.org>,
        "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>,
        "linux-i2c@...r.kernel.org"
	<linux-i2c@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074

> -----Original Message-----
> From: Guenter Roeck <groeck7@...il.com> On Behalf Of Guenter Roeck
> Sent: Friday, February 28, 2025 12:33 AM
> To: Krzysztof Kozlowski <krzk@...nel.org>
> Cc: Rob Herring <robh@...nel.org>; Encarnacion, Cedric justine
> <Cedricjustine.Encarnacion@...log.com>; Krzysztof Kozlowski
> <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Jean Delvare
> <jdelvare@...e.com>; Jonathan Corbet <corbet@....net>; Delphine CC Chiu
> <Delphine_CC_Chiu@...ynn.com>; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; linux-hwmon@...r.kernel.org; linux-
> doc@...r.kernel.org; linux-i2c@...r.kernel.org
> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: add lt3074
> 
> [External]
> 
> On Thu, Feb 27, 2025 at 09:50:23AM +0100, Krzysztof Kozlowski wrote:
> > > > >
> > > > > hwmon code might need some changes, but that's not really
> > > > > relevant for proper hardware description.
> > > >
> > > > Normally, I would agree, but it seems generic pmbus code expects
> > > > this structure. This just came up with changing another binding
> > > > maintained by 'Not Me' to follow this structure. We're stuck with
> > > > the existing way, so I don't know that it is worth supporting 2
> > > > ways forever. OTOH, is it guaranteed that these devices will only
> > > > ever be pmbus devices or that other regulator devices which are
> > > > not handled as pmbus devices currently will be in the future. If
> > > > so, more flexibility in the bindings will be needed.
> > > >
> > >
> > > I would appreciate if someone would explain to me what the problems
> > > with the current PMBus code actually are. I have seen several
> > > comments claiming
> >
> > Not exactly a problem but missing feature. pmbus code (at least one of
> > macros I looked at) expects regulator node and some sort of child of
> > it (vout), while such simple devices should be:
> >
> > regulator {
> > 	compatible = "adi,lt3074";
> > 	regulator-name = "vout";
> > 	regulator-min-microvolt = "100000";
> > 	regulator-max-microvolt = "100000";
> > };
> >
> > so without any of regulators and regulators/vout subnodes.
> >
> > > that the code should be changed, but I have no idea what the
> > > expected changes actually are or, in other words, what the PMBus
> > > code should be doing differently.
> >
> > I did not investigate much into pmbus code, but this might be as
> > simple as accepting arguments for .of_match and .regulators_node and
> > then accepting NULLs as them as well. Or a new macro which assigns
> > NULLs there.
> >
> 
> Unless I am missing something, the following should do the trick.
> 
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index ddb19c9726d6..289767e5d599 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -512,7 +512,6 @@ int pmbus_regulator_init_cb(struct regulator_dev *rdev,
>  	{							\
>  		.name = (_name),				\
>  		.of_match = of_match_ptr(_name),		\
> -		.regulators_node = of_match_ptr("regulators"),	\
>  		.ops = &pmbus_regulator_ops,			\
>  		.type = REGULATOR_VOLTAGE,			\
>  		.owner = THIS_MODULE,				\
> 
> Maybe someone can check if that works.
> 
> Thanks,
> Guenter

I'd like to follow up on this one. As of this writing, my understanding
is that the dt-binding should not expect regulators subnodes for
simple devices like this. There is already a similar binding as
mentioned in this thread particularly
"dt-bindings/regulator/infineon,ir38060". I think a binding without
the subnodes should still work with or without the change above.
With this, I'd like to know what the specific next steps are to continue
this patch series.

Thanks,
Cedric