[U-Boot] [PATCH V4 24/32] power: Add power domain driver for i.MX8

[Peng Fan]

> -----Original Message-----
> From: Lothar Waßmann [mailto:LW at KARO-electronics.de]
> Sent: 2018年9月13日 23:59
> To: Peng Fan <peng.fan at nxp.com>
> Cc: sbabic at denx.de; Fabio Estevam <fabio.estevam at nxp.com>;
> u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH V4 24/32] power: Add power domain driver for
> i.MX8
> 
> Hi,
> 
> On Wed,  5 Sep 2018 10:12:11 +0800 Peng Fan wrote:
> > Add the power domain DM driver for i.MX8, that it depends on the DTB
> > power domain trees to generate the power domain provider devices.
> > Users needs add power domain trees with property "compatible =
> "nxp,imx8-pd";"
> >
> > When power on one PD device, the driver will power on its ancestor PD
> > devices in power domain tree.
> >
> > When power off on PD device, the driver will check its child PD
> > devices first, only all child PD devices are off, then power off the
> > current PD device. Then the driver checks sibling PD devices. If
> > sibling PD devices are off, then it will power off parent PD device.
> >
> > There is no counter maintained in this driver, but a state to hold
> > current on/off state. So the request and free functions are empty.
> >
> > The power domain implementation in i.MX8 DTB set the
> "#power-domain-cells"
> > to 0, so there is no ID binding with each PD device. We don't use "id"
> > variable in struct power_domain. At same time, we have to set of_xlate
> > to empty to bypass standard of_xlate in uclass driver.
> >
> > Signed-off-by: Ye Li <ye.li at nxp.com>
> > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > Cc: Stefano Babic <sbabic at denx.de>
> > ---
> >  arch/arm/include/asm/arch-imx8/power-domain.h |  15 ++
> >  drivers/power/domain/Kconfig                  |   6 +
> >  drivers/power/domain/Makefile                 |   1 +
> >  drivers/power/domain/imx8-power-domain.c      | 312
> ++++++++++++++++++++++++++
> >  4 files changed, 334 insertions(+)
> >  create mode 100644 arch/arm/include/asm/arch-imx8/power-domain.h
> >  create mode 100644 drivers/power/domain/imx8-power-domain.c
> >
> > diff --git a/arch/arm/include/asm/arch-imx8/power-domain.h
> > b/arch/arm/include/asm/arch-imx8/power-domain.h
> > new file mode 100644
> > index 0000000000..1396008877
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-imx8/power-domain.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright 2017 NXP
> > + */
> > +
> > +#ifndef _ASM_ARCH_IMX8_POWER_DOMAIN_H #define
> > +_ASM_ARCH_IMX8_POWER_DOMAIN_H
> > +
> > +#include <asm/arch/sci/types.h>
> > +
> > +struct imx8_power_domain_platdata {
> > +	sc_rsrc_t resource_id;
> > +};
> > +
> > +#endif
> > diff --git a/drivers/power/domain/Kconfig
> > b/drivers/power/domain/Kconfig index 7cfa761498..2a72642a26 100644
> > --- a/drivers/power/domain/Kconfig
> > +++ b/drivers/power/domain/Kconfig
> > @@ -31,4 +31,10 @@ config TEGRA186_POWER_DOMAIN
> >  	  Enable support for manipulating Tegra's on-SoC power domains via IPC
> >  	  requests to the BPMP (Boot and Power Management Processor).
> >
> > +config IMX8_POWER_DOMAIN
> > +	bool "Enable i.MX8 power domain driver"
> > +        depends on ARCH_IMX8
> > +        help
> > +          Enable support for manipulating NXP i.MX8 on-SoC power
> domains via IPC
> > +          requests to the SCU.
> >  endmenu
> > diff --git a/drivers/power/domain/Makefile
> > b/drivers/power/domain/Makefile index 020eee2378..1ef4844c0b 100644
> > --- a/drivers/power/domain/Makefile
> > +++ b/drivers/power/domain/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_BCM6328_POWER_DOMAIN) +=
> > bcm6328-power-domain.o
> >  obj-$(CONFIG_SANDBOX_POWER_DOMAIN) += sandbox-power-domain.o
> >  obj-$(CONFIG_SANDBOX_POWER_DOMAIN) +=
> sandbox-power-domain-test.o
> >  obj-$(CONFIG_TEGRA186_POWER_DOMAIN) += tegra186-power-domain.o
> > +obj-$(CONFIG_IMX8_POWER_DOMAIN) += imx8-power-domain.o
> > diff --git a/drivers/power/domain/imx8-power-domain.c
> > b/drivers/power/domain/imx8-power-domain.c
> > new file mode 100644
> > index 0000000000..be91d626ad
> > --- /dev/null
> > +++ b/drivers/power/domain/imx8-power-domain.c
> > @@ -0,0 +1,312 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2017 NXP
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <power-domain-uclass.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/power-domain.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/device.h>
> > +#include <asm/arch/sci/sci.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +struct imx8_power_domain_priv {
> > +	bool state_on;
> > +};
> > +
> > +static int imx8_power_domain_request(struct power_domain
> > +*power_domain) {
> > +	debug("%s(power_domain=%p)\n", __func__, power_domain);
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx8_power_domain_free(struct power_domain *power_domain)
> > +{
> > +	debug("%s(power_domain=%p)\n", __func__, power_domain);
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx8_power_domain_on(struct power_domain *power_domain) {
> > +	struct udevice *dev = power_domain->dev;
> > +	struct imx8_power_domain_platdata *pdata;
> > +	struct imx8_power_domain_priv *ppriv;
> > +	sc_err_t ret;
> > +
> > +	struct power_domain parent_domain;
> > +	struct udevice *parent = dev_get_parent(dev);
> > +
> > +	/* Need to power on parent node first */
> > +	if (device_get_uclass_id(parent) == UCLASS_POWER_DOMAIN) {
> > +		parent_domain.dev = parent;
> > +		imx8_power_domain_on(&parent_domain);
> >
> What if this fails? Is it actually sensible to continue enabling the power domain,
> when its parent domain could not be enabled?

Actually it should not fail, if it fails, there mush be something wrong.
I'll add a return value check here.

> 
> > +
> > +	pdata = (struct imx8_power_domain_platdata *)dev_get_platdata(dev);
> > +	ppriv = (struct imx8_power_domain_priv *)dev_get_priv(dev);
> > +
> useless type casts.

Drop in next version.

Thanks,
Peng.
> 
> 
> Lothar Waßmann