Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

[Andrew F. Davis]

On 10/24/2015 05:14 PM, Mark Brown wrote:
> On Fri, Oct 23, 2015 at 07:11:56PM -0500, Andrew F. Davis wrote:
> > On 10/23/2015 06:18 PM, Mark Brown wrote:
>
> > > mt6397 doesn't do this, it doesn't have a compatible string at all (it's
> > > doing what I'm recommending that you do).  The SPMI devices are
> > > standalone devices, their parent device is actually functioning as a bus
> > > controller here (it's really a microcontroller inside the SoC).  The
> > > Palmas is part of how we realised this was a problem.
>
> > mt6397: Documentation/devicetree/bindings/mfd/mt6397.txt
> > Doing exactly what I'm doing,
>
> Tbe binding document is buggy and doesn't reflect the code, there's no
> compatible string in the driver.

Sure there is:

drivers/mfd/mt6397-core.c:48:
.of_compatible = "mediatek,mt6397-regulator",

Then mfd_add_devices uses this to find the regulator node and fill
in .of_node, then in the regulator driver:

drivers/regulator/mt6397-regulator.c:48:
.of_match = of_match_ptr(match),

which uses your helper to match the nodes in the filled in .of_node.

> > The Palmas is a great example of why this is a good idea, there are
> > so many spins on this common base, and look how we can re-use sub-nodes:
>
> There's no real reuse here, we have to have a table in both the MFD and
> regulator listing every variant.

That's not true, the only variant that needs its own table is the tps65917,
and that's for the IRQ and regulator differences. The RTC, pwrbutton, and
GPIO nodes are completely variant agnostic and their drivers are reused
by just adding their subnode to a new PMIC device node.

> Remember that the only reason the user
> is having to type most of those subnodes at all is that we pushed things
> into the DT, if someone forgot to include one of the nodes in their
> board DT then they won't be able to use the relevant feature even if
> it's there.

This is what DT is for, we want to push this kind of thing into DT, the
driver should not have to know the devices hardware configuration anymore
that in needs to.

> > > The fact that the SoC DT is not distinct from the board DT is actually
> > > one of the problems with the way we're using DT at the minute, it means
> > > that DTBs are much less stable than they should be since we can enhance
> > > support for SoCs but DTBs need regenerating to take advantage of it.  It
> > > would be much better if the boards just referenced the SoC they use and
> > > pulled in a separate definition of the SoC (DT overlays will make it
> > > much more tractable to implement that if someone has time...).
>
> > I figured this can already be done by keeping the SoC stuff in dtsi files?
>
> That doesn't help with the above issue, include files get processed at
> the time the binary is generated.

Yeah so the board DT and the SoC DT are already mostly separate, it would
then just be a matter enforcing where nodes are defined.

> > Well I have to match the sub-devices on something, it's ether the node name
> > or the compatible string, so they might have to get used to typing :)
>
> No, that's not the case - remember, users don't have to write a new
> driver every time they instantiate a device on a board.  They're going
> to have to list the in-use regulators one way or another but if we have
> the extra compatible for regulators they have to bind both the core
> device (which is going to be required anyway due to the control bus) and
> the subnode saying that it has regulators (which we knew anyway as soon
> as we knew we had the core device).

We don't know what sub-devices the core device has, PMICs are more like
SoCs on a bus than a regular device, the sub-parts change with every spin and
we can represent this in DT like we do with SoCs. Else we would have to have
a new core binding for every spin. We know what devices are on a particular
SoC too, but we still list them and match them in DT so some SoC driver
doesn't have to.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/