On 9/20/21 5:48 PM, Alexandre Ghiti wrote:
This pmic device is present on the SiFive Unmatched board and this
new driver adds the possibility to reset it.
Signed-off-by: Alexandre Ghiti <alexandre.gh...@canonical.com>
---
configs/sifive_unmatched_defconfig | 2 ++
drivers/power/pmic/da9063.c | 49 ++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/configs/sifive_unmatched_defconfig
b/configs/sifive_unmatched_defconfig
index 978818b688..9ab058be39 100644
--- a/configs/sifive_unmatched_defconfig
+++ b/configs/sifive_unmatched_defconfig
@@ -43,3 +43,5 @@ CONFIG_DM_USB=y
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_XHCI_PCI=y
CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_DM_PMIC=y
+CONFIG_DM_PMIC_DA9063=y
diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c
index 25101d18f7..b04879d9c5 100644
--- a/drivers/power/pmic/da9063.c
+++ b/drivers/power/pmic/da9063.c
@@ -10,6 +10,7 @@
#include <dm.h>
#include <i2c.h>
#include <log.h>
+#include <dm/lists.h>
#include <power/pmic.h>
#include <power/regulator.h>
#include <power/da9063_pmic.h>
@@ -87,6 +88,7 @@ static int da9063_bind(struct udevice *dev)
{
ofnode regulators_node;
int children;
+ int ret;
regulators_node = dev_read_subnode(dev, "regulators");
if (!ofnode_valid(regulators_node)) {
@@ -101,6 +103,14 @@ static int da9063_bind(struct udevice *dev)
if (!children)
debug("%s: %s - no child found\n", __func__, dev->name);
+ if (CONFIG_IS_ENABLED(SYSRESET)) {
Thank you for addressing the missing reset driver for the SiFive
Unmatched board.
I imagine some existing or future boards using the DA9063 will have a
GPIO for system reset. Should all boards having a DA9063 PMIC implement
system reset via the power management IC?
We could instead use the devicetree to identify if a board shall use the
DA9063 for system reset.
+ ret = device_bind_driver(dev, "da9063-sysreset",
+ "da9063-sysreset", NULL);
+ if (ret)
+ debug("%s: %s - failed to bind sysreset driver\n",
+ __func__, dev->name);
+ }
+
/* Always return success for this device */
return 0;
}
@@ -129,3 +139,42 @@ U_BOOT_DRIVER(pmic_da9063) = {
.probe = da9063_probe,
.ops = &da9063_ops,
};
+
+#ifdef CONFIG_SYSRESET
The linker will remove functions that are not used automatically. We
have tended to not enclose functions in #ifdef but instead allow the
compiler to check the code.
+#include <sysreset.h>
Please, keep includes at the top of the code.
+
Even though this is a static function I would add a Sphinx style comment
here. Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation
Best regards
Heinrich
+static int da9063_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+ struct udevice *pmic_dev = dev->parent;
+ uint ret;
+
+ if (type != SYSRESET_WARM && type != SYSRESET_COLD)
+ return -EPROTONOSUPPORT;
+
+ ret = pmic_reg_write(pmic_dev, DA9063_REG_PAGE_CON, 0x00);
+ if (ret < 0)
+ return ret;
+
+ /* Sets the WAKE_UP bit */
+ ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_F, 0x04);
+ if (ret < 0)
+ return ret;
+
+ /* Powerdown! */
+ ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_A, 0x68);
+ if (ret < 0)
+ return ret;
+
+ return -EINPROGRESS;
+}
+
+static struct sysreset_ops da9063_sysreset_ops = {
+ .request = da9063_sysreset_request,
+};
+
+U_BOOT_DRIVER(da9063_sysreset) = {
+ .name = "da9063-sysreset",
+ .id = UCLASS_SYSRESET,
+ .ops = &da9063_sysreset_ops,
+};
+#endif
