From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57778C433F5 for ; Tue, 21 Sep 2021 11:04:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3BB9461168 for ; Tue, 21 Sep 2021 11:04:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232242AbhIULGI (ORCPT ); Tue, 21 Sep 2021 07:06:08 -0400 Received: from imap3.hz.codethink.co.uk ([176.9.8.87]:49320 "EHLO imap3.hz.codethink.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232220AbhIULGH (ORCPT ); Tue, 21 Sep 2021 07:06:07 -0400 X-Greylist: delayed 2369 seconds by postgrey-1.27 at vger.kernel.org; Tue, 21 Sep 2021 07:06:07 EDT Received: from [167.98.27.226] (helo=[10.35.5.170]) by imap3.hz.codethink.co.uk with esmtpsa (Exim 4.92 #3 (Debian)) id 1mScxX-0002QA-8i; Tue, 21 Sep 2021 11:25:07 +0100 Subject: Re: [PATCH] drivers: mfd: da9063: Add restart notifier implementation To: Alexandre Ghiti , Support Opensource , Lee Jones , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org References: <20210921053356.1705833-1-alexandre.ghiti@canonical.com> From: Ben Dooks Organization: Codethink Limited. Message-ID: <28072b8e-2c32-e67d-19d3-026913c0bc7f@codethink.co.uk> Date: Tue, 21 Sep 2021 11:25:06 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20210921053356.1705833-1-alexandre.ghiti@canonical.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21/09/2021 06:33, Alexandre Ghiti wrote: > The SiFive Unmatched board uses the da9063 PMIC for reset: add a restart > notifier that will execute a small i2c sequence allowing to reset the > board. > > The original implementation comes from Marcus Comstedt and Anders Montonen > (https://forums.sifive.com/t/reboot-command/4721/28). > > Signed-off-by: Alexandre Ghiti I've got a couple of comments here, mainly is this the right place and has anyone other than you tested. I tried something similar on my Unmatched and it just turned the board off. > --- > drivers/mfd/da9063-core.c | 25 +++++++++++++++++++++++++ > include/linux/mfd/da9063/core.h | 3 +++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c > index df407c3afce3..c87b8d611f20 100644 > --- a/drivers/mfd/da9063-core.c > +++ b/drivers/mfd/da9063-core.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -158,6 +159,18 @@ static int da9063_clear_fault_log(struct da9063 *da9063) > return ret; > } > > +static int da9063_restart_notify(struct notifier_block *this, > + unsigned long mode, void *cmd) > +{ > + struct da9063 *da9063 = container_of(this, struct da9063, restart_handler); > + > + regmap_write(da9063->regmap, DA9063_REG_PAGE_CON, 0x00); > + regmap_write(da9063->regmap, DA9063_REG_CONTROL_F, 0x04); > + regmap_write(da9063->regmap, DA9063_REG_CONTROL_A, 0x68); > + > + return NOTIFY_DONE; > +} > + Firstly, do you also need to force the AUTOBOOT (bit 3, CONTROL_C) to ensure the PMIC does restart. > int da9063_device_init(struct da9063 *da9063, unsigned int irq) > { > int ret; > @@ -197,6 +210,18 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq) > } > } > > + da9063->restart_handler.notifier_call = da9063_restart_notify; > + da9063->restart_handler.priority = 128; > + ret = register_restart_handler(&da9063->restart_handler); > + if (ret) { > + dev_err(da9063->dev, "Failed to register restart handler\n"); > + return ret; > + } > + > + devm_add_action(da9063->dev, > + (void (*)(void *))unregister_restart_handler, > + &da9063->restart_handler); there's devm_register_reboot_notifier() > + > return ret; > } > > diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h > index fa7a43f02f27..1b20c498e340 100644 > --- a/include/linux/mfd/da9063/core.h > +++ b/include/linux/mfd/da9063/core.h > @@ -85,6 +85,9 @@ struct da9063 { > int chip_irq; > unsigned int irq_base; > struct regmap_irq_chip_data *regmap_irq; > + > + /* Restart */ > + struct notifier_block restart_handler; > }; > > int da9063_device_init(struct da9063 *da9063, unsigned int irq); Note, the watchdog driver for the DA9063 also has a restart method although it also does not set the AUTOBOOT bit either. The best thing is to enable the watchdog driver, the SiFive release does not have either the core DA9063 or the watchdog driver for it enabled. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html