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
WARNING: multiple messages have this Message-ID (diff)
From: Abel Vesa <abel.vesa@nxp.com> To: Aisheng Dong <aisheng.dong@nxp.com> Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, Peng Fan <peng.fan@nxp.com>, Sascha Hauer <kernel@pengutronix.de>, "arnd@arndb.de" <arnd@arndb.de>, Anson Huang <anson.huang@nxp.com>, Stephen Boyd <sboyd@kernel.org>, Lee Jones <lee.jones@linaro.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, dl-linux-imx <linux-imx@nxp.com>, Philipp Zabel <p.zabel@pengutronix.de>, Leonard Crestez <leonard.crestez@nxp.com>, Shawn Guo <shawnguo@kernel.org>, "linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Jacky Bai <ping.bai@nxp.com> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-04-30 11:13 UTC|newest] Thread overview: 60+ 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 ` Abel Vesa 2020-04-15 8:02 ` [PATCH v3 01/13] mfd: Add i.MX generic mix support Abel Vesa 2020-04-15 8:02 ` Abel Vesa 2020-04-17 8:07 ` Lee Jones 2020-04-17 8:07 ` Lee Jones 2020-04-22 9:18 ` Abel Vesa 2020-04-22 9:18 ` Abel Vesa 2020-04-23 15:21 ` Aisheng Dong 2020-04-23 15:21 ` Aisheng Dong 2020-04-24 6:27 ` Lee Jones 2020-04-24 6:27 ` Lee Jones 2020-04-30 10:03 ` Abel Vesa 2020-04-30 10:03 ` Abel Vesa 2020-04-30 10:22 ` Aisheng Dong 2020-04-30 10:22 ` Aisheng Dong 2020-04-30 11:13 ` Abel Vesa [this message] 2020-04-30 11:13 ` Abel Vesa 2020-04-15 8:02 ` [PATCH v3 02/13] Documentation: mfd: Add DT bindings for i.MX Mix Abel Vesa 2020-04-15 8:02 ` Abel Vesa 2020-04-16 11:11 ` Arnd Bergmann 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 ` Abel Vesa 2020-04-15 8:02 ` [PATCH v3 04/13] arm64: dts: imx8mp: Add audiomix node Abel Vesa 2020-04-15 8:02 ` Abel Vesa 2020-04-15 8:02 ` [PATCH v3 05/13] clk: imx: gate2: Allow single bit gating clock Abel Vesa 2020-04-15 8:02 ` Abel Vesa 2020-04-22 18:35 ` Stephen Boyd 2020-04-22 18:35 ` Stephen Boyd 2020-04-29 2:00 ` Shawn Guo 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-15 8:02 ` Abel Vesa 2020-04-29 2:00 ` Shawn Guo 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-15 8:02 ` Abel Vesa 2020-04-29 2:01 ` Shawn Guo 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-15 8:02 ` Abel Vesa 2020-04-29 2:16 ` Shawn Guo 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-15 8:02 ` Abel Vesa 2020-04-22 18:38 ` Stephen Boyd 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 ` Abel Vesa 2020-04-15 8:02 ` [PATCH v3 11/13] dt-bindings: reset: imx8mp: Add ids for audiomix reset Abel Vesa 2020-04-15 8:02 ` Abel Vesa 2020-04-16 11:10 ` Arnd Bergmann 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-15 8:02 ` Abel Vesa 2020-04-23 15:47 ` Philipp Zabel 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 2020-04-15 8:02 ` 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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.