linux-kernel - Re: [RFC PATCH 4/5] regulator: add 88pm88x regulators driver

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]

Date: Sun, 07 Jan 2024 13:46:15 +0100
From: "Karel Balej" <karelb@...li.ms.mff.cuni.cz>
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>, "Mark Brown"
 <broonie@...nel.org>
Cc: "Karel Balej" <balejk@...fyz.cz>, "Lee Jones" <lee@...nel.org>, "Rob
 Herring" <robh+dt@...nel.org>, "Krzysztof Kozlowski"
 <krzysztof.kozlowski+dt@...aro.org>, "Conor Dooley" <conor+dt@...nel.org>,
 "Liam Girdwood" <lgirdwood@...il.com>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, Duje Mihanović
 <duje.mihanovic@...le.hr>, <~postmarketos/upstreaming@...ts.sr.ht>,
 <phone-devel@...r.kernel.org>
Subject: Re: [RFC PATCH 4/5] regulator: add 88pm88x regulators driver

Krzysztof,

On Sun Jan 7, 2024 at 11:34 AM CET, Krzysztof Kozlowski wrote:
> On 07/01/2024 10:49, Karel Balej wrote:
> > Mark,
> > 
> > On Fri Jan 5, 2024 at 4:18 PM CET, Mark Brown wrote:
> >> On Thu, Dec 28, 2023 at 10:39:13AM +0100, Karel Balej wrote:
> >>
> >>> @@ -68,6 +68,21 @@ static struct mfd_cell pm886_devs[] = {
> >>>  		.num_resources = ARRAY_SIZE(pm88x_onkey_resources),
> >>>  		.resources = pm88x_onkey_resources,
> >>>  	},
> >>> +	{
> >>> +		.name = "88pm88x-regulator",
> >>> +		.id = PM88X_REGULATOR_ID_LDO2,
> >>> +		.of_compatible = "marvell,88pm88x-regulator",
> >>> +	},
> >>
> >> Why are we adding an of_compatible here?  It's redundant, the MFD split
> >> is a feature of Linux internals not of the hardware, and the existing
> >> 88pm8xx MFD doesn't use them.
> > 
> > in a feedback to my MFD series, Rob Herring pointed out that there is no
> > need to have a devicetree node for a subdevice if it only contains
> > "compatible" as the MFD driver can instantiate subdevices itself. I
> > understood that this is what he was referring to, but now I suspect that
> > it is sufficient for the mfd_cell.name to be set to the subdevice driver
> > name for this - is that correct?
>
> I think Rob was only referring to "no need to have a devicetree node".

yes, but I thought the presence of the compatible in the node is what
triggers instantiation of the driver and that adding it here instead was
necessary for that to happen if the node was to be removed. But like I
said, now I think only the .name property is relevant for that.

> But you added here a devicetree node, plus probably undocumented compatible.
>
> Does it even pass the checkpatch?

It does, but you were correct in your previous messages that I have not
run `make dt_binding_check` for this (or I assume that was what you
meant when you said that I did not test this) because I was not aware of
it when sending the MFD series and because this one would fail with the
same problems as Rob pointed out for that one, which is the main reason
why I only asked for feedback on the new parts. Sorry about that, next
time I will be sure to first fix all already known problems before
building on something.
>
> Best regards,
> Krzysztof

Thank you,
K. B.