All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features
@ 2018-03-27 10:02 Vadim Pasternak
  2018-03-27 10:02 ` [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device Vadim Pasternak
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

This patcheset includes:

Document fixes for mlxreg-hotplug driver and mlxreg header file.

Fix for the race condition in mlxreg-hotplug driver.

Adding support for ODM system types.

Activation of Mellanox LED driver from mlx-platform.

Introduction of new mlxreg-io driver.

Activation of mlxreg-io driver from mlx-platform.

Vadim Pasternak (7):
  platform_data/mlxreg: Document fixes for hotplug device
  platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private
    data
  platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work
    queue
  platform: mellanox: add new ODM system types to mlx-platform
  platform/x86: mlx-platform: Add LED platform driver activation
  platform/mellanox: Introduce support for Mellanox register access
    driver
  platform/x86: mlx-platform: Add mlxreg-io platform driver activation

 drivers/platform/mellanox/Kconfig          |  11 +
 drivers/platform/mellanox/Makefile         |   1 +
 drivers/platform/mellanox/mlxreg-hotplug.c |  23 +-
 drivers/platform/mellanox/mlxreg-io.c      | 221 ++++++++++++++
 drivers/platform/x86/mlx-platform.c        | 447 +++++++++++++++++++++++++++++
 include/linux/platform_data/mlxreg.h       |  66 ++++-
 6 files changed, 765 insertions(+), 4 deletions(-)
 create mode 100644 drivers/platform/mellanox/mlxreg-io.c

-- 
2.1.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-04-13 16:23   ` Darren Hart
  2018-03-27 10:02 ` [PATCH v1 2/7] platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private data Vadim Pasternak
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

Remove redunadant description of label in struct mlxreg_hotplug_device.

Change location of access_mode in struct mlxreg_hotplug_device.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 include/linux/platform_data/mlxreg.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index 2744cff..19f5cb61 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -58,11 +58,10 @@ struct mlxreg_hotplug_device {
  * struct mlxreg_core_data - attributes control data:
  *
  * @label: attribute label;
- * @label: attribute register offset;
  * @reg: attribute register;
  * @mask: attribute access mask;
- * @mode: access mode;
  * @bit: attribute effective bit;
+ * @mode: access mode;
  * @np - pointer to node platform associated with attribute;
  * @hpdev - hotplug device data;
  * @health_cntr: dynamic device health indication counter;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 2/7] platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private data
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
  2018-03-27 10:02 ` [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-04-13 16:27   ` Darren Hart
  2018-03-27 10:02 ` [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue Vadim Pasternak
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

Add missing description of dev, regmap, dwork_irq, after_probe in struct
mlxreg_hotplug_priv_data.

Remove dwork field from the structure mlxreg_hotplug_priv_data itself and
for the descriptions, since it is not used.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/platform/mellanox/mlxreg-hotplug.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
index ea9e7f4..b56953a 100644
--- a/drivers/platform/mellanox/mlxreg-hotplug.c
+++ b/drivers/platform/mellanox/mlxreg-hotplug.c
@@ -59,9 +59,11 @@
 /**
  * struct mlxreg_hotplug_priv_data - platform private data:
  * @irq: platform device interrupt number;
+ * @dev: basic device;
  * @pdev: platform device;
  * @plat: platform data;
- * @dwork: delayed work template;
+ * @regmap: register map handle;
+ * @dwork_irq: delayed work template;
  * @lock: spin lock;
  * @hwmon: hwmon device;
  * @mlxreg_hotplug_attr: sysfs attributes array;
@@ -71,6 +73,7 @@
  * @cell: location of top aggregation interrupt register;
  * @mask: top aggregation interrupt common mask;
  * @aggr_cache: last value of aggregation register status;
+ * @after_probe: flag indication probing completion;
  */
 struct mlxreg_hotplug_priv_data {
 	int irq;
@@ -79,7 +82,6 @@ struct mlxreg_hotplug_priv_data {
 	struct mlxreg_hotplug_platform_data *plat;
 	struct regmap *regmap;
 	struct delayed_work dwork_irq;
-	struct delayed_work dwork;
 	spinlock_t lock; /* sync with interrupt */
 	struct device *hwmon;
 	struct attribute *mlxreg_hotplug_attr[MLXREG_HOTPLUG_ATTRS_MAX + 1];
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
  2018-03-27 10:02 ` [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device Vadim Pasternak
  2018-03-27 10:02 ` [PATCH v1 2/7] platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private data Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-04-13 16:47   ` Darren Hart
  2018-03-27 10:02 ` [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform Vadim Pasternak
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

It adds missed logic for signal acknowledge, by adding an extra run for
work queue in case a signal is received, but no specific signal assertion
is detected. Such case theoretically can happen for example in case
several units are removed or inserted at the same time. In this situation
acknowledge for some signal can be missed at signal top aggreagation
status level. The extra run will allow to handler to acknowledge the
missed signal.

The interrupt handling flow performs the next steps:
(1)
Enter mlxreg_hotplug_work_handler due to signal assertion.
Aggregation status register is changed for example from 0xff to 0xfd
(event signal group related to bit 1).
(2)
Mask aggregation interrupts, read aggregation status register and save it
(0xfd) in aggr_cache, then traverse down to handle signal from groups
related to the changed bit.
(3)
Read and mask group related signal.
Acknowledge and unmask group related signal (acknowledge should clear
aggregation status register from 0xfd back to 0xff).
(4)
Re-schedule work queue for the immediate execution.
(5)
Enter mlxreg_hotplug_work_handler due to re-scheduling.
Aggregation status is changed from previous 0xfd to 0xff.
Go over steps (2) - (5) and in case no new signal assertion
is detected - unmask aggregation interrupts.

The possible race could happen in case new signal from the same group is
asserted after step (3) and prior step (5). In such case aggregation
status will change back from 0xff to 0xfd and the value read from the
aggregation status register will be the same as a value saved in
aggr_cache. As a result the handler will not traverse down and signal
will stay unhandled.
The fix will enforce handler to traverse down in case the signal is
received, but signal assertion is not detected.

Fixes: 1f976f6978bf ("platform/x86: Move Mellanox platform hotplug driver to platform/mellanox")
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/platform/mellanox/mlxreg-hotplug.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
index b56953a..ced81b7 100644
--- a/drivers/platform/mellanox/mlxreg-hotplug.c
+++ b/drivers/platform/mellanox/mlxreg-hotplug.c
@@ -55,6 +55,7 @@
 #define MLXREG_HOTPLUG_RST_CNTR		3
 
 #define MLXREG_HOTPLUG_ATTRS_MAX	24
+#define MLXREG_HOTPLUG_NOT_ASSERT	3
 
 /**
  * struct mlxreg_hotplug_priv_data - platform private data:
@@ -74,6 +75,7 @@
  * @mask: top aggregation interrupt common mask;
  * @aggr_cache: last value of aggregation register status;
  * @after_probe: flag indication probing completion;
+ * @not_asserted: number of entries in workqueue with no signal assertion;
  */
 struct mlxreg_hotplug_priv_data {
 	int irq;
@@ -93,6 +95,7 @@ struct mlxreg_hotplug_priv_data {
 	u32 mask;
 	u32 aggr_cache;
 	bool after_probe;
+	u8 not_asserted;
 };
 
 static int mlxreg_hotplug_device_create(struct mlxreg_hotplug_priv_data *priv,
@@ -410,6 +413,18 @@ static void mlxreg_hotplug_work_handler(struct work_struct *work)
 	aggr_asserted = priv->aggr_cache ^ regval;
 	priv->aggr_cache = regval;
 
+	/*
+	 * Handler is invoked, but no assertion is detected at top aggregation
+	 * status level. Set aggr_asserted to mask value to allow handler extra
+	 * run over all relevant signals to recover any missed signal.
+	 */
+	if (priv->not_asserted == MLXREG_HOTPLUG_NOT_ASSERT) {
+		priv->not_asserted = 0;
+		aggr_asserted = pdata->mask;
+	}
+	if (!aggr_asserted)
+		goto unmask_event;
+
 	/* Handle topology and health configuration changes. */
 	for (i = 0; i < pdata->counter; i++, item++) {
 		if (aggr_asserted & item->aggr_mask) {
@@ -441,6 +456,8 @@ static void mlxreg_hotplug_work_handler(struct work_struct *work)
 		return;
 	}
 
+unmask_event:
+	priv->not_asserted++;
 	/* Unmask aggregation event (no need acknowledge). */
 	ret = regmap_write(priv->regmap, pdata->cell +
 			   MLXREG_HOTPLUG_AGGR_MASK_OFF, pdata->mask);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
                   ` (2 preceding siblings ...)
  2018-03-27 10:02 ` [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-04-13 16:54   ` Darren Hart
  2018-03-27 10:02 ` [PATCH v1 5/7] platform/x86: mlx-platform: Add LED platform driver activation Vadim Pasternak
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

Patch adds new ODM systems, matched according to DMI_BOARD_NAME.
The supported ODM Ids are: VMOD0001, VMOD0002, VMOD0003, VMOD0004,
VMOD0005. It doesn't introduce new systems, but allows to
ODM companies to set DMI_BOARD_VENDOR and DMI_PRODUCT_NAME on
their own. It assumes that ODM company can't change DMI_BOARD_NAME.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/platform/x86/mlx-platform.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 7a0bd24..912f844 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -844,6 +844,36 @@ static const struct dmi_system_id mlxplat_dmi_table[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "SN34"),
 		},
 	},
+	{
+		.callback = mlxplat_dmi_default_matched,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "VMOD0001"),
+		},
+	},
+	{
+		.callback = mlxplat_dmi_msn21xx_matched,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "VMOD0002"),
+		},
+	},
+	{
+		.callback = mlxplat_dmi_msn274x_matched,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "VMOD0003"),
+		},
+	},
+	{
+		.callback = mlxplat_dmi_msn201x_matched,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "VMOD0004"),
+		},
+	},
+	{
+		.callback = mlxplat_dmi_qmb7xx_matched,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "VMOD0005"),
+		},
+	},
 	{ }
 };
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 5/7] platform/x86: mlx-platform: Add LED platform driver activation
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
                   ` (3 preceding siblings ...)
  2018-03-27 10:02 ` [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-03-27 10:02 ` [PATCH v1 6/7] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

This patch adds:
- Support of Mellanox LED driver leds-mlxreg activation from mlx-platform.
  This LED driver uses the same regmap infrastructure as others Mellanox
  platform drivers.
- LED specific registers description.
- Per system type LED description, which is passed to LED driver.
- Static inline functions for adding and removing platform drivers sharing
  the same regmap infrastructure.
  Motivation of adding them as a static inline to the header file is to
  allow reusing of them by not only x86 architecture specific platform
  driver.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/platform/x86/mlx-platform.c  | 290 +++++++++++++++++++++++++++++++++++
 include/linux/platform_data/mlxreg.h |  63 ++++++++
 2 files changed, 353 insertions(+)

diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 912f844..efb605a 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -47,6 +47,11 @@
 /* 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_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_AGGR_OFFSET	0x3a
 #define MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET	0x3b
 #define MLXPLAT_CPLD_LPC_REG_AGGRLO_OFFSET	0x40
@@ -84,6 +89,8 @@
 #define MLXPLAT_CPLD_PWR_MASK		GENMASK(1, 0)
 #define MLXPLAT_CPLD_FAN_MASK		GENMASK(3, 0)
 #define MLXPLAT_CPLD_FAN_NG_MASK	GENMASK(5, 0)
+#define MLXPLAT_CPLD_LED_LO_NIBBLE_MASK	GENMASK(7, 4)
+#define MLXPLAT_CPLD_LED_HI_NIBBLE_MASK	GENMASK(3, 0)
 
 /* Default I2C parent bus number */
 #define MLXPLAT_CPLD_PHYS_ADAPTER_DEF_NR	1
@@ -114,11 +121,13 @@
  * @pdev_i2c - i2c controller platform device
  * @pdev_mux - array of mux platform devices
  * @pdev_hotplug - hotplug platform devices
+ * @platform_items - platform devices container
  */
 struct mlxplat_priv {
 	struct platform_device *pdev_i2c;
 	struct platform_device *pdev_mux[MLXPLAT_CPLD_LPC_MUX_DEVS];
 	struct platform_device *pdev_hotplug;
+	struct mlxreg_core_platform_drivers *platform_items;
 };
 
 /* Regions for LPC I2C controller and LPC base register space */
@@ -592,9 +601,251 @@ struct mlxreg_core_hotplug_platform_data mlxplat_mlxcpld_default_ng_data = {
 	.mask_low = MLXPLAT_CPLD_LOW_AGGR_MASK_LOW,
 };
 
+/* Platform led default data */
+static struct mlxreg_core_data mlxplat_mlxcpld_default_led_data[] = {
+	{
+		.label = "status:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "status:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK
+	},
+	{
+		.label = "psu:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "psu:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan1:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan1:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan2:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan2:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan3:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan3:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan4:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan4:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+};
+
+static struct mlxreg_core_platform_data mlxplat_default_led_data = {
+		.data = mlxplat_mlxcpld_default_led_data,
+		.counter = ARRAY_SIZE(mlxplat_mlxcpld_default_led_data),
+};
+
+/* Platform led MSN21xx system family data */
+static struct mlxreg_core_data mlxplat_mlxcpld_msn21xx_led_data[] = {
+	{
+		.label = "status:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "status:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK
+	},
+	{
+		.label = "fan:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "psu1:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "psu1:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "psu2:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "psu2:red",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "uid:blue",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED5_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+};
+
+static struct mlxreg_core_platform_data mlxplat_msn21xx_led_data = {
+		.data = mlxplat_mlxcpld_msn21xx_led_data,
+		.counter = ARRAY_SIZE(mlxplat_mlxcpld_msn21xx_led_data),
+};
+
+/* Platform led for default data for 200GbE systems */
+static struct mlxreg_core_data mlxplat_mlxcpld_default_ng_led_data[] = {
+	{
+		.label = "status:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "status:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK
+	},
+	{
+		.label = "psu:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "psu:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED1_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan1:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan1:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan2:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan2:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED2_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan3:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan3:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan4:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan4:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED3_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan5:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan5:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_LO_NIBBLE_MASK,
+	},
+	{
+		.label = "fan6:green",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+	{
+		.label = "fan6:orange",
+		.reg = MLXPLAT_CPLD_LPC_REG_LED4_OFFSET,
+		.mask = MLXPLAT_CPLD_LED_HI_NIBBLE_MASK,
+	},
+};
+
+static struct mlxreg_core_platform_data mlxplat_default_ng_led_data = {
+		.data = mlxplat_mlxcpld_default_ng_led_data,
+		.counter = ARRAY_SIZE(mlxplat_mlxcpld_default_ng_led_data),
+};
+
+/* Core platform devices */
+static struct mlxreg_core_platform_driver
+			mlxplat_mlxcpld_default_core_platform_drivers[] = {
+	{
+		.name = "leds-mlxreg",
+		.mlxreg_core_pdata = &mlxplat_default_led_data,
+	},
+};
+
+static struct mlxreg_core_platform_driver
+			mlxplat_mlxcpld_msn21xx_core_platform_drivers[] = {
+	{
+		.name = "leds-mlxreg",
+		.mlxreg_core_pdata = &mlxplat_msn21xx_led_data,
+	},
+};
+
+static struct mlxreg_core_platform_driver
+			mlxplat_mlxcpld_default_ng_core_platform_drivers[] = {
+	{
+		.name = "leds-mlxreg",
+		.mlxreg_core_pdata = &mlxplat_default_ng_led_data,
+	},
+};
+
 static bool mlxplat_mlxcpld_writeable_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
+	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_AGGR_MASK_OFFSET:
 	case MLXPLAT_CPLD_LPC_REG_AGGRLO_MASK_OFFSET:
 	case MLXPLAT_CPLD_LPC_REG_PSU_EVENT_OFFSET:
@@ -611,6 +862,11 @@ 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_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_AGGR_OFFSET:
 	case MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET:
 	case MLXPLAT_CPLD_LPC_REG_AGGRLO_OFFSET:
@@ -632,6 +888,11 @@ 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_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_AGGR_OFFSET:
 	case MLXPLAT_CPLD_LPC_REG_AGGR_MASK_OFFSET:
 	case MLXPLAT_CPLD_LPC_REG_AGGRLO_OFFSET:
@@ -692,6 +953,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_drivers mlxplat_platform_devs;
 
 static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi)
 {
@@ -705,6 +967,10 @@ static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi)
 	mlxplat_hotplug = &mlxplat_mlxcpld_default_data;
 	mlxplat_hotplug->deferred_nr =
 		mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
+	mlxplat_platform_devs.pdrv =
+			mlxplat_mlxcpld_default_core_platform_drivers;
+	mlxplat_platform_devs.pdrvs_num =
+		ARRAY_SIZE(mlxplat_mlxcpld_default_core_platform_drivers);
 
 	return 1;
 };
@@ -721,6 +987,10 @@ static int __init mlxplat_dmi_msn21xx_matched(const struct dmi_system_id *dmi)
 	mlxplat_hotplug = &mlxplat_mlxcpld_msn21xx_data;
 	mlxplat_hotplug->deferred_nr =
 		mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
+	mlxplat_platform_devs.pdrv =
+			mlxplat_mlxcpld_msn21xx_core_platform_drivers;
+	mlxplat_platform_devs.pdrvs_num =
+		ARRAY_SIZE(mlxplat_mlxcpld_msn21xx_core_platform_drivers);
 
 	return 1;
 };
@@ -737,6 +1007,10 @@ static int __init mlxplat_dmi_msn274x_matched(const struct dmi_system_id *dmi)
 	mlxplat_hotplug = &mlxplat_mlxcpld_msn274x_data;
 	mlxplat_hotplug->deferred_nr =
 		mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
+	mlxplat_platform_devs.pdrv =
+			mlxplat_mlxcpld_default_core_platform_drivers;
+	mlxplat_platform_devs.pdrvs_num =
+		ARRAY_SIZE(mlxplat_mlxcpld_default_core_platform_drivers);
 
 	return 1;
 };
@@ -753,6 +1027,10 @@ static int __init mlxplat_dmi_msn201x_matched(const struct dmi_system_id *dmi)
 	mlxplat_hotplug = &mlxplat_mlxcpld_msn201x_data;
 	mlxplat_hotplug->deferred_nr =
 		mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
+	mlxplat_platform_devs.pdrv =
+			mlxplat_mlxcpld_msn21xx_core_platform_drivers;
+	mlxplat_platform_devs.pdrvs_num =
+		ARRAY_SIZE(mlxplat_mlxcpld_msn21xx_core_platform_drivers);
 
 	return 1;
 };
@@ -769,6 +1047,10 @@ static int __init mlxplat_dmi_qmb7xx_matched(const struct dmi_system_id *dmi)
 	mlxplat_hotplug = &mlxplat_mlxcpld_default_ng_data;
 	mlxplat_hotplug->deferred_nr =
 		mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
+	mlxplat_platform_devs.pdrv =
+			mlxplat_mlxcpld_default_ng_core_platform_drivers;
+	mlxplat_platform_devs.pdrvs_num =
+		ARRAY_SIZE(mlxplat_mlxcpld_default_ng_core_platform_drivers);
 
 	return 1;
 };
@@ -990,6 +1272,13 @@ static int __init mlxplat_init(void)
 		goto fail_platform_mux_register;
 	}
 
+	/* Add platform drivers. */
+	err = mlxreg_core_add_platform_drivers(&mlxplat_dev->dev,
+					       &mlxplat_platform_devs,
+					       mlxplat_hotplug->regmap);
+	if (err)
+		goto fail_platform_hotplug_register;
+
 	/* Sync registers with hardware. */
 	regcache_mark_dirty(mlxplat_hotplug->regmap);
 	err = regcache_sync(mlxplat_hotplug->regmap);
@@ -1016,6 +1305,7 @@ static void __exit mlxplat_exit(void)
 	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
 	int i;
 
+	mlxreg_core_remove_platform_drivers(&mlxplat_platform_devs);
 	platform_device_unregister(priv->pdev_hotplug);
 
 	for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index 19f5cb61..efc30a7 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -144,4 +144,67 @@ struct mlxreg_core_hotplug_platform_data {
 	int shift_nr;
 };
 
+/* mlxreg_core_platform_driver - platform driver specification
+ * @name - platform driver name;
+ * @regmap - register map shared between all the drivers;
+ * @pdev - platform device;
+ * @mlxreg_core_pdata - platform driver data;
+ */
+struct mlxreg_core_platform_driver {
+	const char *name;
+	void *regmap;
+	struct platform_device *pdev;
+	struct mlxreg_core_platform_data *mlxreg_core_pdata;
+};
+
+/* mlxreg_core_platform_drivers - platform drivers container
+ * @pdrv - platform driver container;
+ * @pdrvs_num - number of core platform drivers;
+ */
+struct mlxreg_core_platform_drivers {
+	struct mlxreg_core_platform_driver *pdrv;
+	int pdrvs_num;
+};
+
+static inline int
+mlxreg_core_add_platform_drivers(struct device *dev,
+		struct mlxreg_core_platform_drivers *pdrvs, void *regmap)
+{
+	struct mlxreg_core_platform_driver *item;
+	int i, err;
+
+	for (i = 0; i < pdrvs->pdrvs_num; i++, item++) {
+		item = pdrvs->pdrv + i;
+		item->regmap = regmap;
+		item->pdev = platform_device_register_resndata(dev, item->name,
+					PLATFORM_DEVID_NONE, NULL, 0,
+					item->mlxreg_core_pdata,
+					sizeof(*item->mlxreg_core_pdata));
+		if (IS_ERR(item->pdev)) {
+			err = PTR_ERR(item->pdev);
+			goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	while (--i >= 0)
+		platform_device_unregister(--item->pdev);
+
+	return err;
+}
+
+static inline void
+mlxreg_core_remove_platform_drivers(struct mlxreg_core_platform_drivers *pdrvs)
+{
+	struct mlxreg_core_platform_driver *item;
+	int i;
+
+	for (i = pdrvs->pdrvs_num - 1; i >= 0 ; i--) {
+		item = pdrvs->pdrv + i;
+		platform_device_unregister(item->pdev);
+	}
+}
+
 #endif /* __LINUX_PLATFORM_DATA_MLXREG_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 6/7] platform/mellanox: Introduce support for Mellanox register access driver
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
                   ` (4 preceding siblings ...)
  2018-03-27 10:02 ` [PATCH v1 5/7] platform/x86: mlx-platform: Add LED platform driver activation Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-03-27 10:02 ` [PATCH v1 7/7] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

It introduces 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>
---
 drivers/platform/mellanox/Kconfig     |  11 ++
 drivers/platform/mellanox/Makefile    |   1 +
 drivers/platform/mellanox/mlxreg-io.c | 221 ++++++++++++++++++++++++++++++++++
 3 files changed, 233 insertions(+)
 create mode 100644 drivers/platform/mellanox/mlxreg-io.c

diff --git a/drivers/platform/mellanox/Kconfig b/drivers/platform/mellanox/Kconfig
index 591bccd..7fc266e 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..f573b65
--- /dev/null
+++ b/drivers/platform/mellanox/mlxreg-io.c
@@ -0,0 +1,221 @@
+// 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, &regval);
+	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, &regval);
+	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 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;
+
+		/* 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;
+		switch (priv->pdata->data[i].mode) {
+		case 0200:
+			priv->mlxreg_io_dev_attr[i].dev_attr.store =
+							mlxreg_io_attr_store;
+			break;
+
+		case 0444:
+			priv->mlxreg_io_dev_attr[i].dev_attr.show =
+							mlxreg_io_attr_show;
+			break;
+
+		case 0644:
+			priv->mlxreg_io_dev_attr[i].dev_attr.show =
+							mlxreg_io_attr_show;
+			priv->mlxreg_io_dev_attr[i].dev_attr.store =
+							mlxreg_io_attr_store;
+			break;
+
+		default:
+			dev_err(&priv->pdev->dev, "Bad access mode %u for attribute %s.\n",
+				priv->pdata->data[i].mode,
+				priv->mlxreg_io_attr[i]->name);
+			return -EINVAL;
+		}
+
+		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 related	[flat|nested] 16+ messages in thread

* [PATCH v1 7/7] platform/x86: mlx-platform: Add mlxreg-io platform driver activation
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
                   ` (5 preceding siblings ...)
  2018-03-27 10:02 ` [PATCH v1 6/7] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak
@ 2018-03-27 10:02 ` Vadim Pasternak
  2018-04-13 16:17 ` [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Darren Hart
  2018-05-11 15:28 ` Darren Hart
  8 siblings, 0 replies; 16+ messages in thread
From: Vadim Pasternak @ 2018-03-27 10:02 UTC (permalink / raw)
  To: dvhart, andy.shevchenko, gregkh
  Cc: linux-kernel, platform-driver-x86, jiri, michaelsh, ivecera,
	Vadim Pasternak

This patch adds:
- Support for Mellanox mlxreg-io driver activation, which uses the same
  regmap infrastructure as others Mellanox platform drivers.
- Specific registers description for default platform data configuration.
  It includes 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.
- The default values for the register, which should be set to these values
  at initialization time by regmap infrastructure.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/platform/x86/mlx-platform.c | 127 ++++++++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index efb605a..79876d9 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
@@ -813,6 +820,99 @@ 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),
+};
+
 /* Core platform devices */
 static struct mlxreg_core_platform_driver
 			mlxplat_mlxcpld_default_core_platform_drivers[] = {
@@ -828,6 +928,10 @@ static struct mlxreg_core_platform_driver
 		.name = "leds-mlxreg",
 		.mlxreg_core_pdata = &mlxplat_msn21xx_led_data,
 	},
