> -----Original Message----- > From: Lothar Waßmann [mailto:l...@karo-electronics.de] > Sent: 2018年9月13日 23:59 > To: Peng Fan <peng....@nxp.com> > Cc: sba...@denx.de; Fabio Estevam <fabio.este...@nxp.com>; > u-boot@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...@nxp.com> > > Signed-off-by: Peng Fan <peng....@nxp.com> > > Cc: Stefano Babic <sba...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot