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

[Karel Balej]

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.