+	{
+		.name = "mlxreg-io",
+		.mlxreg_core_pdata = &mlxplat_default_regs_io_data,
+	},
 };
 
 static struct mlxreg_core_platform_driver
@@ -846,6 +950,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:
@@ -862,11 +970,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:
@@ -888,11 +1003,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:
@@ -911,6 +1031,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;
 };
@@ -943,6 +1068,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,
 };
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
                   ` (6 preceding siblings ...)
  2018-03-27 10:02 ` [PATCH v1 7/7] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak
@ 2018-04-13 16:17 ` Darren Hart
  2018-05-11 15:28 ` Darren Hart
  8 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2018-04-13 16:17 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri,
	michaelsh, ivecera

On Tue, Mar 27, 2018 at 10:02:00AM +0000, Vadim Pasternak wrote:
> This patcheset includes:
> 
> Document fixes for mlxreg-hotplug driver and mlxreg header file.
> 
> Fix for the race condition in mlxreg-hotplug driver.
> 
> Adding support for ODM system types.
> 
> Activation of Mellanox LED driver from mlx-platform.
> 
> Introduction of new mlxreg-io driver.
> 
> Activation of mlxreg-io driver from mlx-platform.

Hi Vadim,

As you can see, all the patch names you listed above ^ are automatically
populated by the tooling below. Please use the message in patch 0/7 to
provide the reviewer with contextual information about these changes. A
summary of the series and why its needed is a good start. You want to
keep specific information to each patch with the patch. Think of this as
an Introduction to help the reviewer be prepared to review each
individual patch.

