[PATCH] Revert "power: pmic: rk8xx: Support sysreset shutdown method"
From: Chris Morgan macromorgan@hotmail.com
This reverts commit ad607512f5757f4485968efd5bcf2c0245a8a235.
It was found during extensive testing that this causes problems on certain boards. I was able to test this patch on a second device (an Anbernic RG353) and it resulted in similar failures.
Signed-off-by: Chris Morgan macromorgan@hotmail.com --- drivers/power/pmic/rk8xx.c | 50 +------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 25ef621f8d..8d703112c3 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,50 +6,10 @@
#include <common.h> #include <dm.h> -#include <dm/lists.h> #include <errno.h> #include <log.h> #include <power/rk8xx_pmic.h> #include <power/pmic.h> -#include <sysreset.h> - -static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type) -{ - struct rk8xx_priv *priv = dev_get_priv(dev->parent); - - if (type != SYSRESET_POWER_OFF) - return -EPROTONOSUPPORT; - - switch (priv->variant) { - case RK805_ID: - case RK808_ID: - case RK816_ID: - case RK818_ID: - pmic_clrsetbits(dev->parent, REG_DEVCTRL, 0, BIT(0)); - break; - case RK809_ID: - case RK817_ID: - pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0, - BIT(0)); - break; - default: - printf("Unknown PMIC RK%x: Cannot shutdown\n", - priv->variant); - return -EPROTONOSUPPORT; - }; - - return -EINPROGRESS; -} - -static struct sysreset_ops rk8xx_sysreset_ops = { - .request = rk8xx_sysreset_request, -}; - -U_BOOT_DRIVER(rk8xx_sysreset) = { - .name = "rk8xx_sysreset", - .id = UCLASS_SYSRESET, - .ops = &rk8xx_sysreset_ops, -};
/* In the event of a plug-in and the appropriate option has been * selected, we simply shutdown instead of continue the normal boot @@ -133,7 +93,7 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len) static int rk8xx_bind(struct udevice *dev) { ofnode regulators_node; - int children, ret; + int children;
regulators_node = dev_read_subnode(dev, "regulators"); if (!ofnode_valid(regulators_node)) { @@ -144,14 +104,6 @@ static int rk8xx_bind(struct udevice *dev)
debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
- if (CONFIG_IS_ENABLED(SYSRESET)) { - ret = device_bind_driver_to_node(dev, "rk8xx_sysreset", - "rk8xx_sysreset", - dev_ofnode(dev), NULL); - if (ret) - return ret; - } - children = pmic_bind_children(dev, regulators_node, pmic_children_info); if (!children) debug("%s: %s - no child found\n", __func__, dev->name);
Hi Chris,
On Fri, 22 Jul 2022 at 11:32, Chris Morgan macroalpha82@gmail.com wrote:
From: Chris Morgan macromorgan@hotmail.com
This reverts commit ad607512f5757f4485968efd5bcf2c0245a8a235.
It was found during extensive testing that this causes problems on certain boards. I was able to test this patch on a second
Please can you describe the problem in the commit message?
device (an Anbernic RG353) and it resulted in similar failures.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
drivers/power/pmic/rk8xx.c | 50 +------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-)
Regards, Simon
Hello,
On Sat, Jul 23, 2022 at 10:42:36AM -0600, Simon Glass wrote:
Hi Chris,
On Fri, 22 Jul 2022 at 11:32, Chris Morgan macroalpha82@gmail.com wrote:
From: Chris Morgan macromorgan@hotmail.com
This reverts commit ad607512f5757f4485968efd5bcf2c0245a8a235.
It was found during extensive testing that this causes problems on certain boards. I was able to test this patch on a second
Please can you describe the problem in the commit message?
device (an Anbernic RG353) and it resulted in similar failures.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
drivers/power/pmic/rk8xx.c | 50 +------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-)
I would like to know what's wrrong with the code, actually.
I replaced the registration with this code:
debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
if (CONFIG_IS_ENABLED(SYSRESET)) { struct udevice *udev = NULL;
printf("sysresets:"); for (uclass_first_device(UCLASS_SYSRESET, &udev); udev; uclass_next_device(&udev)) { printf(" %s", udev->name); } printf("\n");
device_bind_driver(dev, "rk8xx_sysreset", "rk8xx_sysreset", NULL);
printf("sysresets:"); for (uclass_first_device(UCLASS_SYSRESET, &udev); udev; uclass_next_device(&udev)) { printf(" %s", udev->name); } printf("\n"); }
children = pmic_bind_children(dev, regulators_node, pmic_children_info);
And added soem debug code to uclass:
int uclass_bind_device(struct udevice *dev) { struct uclass *uc; + struct udevice *ud; int ret;
uc = dev->uclass; + debug("Binding %s to %s", dev->name, uc->uc_drv->name); + list_for_each_entry(ud, &uc->dev_head, uclass_node) + debug(" %s", ud->name); + debug("\n"); list_add_tail(&dev->uclass_node, &uc->dev_head); + debug("Bound %s to %s", dev->name, uc->uc_drv->name); + list_for_each_entry(ud, &uc->dev_head, uclass_node) + debug(" %s", ud->name); + debug("\n");
With this I get this output:
Binding pmic@1b to pmic Bound pmic@1b to pmic pmic@1b sysresets: Binding rk8xx_sysreset to sysreset Bound rk8xx_sysreset to sysreset rk8xx_sysreset sysresets:Looking for pmu-clock-controller@ff750000 Looking for pmu-clock-controller@ff750000 - result for pmu-clock-controller@ff750000: (none) (ret=-19) - result for pmu-clock-controller@ff750000: (none) (ret=-19)
Binding DCDC_REG1 to regulator
So the sysreset is actually bound to the class, and then unbound again. When does this happen?
Can also other devices get removed from their class and then failing to get located by code that walks class lists?
Also how does the "Looking for pmu-clock-controller@ff750000" get printed in the middle of the debug print?
Note that the debug print is sysresets:, then emty list of sysresets, and then newline which is printed after the messages about pmu-clock.
Thanks
Michal
participants (3)
-
Chris Morgan -
Michal Suchánek -
Simon Glass