Hi!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core
> @@ -0,0 +1,347 @@
> +Mellanox programmable device control.
> +-------------------------------------
> +This binding defines the device control interface over I2C bus for Mellanox BMC
> +based switches.
You'll need to cc dt maintainers here.
> +Required properties:
> +- compatible = "mellanox,mlxreg-i2c",
> + "mellanox,mlxreg-i2c-16",
> + "mellanox,mlxreg-core";
> +- #address-cells : must be 1;
> +- #size-cells : must be 0;
> +- reg : I2C address;
> + cpu_watchdog {
> + reg = <0x1d>;
> + mask = <0xef>;
> + };
> +
> + cpu_kernel_panic {
> + reg = <0x1d>;
> + mask = <0xef>;
> + };
> + bmc_warm_reset {
> + reg = <0x71>;
> + mask = <0xfe>;
> + };
Extra newline.
> +static ssize_t mlxreg_core_attr_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct mlxreg_core_priv_data *priv = dev_get_drvdata(dev);
> + int index = to_sensor_dev_attr_2(attr)->index;
> + int nr = to_sensor_dev_attr_2(attr)->nr;
> + struct mlxreg_core_data *data;
> + u32 regval = 0;
> + int ret;
> +
> + switch (nr) {
> + case MLXREG_CORE_ATTR_PSU:
> + case MLXREG_CORE_ATTR_FAN:
> + /* Bit = 0 : item is functional. */
> + ret = regmap_read(priv->regmap,
> + priv->item[mlxreg_core_grp[nr].type]->reg,
> + ®val);
> + if (ret)
> + goto access_error;
> +
> + regval = !!!(regval & BIT(mlxreg_core_get_pos(priv,
> + &mlxreg_core_grp[nr],
Uff. You really really mean it. But one ! should be enough.
> +static int
> +mlxreg_core_add_attr_group(struct mlxreg_core_priv_data *priv,
> + struct mlxreg_core_grp *grp, int id)
> +{
> + struct mlxreg_core_data *data = priv->item[grp->type]->data;
> + int i;
> +
> + /* Skip group with negative access value. */
> + if (grp->access < 0)
> + return id;
> +
> + priv->item[grp->type]->ind = id;
> + for (i = 0; i < priv->item[grp->type]->count; i++, id++, data++) {
> + priv->mlxreg_core_attr[id] =
> + &priv->mlxreg_core_dev_attr[id].dev_attr.attr;
> + if (grp->use_grp_name)
> + priv->mlxreg_core_attr[id]->name =
> + devm_kasprintf(priv->dev, GFP_KERNEL,
> + "%s%u", grp->name, id %
> + priv->item[grp->type]->count
> + + 1);
> + else
> + priv->mlxreg_core_attr[id]->name =
> + devm_kasprintf(priv->dev, GFP_KERNEL,
> + data->label);
Is it really neccessary to dynamically generate it? Static table could
be simpler...
> +static void
> +mlxreg_core_work_helper(struct mlxreg_core_priv_data *priv,
> + struct mlxreg_core_item *item, u8 is_inverse,
> + u16 off, u32 mask, u32 *cache)
> +{
> + struct mlxreg_core_data *data;
> + u32 asserted, regval, bit;
> + int ret;
> +
> + /*
> + * Validate if item related to received signal type is valid.
> + * It should never happen, excepted the situation when some
> + * piece of hardware is broken. In such situation just produce
> + * error message and return. Caller must continue to handle the
> + * signals from other devices if any.
> + */
> + if (unlikely(!item)) {
> + dev_err(priv->dev, "False signal is received: register at offset 0x%02x, mask 0x%02x.\n",
> + off, mask);
Slightly too long line.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