This message provides no context, and no introduction, so a reviewer
will have to figure that out as they go - which is another barrier to
getting the code reviewed.

Something for next time. OK, on to the review...

> 
> Vadim Pasternak (7):
>   platform_data/mlxreg: Document fixes for hotplug device
>   platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private
>     data
>   platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work
>     queue
>   platform: mellanox: add new ODM system types to mlx-platform
>   platform/x86: mlx-platform: Add LED platform driver activation
>   platform/mellanox: Introduce support for Mellanox register access
>     driver
>   platform/x86: mlx-platform: Add mlxreg-io platform driver activation
> 
>  drivers/platform/mellanox/Kconfig          |  11 +
>  drivers/platform/mellanox/Makefile         |   1 +
>  drivers/platform/mellanox/mlxreg-hotplug.c |  23 +-
>  drivers/platform/mellanox/mlxreg-io.c      | 221 ++++++++++++++
>  drivers/platform/x86/mlx-platform.c        | 447 +++++++++++++++++++++++++++++
>  include/linux/platform_data/mlxreg.h       |  66 ++++-
>  6 files changed, 765 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/platform/mellanox/mlxreg-io.c
> 
> -- 
> 2.1.4
> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device
  2018-03-27 10:02 ` [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device Vadim Pasternak
@ 2018-04-13 16:23   ` Darren Hart
  0 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2018-04-13 16:23 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri,
	michaelsh, ivecera

