| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 24 Feb 2023 16:05:58 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Esteban Blanc <eblanc@...libre.com>, linus.walleij@...aro.org,
lgirdwood@...il.com, broonie@...nel.org, a.zummo@...ertech.it,
alexandre.belloni@...tlin.com
Cc: linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-rtc@...r.kernel.org, jpanis@...libre.com,
jneanne@...libre.com
Subject: Re: [PATCH INTERNAL v1 3/3] regulator: tps6594-regulator: Add driver
for TI TPS6594 regulators
Hi Esteban,
On 2/24/23 15:31, Esteban Blanc wrote:
> From: Jerome Neanne <jneanne@...libre.com>
>
> This patch adds support for TPS6594 regulators (bucks and LDOs).
> The output voltages are configurable and are meant to supply power
> to the main processor and other components.
> Bucks can be used in single or multiphase mode, depending on PMIC
> part number.
>
> Signed-off-by: Jerome Neanne <jneanne@...libre.com>
> ---
> drivers/regulator/Kconfig | 12 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/tps6594-regulator.c | 559 ++++++++++++++++++++++++++
> 3 files changed, 572 insertions(+)
> create mode 100644 drivers/regulator/tps6594-regulator.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 820c9a0788e5..921540af6958 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1432,6 +1432,18 @@ config REGULATOR_TPS65219
> voltage regulators. It supports software based voltage control
> for different voltage domains.
>
> +config REGULATOR_TPS6594
> + tristate "TI TPS6594 Power regulators"
> + depends on MFD_TPS6594 && OF
> + help
> + This driver supports TPS6594 voltage regulator chips.
> + TPS6594 series of PMICs have 5 BUCKs and 4 LDOs
> + voltage regulators.
> + BUCKs 1,2,3,4 can be used in single phase or multiphase mode.
> + Part number defines which single or multiphase mode is i used.
> + It supports software based voltage control
> + for different voltage domains.
> +
> config REGULATOR_TPS6524X
> tristate "TI TPS6524X Power regulators"
> depends on SPI
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index b9f5eb35bf5f..948b53f6156b 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -171,6 +171,7 @@ obj-$(CONFIG_REGULATOR_TPS6524X) += tps6524x-regulator.o
> obj-$(CONFIG_REGULATOR_TPS6586X) += tps6586x-regulator.o
> obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
> obj-$(CONFIG_REGULATOR_TPS65912) += tps65912-regulator.o
> +obj-$(CONFIG_REGULATOR_TPS6594) += tps6594-regulator.o
> obj-$(CONFIG_REGULATOR_TPS65132) += tps65132-regulator.o
> obj-$(CONFIG_REGULATOR_TPS68470) += tps68470-regulator.o
> obj-$(CONFIG_REGULATOR_TWL4030) += twl-regulator.o twl6030-regulator.o
> diff --git a/drivers/regulator/tps6594-regulator.c b/drivers/regulator/tps6594-regulator.c
> new file mode 100644
> index 000000000000..c099711fd460
> --- /dev/null
> +++ b/drivers/regulator/tps6594-regulator.c
> @@ -0,0 +1,559 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Regulator driver for tps6594 PMIC
> + *
> + * Copyright (C) 2022 BayLibre Incorporated - https://www.baylibre.com/
> + *
> + * This implementation derived from tps65218 authored by "J Keerthy <j-keerthy@...com>"
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +
> +#include <linux/mfd/tps6594.h>
> +
> +#define BUCK_NB 5
> +#define LDO_NB 4
> +#define MULTI_PHASE_NB 4
> +
> +enum tps6594_regulator_id {
> + /* DCDC's */
> + TPS6594_BUCK_1,
> + TPS6594_BUCK_2,
> + TPS6594_BUCK_3,
> + TPS6594_BUCK_4,
> + TPS6594_BUCK_5,
> +
> + /* LDOs */
> + TPS6594_LDO_1,
> + TPS6594_LDO_2,
> + TPS6594_LDO_3,
> + TPS6594_LDO_4,
> +};
> +
> +enum tps6594_multi_regulator_id {
> + /* Multi-phase DCDC's */
> + TPS6594_BUCK_12,
> + TPS6594_BUCK_34,
> + TPS6594_BUCK_123,
> + TPS6594_BUCK_1234,
> +};
> +
> +struct tps6594_regulator_irq_type {
> + const char *irq_name;
> + const char *regulator_name;
> + const char *event_name;
> + unsigned long event;
> +};
> +
> +static struct tps6594_regulator_irq_type tps6594_regulator_irq_types[] = {
> + { TPS6594_IRQ_NAME_BUCK1_OV, "BUCK1", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_BUCK1_UV, "BUCK1", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
You have warning level IRQs - which is cool :)
As warning level IRQs are used for non fatal errors, you probably would
like to also implement a mechanism for consumers to know when the
"warning is over" (assuming the HW provides the status information).
Maybe regulator_get_error_flags() would serve you?
I'd be _really_ interested in hearing if you already have a use-case for
the warnings.
> + { TPS6594_IRQ_NAME_BUCK1_SC, "BUCK1", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_BUCK1_ILIM, "BUCK1", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_BUCK2_OV, "BUCK2", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_BUCK2_UV, "BUCK2", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_BUCK2_SC, "BUCK2", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_BUCK2_ILIM, "BUCK2", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_BUCK3_OV, "BUCK3", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_BUCK3_UV, "BUCK3", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_BUCK3_SC, "BUCK3", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_BUCK3_ILIM, "BUCK3", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_BUCK4_OV, "BUCK4", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_BUCK4_UV, "BUCK4", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_BUCK4_SC, "BUCK4", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_BUCK4_ILIM, "BUCK4", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_BUCK5_OV, "BUCK5", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_BUCK5_UV, "BUCK5", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_BUCK5_SC, "BUCK5", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_BUCK5_ILIM, "BUCK5", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_LDO1_OV, "LDO1", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_LDO1_UV, "LDO1", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_LDO1_SC, "LDO1", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_LDO1_ILIM, "LDO1", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_LDO2_OV, "LDO2", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_LDO2_UV, "LDO2", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_LDO2_SC, "LDO2", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_LDO2_ILIM, "LDO2", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_LDO3_OV, "LDO3", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_LDO3_UV, "LDO3", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_LDO3_SC, "LDO3", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_LDO3_ILIM, "LDO3", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> + { TPS6594_IRQ_NAME_LDO4_OV, "LDO4", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_LDO4_UV, "LDO4", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_LDO4_SC, "LDO4", "short circuit", REGULATOR_EVENT_REGULATION_OUT },
> + { TPS6594_IRQ_NAME_LDO4_ILIM, "LDO4", "reach ilim, overcurrent",
> + REGULATOR_EVENT_OVER_CURRENT },
> +};
> +
> +static struct tps6594_regulator_irq_type tps6594_ext_regulator_irq_types[] = {
> + { TPS6594_IRQ_NAME_VCCA_OV, "VCCA", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_VCCA_UV, "VCCA", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_VMON1_OV, "VMON1", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_VMON1_UV, "VMON1", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_VMON1_RV, "VMON1", "residual voltage",
> + REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_VMON2_OV, "VMON2", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { TPS6594_IRQ_NAME_VMON2_UV, "VMON2", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { TPS6594_IRQ_NAME_VMON2_RV, "VMON2", "residual voltage",
> + REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> +};
> +
> +struct tps6594_regulator_irq_data {
> + struct device *dev;
> + struct tps6594_regulator_irq_type *type;
> + struct regulator_dev *rdev;
> +};
> +
> +struct tps6594_ext_regulator_irq_data {
> + struct device *dev;
> + struct tps6594_regulator_irq_type *type;
> +};
> +
> + for (i = 0; i < ARRAY_SIZE(tps6594_regulator_irq_types); ++i) {
> + irq_type = &tps6594_regulator_irq_types[i];
> +
> + irq = platform_get_irq_byname(pdev, irq_type->irq_name);
> + if (irq < 0)
> + return -EINVAL;
> +
> + irq_data[i].dev = tps->dev;
> + irq_data[i].type = irq_type;
> +
> + tps6594_get_rdev_by_name(irq_type->regulator_name, rdevbucktbl,
> + rdevldotbl, rdev);
> +
> + if (rdev < 0) {
> + dev_err(tps->dev, "Failed to get rdev for %s\n",
> + irq_type->regulator_name);
> + return -EINVAL;
> + }
> + irq_data[i].rdev = rdev;
> +
> + error = devm_request_threaded_irq(tps->dev, irq, NULL,
> + tps6594_regulator_irq_handler,
> + IRQF_ONESHOT,
> + irq_type->irq_name,
> + &irq_data[i]);
> + if (error) {
> + dev_err(tps->dev, "failed to request %s IRQ %d: %d\n",
> + irq_type->irq_name, irq, error);
> + return error;
> + }
> + }
If I read this correctly, you have exact, 1 to 1 mapping from an IRQ to
regulator and event? Maybe you could slightly simplify the driver by
using the devm_regulator_irq_helper() and
regulator_irq_map_event_simple() with it's map-event? And if the
devm_regulator_irq_helper() does not work for you I'll be interested in
hearing if it could be improved.
> +
> + if (tps->chip_id == LP8764X)
> + ext_reg_irq_nb = ARRAY_SIZE(tps6594_ext_regulator_irq_types);
> +
> + irq_ext_reg_data = devm_kmalloc(tps->dev,
> + ext_reg_irq_nb *
> + sizeof(struct tps6594_ext_regulator_irq_data),
> + GFP_KERNEL);
> + if (!irq_ext_reg_data)
> + return -ENOMEM;
> +
> + for (i = 0; i < ext_reg_irq_nb; ++i) {
> + irq_type = &tps6594_ext_regulator_irq_types[i];
> +
> + irq = platform_get_irq_byname(pdev, irq_type->irq_name);
> + if (irq < 0)
> + return -EINVAL;
> +
> + irq_ext_reg_data[i].dev = tps->dev;
> + irq_ext_reg_data[i].type = irq_type;
> +
> + error = devm_request_threaded_irq(tps->dev, irq, NULL,
> + tps6594_regulator_irq_handler,
> + IRQF_ONESHOT,
> + irq_type->irq_name,
> + &irq_ext_reg_data[i]);
> + if (error) {
> + dev_err(tps->dev, "failed to request %s IRQ %d: %d\n",
> + irq_type->irq_name, irq, error);
> + return error;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver tps6594_regulator_driver = {
> + .driver = {
> + .name = "tps6594-regulator",
> + },
> + .probe = tps6594_regulator_probe,
> +};
> +
> +module_platform_driver(tps6594_regulator_driver);
> +
> +MODULE_ALIAS("platform:tps6594-regulator");
> +MODULE_AUTHOR("Jerome Neanne <jneanne@...libre.com>");
> +MODULE_DESCRIPTION("TPS6594 voltage regulator driver");
> +MODULE_LICENSE("GPL");
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Powered by blists - more mailing lists