[PATCH 00/11] i.MX8MM power domain support

[Peng Fan]

> Subject: Re: [PATCH 00/11] i.MX8MM power domain support
> 
> On Sa, 2020-10-10 at 02:16 +0000, Jacky Bai wrote:
> > > -----Original Message-----
> > > From: Lucas Stach [mailto:l.stach at pengutronix.de]
> > > Sent: Friday, October 9, 2020 7:12 PM
> > > To: Jacky Bai <ping.bai at nxp.com>; Shawn Guo <shawnguo at kernel.org>;
> > > Rob Herring <robh+dt at kernel.org>
> > > Cc: dl-linux-imx <linux-imx at nxp.com>; Fabio Estevam
> > > <festevam at gmail.com>; Frieder Schrempf
> > > <frieder.schrempf at kontron.de>; Marek Vasut <marex at denx.de>;
> > > linux-arm-kernel at lists.infradead.org;
> > > devicetree at vger.kernel.org; kernel at pengutronix.de;
> > > patchwork-lst at pengutronix.de
> > > Subject: Re: [PATCH 00/11] i.MX8MM power domain support
> > >
> > > Hi Jacky,
> > >
> > > On Fr, 2020-10-09 at 03:00 +0000, Jacky Bai wrote:
> > > > > -----Original Message-----
> > > > > From: Lucas Stach [mailto:l.stach at pengutronix.de]
> > > > > Sent: Wednesday, September 30, 2020 11:50 PM
> > > > > To: Shawn Guo <shawnguo at kernel.org>; Rob Herring
> > > > > <robh+dt at kernel.org>
> > > > > Cc: dl-linux-imx <linux-imx at nxp.com>; Fabio Estevam
> > > > > <festevam at gmail.com>; Frieder Schrempf
> > > > > <frieder.schrempf at kontron.de>; Marek Vasut <marex at denx.de>;
> > > > > linux-arm-kernel at lists.infradead.org;
> > > > > devicetree at vger.kernel.org; kernel at pengutronix.de;
> > > > > patchwork-lst at pengutronix.de
> > > > > Subject: [PATCH 00/11] i.MX8MM power domain support
> > > > >
> > > > > Hi all,
> > > > >
> > > > > this adds power domain support for the i.MX8MM to the existing
> > > > > GPCv2
> > > driver.
> > > > > It is not complete yet, as it is still missing the VPU and
> > > > > display power domains, as those require support for the BLK_CTL
> > > > > regions of the VPUMIX and DISPLAYMIX domains. A Linux driver for
> > > > > those regions on the i.MX8MP is currently under development and
> > > > > we plan to use this as a template for the i.MX8MM when the dust
> > > > > has settled. The changes in this series have been made with this
> > > > > in mind, so once the BLK_CTL driver exists it should be a matter
> > > > > of hooking things together via DT, with no further changes
> > > > > required on the GPCv2 driver side
> > > (famous last words).
> > > > > Special thanks to Marek Vasut who helped with testing and
> > > > > debugging of early versions of this code.
> > > > >
> > > >
> > > > Lucas,
> > > >
> > > > thanks for working on this, but I think current support for 8MM
> > > > can NOT 100%
> > > work due to HW limitation.
> > > > Maybe, we need further discussion before moving forward,
> > > > otherwise, we will meet awkward situation when NXP doing LTS
> > > > upgrade. Below are some
> > > info shared.
> > > > 1. The GPU & VPU related power domains need to do special handling
> > > > due
> > > to HW limitation, can refer to the power domain sequence
> > > >   In NXP release.
> > >
> > > For the GPU this driver already does the same thing as the TF-A
> > > based implementation by driving the GPU2D and GPU3D domains
> together
> > > and triggering the SRC reset.
> > >
> > > For the VPU I expect that we can do all the necessary syncing with a
> > > proper VPU BLK_CTL driver.
> > >
> >
> > Ok, thanks. I saw the reset handling in this patchset.
> >
> > > > 2. another reason that we do power domain control in TF-A in NXP
> > > > release is
> > > that MAIN NOC power domain can only be controlled by
> > > >   TF-A, and before MAIN NOC power domain, we need to check other
> > > MIXs' power status. If other power domain is controlled by linux
> > > side,
> > > >   It is not easy to cross world status sync.
> > >
> > > This is a valid concern and I want to learn more about this. When do
> > > you turn off MAIN NOC power in the TF-A? Is it just system suspend?
> > > If so I think it's a valid requirement for the kernel driver to shut
> > > down all the peripheral power domains before entering system suspend.
> > >
> >
> > The main NOC will be off just in system suspend case. Main NoC on/off
> > is controlled by GPC HW slot method. As all the MIXs with ADB400
> > bridge are connected on the main NoC, we must make sure that all these
> > ADB400 port in power down status when main NoC power off in system
> > suspend, otherwise system will hang when resume. Previously, all the
> > MIX power domain is controlled by TF-A, then TF-A knows the status of each
> MIX, if any MIX is on, we will skip the NOC power down setting.
> 
> And that just means the SoC doesn't enter the desired low-power state when
> going into suspend, right? I think it would be much easier and less error prone
> to just mandate that all peripheral power domains must be powered down
> before trying to enter system suspend.
> 
> Is there any use-case where you would legitimately want to enter a high
> power suspend state with some of ther peripheral power domains still active?
> 
> > > > 3. either 8MM, 8MN, or 8MP, the power domain design is different,
> > > > I am not
> > > sure if it is the good to add hundreds line of code in GPCv2 each
> > > time
> > > >   a new SOC is added.
> > >
> > > I don't buy into this argument. We have lots of drivers in the Linux
> > > kernel that require some changes for new SoC generations, that's
> > > what Linux drivers are for. The complexity of the hardware doesn't
> > > disappear just because you push some of the driver bits into TF-A,
> > > you just handle the complexity at a different palce and IMHO that
> > > the wrong place. The power domains have complex interactions with
> > > other drivers in the Linux system, so debugging and deplyong fixes
> > > is much easier when the power domain handling is fully done by a kernel
> driver.
> >
> > Actually, due to the security requirement from other system solution
> > provider, for example, Microsoft Azure Sphere, it has strict
> > requirement for power domain to be controlled by secure subsystem(either
> TF-A, TEE or dedicated secure domain controller).
> > Same requirement for reset control, and system critical clock control.
> 
> Yes, I'm aware of those requirements, but to satisfy those you need a full
> implementation of all those parts in the secure subsystem. Doing it just for the
> power domains adds complexity for no gain, as you still won't be able to meet
> all the requirements and frankly I don't think this is a realistic goal to achieve
> with the current i.MX8M family of SoCs.

