LKML Archive on lore.kernel.org help / color / mirror / Atom feed
* [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver @ 2018-05-29 8:59 Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 7/8] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Vadim Pasternak @ 2018-05-29 8:59 UTC (permalink / raw) To: dvhart, andy.shevchenko, gregkh Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera, Vadim Pasternak Introduce new Mellanox platform driver to allow access to Mellanox programmable device register space trough sysfs interface. The driver purpose is to provide sysfs interface for user space for the registers essential for system control and monitoring. The sets of registers for sysfs access are supposed to be defined per system type bases and include the registers related to system resets operation, system reset causes monitoring and some kinds of mux selection. Signed-off-by: Vadim Pasternak <vadimp@mellanox.com> --- v1->v2: Changed added by Vadim: - Change ---help--- to help in Kconfig, according to new requirements; v2->v3: Comments pointed out by Darren: - Remove conditional assignment per attribute mode type, because mode will guard against not permitted access. Verified by Vadim. --- drivers/platform/mellanox/Kconfig | 11 ++ drivers/platform/mellanox/Makefile | 1 + drivers/platform/mellanox/mlxreg-io.c | 203 ++++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+) create mode 100644 drivers/platform/mellanox/mlxreg-io.c diff --git a/drivers/platform/mellanox/Kconfig b/drivers/platform/mellanox/Kconfig index 591bccd..ddfae9fc 100644 --- a/drivers/platform/mellanox/Kconfig +++ b/drivers/platform/mellanox/Kconfig @@ -23,4 +23,15 @@ config MLXREG_HOTPLUG This driver handles hot-plug events for the power suppliers, power cables and fans on the wide range Mellanox IB and Ethernet systems. +config MLXREG_IO + tristate "Mellanox platform register access driver support" + depends on REGMAP + depends on HWMON + help + This driver allows access to Mellanox programmable device register + space trough sysfs interface. The sets of registers for sysfs access + are defined per system type bases and includes the registers related + to system resets operation, system reset causes monitoring and some + kinds of mux selection. + endif # MELLANOX_PLATFORM diff --git a/drivers/platform/mellanox/Makefile b/drivers/platform/mellanox/Makefile index 7c8385e..57074d9c 100644 --- a/drivers/platform/mellanox/Makefile +++ b/drivers/platform/mellanox/Makefile @@ -4,3 +4,4 @@ # Mellanox Platform-Specific Drivers # obj-$(CONFIG_MLXREG_HOTPLUG) += mlxreg-hotplug.o +obj-$(CONFIG_MLXREG_IO) += mlxreg-io.o diff --git a/drivers/platform/mellanox/mlxreg-io.c b/drivers/platform/mellanox/mlxreg-io.c new file mode 100644 index 0000000..b85452f --- /dev/null +++ b/drivers/platform/mellanox/mlxreg-io.c @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Mellanox register access driver + * + * Copyright (C) 2018 Mellanox Technologies + * Copyright (C) 2018 Vadim Pasternak <vadimp@mellanox.com> + */ + +#include <linux/bitops.h> +#include <linux/device.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/platform_data/mlxreg.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +/* Attribute parameters. */ +#define MLXREG_IO_ATT_SIZE 10 +#define MLXREG_IO_ATT_NUM 48 + +/** + * struct mlxreg_io_priv_data - driver's private data: + * + * @pdev: platform device; + * @pdata: platform data; + * @hwmon: hwmon device; + * @mlxreg_io_attr: sysfs attributes array; + * @mlxreg_io_dev_attr: sysfs sensor device attribute array; + * @group: sysfs attribute group; + * @groups: list of sysfs attribute group for hwmon registration; + */ +struct mlxreg_io_priv_data { + struct platform_device *pdev; + struct mlxreg_core_platform_data *pdata; + struct device *hwmon; + struct attribute *mlxreg_io_attr[MLXREG_IO_ATT_NUM + 1]; + struct sensor_device_attribute mlxreg_io_dev_attr[MLXREG_IO_ATT_NUM]; + struct attribute_group group; + const struct attribute_group *groups[2]; +}; + +static ssize_t +mlxreg_io_attr_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct mlxreg_io_priv_data *priv = dev_get_drvdata(dev); + int index = to_sensor_dev_attr(attr)->index; + struct mlxreg_core_data *data = priv->pdata->data + index; + u32 regval = 0; + int ret; + + ret = regmap_read(priv->pdata->regmap, data->reg, ®val); + if (ret) + goto access_error; + + if (!data->bit) + regval = !!(regval & ~data->mask); + + return sprintf(buf, "%u\n", regval); + +access_error: + return ret; +} + +static ssize_t +mlxreg_io_attr_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct mlxreg_io_priv_data *priv = dev_get_drvdata(dev); + int index = to_sensor_dev_attr(attr)->index; + struct mlxreg_core_data *data = priv->pdata->data + index; + u32 val, regval; + int ret; + + ret = kstrtou32(buf, MLXREG_IO_ATT_SIZE, &val); + if (ret) + return ret; + + ret = regmap_read(priv->pdata->regmap, data->reg, ®val); + if (ret) + goto access_error; + + regval &= data->mask; + + val = !!val; + if (val) + regval |= ~data->mask; + else + regval &= data->mask; + + ret = regmap_write(priv->pdata->regmap, data->reg, regval); + if (ret) + goto access_error; + + return len; + +access_error: + dev_err(&priv->pdev->dev, "Bus access error\n"); + return ret; +} + +static struct device_attribute mlxreg_io_devattr_rw = { + .show = mlxreg_io_attr_show, + .store = mlxreg_io_attr_store, +}; + +static int mlxreg_io_attr_init(struct mlxreg_io_priv_data *priv) +{ + int i; + + priv->group.attrs = devm_kzalloc(&priv->pdev->dev, + priv->pdata->counter * + sizeof(struct attribute *), + GFP_KERNEL); + if (!priv->group.attrs) + return -ENOMEM; + + for (i = 0; i < priv->pdata->counter; i++) { + priv->mlxreg_io_attr[i] = + &priv->mlxreg_io_dev_attr[i].dev_attr.attr; + memcpy(&priv->mlxreg_io_dev_attr[i].dev_attr, + &mlxreg_io_devattr_rw, sizeof(struct device_attribute)); + + /* Set attribute name as a label. */ + priv->mlxreg_io_attr[i]->name = + devm_kasprintf(&priv->pdev->dev, GFP_KERNEL, + priv->pdata->data[i].label); + + if (!priv->mlxreg_io_attr[i]->name) { + dev_err(&priv->pdev->dev, "Memory allocation failed for sysfs attribute %d.\n", + i + 1); + return -ENOMEM; + } + + priv->mlxreg_io_dev_attr[i].dev_attr.attr.mode = + priv->pdata->data[i].mode; + priv->mlxreg_io_dev_attr[i].dev_attr.attr.name = + priv->mlxreg_io_attr[i]->name; + priv->mlxreg_io_dev_attr[i].index = i; + sysfs_attr_init(&priv->mlxreg_io_dev_attr[i].dev_attr.attr); + } + + priv->group.attrs = priv->mlxreg_io_attr; + priv->groups[0] = &priv->group; + priv->groups[1] = NULL; + + return 0; +} + +static int mlxreg_io_probe(struct platform_device *pdev) +{ + struct mlxreg_io_priv_data *priv; + int err; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->pdata = dev_get_platdata(&pdev->dev); + if (!priv->pdata) { + dev_err(&pdev->dev, "Failed to get platform data.\n"); + return -EINVAL; + } + + priv->pdev = pdev; + + err = mlxreg_io_attr_init(priv); + if (err) { + dev_err(&priv->pdev->dev, "Failed to allocate attributes: %d\n", + err); + return err; + } + + priv->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev, + "mlxreg_io", + priv, + priv->groups); + if (IS_ERR(priv->hwmon)) { + dev_err(&pdev->dev, "Failed to register hwmon device %ld\n", + PTR_ERR(priv->hwmon)); + return PTR_ERR(priv->hwmon); + } + + dev_set_drvdata(&pdev->dev, priv); + + return 0; +} + +static struct platform_driver mlxreg_io_driver = { + .driver = { + .name = "mlxreg-io", + }, + .probe = mlxreg_io_probe, +}; + +module_platform_driver(mlxreg_io_driver); + +MODULE_AUTHOR("Vadim Pasternak <vadimp@mellanox.com>"); +MODULE_DESCRIPTION("Mellanox regmap I/O access driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:mlxreg-io"); -- 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 7/8] platform/x86: mlx-platform: Add mlxreg-io platform driver activation 2018-05-29 8:59 [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak @ 2018-05-29 8:59 ` Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces Vadim Pasternak 2018-06-05 1:08 ` [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Darren Hart 2 siblings, 0 replies; 6+ messages in thread From: Vadim Pasternak @ 2018-05-29 8:59 UTC (permalink / raw) To: dvhart, andy.shevchenko, gregkh Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera, Vadim Pasternak Add mlxreg-io platform driver activation. Access driver uses the same regmap infrastructure as others Mellanox platform drivers. Specific registers description for default platform data configuration are added to mlx-platform. There are the registers for resets control, reset causes monitoring, programmable devices version reading and mux select control. This platform data is passed to mlxreg-io driver. Also some default values for the register are set at initialization time through the regmap infrastructure, which are necessary for moving write protection from the general purpose registers, which are used by mlxreg-io for write access. Signed-off-by: Vadim Pasternak <vadimp@mellanox.com> --- drivers/platform/x86/mlx-platform.c | 159 +++++++++++++++++++++++++++++++++++- 1 file changed, 157 insertions(+), 2 deletions(-) diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c index a0fd9aa..87ddd3a 100644 --- a/drivers/platform/x86/mlx-platform.c +++ b/drivers/platform/x86/mlx-platform.c @@ -47,11 +47,18 @@ /* LPC bus IO offsets */ #define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR 0x2000 #define MLXPLAT_CPLD_LPC_REG_BASE_ADRR 0x2500 +#define MLXPLAT_CPLD_LPC_REG_CPLD1_VER_OFFSET 0x00 +#define MLXPLAT_CPLD_LPC_REG_CPLD2_VER_OFFSET 0x01 +#define MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET 0x1d #define MLXPLAT_CPLD_LPC_REG_LED1_OFFSET 0x20 #define MLXPLAT_CPLD_LPC_REG_LED2_OFFSET 0x21 #define MLXPLAT_CPLD_LPC_REG_LED3_OFFSET 0x22 #define MLXPLAT_CPLD_LPC_REG_LED4_OFFSET 0x23 #define MLXPLAT_CPLD_LPC_REG_LED5_OFFSET 0x24 +#define MLXPLAT_CPLD_LPC_REG_GP1_OFFSET 0x30 +#define MLXPLAT_CPLD_LPC_REG_WP1_OFFSET 0x31 +#define MLXPLAT_CPLD_LPC_REG_GP2_OFFSET 0x32 +#define MLXPLAT_CPLD_LPC_REG_WP2_OFFSET 0x33 #define MLXPLAT_CPLD_LPC_REG_AGGR_OFFSET 0x3a #define MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET 0x3b #define MLXPLAT_CPLD_LPC_REG_AGGRLO_OFFSET 0x40 @@ -122,12 +129,14 @@ * @pdev_mux - array of mux platform devices * @pdev_hotplug - hotplug platform devices * @pdev_led - led platform devices + * @pdev_io_regs - register access platform devices */ struct mlxplat_priv { struct platform_device *pdev_i2c; struct platform_device *pdev_mux[MLXPLAT_CPLD_LPC_MUX_DEVS]; struct platform_device *pdev_hotplug; struct platform_device *pdev_led; + struct platform_device *pdev_io_regs; }; /* Regions for LPC I2C controller and LPC base register space */ @@ -813,6 +822,98 @@ static struct mlxreg_core_platform_data mlxplat_default_ng_led_data = { .counter = ARRAY_SIZE(mlxplat_mlxcpld_default_ng_led_data), }; +/* Platform register access default */ +static struct mlxreg_core_data mlxplat_mlxcpld_default_regs_io_data[] = { + { + .label = "cpld1_version", + .reg = MLXPLAT_CPLD_LPC_REG_CPLD1_VER_OFFSET, + .bit = GENMASK(7, 0), + .mode = 0444, + }, + { + .label = "cpld2_version", + .reg = MLXPLAT_CPLD_LPC_REG_CPLD2_VER_OFFSET, + .bit = GENMASK(7, 0), + .mode = 0444, + }, + { + .label = "cause_long_pb", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(0), + .mode = 0444, + }, + { + .label = "cause_short_pb", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(1), + .mode = 0444, + }, + { + .label = "cause_aux_pwr_or_ref", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(2), + .mode = 0444, + }, + { + .label = "cause_main_pwr_fail", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(3), + .mode = 0444, + }, + { + .label = "cause_sw_reset", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(4), + .mode = 0444, + }, + { + .label = "cause_fw_reset", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(5), + .mode = 0444, + }, + { + .label = "cause_hotswap_or_wd", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(6), + .mode = 0444, + }, + { + .label = "cause_asic_thermal", + .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(7), + .mode = 0444, + }, + { + .label = "psu1_on", + .reg = MLXPLAT_CPLD_LPC_REG_GP1_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(0), + .mode = 0200, + }, + { + .label = "psu2_on", + .reg = MLXPLAT_CPLD_LPC_REG_GP1_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(1), + .mode = 0200, + }, + { + .label = "pwr_cycle", + .reg = MLXPLAT_CPLD_LPC_REG_GP1_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(2), + .mode = 0200, + }, + { + .label = "select_iio", + .reg = MLXPLAT_CPLD_LPC_REG_GP2_OFFSET, + .mask = GENMASK(7, 0) & ~BIT(6), + .mode = 0644, + }, +}; + +static struct mlxreg_core_platform_data mlxplat_default_regs_io_data = { + .data = mlxplat_mlxcpld_default_regs_io_data, + .counter = ARRAY_SIZE(mlxplat_mlxcpld_default_regs_io_data), +}; static bool mlxplat_mlxcpld_writeable_reg(struct device *dev, unsigned int reg) { @@ -822,6 +923,10 @@ static bool mlxplat_mlxcpld_writeable_reg(struct device *dev, unsigned int reg) case MLXPLAT_CPLD_LPC_REG_LED3_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED4_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED5_OFFSET: + case MLXPLAT_CPLD_LPC_REG_GP1_OFFSET: + case MLXPLAT_CPLD_LPC_REG_WP1_OFFSET: + case MLXPLAT_CPLD_LPC_REG_GP2_OFFSET: + case MLXPLAT_CPLD_LPC_REG_WP2_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGRLO_MASK_OFFSET: case MLXPLAT_CPLD_LPC_REG_PSU_EVENT_OFFSET: @@ -838,11 +943,18 @@ static bool mlxplat_mlxcpld_writeable_reg(struct device *dev, unsigned int reg) static bool mlxplat_mlxcpld_readable_reg(struct device *dev, unsigned int reg) { switch (reg) { + case MLXPLAT_CPLD_LPC_REG_CPLD1_VER_OFFSET: + case MLXPLAT_CPLD_LPC_REG_CPLD2_VER_OFFSET: + case MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED1_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED2_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED3_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED4_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED5_OFFSET: + case MLXPLAT_CPLD_LPC_REG_GP1_OFFSET: + case MLXPLAT_CPLD_LPC_REG_WP1_OFFSET: + case MLXPLAT_CPLD_LPC_REG_GP2_OFFSET: + case MLXPLAT_CPLD_LPC_REG_WP2_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGR_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGRLO_OFFSET: @@ -864,11 +976,16 @@ static bool mlxplat_mlxcpld_readable_reg(struct device *dev, unsigned int reg) static bool mlxplat_mlxcpld_volatile_reg(struct device *dev, unsigned int reg) { switch (reg) { + case MLXPLAT_CPLD_LPC_REG_CPLD1_VER_OFFSET: + case MLXPLAT_CPLD_LPC_REG_CPLD2_VER_OFFSET: + case MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED1_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED2_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED3_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED4_OFFSET: case MLXPLAT_CPLD_LPC_REG_LED5_OFFSET: + case MLXPLAT_CPLD_LPC_REG_GP1_OFFSET: + case MLXPLAT_CPLD_LPC_REG_GP2_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGR_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET: case MLXPLAT_CPLD_LPC_REG_AGGRLO_OFFSET: @@ -887,6 +1004,11 @@ static bool mlxplat_mlxcpld_volatile_reg(struct device *dev, unsigned int reg) return false; } +static const struct reg_default mlxplat_mlxcpld_regmap_default[] = { + { MLXPLAT_CPLD_LPC_REG_WP1_OFFSET, 0x00 }, + { MLXPLAT_CPLD_LPC_REG_WP2_OFFSET, 0x00 }, +}; + struct mlxplat_mlxcpld_regmap_context { void __iomem *base; }; @@ -919,6 +1041,8 @@ static const struct regmap_config mlxplat_mlxcpld_regmap_config = { .writeable_reg = mlxplat_mlxcpld_writeable_reg, .readable_reg = mlxplat_mlxcpld_readable_reg, .volatile_reg = mlxplat_mlxcpld_volatile_reg, + .reg_defaults = mlxplat_mlxcpld_regmap_default, + .num_reg_defaults = ARRAY_SIZE(mlxplat_mlxcpld_regmap_default), .reg_read = mlxplat_mlxcpld_reg_read, .reg_write = mlxplat_mlxcpld_reg_write, }; @@ -930,6 +1054,7 @@ static struct resource mlxplat_mlxcpld_resources[] = { static struct platform_device *mlxplat_dev; static struct mlxreg_core_hotplug_platform_data *mlxplat_hotplug; static struct mlxreg_core_platform_data *mlxplat_led; +static struct mlxreg_core_platform_data *mlxplat_regs_io; static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi) { @@ -944,6 +1069,7 @@ static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi) mlxplat_hotplug->deferred_nr = mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1]; mlxplat_led = &mlxplat_default_led_data; + mlxplat_regs_io = &mlxplat_default_regs_io_data; return 1; }; @@ -978,6 +1104,7 @@ static int __init mlxplat_dmi_msn274x_matched(const struct dmi_system_id *dmi) mlxplat_hotplug->deferred_nr = mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1]; mlxplat_led = &mlxplat_default_led_data; + mlxplat_regs_io = &mlxplat_default_regs_io_data; return 1; }; @@ -1163,7 +1290,7 @@ static int mlxplat_mlxcpld_verify_bus_topology(int *nr) static int __init mlxplat_init(void) { struct mlxplat_priv *priv; - int i, nr, err; + int i, j, nr, err; if (!dmi_check_system(mlxplat_dmi_table)) return -ENODEV; @@ -1233,6 +1360,15 @@ static int __init mlxplat_init(void) goto fail_platform_mux_register; } + /* Set default registers. */ + for (j = 0; j < mlxplat_mlxcpld_regmap_config.num_reg_defaults; j++) { + err = regmap_write(mlxplat_hotplug->regmap, + mlxplat_mlxcpld_regmap_default[j].reg, + mlxplat_mlxcpld_regmap_default[j].def); + if (err) + goto fail_platform_mux_register; + } + /* Add LED driver. */ mlxplat_led->regmap = mlxplat_hotplug->regmap; priv->pdev_led = platform_device_register_resndata( @@ -1244,14 +1380,31 @@ static int __init mlxplat_init(void) goto fail_platform_hotplug_register; } + /* Add registers io access driver. */ + if (mlxplat_regs_io) { + mlxplat_regs_io->regmap = mlxplat_hotplug->regmap; + priv->pdev_io_regs = platform_device_register_resndata( + &mlxplat_dev->dev, "mlxreg-io", + PLATFORM_DEVID_NONE, NULL, 0, + mlxplat_regs_io, + sizeof(*mlxplat_regs_io)); + if (IS_ERR(priv->pdev_io_regs)) { + err = PTR_ERR(priv->pdev_io_regs); + goto fail_platform_led_register; + } + } + /* Sync registers with hardware. */ regcache_mark_dirty(mlxplat_hotplug->regmap); err = regcache_sync(mlxplat_hotplug->regmap); if (err) - goto fail_platform_led_register; + goto fail_platform_io_regs_register; return 0; +fail_platform_io_regs_register: + if (mlxplat_regs_io) + platform_device_unregister(priv->pdev_io_regs); fail_platform_led_register: platform_device_unregister(priv->pdev_led); fail_platform_hotplug_register: @@ -1272,6 +1425,8 @@ static void __exit mlxplat_exit(void) struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev); int i; + if (priv->pdev_io_regs) + platform_device_unregister(priv->pdev_io_regs); platform_device_unregister(priv->pdev_led); platform_device_unregister(priv->pdev_hotplug); -- 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces 2018-05-29 8:59 [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 7/8] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak @ 2018-05-29 8:59 ` Vadim Pasternak 2018-06-05 1:54 ` Darren Hart 2018-06-05 1:08 ` [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Darren Hart 2 siblings, 1 reply; 6+ messages in thread From: Vadim Pasternak @ 2018-05-29 8:59 UTC (permalink / raw) To: dvhart, andy.shevchenko, gregkh Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera, Vadim Pasternak Add documentation for mlxreg-io driver sysfs interfaces for user space access to system's power resets control, reset causes monitoring, programmable devices version reading and devices selection control. Signed-off-by: Vadim Pasternak <vadimp@mellanox.com> --- v4: Comments pointed out by Greg: Add Documentation/ABI/ entries for the new sysfs files. --- Documentation/ABI/stable/sysfs-driver-mlxreg-io | 51 +++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 Documentation/ABI/stable/sysfs-driver-mlxreg-io diff --git a/Documentation/ABI/stable/sysfs-driver-mlxreg-io b/Documentation/ABI/stable/sysfs-driver-mlxreg-io new file mode 100644 index 0000000..fcd659e --- /dev/null +++ b/Documentation/ABI/stable/sysfs-driver-mlxreg-io @@ -0,0 +1,51 @@ +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/ + cause_aux_pwr_or_ref + cause_asic_thermal + cause_hotswap_or_wd + cause_fw_reset + cause_long_pb + cause_main_pwr_fail + cause_short_pb + cause_sw_reset +Date: May 2018 +KernelVersion: 4.18 +Contact: Vadim Pasternak <vadimpmellanox.com> +Description: These files show the system reset cause, as following: power + auxiliary outage or power refresh, ASIC thermal shutdown, + hotswap or watchdog, firmware reset, long press power button, + short press power button, software reset. Value 1 in file means + this is reset cause, 0 - otherwise. Only one of the above + causes could be 1 at the same time, representing only last + reset cause. + + The files are read only. + +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/ + cpld1_version + cpld2_version +Date: May 2018 +KernelVersion: 4.18 +Contact: Vadim Pasternak <vadimpmellanox.com> +Description: These files show with which CPLD versions have been burned + on carrier and switch boards. + + The files are read only. + +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/select_iio +Date: May 2018 +KernelVersion: 4.18 +Contact: Vadim Pasternak <vadimpmellanox.com> +Description: This file allows iio devices selection. + + The file is read/write. + +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/psu1_on + /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/psu2_on + /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/pwr_cycle +Date: May 2018 +KernelVersion: 4.18 +Contact: Vadim Pasternak <vadimpmellanox.com> +Description: These files allow assert system's power cycling and PS units + on/off switching. + + The files are write only. -- 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces 2018-05-29 8:59 ` [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces Vadim Pasternak @ 2018-06-05 1:54 ` Darren Hart 2018-06-05 11:00 ` Vadim Pasternak 0 siblings, 1 reply; 6+ messages in thread From: Darren Hart @ 2018-06-05 1:54 UTC (permalink / raw) To: Vadim Pasternak Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera On Tue, May 29, 2018 at 08:59:07AM +0000, Vadim Pasternak wrote: > Add documentation for mlxreg-io driver sysfs interfaces for user space > access to system's power resets control, reset causes monitoring, > programmable devices version reading and devices selection control. > > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com> Hi Vadim, You mentioned that these can vary by platform, I presume that means that any given platform will present a subset of these attributes - but that all possible attributes are documented here. Is that correct? > --- > v4: > Comments pointed out by Greg: > Add Documentation/ABI/ entries for the new sysfs files. > --- > Documentation/ABI/stable/sysfs-driver-mlxreg-io | 51 +++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/ABI/stable/sysfs-driver-mlxreg-io > > diff --git a/Documentation/ABI/stable/sysfs-driver-mlxreg-io b/Documentation/ABI/stable/sysfs-driver-mlxreg-io > new file mode 100644 > index 0000000..fcd659e > --- /dev/null > +++ b/Documentation/ABI/stable/sysfs-driver-mlxreg-io > @@ -0,0 +1,51 @@ > +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/ > + cause_aux_pwr_or_ref > + cause_asic_thermal > + cause_hotswap_or_wd > + cause_fw_reset > + cause_long_pb > + cause_main_pwr_fail > + cause_short_pb > + cause_sw_reset These property names do not seem to follow any kind of logical pattern with respect to abbreviation. Starting with "cause_" made me think they were event generators at first... Of course, who wants to be able to generate an asic thermal shutdown from sysfs... ;-) The only thing that might make more sense to *me* would be to replace "cause" with "reset". Not required. I'll leave it to you, you may have good reason for the names chosen. > +Date: May 2018 > +KernelVersion: 4.18 > +Contact: Vadim Pasternak <vadimpmellanox.com> > +Description: These files show the system reset cause, as following: power > + auxiliary outage or power refresh, ASIC thermal shutdown, > + hotswap or watchdog, firmware reset, long press power button, > + short press power button, software reset. Value 1 in file means > + this is reset cause, 0 - otherwise. Only one of the above > + causes could be 1 at the same time, representing only last > + reset cause. > + > + The files are read only. > + > +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/ > + cpld1_version > + cpld2_version > +Date: May 2018 > +KernelVersion: 4.18 > +Contact: Vadim Pasternak <vadimpmellanox.com> > +Description: These files show with which CPLD versions have been burned > + on carrier and switch boards. > + > + The files are read only. > + > +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/select_iio > +Date: May 2018 > +KernelVersion: 4.18 > +Contact: Vadim Pasternak <vadimpmellanox.com> > +Description: This file allows iio devices selection. > + > + The file is read/write. > + Some description is appropriate here for what values it accepts and how it is intended to be used. > +What: /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/psu1_on > + /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/psu2_on > + /sys/devices/platform/mlxplat/mlxreg-io/hwmon/hwmon*/pwr_cycle > +Date: May 2018 > +KernelVersion: 4.18 > +Contact: Vadim Pasternak <vadimpmellanox.com> > +Description: These files allow assert system's power cycling and PS units > + on/off switching. This Description is awkward. Consider: These files allow asserting system power cycling and switching power supply units on and off. > + The files are write only. Please provide usage and resulting behavior. -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces 2018-06-05 1:54 ` Darren Hart @ 2018-06-05 11:00 ` Vadim Pasternak 0 siblings, 0 replies; 6+ messages in thread From: Vadim Pasternak @ 2018-06-05 11:00 UTC (permalink / raw) To: Darren Hart Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri, Michael Shych, ivecera > -----Original Message----- > From: Darren Hart [mailto:dvhart@infradead.org] > Sent: Tuesday, June 05, 2018 4:55 AM > To: Vadim Pasternak <vadimp@mellanox.com> > Cc: andy.shevchenko@gmail.com; gregkh@linuxfoundation.org; linux- > kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; jiri@resnulli.us; > Michael Shych <michaelsh@mellanox.com>; ivecera@redhat.com > Subject: Re: [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io > sysfs interfaces > > On Tue, May 29, 2018 at 08:59:07AM +0000, Vadim Pasternak wrote: > > Add documentation for mlxreg-io driver sysfs interfaces for user space > > access to system's power resets control, reset causes monitoring, > > programmable devices version reading and devices selection control. > > > > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com> > > Hi Vadim, > > You mentioned that these can vary by platform, I presume that means that any > given platform will present a subset of these attributes - but that all possible > attributes are documented here. Is that correct? Hi Darren, Thank you for review. These are mostly all attributes, that should be exposed. I add two extra for the coming version. Maybe for the future system it could be slightly extended. Also we have writable attribute for BIOS flash boot selection, which I am still not sure we want expose. Thanks, Vadim. > > > --- > > v4: > > Comments pointed out by Greg: > > Add Documentation/ABI/ entries for the new sysfs files. > > --- > > Documentation/ABI/stable/sysfs-driver-mlxreg-io | 51 > > +++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100644 Documentation/ABI/stable/sysfs-driver-mlxreg-io > > > > diff --git a/Documentation/ABI/stable/sysfs-driver-mlxreg-io > > b/Documentation/ABI/stable/sysfs-driver-mlxreg-io > > new file mode 100644 > > index 0000000..fcd659e > > --- /dev/null > > +++ b/Documentation/ABI/stable/sysfs-driver-mlxreg-io > > @@ -0,0 +1,51 @@ > > +What: /sys/devices/platform/mlxplat/mlxreg- > io/hwmon/hwmon*/ > > + > cause_aux_pwr_or_ref > > + cause_asic_thermal > > + > cause_hotswap_or_wd > > + cause_fw_reset > > + cause_long_pb > > + cause_main_pwr_fail > > + cause_short_pb > > + cause_sw_reset > > These property names do not seem to follow any kind of logical pattern with > respect to abbreviation. Starting with "cause_" made me think they were event > generators at first... Of course, who wants to be able to generate an asic > thermal shutdown from sysfs... ;-) > > The only thing that might make more sense to *me* would be to replace > "cause" with "reset". Not required. I'll leave it to you, you may have good > reason for the names chosen. > > > +Date: May 2018 > > +KernelVersion: 4.18 > > +Contact: Vadim Pasternak <vadimpmellanox.com> > > +Description: These files show the system reset cause, as following: power > > + auxiliary outage or power refresh, ASIC thermal shutdown, > > + hotswap or watchdog, firmware reset, long press power > button, > > + short press power button, software reset. Value 1 in file means > > + this is reset cause, 0 - otherwise. Only one of the above > > + causes could be 1 at the same time, representing only last > > + reset cause. > > + > > + The files are read only. > > + > > +What: /sys/devices/platform/mlxplat/mlxreg- > io/hwmon/hwmon*/ > > + cpld1_version > > + cpld2_version > > +Date: May 2018 > > +KernelVersion: 4.18 > > +Contact: Vadim Pasternak <vadimpmellanox.com> > > +Description: These files show with which CPLD versions have been burned > > + on carrier and switch boards. > > + > > + The files are read only. > > + > > +What: /sys/devices/platform/mlxplat/mlxreg- > io/hwmon/hwmon*/select_iio > > +Date: May 2018 > > +KernelVersion: 4.18 > > +Contact: Vadim Pasternak <vadimpmellanox.com> > > +Description: This file allows iio devices selection. > > + > > + The file is read/write. > > + > > Some description is appropriate here for what values it accepts and how it is > intended to be used. > > > +What: /sys/devices/platform/mlxplat/mlxreg- > io/hwmon/hwmon*/psu1_on > > + /sys/devices/platform/mlxplat/mlxreg- > io/hwmon/hwmon*/psu2_on > > + /sys/devices/platform/mlxplat/mlxreg- > io/hwmon/hwmon*/pwr_cycle > > +Date: May 2018 > > +KernelVersion: 4.18 > > +Contact: Vadim Pasternak <vadimpmellanox.com> > > +Description: These files allow assert system's power cycling and PS units > > + on/off switching. > > This Description is awkward. Consider: > > These files allow asserting system power cycling and switching power supply > units on and off. > > > + The files are write only. > > Please provide usage and resulting behavior. > > -- > Darren Hart > VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver 2018-05-29 8:59 [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 7/8] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces Vadim Pasternak @ 2018-06-05 1:08 ` Darren Hart 2 siblings, 0 replies; 6+ messages in thread From: Darren Hart @ 2018-06-05 1:08 UTC (permalink / raw) To: Vadim Pasternak Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera On Tue, May 29, 2018 at 08:59:05AM +0000, Vadim Pasternak wrote: > Introduce new Mellanox platform driver to allow access to Mellanox > programmable device register space trough sysfs interface. > The driver purpose is to provide sysfs interface for user space for the > registers essential for system control and monitoring. > The sets of registers for sysfs access are supposed to be defined per > system type bases and include the registers related to system resets > operation, system reset causes monitoring and some kinds of mux selection. > > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com> > --- > v1->v2: > Changed added by Vadim: > - Change ---help--- to help in Kconfig, according to new requirements; > v2->v3: > Comments pointed out by Darren: > - Remove conditional assignment per attribute mode type, because mode > will guard against not permitted access. > Verified by Vadim. > --- > drivers/platform/mellanox/Kconfig | 11 ++ > drivers/platform/mellanox/Makefile | 1 + > drivers/platform/mellanox/mlxreg-io.c | 203 ++++++++++++++++++++++++++++++++++ > 3 files changed, 215 insertions(+) > create mode 100644 drivers/platform/mellanox/mlxreg-io.c > > diff --git a/drivers/platform/mellanox/Kconfig b/drivers/platform/mellanox/Kconfig > index 591bccd..ddfae9fc 100644 > --- a/drivers/platform/mellanox/Kconfig > +++ b/drivers/platform/mellanox/Kconfig > @@ -23,4 +23,15 @@ config MLXREG_HOTPLUG > This driver handles hot-plug events for the power suppliers, power > cables and fans on the wide range Mellanox IB and Ethernet systems. > > +config MLXREG_IO > + tristate "Mellanox platform register access driver support" > + depends on REGMAP > + depends on HWMON > + help > + This driver allows access to Mellanox programmable device register > + space trough sysfs interface. The sets of registers for sysfs access > + are defined per system type bases and includes the registers related > + to system resets operation, system reset causes monitoring and some > + kinds of mux selection. > + > endif # MELLANOX_PLATFORM > diff --git a/drivers/platform/mellanox/Makefile b/drivers/platform/mellanox/Makefile > index 7c8385e..57074d9c 100644 > --- a/drivers/platform/mellanox/Makefile > +++ b/drivers/platform/mellanox/Makefile > @@ -4,3 +4,4 @@ > # Mellanox Platform-Specific Drivers > # > obj-$(CONFIG_MLXREG_HOTPLUG) += mlxreg-hotplug.o > +obj-$(CONFIG_MLXREG_IO) += mlxreg-io.o > diff --git a/drivers/platform/mellanox/mlxreg-io.c b/drivers/platform/mellanox/mlxreg-io.c > new file mode 100644 > index 0000000..b85452f > --- /dev/null > +++ b/drivers/platform/mellanox/mlxreg-io.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Mellanox register access driver > + * > + * Copyright (C) 2018 Mellanox Technologies > + * Copyright (C) 2018 Vadim Pasternak <vadimp@mellanox.com> > + */ > + > +#include <linux/bitops.h> > +#include <linux/device.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/platform_data/mlxreg.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +/* Attribute parameters. */ > +#define MLXREG_IO_ATT_SIZE 10 > +#define MLXREG_IO_ATT_NUM 48 > + > +/** > + * struct mlxreg_io_priv_data - driver's private data: > + * > + * @pdev: platform device; > + * @pdata: platform data; > + * @hwmon: hwmon device; > + * @mlxreg_io_attr: sysfs attributes array; > + * @mlxreg_io_dev_attr: sysfs sensor device attribute array; > + * @group: sysfs attribute group; > + * @groups: list of sysfs attribute group for hwmon registration; > + */ > +struct mlxreg_io_priv_data { > + struct platform_device *pdev; > + struct mlxreg_core_platform_data *pdata; > + struct device *hwmon; > + struct attribute *mlxreg_io_attr[MLXREG_IO_ATT_NUM + 1]; > + struct sensor_device_attribute mlxreg_io_dev_attr[MLXREG_IO_ATT_NUM]; > + struct attribute_group group; > + const struct attribute_group *groups[2]; > +}; > + > +static ssize_t > +mlxreg_io_attr_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct mlxreg_io_priv_data *priv = dev_get_drvdata(dev); > + int index = to_sensor_dev_attr(attr)->index; > + struct mlxreg_core_data *data = priv->pdata->data + index; > + u32 regval = 0; > + int ret; > + > + ret = regmap_read(priv->pdata->regmap, data->reg, ®val); > + if (ret) > + goto access_error; > + > + if (!data->bit) I'm finding this difficult to review without any comments regarding what this and similar conditions are checking for. The struct definition isn't much help either: bit: attribute effective bit I don't know what that means either. What I see is: If the data->bit is not set... > + regval = !!(regval & ~data->mask); then I will interpret regval as a boolean. Is that right? Why do we do that? ... > +static ssize_t > +mlxreg_io_attr_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t len) > +{ ... > + ret = kstrtou32(buf, MLXREG_IO_ATT_SIZE, &val); Do we need to verify len is at least as long as MLXREG_IO_ATT_SIZE? (seems like we do) > + if (ret) > + return ret; > + > + ret = regmap_read(priv->pdata->regmap, data->reg, ®val); > + if (ret) > + goto access_error; > + > + regval &= data->mask; > + > + val = !!val; >From what I see, this isn't really necessary, but being explicit is necessarily bad. This isn't a fast path. > + if (val) What does val represent here? Kernel variable naming policy encourages descriptive names, and val appears to only be used for this one thing - so what does it mean? > + regval |= ~data->mask; > + else > + regval &= data->mask; > + ... > +static int mlxreg_io_attr_init(struct mlxreg_io_priv_data *priv) ... > + for (i = 0; i < priv->pdata->counter; i++) { > + priv->mlxreg_io_attr[i] = > + &priv->mlxreg_io_dev_attr[i].dev_attr.attr; > + memcpy(&priv->mlxreg_io_dev_attr[i].dev_attr, > + &mlxreg_io_devattr_rw, sizeof(struct device_attribute)); > + > + /* Set attribute name as a label. */ :-) OK, so this comment is rather obvious from the code.... but where things are really opaque in the processing above, there are no comments. Let's focus the comments where the code is non-obvious. -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-05 11:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-29 8:59 [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 7/8] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak 2018-05-29 8:59 ` [PATCH v4 8/8] Documentation/ABI: Add documentation mlxreg-io sysfs interfaces Vadim Pasternak 2018-06-05 1:54 ` Darren Hart 2018-06-05 11:00 ` Vadim Pasternak 2018-06-05 1:08 ` [PATCH v4 6/8] platform/mellanox: Introduce support for Mellanox register access driver Darren Hart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).