On Tue, Mar 27, 2018 at 10:02:01AM +0000, Vadim Pasternak wrote:
> Remove redunadant description of label in struct mlxreg_hotplug_device.

         ^ redundant

Please configure a spell checker in your text editor. I take care of
these for first time contributors, but as you become a regular
contributor, I shouldn't need to rewrite your commit messages.

I'll handle this one, but please adapt your tooling for future patches.

> 
> Change location of access_mode in struct mlxreg_hotplug_device.
> 
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
>  include/linux/platform_data/mlxreg.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
> index 2744cff..19f5cb61 100644
> --- a/include/linux/platform_data/mlxreg.h
> +++ b/include/linux/platform_data/mlxreg.h
> @@ -58,11 +58,10 @@ struct mlxreg_hotplug_device {
>   * struct mlxreg_core_data - attributes control data:
>   *
>   * @label: attribute label;
> - * @label: attribute register offset;
>   * @reg: attribute register;
>   * @mask: attribute access mask;
> - * @mode: access mode;
>   * @bit: attribute effective bit;
> + * @mode: access mode;
>   * @np - pointer to node platform associated with attribute;
>   * @hpdev - hotplug device data;
>   * @health_cntr: dynamic device health indication counter;
> -- 
> 2.1.4
> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/7] platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private data
  2018-03-27 10:02 ` [PATCH v1 2/7] platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private data Vadim Pasternak
@ 2018-04-13 16:27   ` Darren Hart
  0 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2018-04-13 16:27 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri,
	michaelsh, ivecera

