Re: [patch v1] drivers/platform/x86: introduce support for Mellanox hotplug driver

[Darren Hart]

On Thu, Sep 22, 2016 at 01:14:27PM +0000, vadimp@xxxxxxxxxxxx wrote:
> From: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> 

Hi Vadim,

> Enable system support for the Mellanox Technologies hotplug platform
> driver, which provides support for the next Mellanox basic systems:
> "msx6710", "msx6720", "msb7700", "msn2700", "msx1410", "msn2410",
> "msb7800", "msn2740", "msn2100" and also various number of derivative
> systems from the above basic types.

Some context for those reviewing the code and not familiar with your product
line will help establish context. Those appear to be a mix of Infiniband and
Ethernet switches, typically with dual power supplies, and an x86 processor.

Correct?

> This driver handles hot-plug events for the power suppliers, power
> cables and fans for the above systems.
> 
> The Kconfig currently controlling compilation of this code is:
> driver/platform/x86:config MLX_CPLD_PLATFORM
>                        tristate "Mellanox platform hotplug driver support"

Regarding location and approach. Greg KH asked if there was another to
paramaterize the driver, such as ACPI. You mentioned you didn't have it. I was
curious to know - did you mean you that the Mellanox switch platform firmware
does not use ACPI at all or that you didn't have any ACPI description for this
hardware?

If you use ACPI on the platform, the ACPI _DSD mechanism can provide you with
these sorts of properties, allowing you to describe the platform fully to the
driver without the need for platform specific drivers.

+Rafael

Is there a user of this platform driver coming?

> 
> Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx>

> ---
>  MAINTAINERS                                   |   7 +
>  drivers/platform/x86/Kconfig                  |  10 +
>  drivers/platform/x86/Makefile                 |   1 +
>  drivers/platform/x86/mlxcpld-hotplug.c        | 543 ++++++++++++++++++++++++++
>  include/linux/platform_data/mlxcpld-hotplug.h |  90 +++++
>  5 files changed, 651 insertions(+)
>  create mode 100644 drivers/platform/x86/mlxcpld-hotplug.c
>  create mode 100644 include/linux/platform_data/mlxcpld-hotplug.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a306795..cbfe2a8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7649,6 +7649,13 @@ W:	http://www.mellanox.com
>  Q:	http://patchwork.ozlabs.org/project/netdev/list/
>  F:	drivers/net/ethernet/mellanox/mlxsw/
>  
> +MELLANOX MLX CPLD HOTPLUG DRIVER
> +M:	Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> +L:	platform-driver-x86@xxxxxxxxxxxxxxx
> +S:	Supported
> +F:	drivers/platform/x86/mlxcpld-hotplug.c
> +F:	include/linux/platform_data/mlxcpld-hotplug.h
> +
>  SOFT-ROCE DRIVER (rxe)
>  M:	Moni Shoua <monis@xxxxxxxxxxxx>
>  L:	linux-rdma@xxxxxxxxxxxxxxx
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 81b8dcc..e9be541 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1026,4 +1026,14 @@ config INTEL_TELEMETRY
>  	  used to get various SoC events and parameters
>  	  directly via debugfs files. Various tools may use
>  	  this interface for SoC state monitoring.
> +
> +config MLX_CPLD_PLATFORM
> +        tristate "Mellanox platform hotplug driver support"
> +        default n

Please be consistent with whitespace usage with what is in the file and indent
with tabs.

