mbox series

[RESEND,0/3] Add power domain driver support for imx8m family

Message ID 20190702100832.29718-1-ping.bai@nxp.com
Headers show
Series Add power domain driver support for imx8m family | expand

Message

Jacky Bai July 2, 2019, 10:03 a.m. UTC
I just resend this patchset again to let us rethink & find out a quick way enable
the power domain support in mainline to help other modules upstream.

The GPC module is used for system power management for CPU core & peripheral's
power domain. For the whole i.MX8M family, different SOC has different power
domain design. Some power domains need special on/off flow(need to access the
register out of the GPC module).

It makes us hard to reuse the GPCv2 driver to cover all the different power
up sequence. Each time a new SOC is added, we must modify the GPCv2 power domain
driver to make it resuable for it. We need to add a lot of code for each new chip.
We need to access the SRC & SS's GPR in GPCv2 power domain driver, it is
burden to maintain the GPCv2 power domain driver. For example, in the future
i.MX8MP, there are ~20 power domains, some of the power domain need some special
handling only for this specific chip, same situation on i.MX8MM & i.MX8MN.

THis patchset add a more generic power domain driver that give us the possibility
to use one driver to cover the whole i.MX8M family power domain in kernel side.
kernel side driver don't need to handle the power domain difference time to time.
All the power domain on/off sequence can be abstracted & handled in TF-A side.
it can simplify the power domain handling in kernel side. All the power domain
details can be hiden to TF-A side. TF-A image is SOC specific, we don't need
to care more about the one image principle.

I know some guys suggest to use SCMI to implement the power domains, but it is
a long way, need more time to investigate. especially, for the current SCMI power
domain, it can not meet all our requirement for power domain management. On i.MX8M,
some of the power domain on/off need to handle clock and external regulators, it is
not a generic handling for other SOC vendors, I think.

Additonally, the SiP Service Calls provide interfaces to SoC implementation specific
services on this platform. For example, Secure platform initialization, configuration
and some power control. I don't think it can not be used for specific SOC.

Jacky Bai (3):
  dt-bindings: power: Add power domain binding for i.mx8m family
  soc: imx: Add power domain driver support for i.mx8m family
  arm64: dts: freescale: Add power domain nodes for i.mx8mm

 .../bindings/power/fsl,imx8m-genpd.txt        |  46 ++++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 103 ++++++++
 drivers/soc/imx/Kconfig                       |   6 +
 drivers/soc/imx/Makefile                      |   1 +
 drivers/soc/imx/imx8m_pm_domains.c            | 224 ++++++++++++++++++
 include/soc/imx/imx_sip.h                     |  12 +
 6 files changed, 392 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/fsl,imx8m-genpd.txt
 create mode 100644 drivers/soc/imx/imx8m_pm_domains.c
 create mode 100644 include/soc/imx/imx_sip.h

Comments

Sudeep Holla July 2, 2019, 11:06 a.m. UTC | #1
On Tue, Jul 02, 2019 at 10:03:46AM +0000, Jacky Bai wrote:
> The i.MX8M family is a set of NXP product focus on delivering
> the latest and greatest video and audio experience combining
> state-of-the-art media-specific features with high-performance
> processing while optimized for lowest power consumption.
> 
> i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all
> belong to this family. A GPC module is used to manage all the
> PU power domain on/off. But the situation is that the number of
> power domains & the power up sequence has significate difference
> on those SoCs. Even on the same SoC. The power up sequence still
> has big difference. It makes us hard to reuse the GPCv2 driver to
> cover the whole i.MX8M family. Each time a new SoC is supported in
> the mainline kernel, we need to modify the GPCv2 driver to support
> it. We need to add or modify hundred lines of code in worst case.
> It is a bad practice for the driver maintainability.
> 
> This driver add a more generic power domain driver that the actual
> power on/off is done by TF-A code. the abstraction give us the
> possibility that using one driver to cover the whole i.MX8M family
> in kernel side.
>

TF-A has SCMI support, why can't that be used as abstraction instead
of inventing new. Peng Fan has been working to get SMC mailbox.

Unless you give me strong reasons for not able to pursue that path,
NACK for this patch. I have told this in the recent past.

> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>  drivers/soc/imx/Kconfig            |   6 +
>  drivers/soc/imx/Makefile           |   1 +
>  drivers/soc/imx/imx8m_pm_domains.c | 224 +++++++++++++++++++++++++++++
>  include/soc/imx/imx_sip.h          |  12 ++
>  4 files changed, 243 insertions(+)
>  create mode 100644 drivers/soc/imx/imx8m_pm_domains.c
>  create mode 100644 include/soc/imx/imx_sip.h

[...]

