diff mbox series

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

Message ID 1586937773-5836-2-git-send-email-abel.vesa@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add generic MFD i.MX mix and audiomix support | expand

Commit Message

Abel Vesa April 15, 2020, 8:02 a.m. UTC
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@nxp.com>
---
 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

Comments

Lee Jones April 17, 2020, 8:07 a.m. UTC | #1
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@nxp.com>
> ---
>  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:

  https://www.spinics.net/lists/linux-clk/msg47703.html
Abel Vesa April 22, 2020, 9:18 a.m. UTC | #2
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@nxp.com>
> > ---
> >  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:
>
>  https://www.spinics.net/lists/linux-clk/msg47703.html

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.

> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
Dong Aisheng April 23, 2020, 3:21 p.m. UTC | #3
> From: Abel Vesa <abel.vesa@nxp.com>
> 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@nxp.com>
> > > ---
> > >  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
Lee Jones April 24, 2020, 6:27 a.m. UTC | #4
On Thu, 23 Apr 2020, Aisheng Dong wrote:

> > From: Abel Vesa <abel.vesa@nxp.com>
> > 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@nxp.com>
> > > > ---
> > > >  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?

Sounds okay in principle.  Anything that prevents the existence of a
dummy (a.k.a. pointless) MFD must be seen as a positive move.
Abel Vesa April 30, 2020, 10:03 a.m. UTC | #5
On 20-04-24 07:27:27, Lee Jones wrote:
> On Thu, 23 Apr 2020, Aisheng Dong wrote:
> 
> > > From: Abel Vesa <abel.vesa@nxp.com>
> > > 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@nxp.com>
> > > > > ---
> > > > >  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?
> 
> Sounds okay in principle.  Anything that prevents the existence of a
> dummy (a.k.a. pointless) MFD must be seen as a positive move.
> 

OK, I'll do it in a single driver and single dts node.

But there might be an issue with the placement of this new driver.

drivers/clk/imx/ could be an option, but the driver will use a lot
of different APIs from different subsystems. Not the audiomix, but
the future mixes.

> -- 
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
Dong Aisheng April 30, 2020, 10:22 a.m. UTC | #6
> From: Abel Vesa <abel.vesa@nxp.com>
> Sent: Thursday, April 30, 2020 6:04 PM
> To: Lee Jones <lee.jones@linaro.org>
> On 20-04-24 07:27:27, Lee Jones wrote:
> > On Thu, 23 Apr 2020, Aisheng Dong wrote:
> >
> > > > From: Abel Vesa <abel.vesa@nxp.com>
> > > > 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@nxp.com>
> > > > > > ---
> > > > > >  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?
> >
> > Sounds okay in principle.  Anything that prevents the existence of a
> > dummy (a.k.a. pointless) MFD must be seen as a positive move.
> >
> 
> OK, I'll do it in a single driver and single dts node.
> 
> But there might be an issue with the placement of this new driver.
> 
> drivers/clk/imx/ could be an option, but the driver will use a lot of different APIs
> from different subsystems. Not the audiomix, but the future mixes.

Maybe Stephen could comment whether it's ok to push a combo driver
(e.g. clk&reset&syscon) In drivers/clk/imx.

From what we see, it seems already some similar combo drivers (clk&reset) there.

BTW, not sure if any technical block reasons to put in drivers/clk.
Maybe we can quickly try internally first.

Regards
Aisheng

> 
> > --
> > Lee Jones [李琼斯]
> > Linaro Services Technical Lead
> > Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook
> > | Twitter | Blog
Abel Vesa April 30, 2020, 11:13 a.m. UTC | #7
On 20-04-30 10:22:04, Aisheng Dong wrote:
> > From: Abel Vesa <abel.vesa@nxp.com>
> > Sent: Thursday, April 30, 2020 6:04 PM
> > To: Lee Jones <lee.jones@linaro.org>
> > On 20-04-24 07:27:27, Lee Jones wrote:
> > > On Thu, 23 Apr 2020, Aisheng Dong wrote:
> > >
> > > > > From: Abel Vesa <abel.vesa@nxp.com>
> > > > > 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@nxp.com>
> > > > > > > ---
> > > > > > >  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?
> > >
> > > Sounds okay in principle.  Anything that prevents the existence of a
> > > dummy (a.k.a. pointless) MFD must be seen as a positive move.
> > >
> > 
> > OK, I'll do it in a single driver and single dts node.
> > 
> > But there might be an issue with the placement of this new driver.
> > 
> > drivers/clk/imx/ could be an option, but the driver will use a lot of different APIs
> > from different subsystems. Not the audiomix, but the future mixes.
> 
> Maybe Stephen could comment whether it's ok to push a combo driver
> (e.g. clk&reset&syscon) In drivers/clk/imx.
> 
> From what we see, it seems already some similar combo drivers (clk&reset) there.
> 
> BTW, not sure if any technical block reasons to put in drivers/clk.
> Maybe we can quickly try internally first.
> 

Working on it already.

> Regards
> Aisheng
> 
> > 
> > > --
> > > Lee Jones [李琼斯]
> > > Linaro Services Technical Lead
> > > Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook
> > > | Twitter | Blog
diff mbox series

Patch

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0a59249..7eeb17f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -460,6 +460,17 @@  config MFD_MX25_TSADC
 	  i.MX25 processors. They consist of a conversion queue for general
 	  purpose ADC and a queue for Touchscreens.
 
+config MFD_IMX_MIX
+	tristate "NXP i.MX Generic Mix Control Driver"
+	depends on OF || COMPILE_TEST
+	help
+	  Enable generic mixes support. On some i.MX platforms, there are
+	  devices that are a mix of multiple functionalities like reset
+	  controllers, clock controllers and some others. In order to split
+	  those functionalities between the right drivers, this MFD populates
+	  with virtual devices based on what's found in the devicetree node,
+	  leaving the rest of the behavior control to the dedicated driver.
+
 config MFD_HI6421_PMIC
 	tristate "HiSilicon Hi6421 PMU/Codec IC"
 	depends on OF
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f935d10..5b2ae5d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -113,6 +113,7 @@  obj-$(CONFIG_MFD_TWL4030_AUDIO)	+= twl4030-audio.o
 obj-$(CONFIG_TWL6040_CORE)	+= twl6040.o
 
 obj-$(CONFIG_MFD_MX25_TSADC)	+= fsl-imx25-tsadc.o
+obj-$(CONFIG_MFD_IMX_MIX)	+= imx-mix.o
 
 obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
diff --git a/drivers/mfd/imx-mix.c b/drivers/mfd/imx-mix.c
new file mode 100644
index 00000000..4ea456f
--- /dev/null
+++ b/drivers/mfd/imx-mix.c
@@ -0,0 +1,48 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2019 NXP.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+
+#include <linux/mfd/core.h>
+
+static int imx_mix_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *base;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	dev_set_drvdata(dev, base);
+
+	return devm_of_platform_populate(dev);
+}
+
+static const struct of_device_id imx_mix_of_match[] = {
+	{ .compatible = "fsl,imx8mp-mix" },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, imx_mix_of_match);
+
+static struct platform_driver imx_mix_driver = {
+	.probe = imx_mix_probe,
+	.driver = {
+		.name = "imx-mix",
+		.of_match_table = of_match_ptr(imx_mix_of_match),
+	},
+};
+module_platform_driver(imx_mix_driver);