At least we are moving to that direction.

> 
> Meeting those requirements needs a fully system approach where the secure
> subsystem parts are made sufficiently independent from the non- secure
> parts on a hardware level, which I don't see on the i.MX8M SoC and hardware
> design guide.

CSU could restrict the access permission.

> 
> > For NXP i.MX8M family, it is ok to implement in linux kernel, just a
> > tradeoff to find out a place to hide the complexity ^_^.
> >
> > BTW, for virtualization support, it is better to put the power domain
> > in a central place to simplify the VM implementation.
> 
> Same as above. If you can make all the relevant bits (clock, reset,
> power-domain, regulator) available via a virtualization friendly API, then I
> would see a point in adding complexity for this abstraction. As long as this
> added abstraction only solves a very tiny bit of the overall picture, I just don't
> see the point in the added complexity and (from a Linux PoV) obfuscation.

Could we use SCMI for power domain, system critical clocks, smc watchdog
and etc?

Or we support two approaches, one is let Linux control everything, the other
is using SCMI.

Thoughts?

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
> > BR
> > Jacky Bai
> >
> > > Regards,
> > > Lucas
> > >
> > > > BR
> > > > Jacky Bai
> > > >
> > > > > Regards,
> > > > > Lucas
> > > > >
> > > > > Lucas Stach (11):
> > > > >   soc: imx: gpcv2: move to more ideomatic error handling in probe
> > > > >   soc: imx: gpcv2: move domain mapping to domain driver probe
> > > > >   soc: imx: gpcv2: split power up and power down sequence control
> > > > >   soc: imx: gpcv2: wait for ADB400 handshake
> > > > >   soc: imx: gpcv2: add runtime PM support for power-domains
> > > > >   soc: imx: gpcv2: allow domains without power-sequence control
> > > > >   soc: imx: gpcv2: add support for optional resets
> > > > >   dt-bindings: add defines for i.MX8MM power domains
> > > > >   soc: imx: gpcv2: add support for i.MX8MM power domains
> > > > >   arm64: dts: imx8mm: add GPC node and power domains
> > > > >   arm64: dts: imx8mm: put USB controllers into power-domains
> > > > >
> > > > >  .../bindings/power/fsl,imx-gpcv2.yaml         |   8 +
> > > > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  59 +++
> > > > >  drivers/soc/imx/gpcv2.c                       | 501
> > > > > +++++++++++++++---
> > > > >  include/dt-bindings/power/imx8mm-power.h      |  22 +
> > > > >  4 files changed, 516 insertions(+), 74 deletions(-)  create
> > > > > mode
> > > > > 100644 include/dt-bindings/power/imx8mm-power.h
> > > > >
> > > > > --
> > > > > 2.20.1