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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 54A53C433EF for ; Thu, 19 May 2022 22:24:52 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 691DE83E3C; Fri, 20 May 2022 00:24:50 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=gateworks.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=gateworks-com.20210112.gappssmtp.com header.i=@gateworks-com.20210112.gappssmtp.com header.b="UuRkD7kv"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CC27F83BBF; Fri, 20 May 2022 00:24:47 +0200 (CEST) Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5A78483E3C for ; Fri, 20 May 2022 00:24:42 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=gateworks.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=tharvey@gateworks.com Received: by mail-pg1-x52c.google.com with SMTP id a38so3354537pgl.9 for ; Thu, 19 May 2022 15:24:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gateworks-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xF+Iex7IKh3MiT/5D98Aecy9SjFHr1k/XrTwNed6Km0=; b=UuRkD7kvK7NpCANw73rLx5bzbFhjvOZnjxs1RlYkxMU1Ot7iJ7oeX9J4IAw2hgFcVc 8pYRXki4sbNG1wHFuvbx02JDIdRW3EcGr7+Y1aTPAgJGRcXdCFvhX3Kgnt0SfuQe85Lr kSAqill25GSBwCWpLJmR33cpNFoQUvHLJqrQTTdpq7cgaAWaOv33QjB6FJm2QiVZ3dbT j/ypEwMpy4BELRuMzlUYiT7dx4lwD+7/WGs4bLq6QJpOE4Wi/UwSlcGOsJ8tPTQZ0OVJ uU2iqXBZXoms4p2/OBbkvY+mFYyyHJ19Y3Nsu7O9NzB52x1CrmkkxUCaJSVpVvUcRJ/z QxpQ== 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=xF+Iex7IKh3MiT/5D98Aecy9SjFHr1k/XrTwNed6Km0=; b=4ZDpvb3yigCQOWJrk6YjcvLCBVhyKq/eUr0gXzaK4xIImPPO7M1woK4zAvLGEM+Zp+ ex4oUCKUEYmvvN4wVI+06BrhXUig8a4A39kAzbZiVO2xQLxLrseTD3E959D2b3E2Fpgj mRxsMnrW4DAqckeWgMdIBdIsh2uvw5MrHo0oKsPSwQHmFGLxZPC5yfXSm4W4U9Js93/R Cl7dlFNEd2cNMWPGhTN3GJRyRVH8/GWi4bkzJP5fCyEae9mvxP6bbinCWIPnziS3Qv5h bhyEcJvrLE0w9leIrVySNQ3kikbrCyb3YNgp3Ih8FLRzwB3JQ3IKyPl50+Eov81h+Ubx 6DUA== X-Gm-Message-State: AOAM532I50WvPR6u5t+CdYsn+lxnVvUrWTbJdGlGjodzXWi5suvQKrIQ Ex/ML+s+I06TWYG2NM2FxxDE2JSCAFO9qxbgnaxgkdU5q33tUA== X-Google-Smtp-Source: ABdhPJzL+gaWKXpzILpLDPAQgjL67uBIUbvFNqHHutqWvQfj8nWSGNzQrwmmCrT0HRc8DrR1pIVJOcP+fGGhr4BxvPs= X-Received: by 2002:a63:d244:0:b0:3f2:5897:99c7 with SMTP id t4-20020a63d244000000b003f2589799c7mr5593401pgi.533.1652999080371; Thu, 19 May 2022 15:24:40 -0700 (PDT) MIME-Version: 1.0 References: <20220424214047.51975-1-marex@denx.de> <20220424214047.51975-2-marex@denx.de> In-Reply-To: <20220424214047.51975-2-marex@denx.de> From: Tim Harvey Date: Thu, 19 May 2022 15:24:29 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] pmic: pca9450: Add regulator driver To: Marek Vasut Cc: u-boot , Fabio Estevam , Peng Fan , Stefano Babic Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean On Sun, Apr 24, 2022 at 2:41 PM Marek Vasut wrote: > > Add PCA9450 regulator driver. This is complementary driver for the BUCKn > and LDOn regulators provided by the PCA9450 PMIC driver. Currently the > driver permits reading the settngs and configuring the BUCKn and LDOn > regulators. > > Reviewed-by: Fabio Estevam > Signed-off-by: Marek Vasut > Cc: Fabio Estevam > Cc: Peng Fan > Cc: Stefano Babic > --- > V2: Add RB by Fabio > --- > drivers/power/pmic/pca9450.c | 6 +- > drivers/power/regulator/Kconfig | 15 ++ > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/pca9450.c | 333 ++++++++++++++++++++++++++++++ > include/power/pca9450.h | 11 + > 5 files changed, 363 insertions(+), 3 deletions(-) > create mode 100644 drivers/power/regulator/pca9450.c > > diff --git a/drivers/power/pmic/pca9450.c b/drivers/power/pmic/pca9450.c > index 26c876c9c45..116ac49a8db 100644 > --- a/drivers/power/pmic/pca9450.c > +++ b/drivers/power/pmic/pca9450.c > @@ -83,9 +83,9 @@ static struct dm_pmic_ops pca9450_ops = { > }; > > static const struct udevice_id pca9450_ids[] = { > - { .compatible = "nxp,pca9450a", .data = 0x25, }, > - { .compatible = "nxp,pca9450b", .data = 0x25, }, > - { .compatible = "nxp,pca9450c", .data = 0x25, }, > + { .compatible = "nxp,pca9450a", .data = NXP_CHIP_TYPE_PCA9450A, }, > + { .compatible = "nxp,pca9450b", .data = NXP_CHIP_TYPE_PCA9450BC, }, > + { .compatible = "nxp,pca9450c", .data = NXP_CHIP_TYPE_PCA9450BC, }, > { } > }; > > diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig > index cd253b95f2f..d486bad6bdc 100644 > --- a/drivers/power/regulator/Kconfig > +++ b/drivers/power/regulator/Kconfig > @@ -60,6 +60,21 @@ config SPL_DM_REGULATOR_BD71837 > This config enables implementation of driver-model regulator uclass > features for regulators on ROHM BD71837 and BD71847 in SPL. > > +config DM_REGULATOR_PCA9450 > + bool "Enable Driver Model for NXP PCA9450 regulators" > + depends on DM_REGULATOR && DM_PMIC_PCA9450 > + help > + This config enables implementation of driver-model regulator uclass > + features for regulators on NXP PCA9450 PMICs. PCA9450 contains 6 bucks > + and 5 LDOS. The driver implements get/set api for value and enable. > + > +config SPL_DM_REGULATOR_PCA9450 > + bool "Enable Driver Model for NXP PCA9450 regulators in SPL" > + depends on DM_REGULATOR_PCA9450 > + help > + This config enables implementation of driver-model regulator uclass > + features for regulators on ROHM PCA9450 in SPL. > + > config DM_REGULATOR_DA9063 > bool "Enable Driver Model for REGULATOR DA9063" > depends on DM_REGULATOR && DM_PMIC_DA9063 > diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile > index 4efb32a3228..d2d17f7aed0 100644 > --- a/drivers/power/regulator/Makefile > +++ b/drivers/power/regulator/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_$(SPL_)DM_REGULATOR_DA9063) += da9063.o > obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o > obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_BD71837) += bd71837.o > +obj-$(CONFIG_$(SPL_)DM_REGULATOR_PCA9450) += pca9450.o > obj-$(CONFIG_$(SPL_)REGULATOR_PWM) += pwm_regulator.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_FAN53555) += fan53555.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_COMMON) += regulator_common.o > diff --git a/drivers/power/regulator/pca9450.c b/drivers/power/regulator/pca9450.c > new file mode 100644 > index 00000000000..4847c9f90f0 > --- /dev/null > +++ b/drivers/power/regulator/pca9450.c > @@ -0,0 +1,333 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * NXP PCA9450 regulator driver > + * Copyright (C) 2022 Marek Vasut > + * > + * Largely based on: > + * ROHM BD71837 regulator driver > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HW_STATE_CONTROL 0 > +#define DEBUG > + > +/** > + * struct pca9450_vrange - describe linear range of voltages > + * > + * @min_volt: smallest voltage in range > + * @step: how much voltage changes at each selector step > + * @min_sel: smallest selector in the range > + * @max_sel: maximum selector in the range > + */ > +struct pca9450_vrange { > + unsigned int min_volt; > + unsigned int step; > + u8 min_sel; > + u8 max_sel; > +}; > + > +/** > + * struct pca9450_plat - describe regulator control registers > + * > + * @name: name of the regulator. Used for matching the dt-entry > + * @enable_reg: register address used to enable/disable regulator > + * @enablemask: register mask used to enable/disable regulator > + * @volt_reg: register address used to configure regulator voltage > + * @volt_mask: register mask used to configure regulator voltage > + * @ranges: pointer to ranges of regulator voltages and matching register > + * values > + * @numranges: number of voltage ranges pointed by ranges > + * @dvs: whether the voltage can be changed when regulator is enabled > + */ > +struct pca9450_plat { > + const char *name; > + u8 enable_reg; > + u8 enablemask; > + u8 volt_reg; > + u8 volt_mask; > + struct pca9450_vrange *ranges; > + unsigned int numranges; > + bool dvs; > +}; > + > +#define PCA_RANGE(_min, _vstep, _sel_low, _sel_hi) \ > +{ \ > + .min_volt = (_min), .step = (_vstep), \ > + .min_sel = (_sel_low), .max_sel = (_sel_hi), \ > +} > + > +#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range, _dvs) \ > +{ \ > + .name = (_name), .enable_reg = (enreg), .enablemask = (enmask), \ > + .volt_reg = (vreg), .volt_mask = (vmask), .ranges = (_range), \ > + .numranges = ARRAY_SIZE(_range), .dvs = (_dvs), \ > +} > + > +static struct pca9450_vrange pca9450_buck123_vranges[] = { > + PCA_RANGE(600000, 12500, 0, 0x7f), > +}; > + > +static struct pca9450_vrange pca9450_buck456_vranges[] = { > + PCA_RANGE(600000, 25000, 0, 0x70), > + PCA_RANGE(3400000, 0, 0x71, 0x7f), > +}; > + > +static struct pca9450_vrange pca9450_ldo1_vranges[] = { > + PCA_RANGE(1600000, 100000, 0x0, 0x3), > + PCA_RANGE(3000000, 100000, 0x4, 0x7), > +}; > + > +static struct pca9450_vrange pca9450_ldo2_vranges[] = { > + PCA_RANGE(800000, 50000, 0x0, 0x7), > +}; > + > +static struct pca9450_vrange pca9450_ldo34_vranges[] = { > + PCA_RANGE(800000, 100000, 0x0, 0x19), > + PCA_RANGE(3300000, 0, 0x1a, 0x1f), > +}; > + > +static struct pca9450_vrange pca9450_ldo5_vranges[] = { > + PCA_RANGE(1800000, 100000, 0x0, 0xf), > +}; > + > +/* > + * We use enable mask 'HW_STATE_CONTROL' to indicate that this regulator > + * must not be enabled or disabled by SW. The typical use-case for PCA9450 > + * is powering NXP i.MX8. In this use-case we (for now) only allow control > + * for BUCK4, BUCK5, BUCK6 which are not boot critical. > + */ > +static struct pca9450_plat pca9450_reg_data[] = { > + /* Bucks 1-3 which support dynamic voltage scaling */ > + PCA_DATA("BUCK1", PCA9450_BUCK1CTRL, HW_STATE_CONTROL, > + PCA9450_BUCK1OUT_DVS0, DVS_BUCK_RUN_MASK, > + pca9450_buck123_vranges, true), > + PCA_DATA("BUCK2", PCA9450_BUCK2CTRL, HW_STATE_CONTROL, > + PCA9450_BUCK2OUT_DVS0, DVS_BUCK_RUN_MASK, > + pca9450_buck123_vranges, true), > + PCA_DATA("BUCK3", PCA9450_BUCK3CTRL, HW_STATE_CONTROL, > + PCA9450_BUCK3OUT_DVS0, DVS_BUCK_RUN_MASK, > + pca9450_buck123_vranges, true), > + /* Bucks 4-6 which do not support dynamic voltage scaling */ > + PCA_DATA("BUCK4", PCA9450_BUCK4CTRL, HW_STATE_CONTROL, > + PCA9450_BUCK4OUT, DVS_BUCK_RUN_MASK, > + pca9450_buck456_vranges, false), > + PCA_DATA("BUCK5", PCA9450_BUCK5CTRL, HW_STATE_CONTROL, > + PCA9450_BUCK5OUT, DVS_BUCK_RUN_MASK, > + pca9450_buck456_vranges, false), > + PCA_DATA("BUCK6", PCA9450_BUCK6CTRL, HW_STATE_CONTROL, > + PCA9450_BUCK6OUT, DVS_BUCK_RUN_MASK, > + pca9450_buck456_vranges, false), > + /* LDOs */ > + PCA_DATA("LDO1", PCA9450_LDO1CTRL, HW_STATE_CONTROL, > + PCA9450_LDO1CTRL, PCA9450_LDO12_MASK, > + pca9450_ldo1_vranges, false), > + PCA_DATA("LDO2", PCA9450_LDO2CTRL, HW_STATE_CONTROL, > + PCA9450_LDO2CTRL, PCA9450_LDO12_MASK, > + pca9450_ldo2_vranges, false), > + PCA_DATA("LDO3", PCA9450_LDO3CTRL, HW_STATE_CONTROL, > + PCA9450_LDO3CTRL, PCA9450_LDO34_MASK, > + pca9450_ldo34_vranges, false), > + PCA_DATA("LDO4", PCA9450_LDO4CTRL, HW_STATE_CONTROL, > + PCA9450_LDO4CTRL, PCA9450_LDO34_MASK, > + pca9450_ldo34_vranges, false), > + PCA_DATA("LDO5", PCA9450_LDO5CTRL_H, HW_STATE_CONTROL, > + PCA9450_LDO5CTRL_H, PCA9450_LDO5_MASK, > + pca9450_ldo5_vranges, false), > +}; > + > +static int vrange_find_value(struct pca9450_vrange *r, unsigned int sel, > + unsigned int *val) > +{ > + if (!val || sel < r->min_sel || sel > r->max_sel) > + return -EINVAL; > + > + *val = r->min_volt + r->step * (sel - r->min_sel); > + return 0; > +} > + > +static int vrange_find_selector(struct pca9450_vrange *r, int val, > + unsigned int *sel) > +{ > + int ret = -EINVAL; > + int num_vals = r->max_sel - r->min_sel + 1; > + > + if (val >= r->min_volt && > + val <= r->min_volt + r->step * (num_vals - 1)) { > + if (r->step) { > + *sel = r->min_sel + ((val - r->min_volt) / r->step); > + ret = 0; > + } else { > + *sel = r->min_sel; > + ret = 0; > + } > + } > + return ret; > +} > + > +static int pca9450_get_enable(struct udevice *dev) > +{ > + struct pca9450_plat *plat = dev_get_plat(dev); > + int val; > + > + /* > + * boot critical regulators on pca9450 must not be controlled by sw > + * due to the 'feature' which leaves power rails down if pca9450 is > + * reseted to snvs state. hence we can't get the state here. > + * > + * if we are alive it means we probably are on run state and > + * if the regulator can't be controlled we can assume it is > + * enabled. > + */ > + if (plat->enablemask == HW_STATE_CONTROL) > + return 1; > + > + val = pmic_reg_read(dev->parent, plat->enable_reg); > + if (val < 0) > + return val; > + > + return (val & plat->enablemask); > +} > + > +static int pca9450_set_enable(struct udevice *dev, bool enable) > +{ > + int val = 0; > + struct pca9450_plat *plat = dev_get_plat(dev); > + > + /* > + * boot critical regulators on pca9450 must not be controlled by sw > + * due to the 'feature' which leaves power rails down if pca9450 is > + * reseted to snvs state. Hence we can't set the state here. > + */ > + if (plat->enablemask == HW_STATE_CONTROL) > + return enable ? 0 : -EINVAL; > + > + if (enable) > + val = plat->enablemask; > + > + return pmic_clrsetbits(dev->parent, plat->enable_reg, plat->enablemask, > + val); > +} > + > +static int pca9450_get_value(struct udevice *dev) > +{ > + struct pca9450_plat *plat = dev_get_plat(dev); > + unsigned int reg, tmp; > + int i, ret; > + > + ret = pmic_reg_read(dev->parent, plat->volt_reg); > + if (ret < 0) > + return ret; > + > + reg = ret; > + reg &= plat->volt_mask; > + > + for (i = 0; i < plat->numranges; i++) { > + struct pca9450_vrange *r = &plat->ranges[i]; > + > + if (!vrange_find_value(r, reg, &tmp)) > + return tmp; > + } > + > + pr_err("Unknown voltage value read from pmic\n"); > + > + return -EINVAL; > +} > + > +static int pca9450_set_value(struct udevice *dev, int uvolt) > +{ > + struct pca9450_plat *plat = dev_get_plat(dev); > + unsigned int sel; > + int i, found = 0; > + > + /* > + * An under/overshooting may occur if voltage is changed for other > + * regulators but buck 1,2,3 or 4 when regulator is enabled. Prevent > + * change to protect the HW > + */ > + if (!plat->dvs) > + if (pca9450_get_enable(dev)) { > + /* If the value is already set, skip the warning. */ > + if (pca9450_get_value(dev) == uvolt) > + return 0; > + pr_err("Only DVS bucks can be changed when enabled\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < plat->numranges; i++) { > + struct pca9450_vrange *r = &plat->ranges[i]; > + > + found = !vrange_find_selector(r, uvolt, &sel); > + if (found) { > + unsigned int tmp; > + > + /* > + * We require exactly the requested value to be > + * supported - this can be changed later if needed > + */ > + found = !vrange_find_value(r, sel, &tmp); > + if (found && tmp == uvolt) > + break; > + found = 0; > + } > + } > + > + if (!found) > + return -EINVAL; > + > + return pmic_clrsetbits(dev->parent, plat->volt_reg, > + plat->volt_mask, sel); > +} > + > +static int pca9450_regulator_probe(struct udevice *dev) > +{ > + struct pca9450_plat *plat = dev_get_plat(dev); > + int i, type; > + > + type = dev_get_driver_data(dev_get_parent(dev)); > + > + if (type != NXP_CHIP_TYPE_PCA9450A && type != NXP_CHIP_TYPE_PCA9450BC) { > + debug("Unknown PMIC type\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < ARRAY_SIZE(pca9450_reg_data); i++) { > + if (strcmp(dev->name, pca9450_reg_data[i].name)) > + continue; > + > + /* PCA9450B/PCA9450C uses BUCK1 and BUCK3 in dual-phase */ > + if (type == NXP_CHIP_TYPE_PCA9450BC && > + !strcmp(pca9450_reg_data[i].name, "BUCK3")) { > + continue; > + } > + > + *plat = pca9450_reg_data[i]; > + > + return 0; > + } > + > + pr_err("Unknown regulator '%s'\n", dev->name); > + > + return -ENOENT; > +} > + > +static const struct dm_regulator_ops pca9450_regulator_ops = { > + .get_value = pca9450_get_value, > + .set_value = pca9450_set_value, > + .get_enable = pca9450_get_enable, > + .set_enable = pca9450_set_enable, > +}; > + > +U_BOOT_DRIVER(pca9450_regulator) = { > + .name = PCA9450_REGULATOR_DRIVER, > + .id = UCLASS_REGULATOR, > + .ops = &pca9450_regulator_ops, > + .probe = pca9450_regulator_probe, > + .plat_auto = sizeof(struct pca9450_plat), > +}; > diff --git a/include/power/pca9450.h b/include/power/pca9450.h > index 27703bb1f91..b714fc3477d 100644 > --- a/include/power/pca9450.h > +++ b/include/power/pca9450.h > @@ -56,4 +56,15 @@ enum { > > int power_pca9450_init(unsigned char bus, unsigned char addr); > > +enum { > + NXP_CHIP_TYPE_PCA9450A = 0, > + NXP_CHIP_TYPE_PCA9450BC, > + NXP_CHIP_TYPE_AMOUNT > +}; > + > +#define DVS_BUCK_RUN_MASK 0x7f Marek, Looks like DVS_BUCK_RUN_MASK is a bad choice of names and should be something like PCA9450_DVS_BUCK_RUN_MASK. It's defined differently in include/power/bd71837.h (where it should also likely be changed to something that doesn't collide) $ git grep DVS_BUCK_RUN_MASK include/ include/power/bd71837.h:#define DVS_BUCK_RUN_MASK 0x3f include/power/pca9450.h:#define DVS_BUCK_RUN_MASK 0x7f Granted, board/gateworks/venice/spl.c is the only file I see that includes both of those. Is this merged yet? Best Regards, Tim