RE: [PATCH v3 01/13] mfd: Add i.MX generic mix support

[Aisheng Dong]

> From: Abel Vesa <abel.vesa@xxxxxxx>
> Sent: Wednesday, April 22, 2020 5:19 PM
> On 20-04-17 09:07:47, Lee Jones wrote:
> > On Wed, 15 Apr 2020, Abel Vesa wrote:
> >
> > > Some of the i.MX SoCs have a IP for interfacing the dedicated IPs
> > > with clocks, resets and interrupts, plus some other specific control registers.
> > > To allow the functionality to be split between drivers, this MFD
> > > driver is added that has only two purposes: register the devices and
> > > map the entire register addresses. Everything else is left to the
> > > dedicated drivers that will bind to the registered devices.
> > >
> > > Signed-off-by: Abel Vesa <abel.vesa@xxxxxxx>
> > > ---
> > >  drivers/mfd/Kconfig   | 11 +++++++++++
> > >  drivers/mfd/Makefile  |  1 +
> > >  drivers/mfd/imx-mix.c | 48
> > > ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 60 insertions(+)
> > >  create mode 100644 drivers/mfd/imx-mix.c
> >
> > For completeness - Arnd's reply to this patch:
> >
> 
> I'm replying here to Arnd's reply.
> 
> I'm trying to give here a whole picture of the entire problem while the
> documentation for i.MX8MP is _not yet_ public.
> 
> Historically, each IP would have its own enclosure for all the related GPRs.
> Starting with i.MX8MP some GPRs (and some subparts) from the IP were placed
> inside these mixes.
> 
> Audiomix for example, has multiple SAIs, a PLL, and some reset bits for EARC
> and some GPRs for AudioDSP. This means that i.MX8MP has 7 SAIs, 1 EARC and
> 1 AudioDSP.
> Future platforms might have different numbers of SAIs, EARCs or AudioDSPs.
> The PLL can't be placed in one of those SAIs and it was placed in audiomix.
> The i.MX8MP has at least 4 of these mixes.
> 
> Now, the commonalities between all mixes are:
>  - have their own power domains
>  - driven by dedicated clock slice
>  - contain clocks and resets
>  - some very subsystem specific GPRs
> 
> Knowing that each mix has its own power domain, AFAICT, it needs to be
> registered as a single device. Considering that it can have clocks (audiomix has
> gates, muxes and plls), I believe that needs a clock driver, even more so since the
> muxes need their parents from the platform clock driver. Same principle applies
> to reset bits. The subsystem specific GPRs can be registered as syscon devices
> and taken care of by its counterpart IP (e.g. the AudioDSP specific regs would be
> taken care of by the DSP driver, if there is one).
> 
> Now based on all of the above, by using MFD we take care of the power domain
> control for the entire mix, plus, the MFD doesn't have any kind of functionality
> by its own, relying on its children devices that are populated based on what is in
> the mix MFD devicetree node.
> 

How about doing like this which maybe can address Arnd's concerns?
audiomix: audiomix@30e20000 {
        compatible = "fsl,imx8mp-audiomix", "syscon";
        reg = <0x30e20000 xxx>,
              <0x30e20xxx xxx>;
        reg-names = "audio", "reset", "...";
        #clock-cells = <1>;
        #reset-cells = <1>;
        power-domains = <&audiomix_pd>;
}

That means we have one combo driver registering two controllers (clk/reset), both use
the same power domain as audiomix.
And it can be easily extended to support more services provided by audiomix over syscon
if needed.
Then the 'dummy' MDF driver is not needed anymore.

Jones & Arnd,
How do you think?

Regards
Aisheng

> > --
> > Lee Jones [æçæ]
> > Linaro Services Technical Lead
> > Linaro.org â Open source software for ARM SoCs Follow Linaro: Facebook
> > | Twitter | Blog