> +	depends on MLX_PLATFORM
> +	select I2C
> +        ---help---
> +	  This driver handles hot-plug events for the power suppliers, power
> +	  cables and fans on the wide range Mellanox IB and Ethernet systems.
> +
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2efa86d..1f06b63 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -71,3 +71,4 @@ obj-$(CONFIG_INTEL_TELEMETRY)	+= intel_telemetry_core.o \
>  				   intel_telemetry_pltdrv.o \
>  				   intel_telemetry_debugfs.o
>  obj-$(CONFIG_INTEL_PMC_CORE)    += intel_pmc_core.o
> +obj-$(CONFIG_MLX_CPLD_PLATFORM)	+= mlxcpld-hotplug.o
> diff --git a/drivers/platform/x86/mlxcpld-hotplug.c b/drivers/platform/x86/mlxcpld-hotplug.c
> new file mode 100644
> index 0000000..d53d845
> --- /dev/null
> +++ b/drivers/platform/x86/mlxcpld-hotplug.c
> @@ -0,0 +1,543 @@
> +/*
> + * drivers/platform/x86/mlxcpld-hotplug.c
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + *    contributors may be used to endorse or promote products derived from
> + *    this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_data/mlxcpld-hotplug.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/module.h>
> +#include <linux/wait.h>
> +#include <linux/workqueue.h>

Please scrub this list of includes and only include what you explicitly require
that isn't implicit in one of the others. module.h, for example, is listed
twice. I suspect a few of these could be eliminated.

> +
> +/* Offset of event and mask registers from status register */
> +#define MLXCPLD_HOTPLUG_EVENT_OFF	1
> +#define MLXCPLD_HOTPLUG_MASK_OFF	2
> +#define MLXCPLD_HOTPLUG_AGGR_MASK_OFF	1
> +
> +#define MLXCPLD_HOTPLUG_ATTRS_NUM	8
> +
> +enum mlxcpld_hotplug_attr_type {
> +	MLXCPLD_HOTPLUG_ATTR_TYPE_PSU,
> +	MLXCPLD_HOTPLUG_ATTR_TYPE_PWR,
> +	MLXCPLD_HOTPLUG_ATTR_TYPE_FAN,
> +};
> +
> +/* mlxcpld_hotplug_priv_data - platform private data:

Nit, always lead multi-line comment blocks with:

/*
 * First line...

More than one instance of this in the patch, please correct throughout.

> + * @irq - platform interrupt number;
> + * @pdev - platform device;
> + * @plat - platform data;
> + * @hwmon - hwmon device;
> + * @mlxcpld_hotplug_attr - sysfs attributes array;
> + * @mlxcpld_hotplug_dev_attr - sysfs sensor device attribute array;
> + * @group - sysfs attribute group;
> + * @groups - list of sysfs attribute group for hwmon registration;
> + * @dwork - delayed work template;
> + * @lock - spin lock;
> + * @aggr_cache - last value of aggregation register status;
> + * @psu_cache - last value of PSU register status;
> + * @pwr_cache - last value of power register status;
> + * @fan_cache - last value of FAN register status;
> + */
> +struct mlxcpld_hotplug_priv_data {
> +	int irq;
> +	struct platform_device *pdev;
> +	struct mlxcpld_hotplug_platform_data *plat;
> +	struct device *hwmon;
> +	struct attribute *mlxcpld_hotplug_attr[MLXCPLD_HOTPLUG_ATTRS_NUM + 1];
> +	struct sensor_device_attribute_2
> +			mlxcpld_hotplug_dev_attr[MLXCPLD_HOTPLUG_ATTRS_NUM];
> +	struct attribute_group group;
> +	const struct attribute_group *groups[2];
> +	struct delayed_work dwork;
> +	spinlock_t lock;
> +	u8 aggr_cache;
> +	u8 psu_cache;
> +	u8 pwr_cache;
> +	u8 fan_cache;
> +};
> +
> +static ssize_t mlxcpld_hotplug_attr_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct mlxcpld_hotplug_priv_data *priv = platform_get_drvdata(pdev);
> +	int index = to_sensor_dev_attr_2(attr)->index;
> +	int nr = to_sensor_dev_attr_2(attr)->nr;
> +	u8 reg_val = 0;
> +
> +	switch (nr) {
> +	case MLXCPLD_HOTPLUG_ATTR_TYPE_PSU:
> +		reg_val = inb(priv->plat->psu_reg_offset);
> +		reg_val &= BIT_MASK(index);

So BIT_MASK will mod by BITS_PER_LONG, but you're looking to mask a u8 value, so
would BIT() be just as effective (e.g. not). Is it valid for index to be larger
than the bits in the u8?

> +		reg_val = !!!reg_val; /* Bit = 0 : PSU is present. */

I guess it's a nit, but this seems like a lot of unnecessary breakdown when the
following accomplishes the same thing and isn't any less clear in my opinion.

		reg_val = !!!(inb(priv->plat->psu_reg_offset) & BIT(index));

> +		break;
> +
> +	case MLXCPLD_HOTPLUG_ATTR_TYPE_PWR:
> +		reg_val = inb(priv->plat->pwr_reg_offset);
> +		reg_val &= BIT_MASK(index % priv->plat->pwr_count);

When is it valid for index to need to be modded? Same for fan_count below. Is
this a valid use case, or something that should be checked and rejected earlier?

> +		reg_val = !!reg_val; /* Bit = 1 : power cable is attached. */
> +		break;
> +
> +	case MLXCPLD_HOTPLUG_ATTR_TYPE_FAN:
> +		reg_val = inb(priv->plat->fan_reg_offset);
> +		reg_val &= BIT_MASK(index % priv->plat->fan_count);
> +		reg_val = !!!reg_val; /* Bit = 0 : FAN is present. */
> +		break;
> +	}
> +
> +	return sprintf(buf, "%u\n", reg_val);
> +}
> +
> +static int mlxcpld_hotplug_attr_init(struct mlxcpld_hotplug_priv_data *priv)
> +{
> +	int num_attrs = priv->plat->psu_count + priv->plat->pwr_count +
> +			priv->plat->fan_count;
> +	int i;
> +
> +	priv->group.attrs = devm_kzalloc(&priv->pdev->dev, num_attrs *
> +					 sizeof(struct attribute *),
> +					 GFP_KERNEL);
> +	if (!priv->group.attrs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_attrs; i++) {
> +		priv->mlxcpld_hotplug_attr[i] =
> +			&priv->mlxcpld_hotplug_dev_attr[i].dev_attr.attr;
> +
> +		if (i < priv->plat->psu_count) {
> +			priv->mlxcpld_hotplug_attr[i]->name =
> +				devm_kasprintf(&priv->pdev->dev, GFP_KERNEL,
> +					       "psu%u", i + 1);
> +			priv->mlxcpld_hotplug_dev_attr[i].nr =
> +						MLXCPLD_HOTPLUG_ATTR_TYPE_PSU;


It seems this could be made more legible by doing on for loop for each the three
types (psu, pwr, and fan) and removing one level of indentation, possibly
factoring some of it out into a function to avoid duplicating the error checking
and sysfs dev_attr stuff. Once you've reached 8 levels of indentations.... it's
probably time to refactor a bit. Or perhaps a local alias for the longer names
like priv->mlxcpld_hotplug_dev_attr would do the trick.


> +		} else if (i < priv->plat->psu_count + priv->plat->pwr_count) {
> +			priv->mlxcpld_hotplug_attr[i]->name =
> +				devm_kasprintf(&priv->pdev->dev, GFP_KERNEL,
> +					       "pwr%u", i %
> +						priv->plat->pwr_count + 1);
> +			priv->mlxcpld_hotplug_dev_attr[i].nr =
> +						MLXCPLD_HOTPLUG_ATTR_TYPE_PWR;
> +		} else {
> +			priv->mlxcpld_hotplug_attr[i]->name =
> +				devm_kasprintf(&priv->pdev->dev, GFP_KERNEL,
> +					       "fan%u", i %
> +						priv->plat->fan_count + 1);
> +			priv->mlxcpld_hotplug_dev_attr[i].nr =
> +						MLXCPLD_HOTPLUG_ATTR_TYPE_FAN;
> +		}
> +
> +		if (!priv->mlxcpld_hotplug_attr[i]->name) {
> +			dev_err(&priv->pdev->dev, "Memory allocation failed for sysfs attribute %d.\n",
> +				i + 1);
> +			return -ENOMEM;
> +		}
> +
> +		priv->mlxcpld_hotplug_dev_attr[i].dev_attr.attr.name =
> +			priv->mlxcpld_hotplug_attr[i]->name;
> +		priv->mlxcpld_hotplug_dev_attr[i].dev_attr.attr.mode =
> +								S_IRUGO;
> +		priv->mlxcpld_hotplug_dev_attr[i].dev_attr.show =
> +						mlxcpld_hotplug_attr_show;
> +		priv->mlxcpld_hotplug_dev_attr[i].index = i;
> +		sysfs_attr_init(
> +		&priv->mlxcpld_hotplug_dev_attr[i].dev_attr.attr);
> +	}
> +
> +	priv->group.attrs = priv->mlxcpld_hotplug_attr;
> +	priv->groups[0] = &priv->group;
> +	priv->groups[1] = NULL;
> +
> +	return 0;
> +}
> +
> +static int mlxcpld_hotplug_device_create(struct device *dev,
> +					 struct mlxcpld_hotplug_device *item)
> +{
> +	item->adapter = i2c_get_adapter(item->bus);
> +	if (!item->adapter) {
> +		dev_err(dev, "Failed to get adapter for bus %d\n",
> +			item->bus);
> +		return -EFAULT;
> +	}
> +
> +	item->client = i2c_new_device(item->adapter, &item->brdinfo);
> +	if (!item->client) {
> +		dev_err(dev, "Failed to create client %s at bus %d at addr 0x%02x\n",
> +			item->brdinfo.type, item->bus, item->brdinfo.addr);
> +		i2c_put_adapter(item->adapter);
> +		item->adapter = NULL;
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> +
> +static void mlxcpld_hotplug_device_destroy(struct mlxcpld_hotplug_device *item)
> +{
> +	if (item->client) {
> +		i2c_unregister_device(item->client);
> +		item->client = NULL;
> +	}
> +
> +	if (item->adapter) {
> +		i2c_put_adapter(item->adapter);
> +		item->adapter = NULL;
> +	}
> +}
> +
> +/* mlxcpld_hotplug_work_handler - performs traversing of CPLD interrupt
> + * registers according to the below hierarchy schema:
> + *
> + *                   Aggregation registers (status/mask)
> + * PSU registers:           *---*
> + * *-----------------*      |   |
> + * |status/event/mask|----->| * |
> + * *-----------------*      |   |
> + * Power registers:         |   |
> + * *-----------------*      |   |
> + * |status/event/mask|----->| * |---> CPU
> + * *-----------------*      |   |
> + * FAN registers:
> + * *-----------------*      |   |
> + * |status/event/mask|----->| * |
> + * *-----------------*      |   |
> + *                          *---*
> + * In case some system changed are detected: FAN in/out, PSU in/out, power
> + * cable attached/detached, relevant device is created or destroyed.
> + */
> +static void mlxcpld_hotplug_work_handler(struct work_struct *work)
> +{
> +	struct mlxcpld_hotplug_priv_data *priv = container_of(work,
> +				struct mlxcpld_hotplug_priv_data, dwork.work);
> +	u8 val, aggr_asserted, asserted;
> +	unsigned long flags;
> +	int bit;
> +
> +	/* Mask aggregation event. */
> +	outb(0, priv->plat->top_aggr_offset + MLXCPLD_HOTPLUG_AGGR_MASK_OFF);
> +	/* Read aggregation status. */
> +	val = inb(priv->plat->top_aggr_offset);
> +	val &= priv->plat->top_aggr_mask;
> +	aggr_asserted = priv->aggr_cache ^ val;
> +	priv->aggr_cache = val;
> +
> +	if (aggr_asserted & priv->plat->top_aggr_psu_mask) {
> +		/* Mask psu presense event. */
> +		outb(0, priv->plat->psu_reg_offset + MLXCPLD_HOTPLUG_MASK_OFF);
> +		/* Read psu presense status. */
> +		val = inb(priv->plat->psu_reg_offset);
> +		val &= priv->plat->psu_mask;
> +		asserted = priv->psu_cache ^ val;
> +		priv->psu_cache = val;
> +
> +		if (priv->plat->psu) {
> +			for_each_set_bit(bit, (unsigned long *)&asserted, 8) {
> +				if (val & BIT(bit))
> +					mlxcpld_hotplug_device_destroy(
> +							priv->plat->psu + bit);
> +				else
> +					mlxcpld_hotplug_device_create(
> +							&priv->pdev->dev,
> +							priv->plat->psu + bit);
> +			}
> +		}
> +
> +		/* Acknowledge psu presense event. */
> +		outb(0, priv->plat->psu_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +		/* Unmask psu presense event. */
> +		outb(priv->plat->psu_mask, priv->plat->psu_reg_offset +
> +						MLXCPLD_HOTPLUG_MASK_OFF);
> +
> +	}
> +
> +	if (aggr_asserted & priv->plat->top_aggr_pwr_mask) {
> +		/* Mask power cable event. */
> +		outb(0, priv->plat->pwr_reg_offset + MLXCPLD_HOTPLUG_MASK_OFF);
> +		/* Read power cable status. */
> +		val = inb(priv->plat->pwr_reg_offset);
> +		val &= priv->plat->pwr_mask;
> +		asserted = priv->pwr_cache ^ val;
> +		priv->pwr_cache = val;
> +
> +		if (priv->plat->pwr) {
> +			for_each_set_bit(bit, (unsigned long *)&asserted, 8) {
> +				if (val & BIT(bit))
> +					mlxcpld_hotplug_device_create(
> +							&priv->pdev->dev,
> +							priv->plat->pwr + bit);
> +				else
> +					mlxcpld_hotplug_device_destroy(
> +							priv->plat->pwr + bit);
> +			}
> +		}
> +
> +		/* Acknowledge power cable event. */
> +		outb(0, priv->plat->pwr_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +		/* Unmask power cable event */
> +		outb(priv->plat->pwr_mask, priv->plat->pwr_reg_offset +
> +						MLXCPLD_HOTPLUG_MASK_OFF);
> +
> +	}
> +
> +	if (aggr_asserted & priv->plat->top_aggr_fan_mask) {
> +		/* Mask fan presense event. */
> +		outb(0, priv->plat->fan_reg_offset + MLXCPLD_HOTPLUG_MASK_OFF);
> +		/* Read fan presense status. */
> +		val = inb(priv->plat->fan_reg_offset);
> +		val &= priv->plat->fan_mask;
> +		asserted = priv->fan_cache ^ val;
> +		priv->fan_cache = val;
> +
> +		if (priv->plat->fan) {
> +			for_each_set_bit(bit, (unsigned long *)&asserted, 8) {
> +				if (val & BIT(bit))
> +					mlxcpld_hotplug_device_destroy(
> +							priv->plat->fan + bit);
> +				else
> +					mlxcpld_hotplug_device_create(
> +							&priv->pdev->dev,
> +							priv->plat->fan + bit);
> +			}
> +		}
> +
> +		/* Acknowledge fan presense event. */
> +		outb(0, priv->plat->fan_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +		/* Unmask fan presense event. */
> +		outb(priv->plat->fan_mask, priv->plat->fan_reg_offset +
> +						MLXCPLD_HOTPLUG_MASK_OFF);
> +
> +		/* Unmask aggregation event (no need acknowledge). */
> +		outb(priv->plat->top_aggr_mask, priv->plat->top_aggr_offset +
> +						MLXCPLD_HOTPLUG_AGGR_MASK_OFF);
> +
> +`
> +		spin_lock_irqsave(&priv->lock, flags);
> +
> +		/*
> +		 * It is possible, that some signals have been inserted, while
> +		 * interrupt has been masked by mlxcpld_hotplug_work_handler.
> +		 * In this case such signals will be missed. In order to handle
> +		 * these signals delayed work is canceled and work task
> +		 * re-scheduled for immediate execution. It allows to handle
> +		 * missed signals, if any. In other case work handler just
> +		 * validates that no new signals have been received during
> +		 * masking.
> +		 */
> +		cancel_delayed_work(&priv->dwork);
> +		schedule_delayed_work(&priv->dwork, 0);
> +
> +		spin_unlock_irqrestore(&priv->lock, flags);
> +
> +		return;
> +	}
> +
> +	/* Unmask aggregation event (no need acknowledge). */
> +	outb(priv->plat->top_aggr_mask, priv->plat->top_aggr_offset +
> +						MLXCPLD_HOTPLUG_AGGR_MASK_OFF);
> +}
> +
> +static void mlxcpld_hotplug_set_irq(struct mlxcpld_hotplug_priv_data *priv)
> +{
> +	/* Clear psu presense event. */
> +	outb(0, priv->plat->psu_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +	/* Set psu initial status as mask and unmask psu event. */
> +	priv->psu_cache = priv->plat->psu_mask;
> +	outb(priv->plat->psu_mask, priv->plat->psu_reg_offset +
> +						MLXCPLD_HOTPLUG_MASK_OFF);
> +
> +	/* Clear power cable event. */
> +	outb(0, priv->plat->pwr_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +	/* Keep power initial status as zero and unmask power event. */
> +	outb(priv->plat->pwr_mask, priv->plat->pwr_reg_offset +
> +						MLXCPLD_HOTPLUG_MASK_OFF);
> +
> +	/* Clear fan presense event. */
> +	outb(0, priv->plat->fan_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +	/* Set fan initial status as mask and unmask fan event. */
> +	priv->fan_cache = priv->plat->fan_mask;
> +	outb(priv->plat->fan_mask, priv->plat->fan_reg_offset +
> +						MLXCPLD_HOTPLUG_MASK_OFF);
> +
> +	/* Keep aggregation initial status as zero and unmask events. */
> +	outb(priv->plat->top_aggr_mask, priv->plat->top_aggr_offset +
> +						MLXCPLD_HOTPLUG_AGGR_MASK_OFF);
> +
> +	/* Invoke work handler for initializing hot plug devices setting. */
> +	mlxcpld_hotplug_work_handler(&priv->dwork.work);
> +
> +	enable_irq(priv->irq);
> +}
> +
> +static void mlxcpld_hotplug_unset_irq(struct mlxcpld_hotplug_priv_data *priv)
> +{
> +	int i;
> +
> +	disable_irq(priv->irq);
> +	cancel_delayed_work_sync(&priv->dwork);
> +
> +	/* Mask aggregation event. */
> +	outb(0, priv->plat->top_aggr_offset + MLXCPLD_HOTPLUG_AGGR_MASK_OFF);
> +
> +	/* Mask psu presense event. */
> +	outb(0, priv->plat->psu_reg_offset + MLXCPLD_HOTPLUG_MASK_OFF);
> +	/* Clear psu presense event. */
> +	outb(0, priv->plat->psu_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +
> +	/* Mask power cable event. */
> +	outb(0, priv->plat->pwr_reg_offset + MLXCPLD_HOTPLUG_MASK_OFF);
> +	/* Clear power cable event. */
> +	outb(0, priv->plat->pwr_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +
> +	/* Mask fan presense event. */
> +	outb(0, priv->plat->fan_reg_offset + MLXCPLD_HOTPLUG_MASK_OFF);
> +	/* Clear fan presense event. */
> +	outb(0, priv->plat->fan_reg_offset + MLXCPLD_HOTPLUG_EVENT_OFF);
> +
> +	/* Remove all the attached devices. */
> +	for (i = 0; i < priv->plat->psu_count; i++)
> +		mlxcpld_hotplug_device_destroy(priv->plat->psu + i);
> +
> +	for (i = 0; i < priv->plat->pwr_count; i++)
> +		mlxcpld_hotplug_device_destroy(priv->plat->pwr + i);
> +
> +	for (i = 0; i < priv->plat->fan_count; i++)
> +		mlxcpld_hotplug_device_destroy(priv->plat->fan + i);
> +}
> +
> +static irqreturn_t mlxcpld_hotplug_irq_handler(int irq, void *dev)
> +{
> +	struct mlxcpld_hotplug_priv_data *priv =
> +				(struct mlxcpld_hotplug_priv_data *)dev;
> +
> +	/* Schedule work task for immediate execution.*/
> +	schedule_delayed_work(&priv->dwork, 0);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mlxcpld_hotplug_probe(struct platform_device *pdev)
> +{
> +	struct mlxcpld_hotplug_platform_data *pdata;
> +	struct mlxcpld_hotplug_priv_data *priv;
> +	int err;
> +
> +	pdata = dev_get_platdata(&pdev->dev);
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "Failed to get platform data.\n");
> +		return -EINVAL;
> +	}
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->pdev = pdev;
> +	priv->plat = pdata;
> +
> +	priv->irq = platform_get_irq(pdev, 0);
> +	if (priv->irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get platform irq: %d\n",
> +			priv->irq);
> +		return priv->irq;
> +	}
> +
> +	err = devm_request_irq(&pdev->dev, priv->irq,
> +				mlxcpld_hotplug_irq_handler, 0, pdev->name,
> +				priv);
> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to request irq: %d\n", err);
> +		return err;
> +	}
> +	disable_irq(priv->irq);
> +
> +	INIT_DELAYED_WORK(&priv->dwork, mlxcpld_hotplug_work_handler);
> +	spin_lock_init(&priv->lock);
> +
> +	err = mlxcpld_hotplug_attr_init(priv);
> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to allocate attributes: %d\n", err);
> +		return err;
> +	}
> +
> +	priv->hwmon = devm_hwmon_device_register_with_groups(&pdev->dev,
> +					"mlxcpld_hotplug", 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);
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	/* Perform initial interrupts setup. */
> +	mlxcpld_hotplug_set_irq(priv);
> +
> +	return 0;
> +}
> +
> +static int mlxcpld_hotplug_remove(struct platform_device *pdev)
> +{
> +	struct mlxcpld_hotplug_priv_data *priv = platform_get_drvdata(pdev);
> +
> +	/* Clean interrupts setup. */
> +	mlxcpld_hotplug_unset_irq(priv);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mlxcpld_hotplug_driver = {
> +	.driver = {
> +		.name = "mlxcpld-hotplug",
> +	},
> +	.probe = mlxcpld_hotplug_probe,
> +	.remove = mlxcpld_hotplug_remove,
> +};
> +
> +module_platform_driver(mlxcpld_hotplug_driver);
> +
> +MODULE_AUTHOR("Vadim Pasternak <vadimp@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Mellanox CPLD hotplug platform driver");
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_ALIAS("platform:mlxcpld-hotplug");
> diff --git a/include/linux/platform_data/mlxcpld-hotplug.h b/include/linux/platform_data/mlxcpld-hotplug.h
> new file mode 100644
> index 0000000..ebb942c
> --- /dev/null
> +++ b/include/linux/platform_data/mlxcpld-hotplug.h
> @@ -0,0 +1,90 @@
> +/*
> + * include/linux/platform_data/mlxcpld-hotplug.h
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + *    contributors may be used to endorse or promote products derived from
> + *    this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef __LINUX_PLATFORM_DATA_MLXCPLD_HOTPLUG_H
> +#define __LINUX_PLATFORM_DATA_MLXCPLD_HOTPLUG_H
> +
> +/* mlxcpld_hotplug_device - I2C device data:
> + * @adapter - ;
> + * @client - ;
> + * @brdinfo - ;
> + * @bus - ;

This comment blocks adds no information as it stands, please fill it out, or
remove it.

> + */
> +struct mlxcpld_hotplug_device {
> +	struct i2c_adapter *adapter;
> +	struct i2c_client *client;
> +	struct i2c_board_info brdinfo;
> +	u16 bus;
> +};
> +
> +/* mlxcpld_hotplug_platform_data - device platform data:
> + * @top_aggr_offset - offset of top aggregation interrupt register;
> + * @top_aggr_mask - top aggregation interrupt common mask;
> + * @top_aggr_psu_mask - top aggregation interrupt PSU mask;
> + * @psu_reg_offset - offset of PSU interrupt register;
> + * @psu_mask - PSU interrupt mask;
> + * @psu_count - number of equipped replaceable PSUs;
> + * @psu - pointer to PSU devices data array;
> + * @top_aggr_pwr_mask - top aggregation interrupt power mask;
> + * @pwr_reg_offset - offset of power interrupt register
> + * @pwr_mask - power interrupt mask;
> + * @pwr_count - number of power sources;
> + * @pwr - pointer to power devices data array;
> + * @top_aggr_fan_mask - top aggregation interrupt FAN mask;
> + * @fan_reg_offset - offset of FAN interrupt register;
> + * @fan_mask - FAN interrupt mask;
> + * @fan_count - number of equipped replaceable FANs;
> + * @fan - pointer to FAN devices data array;
> + */
> +struct mlxcpld_hotplug_platform_data {
> +	u16 top_aggr_offset;
> +	u8 top_aggr_mask;
> +	u8 top_aggr_psu_mask;
> +	u16 psu_reg_offset;
> +	u8 psu_mask;
> +	u8 psu_count;
> +	struct mlxcpld_hotplug_device *psu;
> +	u8 top_aggr_pwr_mask;
> +	u16 pwr_reg_offset;
> +	u8 pwr_mask;
> +	u8 pwr_count;
> +	struct mlxcpld_hotplug_device *pwr;
> +	u8 top_aggr_fan_mask;
> +	u16 fan_reg_offset;
> +	u8 fan_mask;
> +	u8 fan_count;
> +	struct mlxcpld_hotplug_device *fan;
> +};
> +
> +#endif /* __LINUX_PLATFORM_DATA_MLXCPLD_HOTPLUG_H */
> -- 
> 2.1.4
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center