> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@xxxxxxx] > Sent: Tuesday, July 2, 2019 7:07 PM > To: Jacky Bai <ping.bai@xxxxxxx> > Cc: robh+dt@xxxxxxxxxx; l.stach@xxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; > festevam@xxxxxxxxx; Leonard Crestez <leonard.crestez@xxxxxxx>; Aisheng > Dong <aisheng.dong@xxxxxxx>; Peng Fan <peng.fan@xxxxxxx>; > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; > kernel@xxxxxxxxxxxxxx; Sudeep Holla <sudeep.holla@xxxxxxx> > 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@xxxxxxx> > > --- > > 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
