From: Abel Vesa <abel.vesa@nxp.com>
To: Aisheng Dong <aisheng.dong@nxp.com>
Cc: Lee Jones <lee.jones@linaro.org>, "arnd@arndb.de" <arnd@arndb.de>,
Shawn Guo <shawnguo@kernel.org>, Peng Fan <peng.fan@nxp.com>,
Philipp Zabel <p.zabel@pengutronix.de>,
Stephen Boyd <sboyd@kernel.org>,
Sascha Hauer <kernel@pengutronix.de>,
Anson Huang <anson.huang@nxp.com>,
Leonard Crestez <leonard.crestez@nxp.com>,
Jacky Bai <ping.bai@nxp.com>, dl-linux-imx <linux-imx@nxp.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 01/13] mfd: Add i.MX generic mix support
Date: Thu, 30 Apr 2020 14:13:42 +0300 [thread overview]
Message-ID: <20200430111342.b2fj7ko3uvffv4wg@fsr-ub1664-175> (raw)
In-Reply-To: <AM6PR04MB496612839A11EA05D60DF6AE80AA0@AM6PR04MB4966.eurprd04.prod.outlook.com>
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
next prev parent reply other threads:[~2020-04-30 11:13 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-15 8:02 [PATCH v3 00/13] Add generic MFD i.MX mix and audiomix support Abel Vesa
2020-04-15 8:02 ` [PATCH v3 01/13] mfd: Add i.MX generic mix support Abel Vesa
2020-04-17 8:07 ` Lee Jones
2020-04-22 9:18 ` Abel Vesa
2020-04-23 15:21 ` Aisheng Dong
2020-04-24 6:27 ` Lee Jones
2020-04-30 10:03 ` Abel Vesa
2020-04-30 10:22 ` Aisheng Dong
2020-04-30 11:13 ` Abel Vesa [this message]
2020-04-15 8:02 ` [PATCH v3 02/13] Documentation: mfd: Add DT bindings for i.MX Mix Abel Vesa
2020-04-16 11:11 ` Arnd Bergmann
2020-04-15 8:02 ` [PATCH v3 03/13] arm64: dts: imx8mp: Add AIPS 4 and 5 Abel Vesa
2020-04-15 8:02 ` [PATCH v3 04/13] arm64: dts: imx8mp: Add audiomix node Abel Vesa
2020-04-15 8:02 ` [PATCH v3 05/13] clk: imx: gate2: Allow single bit gating clock Abel Vesa
2020-04-22 18:35 ` Stephen Boyd
2020-04-29 2:00 ` Shawn Guo
2020-04-15 8:02 ` [PATCH v3 06/13] clk: imx: pll14xx: Add the device as argument when registering Abel Vesa
2020-04-29 2:00 ` Shawn Guo
2020-04-15 8:02 ` [PATCH v3 07/13] clk: imx: Add helpers for passing the device as argument Abel Vesa
2020-04-29 2:01 ` Shawn Guo
2020-04-15 8:02 ` [PATCH v3 08/13] dt-bindings: clocks: imx8mp: Add ids for audiomix clocks Abel Vesa
2020-04-29 2:16 ` Shawn Guo
2020-04-15 8:02 ` [PATCH v3 09/13] clk: imx: Add audiomix clock controller support Abel Vesa
2020-04-22 18:38 ` Stephen Boyd
2020-04-15 8:02 ` [PATCH v3 10/13] arm64: dts: imx8mp: Add audiomix clock controller node Abel Vesa
2020-04-15 8:02 ` [PATCH v3 11/13] dt-bindings: reset: imx8mp: Add ids for audiomix reset Abel Vesa
2020-04-16 11:10 ` Arnd Bergmann
2020-04-15 8:02 ` [PATCH v3 12/13] reset: imx: Add audiomix reset controller support Abel Vesa
2020-04-23 15:47 ` Philipp Zabel
2020-04-15 8:02 ` [PATCH v3 13/13] arm64: dts: imx8mp: Add audiomix reset controller node Abel Vesa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200430111342.b2fj7ko3uvffv4wg@fsr-ub1664-175 \
--to=abel.vesa@nxp.com \
--cc=aisheng.dong@nxp.com \
--cc=anson.huang@nxp.com \
--cc=arnd@arndb.de \
--cc=devicetree@vger.kernel.org \
--cc=kernel@pengutronix.de \
--cc=lee.jones@linaro.org \
--cc=leonard.crestez@nxp.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=peng.fan@nxp.com \
--cc=ping.bai@nxp.com \
--cc=sboyd@kernel.org \
--cc=shawnguo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).