| Message ID | 20190917094556.29530-2-peng.fan@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot,V2,1/2] power: domain: add dev_power_domain_on | expand |
On 17/09/19 2:59 PM, Peng Fan wrote: > When multiple power domains attached to a device, need power on > them all, so use dev_power_domain_on to do that. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V2: > use dev_power_domain_on in patch 1/2 > > drivers/core/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c > index 05dadf98f9..f4d7140698 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -307,7 +307,6 @@ static void *alloc_priv(int size, uint flags) > > int device_probe(struct udevice *dev) > { > - struct power_domain pd; > const struct driver *drv; > int size = 0; > int ret; > @@ -390,8 +389,9 @@ int device_probe(struct udevice *dev) > > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) { I guess we can drop the if condition all together and can directly call dev_power_domain_on(). Rest looks good to me. Thanks and regards, Lokesh > - if (!power_domain_get(dev, &pd)) > - power_domain_on(&pd); > + ret = dev_power_domain_on(dev); > + if (ret) > + goto fail; > } > > ret = uclass_pre_probe_device(dev); >
On 17/09/19 2:59 PM, Peng Fan wrote: > When multiple power domains attached to a device, need power on > them all, so use dev_power_domain_on to do that. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V2: > use dev_power_domain_on in patch 1/2 > > drivers/core/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/core/device.c b/drivers/core/device.c > index 05dadf98f9..f4d7140698 100644 > --- a/drivers/core/device.c > +++ b/drivers/core/device.c > @@ -307,7 +307,6 @@ static void *alloc_priv(int size, uint flags) > > int device_probe(struct udevice *dev) > { > - struct power_domain pd; > const struct driver *drv; > int size = 0; > int ret; > @@ -390,8 +389,9 @@ int device_probe(struct udevice *dev) > > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) { > - if (!power_domain_get(dev, &pd)) > - power_domain_on(&pd); > + ret = dev_power_domain_on(dev); > + if (ret) > + goto fail; Also can you not return here in case of failure? It might be the case that power_domain driver might not be available yet. Thanks and regards, Lokesh > } > > ret = uclass_pre_probe_device(dev); >
Hi Lokesh, > Subject: Re: [PATCH V2 2/2] core: device: use dev_power_domain_on > > > > On 17/09/19 2:59 PM, Peng Fan wrote: > > When multiple power domains attached to a device, need power on them > > all, so use dev_power_domain_on to do that. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V2: > > use dev_power_domain_on in patch 1/2 > > > > drivers/core/device.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/core/device.c b/drivers/core/device.c index > > 05dadf98f9..f4d7140698 100644 > > --- a/drivers/core/device.c > > +++ b/drivers/core/device.c > > @@ -307,7 +307,6 @@ static void *alloc_priv(int size, uint flags) > > > > int device_probe(struct udevice *dev) { > > - struct power_domain pd; > > const struct driver *drv; > > int size = 0; > > int ret; > > @@ -390,8 +389,9 @@ int device_probe(struct udevice *dev) > > > > if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > > device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) { > > - if (!power_domain_get(dev, &pd)) > > - power_domain_on(&pd); > > + ret = dev_power_domain_on(dev); > > + if (ret) > > + goto fail; > > Also can you not return here in case of failure? It might be the case that > power_domain driver might not be available yet Only when POWER_DOMAIN is enabled, dev_power_domain_on will be called. I not follow you not available yet. Thanks, Peng. > > Thanks and regards, > Lokesh > > > } > > > > ret = uclass_pre_probe_device(dev); > >
Hi Peng, On 25/09/19 6:56 AM, Peng Fan wrote: > Hi Lokesh, > >> Subject: Re: [PATCH V2 2/2] core: device: use dev_power_domain_on >> >> >> >> On 17/09/19 2:59 PM, Peng Fan wrote: >>> When multiple power domains attached to a device, need power on them >>> all, so use dev_power_domain_on to do that. >>> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com> >>> --- >>> >>> V2: >>> use dev_power_domain_on in patch 1/2 >>> >>> drivers/core/device.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/core/device.c b/drivers/core/device.c index >>> 05dadf98f9..f4d7140698 100644 >>> --- a/drivers/core/device.c >>> +++ b/drivers/core/device.c >>> @@ -307,7 +307,6 @@ static void *alloc_priv(int size, uint flags) >>> >>> int device_probe(struct udevice *dev) { >>> - struct power_domain pd; >>> const struct driver *drv; >>> int size = 0; >>> int ret; >>> @@ -390,8 +389,9 @@ int device_probe(struct udevice *dev) >>> >>> if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && >>> device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) { >>> - if (!power_domain_get(dev, &pd)) >>> - power_domain_on(&pd); >>> + ret = dev_power_domain_on(dev); >>> + if (ret) >>> + goto fail; >> >> Also can you not return here in case of failure? It might be the case that >> power_domain driver might not be available yet > > Only when POWER_DOMAIN is enabled, dev_power_domain_on will be called. > I not follow you not available yet. Never mind, it is an issue with enabling power domain on my end. Ill take back my comment, Sorry for the noise. Thanks and regards, Lokesh > > Thanks, > Peng. > >> >> Thanks and regards, >> Lokesh >> >>> } >>> >>> ret = uclass_pre_probe_device(dev); >>>
On Tue, 24 Sep 2019 at 22:25, Lokesh Vutla <lokeshvutla@ti.com> wrote: > > Hi Peng, > > On 25/09/19 6:56 AM, Peng Fan wrote: > > Hi Lokesh, > > > >> Subject: Re: [PATCH V2 2/2] core: device: use dev_power_domain_on > >> > >> > >> > >> On 17/09/19 2:59 PM, Peng Fan wrote: > >>> When multiple power domains attached to a device, need power on them > >>> all, so use dev_power_domain_on to do that. > >>> > >>> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >>> --- > >>> > >>> V2: > >>> use dev_power_domain_on in patch 1/2 > >>> > >>> drivers/core/device.c | 6 +++--- > >>> 1 file changed, 3 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/core/device.c b/drivers/core/device.c index > >>> 05dadf98f9..f4d7140698 100644 > >>> --- a/drivers/core/device.c > >>> +++ b/drivers/core/device.c > >>> @@ -307,7 +307,6 @@ static void *alloc_priv(int size, uint flags) > >>> > >>> int device_probe(struct udevice *dev) { > >>> - struct power_domain pd; > >>> const struct driver *drv; > >>> int size = 0; > >>> int ret; > >>> @@ -390,8 +389,9 @@ int device_probe(struct udevice *dev) > >>> > >>> if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && > >>> device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) { > >>> - if (!power_domain_get(dev, &pd)) > >>> - power_domain_on(&pd); > >>> + ret = dev_power_domain_on(dev); > >>> + if (ret) > >>> + goto fail; > >> > >> Also can you not return here in case of failure? It might be the case that > >> power_domain driver might not be available yet > > > > Only when POWER_DOMAIN is enabled, dev_power_domain_on will be called. > > I not follow you not available yet. > > Never mind, it is an issue with enabling power domain on my end. Ill take back > my comment, Sorry for the noise. Reviewed-by: Simon Glass <sjg@chromium.org>
diff --git a/drivers/core/device.c b/drivers/core/device.c index 05dadf98f9..f4d7140698 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -307,7 +307,6 @@ static void *alloc_priv(int size, uint flags) int device_probe(struct udevice *dev) { - struct power_domain pd; const struct driver *drv; int size = 0; int ret; @@ -390,8 +389,9 @@ int device_probe(struct udevice *dev) if (CONFIG_IS_ENABLED(POWER_DOMAIN) && dev->parent && device_get_uclass_id(dev) != UCLASS_POWER_DOMAIN) { - if (!power_domain_get(dev, &pd)) - power_domain_on(&pd); + ret = dev_power_domain_on(dev); + if (ret) + goto fail; } ret = uclass_pre_probe_device(dev);
When multiple power domains attached to a device, need power on them all, so use dev_power_domain_on to do that. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- V2: use dev_power_domain_on in patch 1/2 drivers/core/device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)