| 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
| ||
|
Message-ID: <Yg5PALyUv6qHPz//@kroah.com>
Date: Thu, 17 Feb 2022 14:34:56 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zev Weiss <zev@...ilderbeest.net>
Cc: Arnd Bergmann <arnd@...db.de>, openbmc@...ts.ozlabs.org,
linux-kernel@...r.kernel.org, Guenter Roeck <linux@...ck-us.net>,
Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>
Subject: Re: [PATCH 4/4] misc: Add power-efuse driver
On Thu, Feb 17, 2022 at 02:44:44AM -0800, Zev Weiss wrote:
> This driver provides a sysfs interface to access the on/off state and
> error flags of a regulator supplying a power output controlled by the
> system.
>
> Signed-off-by: Zev Weiss <zev@...ilderbeest.net>
> ---
> MAINTAINERS | 5 +
> drivers/misc/Kconfig | 15 +++
> drivers/misc/Makefile | 1 +
> drivers/misc/power-efuse.c | 221 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 242 insertions(+)
> create mode 100644 drivers/misc/power-efuse.c
You add sysfs files, yet have no Documentation/ABI/ entry updates
documenting what those sysfs files do? Please fix.
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fca970a46e77..d1153a0389d2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7064,6 +7064,11 @@ S: Orphan
> W: http://aeschi.ch.eu.org/efs/
> F: fs/efs/
>
> +POWER EFUSE DRIVER
> +M: Zev Weiss <zev@...ilderbeest.net>
> +S: Maintained
> +F: drivers/misc/power-efuse.c
> +
> EHEA (IBM pSeries eHEA 10Gb ethernet adapter) DRIVER
> M: Douglas Miller <dougmill@...ux.ibm.com>
> L: netdev@...r.kernel.org
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 0f5a49fc7c9e..45fc3e8ad35d 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -470,6 +470,21 @@ config HISI_HIKEY_USB
> switching between the dual-role USB-C port and the USB-A host ports
> using only one USB controller.
>
> +config POWER_EFUSE
> + tristate "Power efuse driver support"
> + depends on OF && REGULATOR
> + help
> + This driver supports a regulator device functioning as a
> + power efuse, with status bits and an on/off switch available
> + via sysfs.
> +
> + A typical use for this would be for an efuse controlling a
> + generic power output for supplying power to devices external
> + to the system running this driver (such as in the management
> + controller of a "smart" PDU or similar), allowing the
> + operator to manually turn the output on and off, check if
> + the efuse has tripped due to overload, etc.
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index a086197af544..7bd784b89ef8 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -59,3 +59,4 @@ obj-$(CONFIG_UACCE) += uacce/
> obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
> +obj-$(CONFIG_POWER_EFUSE) += power-efuse.o
> diff --git a/drivers/misc/power-efuse.c b/drivers/misc/power-efuse.c
> new file mode 100644
> index 000000000000..e974dde57615
> --- /dev/null
> +++ b/drivers/misc/power-efuse.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This module provides a thin wrapper around a regulator device that exposes
> + * status bits and on/off state via sysfs.
> + *
> + * Copyright (C) 2022 Zev Weiss <zev@...ilderbeest.net>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +struct efuse {
> + struct regulator *reg;
> + struct {
> + unsigned int cache;
> + unsigned long ttl;
> + unsigned long fetch_time;
> + struct mutex lock;
> + } error_flags;
> +};
> +
> +/* Ensure that the next error_flags access fetches them from the device */
> +static void efuse_invalidate_error_flags(struct efuse *efuse)
> +{
> + mutex_lock(&efuse->error_flags.lock);
> + efuse->error_flags.fetch_time = 0;
> + mutex_unlock(&efuse->error_flags.lock);
> +}
> +
> +static ssize_t efuse_show_operstate(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct efuse *efuse = dev_get_drvdata(dev);
> + int status = regulator_is_enabled(efuse->reg);
> +
> + if (status < 0)
> + return status;
> +
> + return sysfs_emit(buf, "%s\n", status ? "on" : "off");
> +}
> +
> +static ssize_t efuse_set_operstate(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int status, wantstate;
> + struct efuse *efuse = dev_get_drvdata(dev);
> + struct regulator *reg = efuse->reg;
> +
> + if (sysfs_streq(buf, "on"))
> + wantstate = 1;
> + else if (sysfs_streq(buf, "off"))
> + wantstate = 0;
> + else
> + return -EINVAL;
> +
> + status = regulator_is_enabled(reg);
> +
> + /*
> + * We need to ensure our enable/disable calls don't get imbalanced, so
> + * bail if we can't determine the current state.
> + */
> + if (status < 0)
> + return status;
> +
> + /* Return early if we're already in the desired state */
> + if (!!status == wantstate)
> + return count;
> +
> + if (wantstate)
> + status = regulator_enable(reg);
> + else
> + status = regulator_disable(reg);
> +
> + /*
> + * Toggling operstate can reset latched status flags, so invalidate
> + * the cached value.
> + */
> + efuse_invalidate_error_flags(efuse);
> +
> + if (!status && regulator_is_enabled(reg) != wantstate) {
> + /*
> + * We could do
> + *
> + * if (!wantstate)
> + * regulator_force_disable(reg);
> + *
> + * here, but it's likely to leave it such that it can't then
> + * be re-enabled, so we'll just report the error and leave it
> + * as it is (and hopefully as long as our enable/disable calls
> + * remain balanced and nobody registers another consumer for
> + * the same supply we won't end up in this situation anyway).
> + */
> + dev_err(dev, "regulator_%sable() didn't take effect\n", wantstate ? "en" : "dis");
> + status = -EIO;
> + }
> +
> + return status ? : count;
> +}
> +
> +static int efuse_update_error_flags(struct efuse *efuse)
> +{
> + int status = 0;
> + unsigned long cache_expiry;
> +
> + mutex_lock(&efuse->error_flags.lock);
> +
> + cache_expiry = efuse->error_flags.fetch_time + efuse->error_flags.ttl;
> +
> + if (!efuse->error_flags.ttl || !efuse->error_flags.fetch_time ||
> + time_after(jiffies, cache_expiry)) {
> + status = regulator_get_error_flags(efuse->reg, &efuse->error_flags.cache);
> + if (!status)
> + efuse->error_flags.fetch_time = jiffies;
> + }
> +
> + mutex_unlock(&efuse->error_flags.lock);
> +
> + return status;
> +}
> +
> +static DEVICE_ATTR(operstate, 0644, efuse_show_operstate, efuse_set_operstate);
> +
> +#define EFUSE_ERROR_ATTR(name, bit) \
> + static ssize_t efuse_show_##name(struct device *dev, struct device_attribute *attr, \
> + char *buf) \
> + { \
> + struct efuse *efuse = dev_get_drvdata(dev); \
> + int status = efuse_update_error_flags(efuse); \
> + if (status) \
> + return status; \
> + return sysfs_emit(buf, "%d\n", !!(efuse->error_flags.cache & bit)); \
> + } \
> + static DEVICE_ATTR(name, 0444, efuse_show_##name, NULL)
> +
> +EFUSE_ERROR_ATTR(under_voltage, REGULATOR_ERROR_UNDER_VOLTAGE);
> +EFUSE_ERROR_ATTR(over_current, REGULATOR_ERROR_OVER_CURRENT);
> +EFUSE_ERROR_ATTR(regulation_out, REGULATOR_ERROR_REGULATION_OUT);
> +EFUSE_ERROR_ATTR(fail, REGULATOR_ERROR_FAIL);
> +EFUSE_ERROR_ATTR(over_temp, REGULATOR_ERROR_OVER_TEMP);
> +EFUSE_ERROR_ATTR(under_voltage_warn, REGULATOR_ERROR_UNDER_VOLTAGE_WARN);
> +EFUSE_ERROR_ATTR(over_current_warn, REGULATOR_ERROR_OVER_CURRENT_WARN);
> +EFUSE_ERROR_ATTR(over_voltage_warn, REGULATOR_ERROR_OVER_VOLTAGE_WARN);
> +EFUSE_ERROR_ATTR(over_temp_warn, REGULATOR_ERROR_OVER_TEMP_WARN);
> +
> +static struct attribute *attributes[] = {
> + &dev_attr_operstate.attr,
> + &dev_attr_under_voltage.attr,
> + &dev_attr_over_current.attr,
> + &dev_attr_regulation_out.attr,
> + &dev_attr_fail.attr,
> + &dev_attr_over_temp.attr,
> + &dev_attr_under_voltage_warn.attr,
> + &dev_attr_over_current_warn.attr,
> + &dev_attr_over_voltage_warn.attr,
> + &dev_attr_over_temp_warn.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group attr_group = {
> + .attrs = attributes,
> +};
ATTRIBUTE_GROUPS()?
> +
> +static int efuse_probe(struct platform_device *pdev)
> +{
> + int status;
> + struct regulator *reg;
> + struct efuse *efuse;
> + u32 cache_ttl_ms;
> +
> + reg = devm_regulator_get(&pdev->dev, "vout");
> + if (IS_ERR(reg))
> + return PTR_ERR(reg);
> +
> + status = regulator_enable(reg);
> + if (status) {
> + dev_err(&pdev->dev, "failed to enable regulator\n");
> + return status;
> + }
> +
> + efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
> + if (!efuse)
> + return -ENOMEM;
> +
> + efuse->reg = reg;
> + mutex_init(&efuse->error_flags.lock);
> +
> + if (!of_property_read_u32(pdev->dev.of_node, "error-flags-cache-ttl-ms", &cache_ttl_ms))
> + efuse->error_flags.ttl = msecs_to_jiffies(cache_ttl_ms);
> +
> + platform_set_drvdata(pdev, efuse);
> +
> + return sysfs_create_group(&pdev->dev.kobj, &attr_group);
You just raced with userspace and lost :(
Set the default groups for your platform driver and then the driver core
will automatically create/remove them for you, no need for you to do
anything directly with them at all.
thanks,
greg k-h
Powered by blists - more mailing lists