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.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham 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 39116C433F5 for ; Wed, 22 Sep 2021 07:14:25 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 218C161156 for ; Wed, 22 Sep 2021 07:14:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 218C161156 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=canonical.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id CDA908329C; Wed, 22 Sep 2021 09:14:20 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.b="B/pg6ifS"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 1BE3182EE0; Wed, 22 Sep 2021 09:14:18 +0200 (CEST) Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 32A9082EE0 for ; Wed, 22 Sep 2021 09:14:14 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=alexandre.ghiti@canonical.com Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 6C817402D8 for ; Wed, 22 Sep 2021 07:14:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1632294850; bh=8scdzMO6K65Jd+j/AJuvdYM+xBemCoWFdUD5UYWYG3o=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=B/pg6ifSOMhIpxQAwZnwbvXpGh7tH2pwWqKj/7Ig/oULtBSZkTJniTOxi3wCaHgT/ yZ2kv+uqtCi2TXGZ8bLFgZRX156UrUvy2EtRjILy3biVKZQhYMEm58y8WbKDZ4p027 mGqK0Hg+oWuuZ46tCoGBAj3tw4isG4KCb9WnJDsRlthW5iZdPTsOe4bjeirySV81X+ c0EjoDmwpaivKfK03kvwaUc9EcdB38hefk3WLxHV/LMzwZ3s0By1LogLNQNE3hhcog 9zEcBuwt+6h/n4gOIAODFMhZrOWvqebDc2uyXGia0ALaUe4k6Lcl7YoM5/UYwUE7vA nZV9J3hT7LXTg== Received: by mail-ed1-f72.google.com with SMTP id d1-20020a50f681000000b003d860fcf4ffso1874544edn.22 for ; Wed, 22 Sep 2021 00:14:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8scdzMO6K65Jd+j/AJuvdYM+xBemCoWFdUD5UYWYG3o=; b=z9T1Y8IncMLqhdlplRoSJ2s0J8UWK5qefiQ3jU0VnYEEYZVpWeNgUsTUD93BrW8rDI hesqoqFA0xdD2IIg4lqMoF3Tno7Nxttj5NlX0RFDjKnGWyhMgoNnme4v9/uVV5atYYcs vL3YQqjSFUeNGcmH966PpmyZixsK+jhuDTFruuAV3Hb5+fBCjgLAjLluly7KP8jrqPcn 4uG00Ro9SZHXfIXE33sBzsUky33Utw9xjQs0wswiHCS2x4pENW9u5OTM45lH8ZDFK+Yy e1tJ9/zbxXNYQ1dbel4zSwdvMcS98jlUAdUFmiXlH0v+qsTaI7kBOKwFU28ni9jXE3LE praQ== X-Gm-Message-State: AOAM532yrXdydiWo2dFurqsguD11oc1LhuWMlzu0w8G6gjW8usZ1a/89 X12coV9+ZMvc8w7H3VOngn+y3X1sBOXc0gtEpDQh2MrftBMS3FM+XBq0nT3ksVhDO7ax/0obyl5 fG2LKQsCRPwhUmVUUELNtF0GsXG51ViCzQaB1cUBRmDyKCDs= X-Received: by 2002:a05:6402:1ac5:: with SMTP id ba5mr40810139edb.20.1632294848843; Wed, 22 Sep 2021 00:14:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJye5vPK9wKQPxjBcWQMdbkkT/FKMOYROwPQyfJwjqkxDnO3OZZwsGWzlgEpn1PkhLC+gyDYfZE3PBUn+a/wCAM= X-Received: by 2002:a05:6402:1ac5:: with SMTP id ba5mr40810121edb.20.1632294848632; Wed, 22 Sep 2021 00:14:08 -0700 (PDT) MIME-Version: 1.0 References: <20210920154809.1695453-1-alexandre.ghiti@canonical.com> <396c1517-b871-5768-40f8-8e312879041f@canonical.com> In-Reply-To: <396c1517-b871-5768-40f8-8e312879041f@canonical.com> From: Alexandre Ghiti Date: Wed, 22 Sep 2021 09:13:56 +0200 Message-ID: Subject: Re: [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device To: Heinrich Schuchardt Cc: Paul Walmsley , Pragnesh Patel , Green Wan , Jaehoon Chung , u-boot@lists.denx.de Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Tue, Sep 21, 2021 at 9:23 AM Heinrich Schuchardt wrote: > > > > 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 > > --- > > 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 > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -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. Indeed, with this patch, any board with this chip may use this reset handler, I'm not sure in which order the sysreset drivers are called though. We could add a new device like the rtc/watchdog ones, I'll give it a try, that sounds way cleaner. > > > + 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. > Ok thanks! > > +#include > > 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 > > Alex