linux-kernel - Re: [patch v4 3/3] platform/mellanox: mlxreg-hotplug: modify to use regmap intreface

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHp75VfaU-GUvgFwx=+M-=DX2NRO9A3uQkxcmtY0FVRow+S9zA@mail.gmail.com>
Date:   Tue, 17 Oct 2017 22:24:07 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Vadim Pasternak <vadimp@...lanox.com>
Cc:     "dvhart@...radead.org" <dvhart@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Jiří Pírko <jiri@...nulli.us>
Subject: Re: [patch v4 3/3] platform/mellanox: mlxreg-hotplug: modify to use
 regmap intreface

On Tue, Oct 17, 2017 at 9:22 PM, Vadim Pasternak <vadimp@...lanox.com> wrote:
> It includes:
> - restructuring  of mlxreg header file for unification of hotplug items
>   definition;
> - unification of hotplug items to allow any kind of item (power
>   controller, fan eeprom, psu eeprom, asic health in common way);
> - utilization of regmap interface to allow independency of underlying
>   hardware. It allows to support hotplug event functionality over
>   programmable devices logic, when these devices can be attached to
>   different interfaces types, like I2C, LPC, SPI.
>
> Signed-off-by: Vadim Pasternak <vadimp@...lanox.com>
> Acked-by: Andy Shevchenko <andy.shevchenko@...il.com>

> -#define PRIV_ATTR(i) priv->mlxreg_hotplug_attr[i]
> -#define PRIV_DEV_ATTR(i) priv->mlxreg_hotplug_dev_attr[i]

> -static int mlxreg_hotplug_attr_init(struct mlxreg_hotplug_priv_data *priv)
> +static int
> +mlxreg_hotplug_attr_init(struct mlxreg_hotplug_priv_data *priv)

This is kinda noisy change.

>  {

> -       for (i = 0; i < num_attrs; i++) {
> -               PRIV_ATTR(i) = &PRIV_DEV_ATTR(i).dev_attr.attr;
> -
> -               if (i < priv->plat->psu_count) {
> -                       PRIV_ATTR(i)->name = devm_kasprintf(&priv->pdev->dev,
> -                                               GFP_KERNEL, "psu%u", i + 1);
> -                       PRIV_DEV_ATTR(i).nr = MLXREG_HOTPLUG_ATTR_TYPE_PSU;
> -               } else if (i < priv->plat->psu_count + priv->plat->pwr_count) {
> -                       PRIV_ATTR(i)->name = devm_kasprintf(&priv->pdev->dev,
> -                                               GFP_KERNEL, "pwr%u", i %
> -                                               priv->plat->pwr_count + 1);
> -                       PRIV_DEV_ATTR(i).nr = MLXREG_HOTPLUG_ATTR_TYPE_PWR;
> -               } else {
> -                       PRIV_ATTR(i)->name = devm_kasprintf(&priv->pdev->dev,
> -                                               GFP_KERNEL, "fan%u", i %
> -                                               priv->plat->fan_count + 1);
> -                       PRIV_DEV_ATTR(i).nr = MLXREG_HOTPLUG_ATTR_TYPE_FAN;
> -               }
> -
> -               if (!PRIV_ATTR(i)->name) {
> -                       dev_err(&priv->pdev->dev, "Memory allocation failed for sysfs attribute %d.\n",
> -                               i + 1);
> -                       return -ENOMEM;
> +       for (i = 0; i < pdata->counter; i++, item++) {
> +               data = item->data;
> +               for (j = 0; j < item->count; j++, data++, id++) {
> +                       priv->mlxreg_hotplug_attr[id] =
> +                       &priv->mlxreg_hotplug_dev_attr[id].dev_attr.attr;
> +                       priv->mlxreg_hotplug_attr[id]->name =
> +                               devm_kasprintf(&priv->pdev->dev, GFP_KERNEL,
> +                                              data->label);
> +
> +                       if (!priv->mlxreg_hotplug_attr[id]->name) {
> +                               dev_err(priv->dev, "Memory allocation failed for attr %d.\n",
> +                                       id);
> +                               return -ENOMEM;
> +                       }
> +

> +                       priv->mlxreg_hotplug_dev_attr[id].dev_attr.attr.mode =
> +                                                                       0444;
> +                       priv->mlxreg_hotplug_dev_attr[id].dev_attr.show =
> +                                               mlxreg_hotplug_attr_show;
> +                       priv->mlxreg_hotplug_dev_attr[id].nr = i;
> +                       priv->mlxreg_hotplug_dev_attr[id].index = j;

Why you drop macros usage here?

>                 }
> -
> -               PRIV_DEV_ATTR(i).dev_attr.attr.name = PRIV_ATTR(i)->name;
> -               PRIV_DEV_ATTR(i).dev_attr.attr.mode = S_IRUGO;
> -               PRIV_DEV_ATTR(i).dev_attr.show = mlxreg_hotplug_attr_show;
> -               PRIV_DEV_ATTR(i).index = i;
> -               sysfs_attr_init(&PRIV_DEV_ATTR(i).dev_attr.attr);


>  #define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR         0x2000
>  #define MLXPLAT_CPLD_LPC_REG_BASE_ADRR         0x2500
> -#define MLXPLAT_CPLD_LPC_REG_AGGR_ADRR         0x253a
> -#define MLXPLAT_CPLD_LPC_REG_PSU_ADRR          0x2558
> -#define MLXPLAT_CPLD_LPC_REG_PWR_ADRR          0x2564
> -#define MLXPLAT_CPLD_LPC_REG_FAN_ADRR          0x2588

> +#define MLXPLAT_CPLD_LPC_REG_AGGR_ADRR         0x3a
> +#define MLXPLAT_CPLD_LPC_REG_PSU_ADRR          0x58
> +#define MLXPLAT_CPLD_LPC_REG_PWR_ADRR          0x64
> +#define MLXPLAT_CPLD_LPC_REG_FAN_ADRR          0x88

These now by the fact *offsets*.

-- 
With Best Regards,
Andy Shevchenko