On 24/04/2023 11:52, Lee Jones wrote:
>> include/linux/mfd/pef2256.h | 52 ++ >> 5 files changed, 1269 insertions(+) >> create mode 100644 drivers/mfd/pef2256-regs.h >> create mode 100644 drivers/mfd/pef2256.c >> create mode 100644 include/linux/mfd/pef2256.h > > [...] > >> +static int pef2256_add_audio_devices(struct pef2256 *pef2256) >> +{ >> + const char *compatible = "lantiq,pef2256-codec"; >> + struct mfd_cell *audio_devs; >> + struct device_node *np; >> + unsigned int count = 0; >> + unsigned int i; >> + int ret; >> + >> + for_each_available_child_of_node(pef2256->dev->of_node, np) { >> + if (of_device_is_compatible(np, compatible)) >> + count++; >> + } > > Converting Device Tree nodes into MFD cells to register with the > Platform Device API is not a reasonable use-case of MFD. > > Have the CODEC driver match on "lantiq,pef2256-codec" and let it > instantiate itself.
As the framer is going to used by several subsystem, I cannot instantiate it in the specific ASoC subsystem.
> > Your first version using of_platform_populate() was closer to the mark.
The issue was that I need MFD cells for the pinctrl part.
Why can't it be represented in DT?
The pinctrl part has no specific compatible string. Not sure that a compatible string for pinctrl can be accepted as there is only one pinctrl subnode and no specific reg for this subnode.
The DT looks like this: framer@2000000 { compatible = "lantiq,pef2256"; reg = <0x2000000 0x100>; ... pinctrl { pef2256_rpa_sypr: rpa-pins { pins = "RPA"; function = "SYPR"; }; };
pef2256_codec0: codec-0 { compatible = "lantiq,pef2256-codec"; #sound-dai-cells = <0>; sound-name-prefix = "PEF2256_0"; }; };Krzysztof, is it acceptable to have a compatible string in the pinctrl node ?
Why wouldn't it be?
$ git grep ".compatible" -- drivers/pinctrl/
In this case, it will looks like this: framer@2000000 { compatible = "lantiq,pef2256"; reg = <0x2000000 0x100>; ... pinctrl { compatible = "lantiq,pef2256-pinctrl";
If you do not have any resources, there is no point in having separate compatible for separate device node.
That's a new rule. Is that documented somewhere? I'm sure we already have device nodes for devices whom only operate on shared resources.
Let me clarify - no need for separate node for such case, when this is in general one device and it's sub-block does not look re-usable. For SoC blocks it is a bit different. For PMICs which pretty often re-use pieces between different devices, as well.
But here there is not much benefit of separate device node for pinctrl.
Whether rule is new? Dunno, depends, I saw it from reviews from Rob since long time, e.g.: https://lore.kernel.org/all/20220902172808.GB52527-robh@kernel.org/
Maybe this is a bit different because of children - pinconf settings? But I would still look at this as: 1. For a re-usable sub-block: separate device node and compatible is useful, 2. Non-reusable but having a child node only to group children like pin configuration nodes: no need for compatible.
Best regards, Krzysztof