On Tue, Aug 29, 2017 at 06:00:15PM +0000, Vadim Pasternak wrote:
> This patch adds core regmap platform driver for Mellanox BMC cards with
> the programmable devices based chassis control. The device logics,
> controlled by software includes:
> - Interrupt handling for chassis, ASIC, CPU events;
> - LED handling;
> - Exposes through sysfs mux, reset signals, reset cause notification;
> The patch provides support for the access to programmable device through
> the register map and allows dynamic device tree manipulation at runtime
> for removable devices.
>
> This driver requires activator driver, which responsibility is to create
> register map and pass it to regmap core. Such activator could be based for
> example on I2C, SPI or MMIO interface.
>
> Drivers exposes the number of hwmon attributes to sysfs according to the
> attribute groups, defined in the device tree. These attributes will be
> located for example in /sys/class/hwmon/hwmonX folder, which is a symbolic
> link to for example:
> /sys/bus/i2c/devices/4-0072/mlxreg-core.1138/hwmon/hwmon10.
> The attributes are divided to the groups, like in the below example:
> MUX nodes
> - safe_bios_disable
> - spi_burn_bios_ci
> - spi_burn_ncsi
> - uart_sel
> Reset nodes:
> - sys_power_cycle
> - bmc_upgrade
> - asic_reset
> Cause of reset nodes:
> - cpu_kernel_panic
> - cpu_shutdown
> - bmc_warm_reset
> General purpose RW nodes:
> - version
>
> Drivers also probes LED and hotplug drivers, in case device tree
> description contains LED and hotplug nodes.
>
> Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> ---
> v3->v4:
> Comments pointed out by Lee:
> - Split interrupt related logic into separate module and place this logic
> within the existing Mellanox hotplug driver. Move for this reason this
> driver from drivers/platform/x86 to drivers/platform/mellanox folder;
> Comments pointed out by Rob:
> - Make a separate patch /devicetree/bindings/vendor-prefixes.txt;
> - Add .txt to Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core
> and send it within this series;
Why did you combine this? Bindings should be separate patches.
> - Modify "compatible" property;
> - Modify explanation for "deferred" property;
> - Describe each subnode by its own section;
> - Don't use underscore in attribute names;
> Changes added by Vadim:
> - Add mlxreg_hotplug_device and mlxreg_core_item structures to mlxreg.h
> and modify mlxreg_core_led_platform_data structure;
> v2->v3:
> Changes added by Vadim:
> - Change name of field led data in struct mlxreg_core_led_platform_data
> in mlxreg.h;
> - fix data position assignment in mlxreg_core_get;
> - update name of field led data in mlxreg_core_set_led;
> v1->v2:
> Comments pointed out by Pavel:
> - Remove extra new line in mellanox,mlxreg-core;
> - Replace three NOT with one in mlxreg_core_attr_show;
> - Make error message in mlxreg_core_work_helper shorter;
> - Make attribute assignment more readable;
> - Separate mellanox,mlxreg-core file for sending to DT maintainers.
> Comments pointed out by Jacek:
> - Since brightness_set_blocking is used instead of
> brightness_set, three fields from mlxreg_core_led_data are removed.
> ---
> .../bindings/mfd/mellanox,mlxreg-core.txt | 367 +++++++++
> MAINTAINERS | 7 +
> drivers/mfd/Kconfig | 15 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/mlxreg-core.c | 839 +++++++++++++++++++++
> include/linux/platform_data/mlxreg.h | 138 ++++
> 6 files changed, 1367 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core.txt
> create mode 100644 drivers/mfd/mlxreg-core.c
> create mode 100644 include/linux/platform_data/mlxreg.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core.txt b/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core.txt
> new file mode 100644
> index 0000000..f8c776a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core.txt
> @@ -0,0 +1,367 @@
> +Mellanox programmable device control.
> +-------------------------------------
> +This binding defines the device control interface over I2C bus for Mellanox BMC
> +based switches.
> +
> +Required properties:
> +- compatible = "mellanox,mlxreg-i2c" or
> + "mellanox,mlxreg-i2c-16"
> +
> +- #address-cells : must be 1;
> +- #size-cells : must be 0;
> +- reg : I2C address;
> +
> +Optional properties:
> +- interrupt-parent : phandle of parent interrupt controller;
> +- interrupts : interrupt line;
> +- deferred : I2C deferred bus phandle;
> + I2C bus activation order enforce for the cases when hot-plug
> + devices are attached to I2C bus, which is initialized after the
> + I2C bus, where programmable device is attached;
> +- cell : top aggregation register offset;
> +- mask : top aggregation register mask;
> +
> +Optional nodes and their properties:
> + - psu : power supply unit nodes:
> + Required properties:
> + - aggr : effective bit in aggregation mask;
> + - reg : register offset for all group members;
> + - mask : register mask;
> + - phandles - list of relevant device nodes;
> + - fan : fan unit nodes:
> + Required properties:
> + - aggr : effective bit in aggregation mask;
> + - reg : register offset for all group members;
> + - mask : register mask;
> + - phandles - list of relevant device nodes;
> + - pwr : power cable nodes:
> + Required properties:
> + - aggr : effective bit in aggregation mask;
> + - reg : register offset for all group members;
> + - mask : register mask;
> + - phandles : list of relevant device nodes;
> + - host : host side nodes (CPU host side for BMC):
> + Required properties:
> + - aggr : effective bit in aggregation mask;
> + - reg : register offset for all group members;
> + - mask : register mask;
> + - phandles : list of relevant device nodes;
> + - asic : asic nodes:
> + Required properties:
> + - aggr : effective bit in aggregation mask;
> + - regs : register offsets array per group member;
> + - masks : register masks array per group member;
> + - phandles : list of relevant device nodes;
> + - reset : reset nodes:
> + Required properties:
> + - #address-cells : must be 1;
> + - #size-cells : must be 0;
> + - reset control subnodes:
> + Required properties:
> + - reg : register offset;
> + - mask : attribute mask;
> + - cause - reset cause nodes:
> + Required properties:
> + - #address-cells : must be 1;
> + - #size-cells : must be 0;
> + - cause info subnodes:
> + Required properties:
> + - reg : register offset;
> + - mask : attribute mask;
> + - mux - mux nodes:
> + Required properties:
> + - #address-cells : must be 1;
> + - #size-cells : must be 0;
> + - mux control subnodes:
> + Required properties:
> + - reg : register offset;
> + - mask : attribute mask;
> + - bit : effective bit;
> + Optional property:
> + - phandles : relevant device node;
> + - gprw - general purpose register nodes:
> + Required properties:
> + - #address-cells : must be 1;
> + - #size-cells : must be 0;
> + - general purpose read-write register subnodes:
> + Required properties:
> + - reg : register offset;
> + - mask : attribute mask;
> + - gpro - general purpose register nodes:
> + Required properties:
> + - #address-cells : must be 1;
> + - #size-cells : must be 0;
> + - general purpose read only register subnodes:
> + Required properties:
> + - reg : register offset.
> + - mask : attribute mask.
> + - led - led nodes:
> + Required properties:
> + - #address-cells : must be 1;
> + - #size-cells : must be 0;
> + - led control nodes:
> + Required properties:
> + - label : symbolic name;
> + - reg : register offset;
> + - mask : attribute mask;
> +
> +Example:
> + mlxcpld-mng-ctrl@71 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupt-parent = <&gpio>;
> + interrupts = <ASPEED_GPIO(S, 1) 2>;
> + compatible = "mellanox,mlxcpld-ctrl-i2c";
> + reg = <0x71>;
> + deferred = <&i2c6>;
> + cell = <0x3a>;
> + mask = <0x4c>;
> +
> + psu {
> + aggr = <0x08>;
> + reg = <0x58>;
> + mask = <0x03>;
> + phandles = <&psu1_eeprom &psu2_eeprom>;
> + };
> +
> + pwr {
> + aggr = <0x08>;
> + reg = <0x64>;
> + mask = <0x03>;
> + phandles = <&psu1_ctrl &psu2_ctrl>;
> + };
> +
> + fan {
> + aggr = <0x40>;
> + reg = <0x88>;
> + mask = <0x0f>;
> + phandles = <&fan1_eeprom &fan2_eeprom &fan3_eeprom
> + &fan4_eeprom>;
> + };
> +
> + mux {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + uart_sel {
> + reg = <0x30>;
> + mask = <0xef>;
> + bit = <0x00>;
> + };
> + spi_burn_bios_ci {
> + reg = <0x32>;
> + mask = <0xfe>;
> + bit = <0x00>;
> + phandles = <&spi2>;
> + };
> + spi_burn_ncsi {
> + reg = <0x32>;
You can't have duplicate reg values. This will also generate warnings.
Build your dtb with "W=2".
Again, this all looks too low level to me. You described the high level
differences in terms of number of cards, fans, PSUs, etc., but I don't
see that here. This isn't reviewable for anyone not familiar with your
systems.
> + mask = <0xfd>;
> + bit = <0x00>;
> + phandles = <&spi2>;
Not a good name other than giving type information. Name it for what
function is being provided (e.g. clocks, interrupts, gpios).
> + };
> + bmc_uart_en {
> + reg = <0x32>;
> + mask = <0xdf>;
> + bit = <0x00>;
> + };
> + safe_bios1 {
> + reg = <0x35>;
> + mask = <0xfb>;
> + bit = <0x01>;
> + };
> + safe_bios2 {
> + reg = <0x35>;
> + mask = <0xf7>;
> + bit = <0x01>;
> + };
> + safe_bios_disable {
> + reg = <0x35>;
> + mask = <0xdf>;
> + bit = <0x01>;
> + };
> + };
> +
> + led {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status-green {
> + reg = <0x20>;
> + mask = <0xf0>;
> + };
> + status-red {
> + reg = <0x20>;
> + mask = <0xf0>;
> + };
> + status-amber {
> + reg = <0x20>;
> + mask = <0xf0>;
> + };
> + fan1-green {
> + reg = <0x21>;
> + mask = <0xf0>;
> + };
> + fan1-red {
> + reg = <0x21>;
> + mask = <0xf0>;
> + };
> + fan2-green {
> + reg = <0x21>;
> + mask = <0x0f>;
> + };
> + fan2-red {
> + reg = <0x21>;
> + mask = <0x0f>;
> + };
> + fan3-green {
> + reg = <0x22>;
> + mask = <0xf0>;
> + };
> + fan3-red {
> + label = "fan3:red";
> + reg = <0x22>;
> + mask = <0xf0>;
> + };
> + fan4-green {
> + reg = <0x22>;
> + mask = <0x0f>;
> + };
> + fan4-red {
> + reg = <0x22>;
> + mask = <0x0f>;
> + };
> + };
> +
> + reset {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + bmc_reset_soft {
> + reg = <0x17>;
> + mask = <0xfd>;
> + };
> + system_reset_hard {
> + reg = <0x17>;
> + mask = <0xfe>;
> + };
> + cpu_reset_soft {
> + reg = <0x17>;
> + mask = <0xf7>;
> + };
> + ps1_on {
> + reg = <0x30>;
> + mask = <0xfe>;
> + };
> + ps2_on {
> + label = "ps2_on";
> + reg = <0x30>;
> + mask = <0xfd>;
> + };
> + sys_power_cycle {
> + reg = <0x30>;
> + mask = <0xfb>;
> + };
> + mng_pg_ovrd {
> + reg = <0x30>;
> + mask = <0xf7>;
> + };
> + cpu_reset_hard {
> + reg = <0x30>;
> + mask = <0xdf>;
> + };
> + };
> +
> + cause {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ac_power_cycle {
> + reg = <0x1d>;
> + mask = <0xfe>;
> + };
> + dc_power_cycle {
> + reg = <0x1d>;
> + mask = <0xfd>;
> + };
> + cpu_power_down {
> + reg = <0x1d>;
> + mask = <0xfb>;
> + };
> + cpu_reboot {
> + reg = <0x1d>;
> + mask = <0xf7>;
> + };
> + cpu_shutdown {
> + reg = <0x1d>;
> + mask = <0xef>;
> + };
> + cpu_watchdog {
> + reg = <0x1d>;
> + mask = <0xef>;
> + };
> + cpu_kernel_panic {
> + reg = <0x1d>;
> + mask = <0xef>;
> + };
> + bmc_warm_reset {
> + reg = <0x71>;
> + mask = <0xfe>;
> + };
> + bmc_upgrade {
> + reg = <0x2e>;
> + mask = <0xf7>;
> + };
> + };
> +
> + gpro {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + version {
> + reg = <0x00>;
> + mask = <0xff>;
> + bit = <0xff>;
> + };
> + };
> + };
> +
> + mlxcpld-swb-ctrl@72 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupt-parent = <&gpio>;
> + interrupts = <ASPEED_GPIO(S, 1) 2>;
> + compatible = "mellanox,mlxcpld-ctrl-i2c";
> + reg = <0x72>;
> + deferred = <&i2c12>;
> + cell = <0x40>;
> + mask = <0x01>;
> +
> + asic {
> + aggr = <0x01>;
> + regs = <0x50>;
> + masks = <0x03>;
> + phandles = <&asic_thermal>;
> + };
> +
> + gpro {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + version {
> + reg = <0x01>;
> + mask = <0xff>;
> + bit = <0xff>;
> + };
> + };
> +
> + reset {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + asic_reset {
> + reg = <0x19>;
> + mask = <0xf7>;
> + };
> + phy_reset {
> + reg = <0x19>;
> + mask = <0xef>;
> + };
> + };
> +
> + };