> +
> +	mutex_lock(&gpc_pd_mutex);
> +	arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_CONFIG_GPC_PM_DOMAIN, domain->domain_index,
> +		      PD_STATE_ON, 0, 0, 0, 0, &res);
> +	mutex_unlock(&gpc_pd_mutex);
> +
> +	return 0;
> +}
> +
> +static int imx8m_pd_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct imx8m_pm_domain *domain = to_imx8m_pm_domain(genpd);
> +	struct arm_smccc_res res;
> +	int index, ret = 0;
> +
> +	mutex_lock(&gpc_pd_mutex);
> +	arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_CONFIG_GPC_PM_DOMAIN, domain->domain_index,
> +		      PD_STATE_OFF, 0, 0, 0, 0, &res);

How big is this IMX SMC SIP ? I keep seeing that it's ever growing.
I don't want to see this for any future products as they seem to be
designed "ON THE GO" as and when needed rather than completely thought
through.

--
Regards,
Sudeep
Jacky Bai July 3, 2019, 1:15 a.m. UTC | #2
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: Tuesday, July 2, 2019 7:07 PM
> To: Jacky Bai <ping.bai@nxp.com>
> Cc: robh+dt@kernel.org; l.stach@pengutronix.de; shawnguo@kernel.org;
> festevam@gmail.com; Leonard Crestez <leonard.crestez@nxp.com>; Aisheng
> Dong <aisheng.dong@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> kernel@pengutronix.de; Sudeep Holla <sudeep.holla@arm.com>
> Subject: Re: [RESEND PATCH 2/3] soc: imx: Add power domain driver support
> for i.mx8m family
> 
> On Tue, Jul 02, 2019 at 10:03:46AM +0000, Jacky Bai wrote:
> > The i.MX8M family is a set of NXP product focus on delivering the
> > latest and greatest video and audio experience combining
> > state-of-the-art media-specific features with high-performance
> > processing while optimized for lowest power consumption.
> >
> > i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> > this family. A GPC module is used to manage all the PU power domain
> > on/off. But the situation is that the number of power domains & the
> > power up sequence has significate difference on those SoCs. Even on
> > the same SoC. The power up sequence still has big difference. It makes
> > us hard to reuse the GPCv2 driver to cover the whole i.MX8M family.
> > Each time a new SoC is supported in the mainline kernel, we need to
> > modify the GPCv2 driver to support it. We need to add or modify
> > hundred lines of code in worst case.
> > It is a bad practice for the driver maintainability.
> >
> > This driver add a more generic power domain driver that the actual
> > power on/off is done by TF-A code. the abstraction give us the
> > possibility that using one driver to cover the whole i.MX8M family in
> > kernel side.
> >
> 
> TF-A has SCMI support, why can't that be used as abstraction instead of
> inventing new. Peng Fan has been working to get SMC mailbox.
> 
> Unless you give me strong reasons for not able to pursue that path, NACK for
> this patch. I have told this in the recent past.
> 

For some of the power domains, we need to handle the external regulator.
In current SCMI power domain driver, there is not such support. If we use
the SCMI power domain driver, how to handle regulator on/off, in TF-A?
currently, all the regulator is managed by kernel side. most of the regulator
is controlled by I2C bus. Accessing the I2C from kernel & TF-A both is not feasible.
if regulator need to be handled in TF-A, I am not sure if it is necessary to extend the
SCMI spec to include a regulator protocol.

Another concern is that moving all currently implementation to SCMI compatible is
a huge work, waiting for SCMI implementation ready will block other peripherals
upstream for a very long time.

Anyway, if current implementation can NOT be accepted, we can try to switch SCMI
implementation.

> > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > ---
> >  drivers/soc/imx/Kconfig            |   6 +
> >  drivers/soc/imx/Makefile           |   1 +
> >  drivers/soc/imx/imx8m_pm_domains.c | 224
> +++++++++++++++++++++++++++++
> >  include/soc/imx/imx_sip.h          |  12 ++
> >  4 files changed, 243 insertions(+)
> >  create mode 100644 drivers/soc/imx/imx8m_pm_domains.c
> >  create mode 100644 include/soc/imx/imx_sip.h
> 
> [...]
> 
> > +
> > +	mutex_lock(&gpc_pd_mutex);
> > +	arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_CONFIG_GPC_PM_DOMAIN,
> domain->domain_index,
> > +		      PD_STATE_ON, 0, 0, 0, 0, &res);
> > +	mutex_unlock(&gpc_pd_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx8m_pd_power_off(struct generic_pm_domain *genpd) {
> > +	struct imx8m_pm_domain *domain = to_imx8m_pm_domain(genpd);
> > +	struct arm_smccc_res res;
> > +	int index, ret = 0;
> > +
> > +	mutex_lock(&gpc_pd_mutex);
> > +	arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_CONFIG_GPC_PM_DOMAIN,
> domain->domain_index,
> > +		      PD_STATE_OFF, 0, 0, 0, 0, &res);
> 
> How big is this IMX SMC SIP ? I keep seeing that it's ever growing.
> I don't want to see this for any future products as they seem to be designed
> "ON THE GO" as and when needed rather than completely thought through.
> 
> --
> Regards,
> Sudeep