On Tue, Mar 27, 2018 at 10:02:02AM +0000, Vadim Pasternak wrote:
> Add missing description of dev, regmap, dwork_irq, after_probe in struct
> mlxreg_hotplug_priv_data.
> 
> Remove dwork field from the structure mlxreg_hotplug_priv_data itself and
> for the descriptions, since it is not used.
> 

A bit of a nit, but "Document..." implies no functional changes, but you are
changing the physical structure. This can have negative consequences in some
cases, even if the removed field is unused.

Someone reading through the git history might skip over patches starting with
"Document..." when looking for change which impacted behavior.

Again, for future submissions.

> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
>  drivers/platform/mellanox/mlxreg-hotplug.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
> index ea9e7f4..b56953a 100644
> --- a/drivers/platform/mellanox/mlxreg-hotplug.c
> +++ b/drivers/platform/mellanox/mlxreg-hotplug.c
> @@ -59,9 +59,11 @@
>  /**
>   * struct mlxreg_hotplug_priv_data - platform private data:
>   * @irq: platform device interrupt number;
> + * @dev: basic device;
>   * @pdev: platform device;
>   * @plat: platform data;
> - * @dwork: delayed work template;
> + * @regmap: register map handle;
> + * @dwork_irq: delayed work template;
>   * @lock: spin lock;
>   * @hwmon: hwmon device;
>   * @mlxreg_hotplug_attr: sysfs attributes array;
> @@ -71,6 +73,7 @@
>   * @cell: location of top aggregation interrupt register;
>   * @mask: top aggregation interrupt common mask;
>   * @aggr_cache: last value of aggregation register status;
> + * @after_probe: flag indication probing completion;
>   */
>  struct mlxreg_hotplug_priv_data {
>  	int irq;
> @@ -79,7 +82,6 @@ struct mlxreg_hotplug_priv_data {
>  	struct mlxreg_hotplug_platform_data *plat;
>  	struct regmap *regmap;
>  	struct delayed_work dwork_irq;
> -	struct delayed_work dwork;
>  	spinlock_t lock; /* sync with interrupt */
>  	struct device *hwmon;
>  	struct attribute *mlxreg_hotplug_attr[MLXREG_HOTPLUG_ATTRS_MAX + 1];
> -- 
> 2.1.4
> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue
  2018-03-27 10:02 ` [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue Vadim Pasternak
@ 2018-04-13 16:47   ` Darren Hart
  2018-04-13 19:39     ` Vadim Pasternak
  0 siblings, 1 reply; 16+ messages in thread
From: Darren Hart @ 2018-04-13 16:47 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri,
	michaelsh, ivecera

On Tue, Mar 27, 2018 at 10:02:03AM +0000, Vadim Pasternak wrote:
> It adds missed logic for signal acknowledge, by adding an extra run for
> work queue in case a signal is received, but no specific signal assertion
> is detected. Such case theoretically can happen for example in case
> several units are removed or inserted at the same time. In this situation
> acknowledge for some signal can be missed at signal top aggreagation
> status level.

Why can they be missed? What does "signal top aggregation status level"
mean? I'm asking to confirm that we are fixing this at the right place,
and not just applying a suboptimal bandaid by running the workqueue
more.

...

> 
> Fixes: 1f976f6978bf ("platform/x86: Move Mellanox platform hotplug driver to platform/mellanox")

You didn't mention above how this commit caused this - how did moving
the driver create this problem? Does this need to go to stable? I'm
assuming not as you've called it theoretical - not something you've
observed in practice?

...

>  static int mlxreg_hotplug_device_create(struct mlxreg_hotplug_priv_data *priv,
> @@ -410,6 +413,18 @@ static void mlxreg_hotplug_work_handler(struct work_struct *work)
>  	aggr_asserted = priv->aggr_cache ^ regval;
>  	priv->aggr_cache = regval;
>  
> +	/*
> +	 * Handler is invoked, but no assertion is detected at top aggregation
> +	 * status level. Set aggr_asserted to mask value to allow handler extra
> +	 * run over all relevant signals to recover any missed signal.
> +	 */
> +	if (priv->not_asserted == MLXREG_HOTPLUG_NOT_ASSERT) {
> +		priv->not_asserted = 0;
> +		aggr_asserted = pdata->mask;
> +	}
> +	if (!aggr_asserted)

We seem to check aggr_asserted in several places in this routine now.
Looks like something we could simplify. If you check it here, can you
drop the check lower in the routine? Can you remove it from the for loop
if conditional entirely? Please consider how to simplify.

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform
  2018-03-27 10:02 ` [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform Vadim Pasternak
@ 2018-04-13 16:54   ` Darren Hart
  2018-04-13 19:53     ` Vadim Pasternak
  0 siblings, 1 reply; 16+ messages in thread
From: Darren Hart @ 2018-04-13 16:54 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri,
	michaelsh, ivecera

On Tue, Mar 27, 2018 at 10:02:04AM +0000, Vadim Pasternak wrote:
> Patch adds new ODM systems, matched according to DMI_BOARD_NAME.

General nit. When writing your messages, please use the imperative (command)
form. Rather than:

"Patch adds" or "It changes" use the same form you use in the subject lines:

"Add new ODM systems", "Fix struct field documentation", etc.

Again, I've been rewriting these, but as a regular contributor, this will help
reduce the overhead of reviewing your patches - good for you, good for me :-)

> The supported ODM Ids are: VMOD0001, VMOD0002, VMOD0003, VMOD0004,
> VMOD0005. It doesn't introduce new systems, but allows to
> ODM companies to set DMI_BOARD_VENDOR and DMI_PRODUCT_NAME on
> their own. It assumes that ODM company can't change DMI_BOARD_NAME.

You said "it assumes that ODM companies can't change DMI_BOARD_NAME". Is that an
assumption, or is that how ODMs work with Mellanox?
-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue
  2018-04-13 16:47   ` Darren Hart
@ 2018-04-13 19:39     ` Vadim Pasternak
  0 siblings, 0 replies; 16+ messages in thread
From: Vadim Pasternak @ 2018-04-13 19:39 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: Friday, April 13, 2018 7:47 PM
> 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 v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle
> for hotplug work queue
> 
> On Tue, Mar 27, 2018 at 10:02:03AM +0000, Vadim Pasternak wrote:
> > It adds missed logic for signal acknowledge, by adding an extra run
> > for work queue in case a signal is received, but no specific signal
> > assertion is detected. Such case theoretically can happen for example
> > in case several units are removed or inserted at the same time. In
> > this situation acknowledge for some signal can be missed at signal top
> > aggreagation status level.
> 
> Why can they be missed? What does "signal top aggregation status level"
> mean? I'm asking to confirm that we are fixing this at the right place, and not
> just applying a suboptimal bandaid by running the workqueue more.
> 

Hi Darren,

Thank for review.

It could happen within the next flow:

The signal routing flow is as following (f.e. for of FANi removing):
 - FAN status and event registers related bit is changed;
 -- intermediate aggregation status register is changed;
 --- top aggregation status register is changed;
 ---- interrupt routed to CPU and interrupt handler is invoked.

When interrupt handler is invoked it follows the next simple logic (f.e
FAN3 is removed):
 (a1) mask top aggregation interrupt mask register;
 (a2) read top aggregation interrupt status register and test to which
      underling group belongs a signal (FANs in this case and is changed
	  from 0xff to 0xfb and 0xfb is saved as a last status value);
   (b1) mask FANs interrupt mask register;
   (b2) read FANs status register and test which FAN has been changed (FAN3 in
        this example);
     (c1) perform relevant action;
              <--------------- (FAN2 is removed at this point)
   (b3) clear FANs interrupt event register to acknowledge FAN3 signal;
   (b4) unmask FANs interrupt mask register
 (a3) unmask top aggregation interrupt mask register;
 
 An interrupt handler is invoked, since FAN2 interrupt is not acknowledge.
 It should set top aggregation interrupt status register bit 6 (0xfb).
 In step (a2)
 (a2) read top aggregation interrupt and comparing it with saved value doesn't
      show change (same 0xfb) and after (a2) execution jumps to (a3) and
	  signal leaved unhandled.

> ...
> 
> >
> > Fixes: 1f976f6978bf ("platform/x86: Move Mellanox platform hotplug
> > driver to platform/mellanox")
> 
> You didn't mention above how this commit caused this - how did moving the
> driver create this problem? 

Actually I should reference to 
07b89c2b2a5e ("platform/x86: Introduce support for Mellanox hotplug driver")
which was initial driver commit, before it has been relocated. 

Does this need to go to stable? I'm assuming not as
> you've called it theoretical - not something you've observed in practice?
> 

It's not necessary to go to stable.

> ...
> 
> >  static int mlxreg_hotplug_device_create(struct
> > mlxreg_hotplug_priv_data *priv, @@ -410,6 +413,18 @@ static void
> mlxreg_hotplug_work_handler(struct work_struct *work)
> >  	aggr_asserted = priv->aggr_cache ^ regval;
> >  	priv->aggr_cache = regval;
> >
> > +	/*
> > +	 * Handler is invoked, but no assertion is detected at top aggregation
> > +	 * status level. Set aggr_asserted to mask value to allow handler extra
> > +	 * run over all relevant signals to recover any missed signal.
> > +	 */
> > +	if (priv->not_asserted == MLXREG_HOTPLUG_NOT_ASSERT) {
> > +		priv->not_asserted = 0;
> > +		aggr_asserted = pdata->mask;
> > +	}
> > +	if (!aggr_asserted)
> 
> We seem to check aggr_asserted in several places in this routine now.
> Looks like something we could simplify. If you check it here, can you drop the
> check lower in the routine? Can you remove it from the for loop if conditional
> entirely? Please consider how to simplify.

OK, will review this code.

> 
> --
> Darren Hart
> VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform
  2018-04-13 16:54   ` Darren Hart
@ 2018-04-13 19:53     ` Vadim Pasternak
  0 siblings, 0 replies; 16+ messages in thread
From: Vadim Pasternak @ 2018-04-13 19:53 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: Friday, April 13, 2018 7:55 PM
> 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 v1 4/7] platform: mellanox: add new ODM system types to
> mlx-platform
> 
> On Tue, Mar 27, 2018 at 10:02:04AM +0000, Vadim Pasternak wrote:
> > Patch adds new ODM systems, matched according to DMI_BOARD_NAME.
> 
> General nit. When writing your messages, please use the imperative (command)
> form. Rather than:
> 
> "Patch adds" or "It changes" use the same form you use in the subject lines:
> 
> "Add new ODM systems", "Fix struct field documentation", etc.
> 
> Again, I've been rewriting these, but as a regular contributor, this will help
> reduce the overhead of reviewing your patches - good for you, good for me :-)
> 
> > The supported ODM Ids are: VMOD0001, VMOD0002, VMOD0003,
> VMOD0004,
> > VMOD0005. It doesn't introduce new systems, but allows to ODM
> > companies to set DMI_BOARD_VENDOR and DMI_PRODUCT_NAME on their
> own.
> > It assumes that ODM company can't change DMI_BOARD_NAME.
> 
> You said "it assumes that ODM companies can't change DMI_BOARD_NAME". Is
> that an assumption, or is that how ODMs work with Mellanox?

Till now ODM companies didn't care about SMBIOS content.
But now we have two, which want to overwrite BOARD_VENDOR and 
PRODUCT_NAME (and some other fields).
System manufacturing for ODM is provided by Mellanox, so Mellanox
production team is responsible for setting all these fields.

Both don't care about BOARD_NAME and it has been closed with them,
that content of this filed will be defined by Mellanox.

> --
> Darren Hart
> VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features
  2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
                   ` (7 preceding siblings ...)
  2018-04-13 16:17 ` [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Darren Hart
@ 2018-05-11 15:28 ` Darren Hart
  8 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2018-05-11 15:28 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: andy.shevchenko, gregkh, linux-kernel, platform-driver-x86, jiri,
	michaelsh, ivecera

On Tue, Mar 27, 2018 at 10:02:00AM +0000, Vadim Pasternak wrote:
> This patcheset includes:
> 
> Document fixes for mlxreg-hotplug driver and mlxreg header file.
> 
> Fix for the race condition in mlxreg-hotplug driver.
> 
> Adding support for ODM system types.
> 
> Activation of Mellanox LED driver from mlx-platform.
> 
> Introduction of new mlxreg-io driver.
> 
> Activation of mlxreg-io driver from mlx-platform.

Patches 1,2 are queued up. Dropping 3-7 and reviewing them now from v2.

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-05-11 15:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 10:02 [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Vadim Pasternak
2018-03-27 10:02 ` [PATCH v1 1/7] platform_data/mlxreg: Document fixes for hotplug device Vadim Pasternak
2018-04-13 16:23   ` Darren Hart
2018-03-27 10:02 ` [PATCH v1 2/7] platform/mellanox: mlxreg-hotplug: Document fixes for hotplug private data Vadim Pasternak
2018-04-13 16:27   ` Darren Hart
2018-03-27 10:02 ` [PATCH v1 3/7] platform/mellanox: mlxreg-hotplug: add extra cycle for hotplug work queue Vadim Pasternak
2018-04-13 16:47   ` Darren Hart
2018-04-13 19:39     ` Vadim Pasternak
2018-03-27 10:02 ` [PATCH v1 4/7] platform: mellanox: add new ODM system types to mlx-platform Vadim Pasternak
2018-04-13 16:54   ` Darren Hart
2018-04-13 19:53     ` Vadim Pasternak
2018-03-27 10:02 ` [PATCH v1 5/7] platform/x86: mlx-platform: Add LED platform driver activation Vadim Pasternak
2018-03-27 10:02 ` [PATCH v1 6/7] platform/mellanox: Introduce support for Mellanox register access driver Vadim Pasternak
2018-03-27 10:02 ` [PATCH v1 7/7] platform/x86: mlx-platform: Add mlxreg-io platform driver activation Vadim Pasternak
2018-04-13 16:17 ` [PATCH v1 0/7] platform/x86: Mellanox add fixes and new features Darren Hart
2018-05-11 15:28 ` Darren Hart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.