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

[Adam Ford <aford173@gmail.com>]

On Fri, Oct 9, 2020 at 6:16 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Jacky,
>
> On Fr, 2020-10-09 at 03:00 +0000, Jacky Bai wrote:
> > > -----Original Message-----
> > > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > > Sent: Wednesday, September 30, 2020 11:50 PM
> > > To: Shawn Guo <shawnguo@kernel.org>; Rob Herring <robh+dt@kernel.org>
> > > Cc: dl-linux-imx <linux-imx@nxp.com>; Fabio Estevam
> > > <festevam@gmail.com>; Frieder Schrempf <frieder.schrempf@kontron.de>;
> > > Marek Vasut <marex@denx.de>; linux-arm-kernel@lists.infradead.org;
> > > devicetree@vger.kernel.org; kernel@pengutronix.de;
> > > patchwork-lst@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.
>
> > 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.
>
> > 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.

In an effort to keep the code size manageable, what if we were to
propose a gpc-core configured to be a generic function common to all
SoC's, and move the tables for each unique SoC into separate files.
Making each SoC's GPC a Kconfig option could give people the ability
to disable the various options that don't apply to their specific
application, and the setup and configuration of the tables should be
easier to read.  I know of at least one touchscreen driver that does
this (tsc200x).

adam
>
> 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
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel