All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  6:29 [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform vadimp
@ 2016-09-12  4:41 ` H. Peter Anvin
  2016-09-12  4:52     ` Vadim Pasternak
  2016-09-12  5:34 ` Guenter Roeck
  2016-09-12  6:12 ` Greg KH
  2 siblings, 1 reply; 39+ messages in thread
From: H. Peter Anvin @ 2016-09-12  4:41 UTC (permalink / raw)
  To: vadimp, tglx
  Cc: mingo, davem, geert, akpm, gregkh, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On September 11, 2016 11:29:58 PM PDT, vadimp@mellanox.com wrote:
>From: Vadim Pasternak <vadimp@mellanox.com>
>
>Enable system support for the Mellanox Technologies platform, 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.
>
>The Kconfig currently controlling compilation of this code is:
>arch/x86/platform:config MLX_PLATFORM
>arch/x86/platform:      tristate "Mellanox Technologies platform
>support"
>
>Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
>---
> MAINTAINERS                               |   7 +
> arch/x86/Kconfig                          |  23 ++
> arch/x86/platform/Makefile                |   1 +
> arch/x86/platform/mellanox/Makefile       |   1 +
>arch/x86/platform/mellanox/mlx-platform.c | 501
>++++++++++++++++++++++++++++++
> 5 files changed, 533 insertions(+)
> create mode 100644 arch/x86/platform/mellanox/Makefile
> create mode 100644 arch/x86/platform/mellanox/mlx-platform.c
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 4705c94..8a675de 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -7664,6 +7664,13 @@ W:	http://www.mellanox.com
> Q:	http://patchwork.ozlabs.org/project/netdev/list/
> F:	drivers/net/ethernet/mellanox/mlxsw/
> 
>+MELLANOX PLATFORM DRIVER
>+M:      Vadim Pasternak <vadimp@mellanox.com>
>+L:      platform-driver-x86@vger.kernel.org
>+S:      Supported
>+W:      http://www.mellanox.com
>+F:      arch/x86/platform/mellanox/mlx-platform.c
>+
> SOFT-ROCE DRIVER (rxe)
> M:	Moni Shoua <monis@mellanox.com>
> L:	linux-rdma@vger.kernel.org
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index c580d8c..cc7efdd9 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -2631,6 +2631,29 @@ config TS5500
> 
> endif # X86_32
> 
>+config MLX_PLATFORM
>+	tristate "Mellanox Technologies platform support"
>+	depends on X86_64
>+	depends on PCI
>+	depends on DMI
>+	depends on I2C_MLXCPLD
>+	depends on I2C_MUX_REG
>+	select HWMON
>+	select PMBUS
>+	select LM75
>+	select NEW_LEDS
>+	select LEDS_CLASS
>+	select LEDS_TRIGGERS
>+	select LEDS_TRIGGER_TIMER
>+	select LEDS_MLXCPLD
>+	---help---
>+	  This option enables system support for the Mellanox Technologies
>+	  platform.
>+
>+	  Say Y here if you are building a kernel for Mellanox system.
>+
>+	  Otherwise, say N.
>+
> config AMD_NB
> 	def_bool y
> 	depends on CPU_SUP_AMD && PCI
>diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
>index 184842e..3c3c19e 100644
>--- a/arch/x86/platform/Makefile
>+++ b/arch/x86/platform/Makefile
>@@ -8,6 +8,7 @@ obj-y	+= iris/
> obj-y	+= intel/
> obj-y	+= intel-mid/
> obj-y	+= intel-quark/
>+obj-y	+= mellanox/
> obj-y	+= olpc/
> obj-y	+= scx200/
> obj-y	+= sfi/
>diff --git a/arch/x86/platform/mellanox/Makefile
>b/arch/x86/platform/mellanox/Makefile
>new file mode 100644
>index 0000000..f43c931
>--- /dev/null
>+++ b/arch/x86/platform/mellanox/Makefile
>@@ -0,0 +1 @@
>+obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
>diff --git a/arch/x86/platform/mellanox/mlx-platform.c
>b/arch/x86/platform/mellanox/mlx-platform.c
>new file mode 100644
>index 0000000..02afa89
>--- /dev/null
>+++ b/arch/x86/platform/mellanox/mlx-platform.c
>@@ -0,0 +1,501 @@
>+/*
>+ * arch/x86/platform/mellanox/mlx-platform.c
>+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
>+ * Copyright (c) 2016 Vadim Pasternak <vadimp@mellanox.com>
>+ *
>+ * 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/acpi.h>
>+#include <linux/device.h>
>+#include <linux/i2c.h>
>+#include <linux/i2c-mux.h>
>+#include <linux/module.h>
>+#include <linux/of_platform.h>
>+#include <linux/platform_device.h>
>+#include <linux/platform_data/i2c-mux-reg.h>
>+#include <linux/pci.h>
>+#include <linux/slab.h>
>+#include <linux/version.h>
>+
>+#define MLX_PLAT_DEVICE_NAME		"mlxplat"
>+
>+/* LPC IFC in PCH defines */
>+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR		0x2000
>+#define MLXPLAT_CPLD_LPC_REG_BASE_ADRR		0x2500
>+#define MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID	0
>+#define MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID	31
>+#define MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID	0
>+#define MLXPLAT_CPLD_LPC_QM67_DEV_ID		0x1c4f
>+#define MLXPLAT_CPLD_LPC_QM77_DEV_ID		0x1e55
>+#define MLXPLAT_CPLD_LPC_RNG_DEV_ID		0x1f38
>+#define MLXPLAT_CPLD_LPC_I2C_CH1_OFF		0xdb
>+#define MLXPLAT_CPLD_LPC_I2C_CH2_OFF		0xda
>+#define MLXPLAT_CPLD_LPC_PIO_OFFSET		0x10000UL
>+#define MLXPLAT_CPLD_LPC_REG1	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
>+				  MLXPLAT_CPLD_LPC_I2C_CH1_OFF) | \
>+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
>+#define MLXPLAT_CPLD_LPC_REG2	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
>+				  MLXPLAT_CPLD_LPC_I2C_CH2_OFF) | \
>+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
>+
>+/* Use generic decode range 4 for CPLD LPC */
>+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGE4	0x90
>+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE	0x84
>+#define MLXPLAT_CPLD_LPC_RNG_CTRL		0x84
>+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES	4
>+#define MLXPLAT_CPLD_LPC_I2C_RANGE		2
>+#define MLXPLAT_CPLD_LPC_RANGE			3
>+#define MLXPLAT_CPLD_LPC_CLKS_EN		0
>+#define MLXPLAT_CPLD_LPC_IO_RANGE		0x100
>+
>+/* Start channel numbers */
>+#define MLXPLAT_CPLD_CH1			2
>+#define MLXPLAT_CPLD_CH2			10
>+
>+/* mlxplat_priv - board private data
>+ * @lpc_reg - register space
>+ * @dev_id - platform device id
>+ * @lpc_i2c_res- lpc cpld i2c resource space
>+ * @lpc_cpld_res - lpc cpld register resource space
>+ * @pdev - platform device
>+ */
>+struct mlxplat_priv {
>+	u32 lpc_reg[MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES];
>+	u16 dev_id;
>+	struct resource *lpc_i2c_res;
>+	struct resource *lpc_cpld_res;
>+	struct platform_device *pdev;
>+	struct platform_device *pdev_i2c;
>+};
>+
>+/* Regions for LPC I2C controller and LPC base register space */
>+static struct resource mlxplat_lpc_resources[] = {
>+	[0] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_I2C_BASE_ADRR,
>+			       MLXPLAT_CPLD_LPC_IO_RANGE,
>+			       "mlxplat_cpld_lpc_i2c_ctrl", IORESOURCE_IO),
>+	[1] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_REG_BASE_ADRR,
>+			       MLXPLAT_CPLD_LPC_IO_RANGE,
>+			       "mlxplat_cpld_lpc_regs",
>+			       IORESOURCE_IO),
>+};
>+
>+/* Platfform channels */
>+static int mlxplat_channels[][8] = {
>+	{
>+		MLXPLAT_CPLD_CH1, MLXPLAT_CPLD_CH1 + 1, MLXPLAT_CPLD_CH1 + 2,
>+		MLXPLAT_CPLD_CH1 + 3, MLXPLAT_CPLD_CH1 + 4, MLXPLAT_CPLD_CH1 +
>+		5, MLXPLAT_CPLD_CH1 + 6, MLXPLAT_CPLD_CH1 + 7
>+	},
>+	{
>+		MLXPLAT_CPLD_CH2, MLXPLAT_CPLD_CH2 + 1, MLXPLAT_CPLD_CH2 + 2,
>+		MLXPLAT_CPLD_CH2 + 3, MLXPLAT_CPLD_CH2 + 4, MLXPLAT_CPLD_CH2 +
>+		+ 5, MLXPLAT_CPLD_CH2 + 6, MLXPLAT_CPLD_CH2 + 7
>+	},
>+};
>+
>+/* Platform mux data */
>+struct i2c_mux_reg_platform_data mlxplat_mux_data[] = {
>+	{
>+		.parent = 1,
>+		.base_nr = MLXPLAT_CPLD_CH1,
>+		.write_only = 1,
>+		.values = mlxplat_channels[0],
>+		.n_values = ARRAY_SIZE(mlxplat_channels[0]),
>+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG1,
>+		.reg_size = 1,
>+		.idle_in_use = 1,
>+	},
>+	{
>+		.parent = 1,
>+		.base_nr = MLXPLAT_CPLD_CH2,
>+		.write_only = 1,
>+		.values = mlxplat_channels[1],
>+		.n_values = ARRAY_SIZE(mlxplat_channels[1]),
>+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG2,
>+		.reg_size = 1,
>+		.idle_in_use = 1,
>+	},
>+
>+};
>+
>+/* mlxplat_topology - platform entry data:
>+ * @pdev - platform device
>+ * @name - platform device name
>+ */
>+struct mlxplat_topology {
>+	struct platform_device *pdev;
>+	const char *name;
>+};
>+
>+/* Platform topology */
>+struct mlxplat_topology mlxplat_topo[] = {
>+	{
>+		.name = "i2c-mux-reg",
>+	},
>+	{
>+		.name = "i2c-mux-reg",
>+	},
>+};
>+
>+struct platform_device *mlxplat_dev;
>+
>+static int mlxplat_lpc_i2c_dec_range_config(struct mlxplat_priv *priv,
>+					    struct pci_dev *pdev, u8 range,
>+					    u16 base_addr)
>+{
>+	u16 rng_reg;
>+	u32 val;
>+	int err;
>+
>+	if (range >= MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES) {
>+		dev_err(&priv->pdev->dev, "Incorrect LPC decode range %d > %d\n",
>+			range, MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES);
>+		return -ERANGE;
>+	}
>+
>+	rng_reg = MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE + 4 * range;
>+	err = pci_read_config_dword(pdev, rng_reg, &val);
>+	if (err) {
>+		dev_err(&priv->pdev->dev, "Access to LPC_PCH config failed, err
>%d\n",
>+			err);
>+		return -EFAULT;
>+	}
>+	priv->lpc_reg[range] = val;
>+
>+	/* Clean all bits excepted reserved (reserved: 2, 16, 17 , 24 - 31).
>*/
>+	val &= 0xff030002;
>+	/* Set bits 18 - 23 to allow decode range address mask, set bit 1 to
>+	 * enable decode range, clear bit 1,2 in base address.
>+	 */
>+	val |= 0xfc0001 | (base_addr & 0xfff3);
>+	err = pci_write_config_dword(pdev, rng_reg, val);
>+	if (err)
>+		dev_err(&priv->pdev->dev, "Config of LPC_PCH Generic Decode Range %d
>failed, err %d\n",
>+			range, err);
>+
>+	return err;
>+}
>+
>+static void mlxplat_lpc_dec_rng_config_clean(struct pci_dev *pdev, u32
>val,
>+					     u8 range)
>+{
>+	/* Restore old value */
>+	if (pci_write_config_dword(pdev, (MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE +
>+				   range * 4), val))
>+		dev_err(&pdev->dev, "Deconfig of LPC_PCH Generic Decode Range %x
>failed\n",
>+			range);
>+
>+}
>+
>+static int mlxplat_lpc_request_region(struct mlxplat_priv *priv,
>+				      struct resource *res)
>+{
>+	resource_size_t size = resource_size(res);
>+
>+	if (!devm_request_region(&priv->pdev->dev, res->start, size,
>+				 res->name)) {
>+		devm_release_region(&priv->pdev->dev, res->start, size);
>+
>+		if (!devm_request_region(&priv->pdev->dev, res->start, size,
>+					 res->name)) {
>+			dev_err(&priv->pdev->dev, "Request ioregion 0x%llx len 0x%llx for
>%s fail\n",
>+				res->start, size, res->name);
>+			return -EIO;
>+		}
>+	}
>+
>+	return 0;
>+}
>+
>+static int mlxplat_lpc_request_regions(struct mlxplat_priv *priv)
>+{
>+	int i;
>+	int err;
>+
>+	for (i = 0; i < ARRAY_SIZE(mlxplat_lpc_resources); i++) {
>+		err = mlxplat_lpc_request_region(priv,
>+						 &mlxplat_lpc_resources[i]);
>+		if (err)
>+			return err;
>+	}
>+
>+	priv->lpc_i2c_res = &mlxplat_lpc_resources[0];
>+	priv->lpc_cpld_res = &mlxplat_lpc_resources[1];
>+
>+	return 0;
>+}
>+
>+static int mlxplat_lpc_ivb_config(struct mlxplat_priv *priv,
>+				  struct pci_dev *pdev)
>+{
>+	int err;
>+
>+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
>+					       MLXPLAT_CPLD_LPC_I2C_RANGE,
>+					       MLXPLAT_CPLD_LPC_I2C_BASE_ADRR);
>+	if (err) {
>+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed, err
>%d\n",
>+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
>+		pci_dev_put(pdev);
>+		return -EFAULT;
>+	}
>+
>+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
>+					       MLXPLAT_CPLD_LPC_RANGE,
>+					       MLXPLAT_CPLD_LPC_REG_BASE_ADRR);
>+	if (err) {
>+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed, err
>%d\n",
>+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
>+		return -EFAULT;
>+	}
>+
>+	return err;
>+}
>+
>+static void mlxplat_lpc_ivb_config_clean(struct mlxplat_priv *priv,
>+					 struct pci_dev *pdev)
>+{
>+	mlxplat_lpc_dec_rng_config_clean(pdev,
>+				priv->lpc_reg[MLXPLAT_CPLD_LPC_RANGE],
>+				MLXPLAT_CPLD_LPC_RANGE);
>+	mlxplat_lpc_dec_rng_config_clean(pdev,
>+				priv->lpc_reg[MLXPLAT_CPLD_LPC_I2C_RANGE],
>+				MLXPLAT_CPLD_LPC_I2C_RANGE);
>+
>+}
>+
>+static int mlxplat_lpc_range_config(struct mlxplat_priv *priv,
>+				    struct pci_dev *pdev)
>+{
>+	u32 val, lpc_clks;
>+	int err;
>+
>+	err = pci_read_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL, &val);
>+	if (err) {
>+		dev_err(&priv->pdev->dev, "Access to LPC Ctrl reg failed, err %d\n",
>+			err);
>+		return -EFAULT;
>+	}
>+	lpc_clks = val & 0x3;
>+	if (lpc_clks != MLXPLAT_CPLD_LPC_CLKS_EN) {
>+		val &= 0xFFFFFFFC;
>+		err = pci_write_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL,
>+					     val);
>+		if (err) {
>+			dev_err(&priv->pdev->dev, "Config LPC CLKS CTRL failed, err %d\n",
>+				err);
>+			return -EFAULT;
>+		}
>+	}
>+
>+	return err;
>+}
>+
>+static int mlxplat_lpc_config(struct mlxplat_priv *priv)
>+{
>+	struct pci_dev *pdev = NULL;
>+	u16 dev_id;
>+	int err;
>+
>+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
>+				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
>+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
>+
>+	if (!pdev) {
>+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d func:%d not
>found\n",
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
>+		return -EFAULT;
>+	}
>+
>+	err = pci_read_config_word(pdev, 2, &dev_id);
>+	if (err) {
>+		dev_err(&priv->pdev->dev, "Access PCIe LPC interface failed, err
>%d\n",
>+			err);
>+		goto failure;
>+	}
>+
>+	switch (dev_id) {
>+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
>+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
>+		err = mlxplat_lpc_ivb_config(priv, pdev);
>+		break;
>+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
>+		err = mlxplat_lpc_range_config(priv, pdev);
>+		break;
>+	default:
>+		err = -ENXIO;
>+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d slot:%d
>func:%d\n",
>+			dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
>+		goto failure;
>+	}
>+	priv->dev_id = dev_id;
>+
>+failure:
>+	pci_dev_put(pdev);
>+	return err;
>+}
>+
>+static int mlxplat_lpc_config_clean(struct mlxplat_priv *priv)
>+{
>+	struct pci_dev *pdev = NULL;
>+	int err = 0;
>+
>+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
>+				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
>+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
>+	if (!pdev) {
>+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d func:%d not
>found\n",
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
>+		return -EFAULT;
>+	}
>+
>+	switch (priv->dev_id) {
>+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
>+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
>+		mlxplat_lpc_ivb_config_clean(priv, pdev);
>+		break;
>+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
>+		break;
>+	default:
>+		err = -ENXIO;
>+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d slot:%d
>func:%d\n",
>+			priv->dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
>+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
>+		break;
>+	}
>+
>+	pci_dev_put(pdev);
>+
>+	return err;
>+}
>+
>+static int __init mlxplat_init(void)
>+{
>+	struct mlxplat_priv *priv;
>+	struct device *dev;
>+	int i, j;
>+	int err;
>+
>+	mlxplat_dev = platform_device_alloc(MLX_PLAT_DEVICE_NAME, -1);
>+	if (!mlxplat_dev) {
>+		pr_err("Alloc %s platform device failed\n",
>+			MLX_PLAT_DEVICE_NAME);
>+		return -ENOMEM;
>+	}
>+
>+	err = platform_device_add(mlxplat_dev);
>+	if (err) {
>+		pr_err("Add %s platform device failed (%d)\n",
>+			MLX_PLAT_DEVICE_NAME, err);
>+		goto fail_platform_device_add;
>+	}
>+	dev = &mlxplat_dev->dev;
>+
>+	priv = devm_kzalloc(dev, sizeof(struct mlxplat_priv), GFP_KERNEL);
>+	if (!priv) {
>+		err = -ENOMEM;
>+		dev_err(dev, "Failed to allocate mlxplat_priv\n");
>+		goto fail_alloc;
>+	}
>+	platform_set_drvdata(mlxplat_dev, priv);
>+	priv->pdev = mlxplat_dev;
>+
>+	err = mlxplat_lpc_config(priv);
>+	if (err) {
>+		dev_err(dev, "Failed to configure LPC interface\n");
>+		goto fail_alloc;
>+	}
>+
>+	err = mlxplat_lpc_request_regions(priv);
>+	if (err) {
>+		dev_err(dev, "Request ioregion failed (%d)\n", err);
>+		goto fail_alloc;
>+	}
>+
>+	priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", -1,
>+							 NULL, 0);
>+	if (IS_ERR(priv->pdev_i2c)) {
>+		err = PTR_ERR(priv->pdev_i2c);
>+		goto fail_alloc;
>+	};
>+
>+	for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) {
>+		mlxplat_topo[i].pdev = platform_device_register_resndata(dev,
>+						mlxplat_topo[i].name, i, NULL,
>+						0, &mlxplat_mux_data[i],
>+						sizeof(mlxplat_mux_data[i]));
>+		if (IS_ERR(mlxplat_topo[i].pdev)) {
>+			err = PTR_ERR(mlxplat_topo[i].pdev);
>+			goto fail_platform_mux_register;
>+		}
>+	}
>+
>+	return err;
>+
>+fail_platform_mux_register:
>+	for (j = i; j > 0 ; j--)
>+		platform_device_unregister(mlxplat_topo[j].pdev);
>+	platform_device_unregister(priv->pdev_i2c);
>+fail_alloc:
>+	platform_device_del(mlxplat_dev);
>+fail_platform_device_add:
>+	platform_device_put(mlxplat_dev);
>+
>+	return err;
>+}
>+
>+static void __exit mlxplat_exit(void)
>+{
>+	int i;
>+	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
>+
>+	for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
>+		platform_device_unregister(mlxplat_topo[i].pdev);
>+
>+	platform_device_unregister(priv->pdev_i2c);
>+	mlxplat_lpc_config_clean(priv);
>+	platform_device_del(mlxplat_dev);
>+	platform_device_put(mlxplat_dev);
>+}
>+
>+module_init(mlxplat_init);
>+module_exit(mlxplat_exit);
>+
>+MODULE_AUTHOR("Vadim Pasternak (vadimp@mellanox.com)");
>+MODULE_DESCRIPTION("Mellanox platform driver");
>+MODULE_LICENSE("GPL v2");
>+MODULE_ALIAS("platform:mlx-platform");

This is a horrifically uninformative piece of code.  What does this driver do?  Why is it needed?  What makes it a "platform" in the sense of the way the term is used in the Linux kernel?  How does it affect the overall system, and in what sense does it diverge from a standard x86?
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  4:41 ` H. Peter Anvin
@ 2016-09-12  4:52     ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  4:52 UTC (permalink / raw)
  To: H. Peter Anvin, tglx
  Cc: mingo, davem, geert, akpm, gregkh, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@zytor.com]
> Sent: Monday, September 12, 2016 7:42 AM
> To: Vadim Pasternak <vadimp@mellanox.com>; tglx@linutronix.de
> Cc: mingo@redhat.com; davem@davemloft.net; geert@linux-m68k.org;
> akpm@linux-foundation.org; gregkh@linuxfoundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On September 11, 2016 11:29:58 PM PDT, vadimp@mellanox.com wrote:
> >From: Vadim Pasternak <vadimp@mellanox.com>
> >
> >Enable system support for the Mellanox Technologies platform, 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.
> >
> >The Kconfig currently controlling compilation of this code is:
> >arch/x86/platform:config MLX_PLATFORM
> >arch/x86/platform:      tristate "Mellanox Technologies platform
> >support"
> >
> >Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> >---
> > MAINTAINERS                               |   7 +
> > arch/x86/Kconfig                          |  23 ++
> > arch/x86/platform/Makefile                |   1 +
> > arch/x86/platform/mellanox/Makefile       |   1 +
> >arch/x86/platform/mellanox/mlx-platform.c | 501
> >++++++++++++++++++++++++++++++
> > 5 files changed, 533 insertions(+)
> > create mode 100644 arch/x86/platform/mellanox/Makefile
> > create mode 100644 arch/x86/platform/mellanox/mlx-platform.c
> >
> >diff --git a/MAINTAINERS b/MAINTAINERS
> >index 4705c94..8a675de 100644
> >--- a/MAINTAINERS
> >+++ b/MAINTAINERS
> >@@ -7664,6 +7664,13 @@ W:	http://www.mellanox.com
> > Q:	http://patchwork.ozlabs.org/project/netdev/list/
> > F:	drivers/net/ethernet/mellanox/mlxsw/
> >
> >+MELLANOX PLATFORM DRIVER
> >+M:      Vadim Pasternak <vadimp@mellanox.com>
> >+L:      platform-driver-x86@vger.kernel.org
> >+S:      Supported
> >+W:      http://www.mellanox.com
> >+F:      arch/x86/platform/mellanox/mlx-platform.c
> >+
> > SOFT-ROCE DRIVER (rxe)
> > M:	Moni Shoua <monis@mellanox.com>
> > L:	linux-rdma@vger.kernel.org
> >diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
> >c580d8c..cc7efdd9 100644
> >--- a/arch/x86/Kconfig
> >+++ b/arch/x86/Kconfig
> >@@ -2631,6 +2631,29 @@ config TS5500
> >
> > endif # X86_32
> >
> >+config MLX_PLATFORM
> >+	tristate "Mellanox Technologies platform support"
> >+	depends on X86_64
> >+	depends on PCI
> >+	depends on DMI
> >+	depends on I2C_MLXCPLD
> >+	depends on I2C_MUX_REG
> >+	select HWMON
> >+	select PMBUS
> >+	select LM75
> >+	select NEW_LEDS
> >+	select LEDS_CLASS
> >+	select LEDS_TRIGGERS
> >+	select LEDS_TRIGGER_TIMER
> >+	select LEDS_MLXCPLD
> >+	---help---
> >+	  This option enables system support for the Mellanox Technologies
> >+	  platform.
> >+
> >+	  Say Y here if you are building a kernel for Mellanox system.
> >+
> >+	  Otherwise, say N.
> >+
> > config AMD_NB
> > 	def_bool y
> > 	depends on CPU_SUP_AMD && PCI
> >diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
> >index 184842e..3c3c19e 100644
> >--- a/arch/x86/platform/Makefile
> >+++ b/arch/x86/platform/Makefile
> >@@ -8,6 +8,7 @@ obj-y	+= iris/
> > obj-y	+= intel/
> > obj-y	+= intel-mid/
> > obj-y	+= intel-quark/
> >+obj-y	+= mellanox/
> > obj-y	+= olpc/
> > obj-y	+= scx200/
> > obj-y	+= sfi/
> >diff --git a/arch/x86/platform/mellanox/Makefile
> >b/arch/x86/platform/mellanox/Makefile
> >new file mode 100644
> >index 0000000..f43c931
> >--- /dev/null
> >+++ b/arch/x86/platform/mellanox/Makefile
> >@@ -0,0 +1 @@
> >+obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
> >diff --git a/arch/x86/platform/mellanox/mlx-platform.c
> >b/arch/x86/platform/mellanox/mlx-platform.c
> >new file mode 100644
> >index 0000000..02afa89
> >--- /dev/null
> >+++ b/arch/x86/platform/mellanox/mlx-platform.c
> >@@ -0,0 +1,501 @@
> >+/*
> >+ * arch/x86/platform/mellanox/mlx-platform.c
> >+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> >+ * Copyright (c) 2016 Vadim Pasternak <vadimp@mellanox.com>
> >+ *
> >+ * 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/acpi.h>
> >+#include <linux/device.h>
> >+#include <linux/i2c.h>
> >+#include <linux/i2c-mux.h>
> >+#include <linux/module.h>
> >+#include <linux/of_platform.h>
> >+#include <linux/platform_device.h>
> >+#include <linux/platform_data/i2c-mux-reg.h>
> >+#include <linux/pci.h>
> >+#include <linux/slab.h>
> >+#include <linux/version.h>
> >+
> >+#define MLX_PLAT_DEVICE_NAME		"mlxplat"
> >+
> >+/* LPC IFC in PCH defines */
> >+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR		0x2000
> >+#define MLXPLAT_CPLD_LPC_REG_BASE_ADRR		0x2500
> >+#define MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID	0
> >+#define MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID	31
> >+#define MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID	0
> >+#define MLXPLAT_CPLD_LPC_QM67_DEV_ID		0x1c4f
> >+#define MLXPLAT_CPLD_LPC_QM77_DEV_ID		0x1e55
> >+#define MLXPLAT_CPLD_LPC_RNG_DEV_ID		0x1f38
> >+#define MLXPLAT_CPLD_LPC_I2C_CH1_OFF		0xdb
> >+#define MLXPLAT_CPLD_LPC_I2C_CH2_OFF		0xda
> >+#define MLXPLAT_CPLD_LPC_PIO_OFFSET		0x10000UL
> >+#define MLXPLAT_CPLD_LPC_REG1
> 	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
> >+				  MLXPLAT_CPLD_LPC_I2C_CH1_OFF) | \
> >+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
> >+#define MLXPLAT_CPLD_LPC_REG2
> 	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
> >+				  MLXPLAT_CPLD_LPC_I2C_CH2_OFF) | \
> >+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
> >+
> >+/* Use generic decode range 4 for CPLD LPC */
> >+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGE4	0x90
> >+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE	0x84
> >+#define MLXPLAT_CPLD_LPC_RNG_CTRL		0x84
> >+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES	4
> >+#define MLXPLAT_CPLD_LPC_I2C_RANGE		2
> >+#define MLXPLAT_CPLD_LPC_RANGE			3
> >+#define MLXPLAT_CPLD_LPC_CLKS_EN		0
> >+#define MLXPLAT_CPLD_LPC_IO_RANGE		0x100
> >+
> >+/* Start channel numbers */
> >+#define MLXPLAT_CPLD_CH1			2
> >+#define MLXPLAT_CPLD_CH2			10
> >+
> >+/* mlxplat_priv - board private data
> >+ * @lpc_reg - register space
> >+ * @dev_id - platform device id
> >+ * @lpc_i2c_res- lpc cpld i2c resource space
> >+ * @lpc_cpld_res - lpc cpld register resource space
> >+ * @pdev - platform device
> >+ */
> >+struct mlxplat_priv {
> >+	u32 lpc_reg[MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES];
> >+	u16 dev_id;
> >+	struct resource *lpc_i2c_res;
> >+	struct resource *lpc_cpld_res;
> >+	struct platform_device *pdev;
> >+	struct platform_device *pdev_i2c;
> >+};
> >+
> >+/* Regions for LPC I2C controller and LPC base register space */
> >+static struct resource mlxplat_lpc_resources[] = {
> >+	[0] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_I2C_BASE_ADRR,
> >+			       MLXPLAT_CPLD_LPC_IO_RANGE,
> >+			       "mlxplat_cpld_lpc_i2c_ctrl", IORESOURCE_IO),
> >+	[1] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_REG_BASE_ADRR,
> >+			       MLXPLAT_CPLD_LPC_IO_RANGE,
> >+			       "mlxplat_cpld_lpc_regs",
> >+			       IORESOURCE_IO),
> >+};
> >+
> >+/* Platfform channels */
> >+static int mlxplat_channels[][8] = {
> >+	{
> >+		MLXPLAT_CPLD_CH1, MLXPLAT_CPLD_CH1 + 1,
> MLXPLAT_CPLD_CH1 + 2,
> >+		MLXPLAT_CPLD_CH1 + 3, MLXPLAT_CPLD_CH1 + 4,
> MLXPLAT_CPLD_CH1 +
> >+		5, MLXPLAT_CPLD_CH1 + 6, MLXPLAT_CPLD_CH1 + 7
> >+	},
> >+	{
> >+		MLXPLAT_CPLD_CH2, MLXPLAT_CPLD_CH2 + 1,
> MLXPLAT_CPLD_CH2 + 2,
> >+		MLXPLAT_CPLD_CH2 + 3, MLXPLAT_CPLD_CH2 + 4,
> MLXPLAT_CPLD_CH2 +
> >+		+ 5, MLXPLAT_CPLD_CH2 + 6, MLXPLAT_CPLD_CH2 + 7
> >+	},
> >+};
> >+
> >+/* Platform mux data */
> >+struct i2c_mux_reg_platform_data mlxplat_mux_data[] = {
> >+	{
> >+		.parent = 1,
> >+		.base_nr = MLXPLAT_CPLD_CH1,
> >+		.write_only = 1,
> >+		.values = mlxplat_channels[0],
> >+		.n_values = ARRAY_SIZE(mlxplat_channels[0]),
> >+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG1,
> >+		.reg_size = 1,
> >+		.idle_in_use = 1,
> >+	},
> >+	{
> >+		.parent = 1,
> >+		.base_nr = MLXPLAT_CPLD_CH2,
> >+		.write_only = 1,
> >+		.values = mlxplat_channels[1],
> >+		.n_values = ARRAY_SIZE(mlxplat_channels[1]),
> >+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG2,
> >+		.reg_size = 1,
> >+		.idle_in_use = 1,
> >+	},
> >+
> >+};
> >+
> >+/* mlxplat_topology - platform entry data:
> >+ * @pdev - platform device
> >+ * @name - platform device name
> >+ */
> >+struct mlxplat_topology {
> >+	struct platform_device *pdev;
> >+	const char *name;
> >+};
> >+
> >+/* Platform topology */
> >+struct mlxplat_topology mlxplat_topo[] = {
> >+	{
> >+		.name = "i2c-mux-reg",
> >+	},
> >+	{
> >+		.name = "i2c-mux-reg",
> >+	},
> >+};
> >+
> >+struct platform_device *mlxplat_dev;
> >+
> >+static int mlxplat_lpc_i2c_dec_range_config(struct mlxplat_priv *priv,
> >+					    struct pci_dev *pdev, u8 range,
> >+					    u16 base_addr)
> >+{
> >+	u16 rng_reg;
> >+	u32 val;
> >+	int err;
> >+
> >+	if (range >= MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES) {
> >+		dev_err(&priv->pdev->dev, "Incorrect LPC decode range %d >
> %d\n",
> >+			range, MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES);
> >+		return -ERANGE;
> >+	}
> >+
> >+	rng_reg = MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE + 4 * range;
> >+	err = pci_read_config_dword(pdev, rng_reg, &val);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "Access to LPC_PCH config failed,
> err
> >%d\n",
> >+			err);
> >+		return -EFAULT;
> >+	}
> >+	priv->lpc_reg[range] = val;
> >+
> >+	/* Clean all bits excepted reserved (reserved: 2, 16, 17 , 24 - 31).
> >*/
> >+	val &= 0xff030002;
> >+	/* Set bits 18 - 23 to allow decode range address mask, set bit 1 to
> >+	 * enable decode range, clear bit 1,2 in base address.
> >+	 */
> >+	val |= 0xfc0001 | (base_addr & 0xfff3);
> >+	err = pci_write_config_dword(pdev, rng_reg, val);
> >+	if (err)
> >+		dev_err(&priv->pdev->dev, "Config of LPC_PCH Generic Decode
> Range %d
> >failed, err %d\n",
> >+			range, err);
> >+
> >+	return err;
> >+}
> >+
> >+static void mlxplat_lpc_dec_rng_config_clean(struct pci_dev *pdev, u32
> >val,
> >+					     u8 range)
> >+{
> >+	/* Restore old value */
> >+	if (pci_write_config_dword(pdev,
> (MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE +
> >+				   range * 4), val))
> >+		dev_err(&pdev->dev, "Deconfig of LPC_PCH Generic Decode
> Range %x
> >failed\n",
> >+			range);
> >+
> >+}
> >+
> >+static int mlxplat_lpc_request_region(struct mlxplat_priv *priv,
> >+				      struct resource *res)
> >+{
> >+	resource_size_t size = resource_size(res);
> >+
> >+	if (!devm_request_region(&priv->pdev->dev, res->start, size,
> >+				 res->name)) {
> >+		devm_release_region(&priv->pdev->dev, res->start, size);
> >+
> >+		if (!devm_request_region(&priv->pdev->dev, res->start, size,
> >+					 res->name)) {
> >+			dev_err(&priv->pdev->dev, "Request ioregion 0x%llx len
> 0x%llx for
> >%s fail\n",
> >+				res->start, size, res->name);
> >+			return -EIO;
> >+		}
> >+	}
> >+
> >+	return 0;
> >+}
> >+
> >+static int mlxplat_lpc_request_regions(struct mlxplat_priv *priv) {
> >+	int i;
> >+	int err;
> >+
> >+	for (i = 0; i < ARRAY_SIZE(mlxplat_lpc_resources); i++) {
> >+		err = mlxplat_lpc_request_region(priv,
> >+						 &mlxplat_lpc_resources[i]);
> >+		if (err)
> >+			return err;
> >+	}
> >+
> >+	priv->lpc_i2c_res = &mlxplat_lpc_resources[0];
> >+	priv->lpc_cpld_res = &mlxplat_lpc_resources[1];
> >+
> >+	return 0;
> >+}
> >+
> >+static int mlxplat_lpc_ivb_config(struct mlxplat_priv *priv,
> >+				  struct pci_dev *pdev)
> >+{
> >+	int err;
> >+
> >+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
> >+					       MLXPLAT_CPLD_LPC_I2C_RANGE,
> >+
> MLXPLAT_CPLD_LPC_I2C_BASE_ADRR);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed,
> err
> >%d\n",
> >+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
> >+		pci_dev_put(pdev);
> >+		return -EFAULT;
> >+	}
> >+
> >+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
> >+					       MLXPLAT_CPLD_LPC_RANGE,
> >+
> MLXPLAT_CPLD_LPC_REG_BASE_ADRR);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed,
> err
> >%d\n",
> >+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
> >+		return -EFAULT;
> >+	}
> >+
> >+	return err;
> >+}
> >+
> >+static void mlxplat_lpc_ivb_config_clean(struct mlxplat_priv *priv,
> >+					 struct pci_dev *pdev)
> >+{
> >+	mlxplat_lpc_dec_rng_config_clean(pdev,
> >+				priv->lpc_reg[MLXPLAT_CPLD_LPC_RANGE],
> >+				MLXPLAT_CPLD_LPC_RANGE);
> >+	mlxplat_lpc_dec_rng_config_clean(pdev,
> >+				priv-
> >lpc_reg[MLXPLAT_CPLD_LPC_I2C_RANGE],
> >+				MLXPLAT_CPLD_LPC_I2C_RANGE);
> >+
> >+}
> >+
> >+static int mlxplat_lpc_range_config(struct mlxplat_priv *priv,
> >+				    struct pci_dev *pdev)
> >+{
> >+	u32 val, lpc_clks;
> >+	int err;
> >+
> >+	err = pci_read_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL,
> &val);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "Access to LPC Ctrl reg failed, err
> %d\n",
> >+			err);
> >+		return -EFAULT;
> >+	}
> >+	lpc_clks = val & 0x3;
> >+	if (lpc_clks != MLXPLAT_CPLD_LPC_CLKS_EN) {
> >+		val &= 0xFFFFFFFC;
> >+		err = pci_write_config_dword(pdev,
> MLXPLAT_CPLD_LPC_RNG_CTRL,
> >+					     val);
> >+		if (err) {
> >+			dev_err(&priv->pdev->dev, "Config LPC CLKS CTRL
> failed, err %d\n",
> >+				err);
> >+			return -EFAULT;
> >+		}
> >+	}
> >+
> >+	return err;
> >+}
> >+
> >+static int mlxplat_lpc_config(struct mlxplat_priv *priv) {
> >+	struct pci_dev *pdev = NULL;
> >+	u16 dev_id;
> >+	int err;
> >+
> >+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+
> 	PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> >+
> >+	if (!pdev) {
> >+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d
> func:%d not
> >found\n",
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		return -EFAULT;
> >+	}
> >+
> >+	err = pci_read_config_word(pdev, 2, &dev_id);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "Access PCIe LPC interface failed,
> err
> >%d\n",
> >+			err);
> >+		goto failure;
> >+	}
> >+
> >+	switch (dev_id) {
> >+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
> >+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
> >+		err = mlxplat_lpc_ivb_config(priv, pdev);
> >+		break;
> >+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
> >+		err = mlxplat_lpc_range_config(priv, pdev);
> >+		break;
> >+	default:
> >+		err = -ENXIO;
> >+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d
> slot:%d
> >func:%d\n",
> >+			dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		goto failure;
> >+	}
> >+	priv->dev_id = dev_id;
> >+
> >+failure:
> >+	pci_dev_put(pdev);
> >+	return err;
> >+}
> >+
> >+static int mlxplat_lpc_config_clean(struct mlxplat_priv *priv) {
> >+	struct pci_dev *pdev = NULL;
> >+	int err = 0;
> >+
> >+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+
> 	PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> >+	if (!pdev) {
> >+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d
> func:%d not
> >found\n",
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		return -EFAULT;
> >+	}
> >+
> >+	switch (priv->dev_id) {
> >+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
> >+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
> >+		mlxplat_lpc_ivb_config_clean(priv, pdev);
> >+		break;
> >+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
> >+		break;
> >+	default:
> >+		err = -ENXIO;
> >+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d
> slot:%d
> >func:%d\n",
> >+			priv->dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		break;
> >+	}
> >+
> >+	pci_dev_put(pdev);
> >+
> >+	return err;
> >+}
> >+
> >+static int __init mlxplat_init(void)
> >+{
> >+	struct mlxplat_priv *priv;
> >+	struct device *dev;
> >+	int i, j;
> >+	int err;
> >+
> >+	mlxplat_dev = platform_device_alloc(MLX_PLAT_DEVICE_NAME, -1);
> >+	if (!mlxplat_dev) {
> >+		pr_err("Alloc %s platform device failed\n",
> >+			MLX_PLAT_DEVICE_NAME);
> >+		return -ENOMEM;
> >+	}
> >+
> >+	err = platform_device_add(mlxplat_dev);
> >+	if (err) {
> >+		pr_err("Add %s platform device failed (%d)\n",
> >+			MLX_PLAT_DEVICE_NAME, err);
> >+		goto fail_platform_device_add;
> >+	}
> >+	dev = &mlxplat_dev->dev;
> >+
> >+	priv = devm_kzalloc(dev, sizeof(struct mlxplat_priv), GFP_KERNEL);
> >+	if (!priv) {
> >+		err = -ENOMEM;
> >+		dev_err(dev, "Failed to allocate mlxplat_priv\n");
> >+		goto fail_alloc;
> >+	}
> >+	platform_set_drvdata(mlxplat_dev, priv);
> >+	priv->pdev = mlxplat_dev;
> >+
> >+	err = mlxplat_lpc_config(priv);
> >+	if (err) {
> >+		dev_err(dev, "Failed to configure LPC interface\n");
> >+		goto fail_alloc;
> >+	}
> >+
> >+	err = mlxplat_lpc_request_regions(priv);
> >+	if (err) {
> >+		dev_err(dev, "Request ioregion failed (%d)\n", err);
> >+		goto fail_alloc;
> >+	}
> >+
> >+	priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", -1,
> >+							 NULL, 0);
> >+	if (IS_ERR(priv->pdev_i2c)) {
> >+		err = PTR_ERR(priv->pdev_i2c);
> >+		goto fail_alloc;
> >+	};
> >+
> >+	for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) {
> >+		mlxplat_topo[i].pdev = platform_device_register_resndata(dev,
> >+						mlxplat_topo[i].name, i, NULL,
> >+						0, &mlxplat_mux_data[i],
> >+						sizeof(mlxplat_mux_data[i]));
> >+		if (IS_ERR(mlxplat_topo[i].pdev)) {
> >+			err = PTR_ERR(mlxplat_topo[i].pdev);
> >+			goto fail_platform_mux_register;
> >+		}
> >+	}
> >+
> >+	return err;
> >+
> >+fail_platform_mux_register:
> >+	for (j = i; j > 0 ; j--)
> >+		platform_device_unregister(mlxplat_topo[j].pdev);
> >+	platform_device_unregister(priv->pdev_i2c);
> >+fail_alloc:
> >+	platform_device_del(mlxplat_dev);
> >+fail_platform_device_add:
> >+	platform_device_put(mlxplat_dev);
> >+
> >+	return err;
> >+}
> >+
> >+static void __exit mlxplat_exit(void)
> >+{
> >+	int i;
> >+	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
> >+
> >+	for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
> >+		platform_device_unregister(mlxplat_topo[i].pdev);
> >+
> >+	platform_device_unregister(priv->pdev_i2c);
> >+	mlxplat_lpc_config_clean(priv);
> >+	platform_device_del(mlxplat_dev);
> >+	platform_device_put(mlxplat_dev);
> >+}
> >+
> >+module_init(mlxplat_init);
> >+module_exit(mlxplat_exit);
> >+
> >+MODULE_AUTHOR("Vadim Pasternak (vadimp@mellanox.com)");
> >+MODULE_DESCRIPTION("Mellanox platform driver"); MODULE_LICENSE("GPL
> >+v2"); MODULE_ALIAS("platform:mlx-platform");
> 
> This is a horrifically uninformative piece of code.  What does this driver do?
> Why is it needed?  What makes it a "platform" in the sense of the way the term
> is used in the Linux kernel?  How does it affect the overall system, and in what
> sense does it diverge from a standard x86?

Hi Peter,

Thank for your reply.
This driver provides support for the number of Mellanox systems (switches), based on I7, Celeron, ATOM processors and equipped with Mellanox ASICs for Eth 100G/50G/40G/25G support.
It configures relevant PCI ranges and create basic infrastructure for switch.

If the location in arch/x86/platform is not correct for such code, maybe you could suggest another appropriate location?

Thanks,
Vadim.

> --
> Sent from my Android device with K-9 Mail. Please excuse brevity and
> formatting.

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  4:52     ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  4:52 UTC (permalink / raw)
  To: H. Peter Anvin, tglx
  Cc: mingo, davem, geert, akpm, gregkh, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@zytor.com]
> Sent: Monday, September 12, 2016 7:42 AM
> To: Vadim Pasternak <vadimp@mellanox.com>; tglx@linutronix.de
> Cc: mingo@redhat.com; davem@davemloft.net; geert@linux-m68k.org;
> akpm@linux-foundation.org; gregkh@linuxfoundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On September 11, 2016 11:29:58 PM PDT, vadimp@mellanox.com wrote:
> >From: Vadim Pasternak <vadimp@mellanox.com>
> >
> >Enable system support for the Mellanox Technologies platform, 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.
> >
> >The Kconfig currently controlling compilation of this code is:
> >arch/x86/platform:config MLX_PLATFORM
> >arch/x86/platform:      tristate "Mellanox Technologies platform
> >support"
> >
> >Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> >---
> > MAINTAINERS                               |   7 +
> > arch/x86/Kconfig                          |  23 ++
> > arch/x86/platform/Makefile                |   1 +
> > arch/x86/platform/mellanox/Makefile       |   1 +
> >arch/x86/platform/mellanox/mlx-platform.c | 501
> >++++++++++++++++++++++++++++++
> > 5 files changed, 533 insertions(+)
> > create mode 100644 arch/x86/platform/mellanox/Makefile
> > create mode 100644 arch/x86/platform/mellanox/mlx-platform.c
> >
> >diff --git a/MAINTAINERS b/MAINTAINERS
> >index 4705c94..8a675de 100644
> >--- a/MAINTAINERS
> >+++ b/MAINTAINERS
> >@@ -7664,6 +7664,13 @@ W:	http://www.mellanox.com
> > Q:	http://patchwork.ozlabs.org/project/netdev/list/
> > F:	drivers/net/ethernet/mellanox/mlxsw/
> >
> >+MELLANOX PLATFORM DRIVER
> >+M:      Vadim Pasternak <vadimp@mellanox.com>
> >+L:      platform-driver-x86@vger.kernel.org
> >+S:      Supported
> >+W:      http://www.mellanox.com
> >+F:      arch/x86/platform/mellanox/mlx-platform.c
> >+
> > SOFT-ROCE DRIVER (rxe)
> > M:	Moni Shoua <monis@mellanox.com>
> > L:	linux-rdma@vger.kernel.org
> >diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
> >c580d8c..cc7efdd9 100644
> >--- a/arch/x86/Kconfig
> >+++ b/arch/x86/Kconfig
> >@@ -2631,6 +2631,29 @@ config TS5500
> >
> > endif # X86_32
> >
> >+config MLX_PLATFORM
> >+	tristate "Mellanox Technologies platform support"
> >+	depends on X86_64
> >+	depends on PCI
> >+	depends on DMI
> >+	depends on I2C_MLXCPLD
> >+	depends on I2C_MUX_REG
> >+	select HWMON
> >+	select PMBUS
> >+	select LM75
> >+	select NEW_LEDS
> >+	select LEDS_CLASS
> >+	select LEDS_TRIGGERS
> >+	select LEDS_TRIGGER_TIMER
> >+	select LEDS_MLXCPLD
> >+	---help---
> >+	  This option enables system support for the Mellanox Technologies
> >+	  platform.
> >+
> >+	  Say Y here if you are building a kernel for Mellanox system.
> >+
> >+	  Otherwise, say N.
> >+
> > config AMD_NB
> > 	def_bool y
> > 	depends on CPU_SUP_AMD && PCI
> >diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
> >index 184842e..3c3c19e 100644
> >--- a/arch/x86/platform/Makefile
> >+++ b/arch/x86/platform/Makefile
> >@@ -8,6 +8,7 @@ obj-y	+= iris/
> > obj-y	+= intel/
> > obj-y	+= intel-mid/
> > obj-y	+= intel-quark/
> >+obj-y	+= mellanox/
> > obj-y	+= olpc/
> > obj-y	+= scx200/
> > obj-y	+= sfi/
> >diff --git a/arch/x86/platform/mellanox/Makefile
> >b/arch/x86/platform/mellanox/Makefile
> >new file mode 100644
> >index 0000000..f43c931
> >--- /dev/null
> >+++ b/arch/x86/platform/mellanox/Makefile
> >@@ -0,0 +1 @@
> >+obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
> >diff --git a/arch/x86/platform/mellanox/mlx-platform.c
> >b/arch/x86/platform/mellanox/mlx-platform.c
> >new file mode 100644
> >index 0000000..02afa89
> >--- /dev/null
> >+++ b/arch/x86/platform/mellanox/mlx-platform.c
> >@@ -0,0 +1,501 @@
> >+/*
> >+ * arch/x86/platform/mellanox/mlx-platform.c
> >+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> >+ * Copyright (c) 2016 Vadim Pasternak <vadimp@mellanox.com>
> >+ *
> >+ * 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/acpi.h>
> >+#include <linux/device.h>
> >+#include <linux/i2c.h>
> >+#include <linux/i2c-mux.h>
> >+#include <linux/module.h>
> >+#include <linux/of_platform.h>
> >+#include <linux/platform_device.h>
> >+#include <linux/platform_data/i2c-mux-reg.h>
> >+#include <linux/pci.h>
> >+#include <linux/slab.h>
> >+#include <linux/version.h>
> >+
> >+#define MLX_PLAT_DEVICE_NAME		"mlxplat"
> >+
> >+/* LPC IFC in PCH defines */
> >+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR		0x2000
> >+#define MLXPLAT_CPLD_LPC_REG_BASE_ADRR		0x2500
> >+#define MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID	0
> >+#define MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID	31
> >+#define MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID	0
> >+#define MLXPLAT_CPLD_LPC_QM67_DEV_ID		0x1c4f
> >+#define MLXPLAT_CPLD_LPC_QM77_DEV_ID		0x1e55
> >+#define MLXPLAT_CPLD_LPC_RNG_DEV_ID		0x1f38
> >+#define MLXPLAT_CPLD_LPC_I2C_CH1_OFF		0xdb
> >+#define MLXPLAT_CPLD_LPC_I2C_CH2_OFF		0xda
> >+#define MLXPLAT_CPLD_LPC_PIO_OFFSET		0x10000UL
> >+#define MLXPLAT_CPLD_LPC_REG1
> 	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
> >+				  MLXPLAT_CPLD_LPC_I2C_CH1_OFF) | \
> >+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
> >+#define MLXPLAT_CPLD_LPC_REG2
> 	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
> >+				  MLXPLAT_CPLD_LPC_I2C_CH2_OFF) | \
> >+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
> >+
> >+/* Use generic decode range 4 for CPLD LPC */
> >+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGE4	0x90
> >+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE	0x84
> >+#define MLXPLAT_CPLD_LPC_RNG_CTRL		0x84
> >+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES	4
> >+#define MLXPLAT_CPLD_LPC_I2C_RANGE		2
> >+#define MLXPLAT_CPLD_LPC_RANGE			3
> >+#define MLXPLAT_CPLD_LPC_CLKS_EN		0
> >+#define MLXPLAT_CPLD_LPC_IO_RANGE		0x100
> >+
> >+/* Start channel numbers */
> >+#define MLXPLAT_CPLD_CH1			2
> >+#define MLXPLAT_CPLD_CH2			10
> >+
> >+/* mlxplat_priv - board private data
> >+ * @lpc_reg - register space
> >+ * @dev_id - platform device id
> >+ * @lpc_i2c_res- lpc cpld i2c resource space
> >+ * @lpc_cpld_res - lpc cpld register resource space
> >+ * @pdev - platform device
> >+ */
> >+struct mlxplat_priv {
> >+	u32 lpc_reg[MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES];
> >+	u16 dev_id;
> >+	struct resource *lpc_i2c_res;
> >+	struct resource *lpc_cpld_res;
> >+	struct platform_device *pdev;
> >+	struct platform_device *pdev_i2c;
> >+};
> >+
> >+/* Regions for LPC I2C controller and LPC base register space */
> >+static struct resource mlxplat_lpc_resources[] = {
> >+	[0] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_I2C_BASE_ADRR,
> >+			       MLXPLAT_CPLD_LPC_IO_RANGE,
> >+			       "mlxplat_cpld_lpc_i2c_ctrl", IORESOURCE_IO),
> >+	[1] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_REG_BASE_ADRR,
> >+			       MLXPLAT_CPLD_LPC_IO_RANGE,
> >+			       "mlxplat_cpld_lpc_regs",
> >+			       IORESOURCE_IO),
> >+};
> >+
> >+/* Platfform channels */
> >+static int mlxplat_channels[][8] = {
> >+	{
> >+		MLXPLAT_CPLD_CH1, MLXPLAT_CPLD_CH1 + 1,
> MLXPLAT_CPLD_CH1 + 2,
> >+		MLXPLAT_CPLD_CH1 + 3, MLXPLAT_CPLD_CH1 + 4,
> MLXPLAT_CPLD_CH1 +
> >+		5, MLXPLAT_CPLD_CH1 + 6, MLXPLAT_CPLD_CH1 + 7
> >+	},
> >+	{
> >+		MLXPLAT_CPLD_CH2, MLXPLAT_CPLD_CH2 + 1,
> MLXPLAT_CPLD_CH2 + 2,
> >+		MLXPLAT_CPLD_CH2 + 3, MLXPLAT_CPLD_CH2 + 4,
> MLXPLAT_CPLD_CH2 +
> >+		+ 5, MLXPLAT_CPLD_CH2 + 6, MLXPLAT_CPLD_CH2 + 7
> >+	},
> >+};
> >+
> >+/* Platform mux data */
> >+struct i2c_mux_reg_platform_data mlxplat_mux_data[] = {
> >+	{
> >+		.parent = 1,
> >+		.base_nr = MLXPLAT_CPLD_CH1,
> >+		.write_only = 1,
> >+		.values = mlxplat_channels[0],
> >+		.n_values = ARRAY_SIZE(mlxplat_channels[0]),
> >+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG1,
> >+		.reg_size = 1,
> >+		.idle_in_use = 1,
> >+	},
> >+	{
> >+		.parent = 1,
> >+		.base_nr = MLXPLAT_CPLD_CH2,
> >+		.write_only = 1,
> >+		.values = mlxplat_channels[1],
> >+		.n_values = ARRAY_SIZE(mlxplat_channels[1]),
> >+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG2,
> >+		.reg_size = 1,
> >+		.idle_in_use = 1,
> >+	},
> >+
> >+};
> >+
> >+/* mlxplat_topology - platform entry data:
> >+ * @pdev - platform device
> >+ * @name - platform device name
> >+ */
> >+struct mlxplat_topology {
> >+	struct platform_device *pdev;
> >+	const char *name;
> >+};
> >+
> >+/* Platform topology */
> >+struct mlxplat_topology mlxplat_topo[] = {
> >+	{
> >+		.name = "i2c-mux-reg",
> >+	},
> >+	{
> >+		.name = "i2c-mux-reg",
> >+	},
> >+};
> >+
> >+struct platform_device *mlxplat_dev;
> >+
> >+static int mlxplat_lpc_i2c_dec_range_config(struct mlxplat_priv *priv,
> >+					    struct pci_dev *pdev, u8 range,
> >+					    u16 base_addr)
> >+{
> >+	u16 rng_reg;
> >+	u32 val;
> >+	int err;
> >+
> >+	if (range >= MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES) {
> >+		dev_err(&priv->pdev->dev, "Incorrect LPC decode range %d >
> %d\n",
> >+			range, MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES);
> >+		return -ERANGE;
> >+	}
> >+
> >+	rng_reg = MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE + 4 * range;
> >+	err = pci_read_config_dword(pdev, rng_reg, &val);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "Access to LPC_PCH config failed,
> err
> >%d\n",
> >+			err);
> >+		return -EFAULT;
> >+	}
> >+	priv->lpc_reg[range] = val;
> >+
> >+	/* Clean all bits excepted reserved (reserved: 2, 16, 17 , 24 - 31).
> >*/
> >+	val &= 0xff030002;
> >+	/* Set bits 18 - 23 to allow decode range address mask, set bit 1 to
> >+	 * enable decode range, clear bit 1,2 in base address.
> >+	 */
> >+	val |= 0xfc0001 | (base_addr & 0xfff3);
> >+	err = pci_write_config_dword(pdev, rng_reg, val);
> >+	if (err)
> >+		dev_err(&priv->pdev->dev, "Config of LPC_PCH Generic Decode
> Range %d
> >failed, err %d\n",
> >+			range, err);
> >+
> >+	return err;
> >+}
> >+
> >+static void mlxplat_lpc_dec_rng_config_clean(struct pci_dev *pdev, u32
> >val,
> >+					     u8 range)
> >+{
> >+	/* Restore old value */
> >+	if (pci_write_config_dword(pdev,
> (MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE +
> >+				   range * 4), val))
> >+		dev_err(&pdev->dev, "Deconfig of LPC_PCH Generic Decode
> Range %x
> >failed\n",
> >+			range);
> >+
> >+}
> >+
> >+static int mlxplat_lpc_request_region(struct mlxplat_priv *priv,
> >+				      struct resource *res)
> >+{
> >+	resource_size_t size = resource_size(res);
> >+
> >+	if (!devm_request_region(&priv->pdev->dev, res->start, size,
> >+				 res->name)) {
> >+		devm_release_region(&priv->pdev->dev, res->start, size);
> >+
> >+		if (!devm_request_region(&priv->pdev->dev, res->start, size,
> >+					 res->name)) {
> >+			dev_err(&priv->pdev->dev, "Request ioregion 0x%llx len
> 0x%llx for
> >%s fail\n",
> >+				res->start, size, res->name);
> >+			return -EIO;
> >+		}
> >+	}
> >+
> >+	return 0;
> >+}
> >+
> >+static int mlxplat_lpc_request_regions(struct mlxplat_priv *priv) {
> >+	int i;
> >+	int err;
> >+
> >+	for (i = 0; i < ARRAY_SIZE(mlxplat_lpc_resources); i++) {
> >+		err = mlxplat_lpc_request_region(priv,
> >+						 &mlxplat_lpc_resources[i]);
> >+		if (err)
> >+			return err;
> >+	}
> >+
> >+	priv->lpc_i2c_res = &mlxplat_lpc_resources[0];
> >+	priv->lpc_cpld_res = &mlxplat_lpc_resources[1];
> >+
> >+	return 0;
> >+}
> >+
> >+static int mlxplat_lpc_ivb_config(struct mlxplat_priv *priv,
> >+				  struct pci_dev *pdev)
> >+{
> >+	int err;
> >+
> >+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
> >+					       MLXPLAT_CPLD_LPC_I2C_RANGE,
> >+
> MLXPLAT_CPLD_LPC_I2C_BASE_ADRR);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed,
> err
> >%d\n",
> >+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
> >+		pci_dev_put(pdev);
> >+		return -EFAULT;
> >+	}
> >+
> >+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
> >+					       MLXPLAT_CPLD_LPC_RANGE,
> >+
> MLXPLAT_CPLD_LPC_REG_BASE_ADRR);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed,
> err
> >%d\n",
> >+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
> >+		return -EFAULT;
> >+	}
> >+
> >+	return err;
> >+}
> >+
> >+static void mlxplat_lpc_ivb_config_clean(struct mlxplat_priv *priv,
> >+					 struct pci_dev *pdev)
> >+{
> >+	mlxplat_lpc_dec_rng_config_clean(pdev,
> >+				priv->lpc_reg[MLXPLAT_CPLD_LPC_RANGE],
> >+				MLXPLAT_CPLD_LPC_RANGE);
> >+	mlxplat_lpc_dec_rng_config_clean(pdev,
> >+				priv-
> >lpc_reg[MLXPLAT_CPLD_LPC_I2C_RANGE],
> >+				MLXPLAT_CPLD_LPC_I2C_RANGE);
> >+
> >+}
> >+
> >+static int mlxplat_lpc_range_config(struct mlxplat_priv *priv,
> >+				    struct pci_dev *pdev)
> >+{
> >+	u32 val, lpc_clks;
> >+	int err;
> >+
> >+	err = pci_read_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL,
> &val);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "Access to LPC Ctrl reg failed, err
> %d\n",
> >+			err);
> >+		return -EFAULT;
> >+	}
> >+	lpc_clks = val & 0x3;
> >+	if (lpc_clks != MLXPLAT_CPLD_LPC_CLKS_EN) {
> >+		val &= 0xFFFFFFFC;
> >+		err = pci_write_config_dword(pdev,
> MLXPLAT_CPLD_LPC_RNG_CTRL,
> >+					     val);
> >+		if (err) {
> >+			dev_err(&priv->pdev->dev, "Config LPC CLKS CTRL
> failed, err %d\n",
> >+				err);
> >+			return -EFAULT;
> >+		}
> >+	}
> >+
> >+	return err;
> >+}
> >+
> >+static int mlxplat_lpc_config(struct mlxplat_priv *priv) {
> >+	struct pci_dev *pdev = NULL;
> >+	u16 dev_id;
> >+	int err;
> >+
> >+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+
> 	PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> >+
> >+	if (!pdev) {
> >+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d
> func:%d not
> >found\n",
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		return -EFAULT;
> >+	}
> >+
> >+	err = pci_read_config_word(pdev, 2, &dev_id);
> >+	if (err) {
> >+		dev_err(&priv->pdev->dev, "Access PCIe LPC interface failed,
> err
> >%d\n",
> >+			err);
> >+		goto failure;
> >+	}
> >+
> >+	switch (dev_id) {
> >+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
> >+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
> >+		err = mlxplat_lpc_ivb_config(priv, pdev);
> >+		break;
> >+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
> >+		err = mlxplat_lpc_range_config(priv, pdev);
> >+		break;
> >+	default:
> >+		err = -ENXIO;
> >+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d
> slot:%d
> >func:%d\n",
> >+			dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		goto failure;
> >+	}
> >+	priv->dev_id = dev_id;
> >+
> >+failure:
> >+	pci_dev_put(pdev);
> >+	return err;
> >+}
> >+
> >+static int mlxplat_lpc_config_clean(struct mlxplat_priv *priv) {
> >+	struct pci_dev *pdev = NULL;
> >+	int err = 0;
> >+
> >+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+
> 	PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> >+	if (!pdev) {
> >+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d
> func:%d not
> >found\n",
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		return -EFAULT;
> >+	}
> >+
> >+	switch (priv->dev_id) {
> >+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
> >+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
> >+		mlxplat_lpc_ivb_config_clean(priv, pdev);
> >+		break;
> >+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
> >+		break;
> >+	default:
> >+		err = -ENXIO;
> >+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d
> slot:%d
> >func:%d\n",
> >+			priv->dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> >+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> >+		break;
> >+	}
> >+
> >+	pci_dev_put(pdev);
> >+
> >+	return err;
> >+}
> >+
> >+static int __init mlxplat_init(void)
> >+{
> >+	struct mlxplat_priv *priv;
> >+	struct device *dev;
> >+	int i, j;
> >+	int err;
> >+
> >+	mlxplat_dev = platform_device_alloc(MLX_PLAT_DEVICE_NAME, -1);
> >+	if (!mlxplat_dev) {
> >+		pr_err("Alloc %s platform device failed\n",
> >+			MLX_PLAT_DEVICE_NAME);
> >+		return -ENOMEM;
> >+	}
> >+
> >+	err = platform_device_add(mlxplat_dev);
> >+	if (err) {
> >+		pr_err("Add %s platform device failed (%d)\n",
> >+			MLX_PLAT_DEVICE_NAME, err);
> >+		goto fail_platform_device_add;
> >+	}
> >+	dev = &mlxplat_dev->dev;
> >+
> >+	priv = devm_kzalloc(dev, sizeof(struct mlxplat_priv), GFP_KERNEL);
> >+	if (!priv) {
> >+		err = -ENOMEM;
> >+		dev_err(dev, "Failed to allocate mlxplat_priv\n");
> >+		goto fail_alloc;
> >+	}
> >+	platform_set_drvdata(mlxplat_dev, priv);
> >+	priv->pdev = mlxplat_dev;
> >+
> >+	err = mlxplat_lpc_config(priv);
> >+	if (err) {
> >+		dev_err(dev, "Failed to configure LPC interface\n");
> >+		goto fail_alloc;
> >+	}
> >+
> >+	err = mlxplat_lpc_request_regions(priv);
> >+	if (err) {
> >+		dev_err(dev, "Request ioregion failed (%d)\n", err);
> >+		goto fail_alloc;
> >+	}
> >+
> >+	priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", -1,
> >+							 NULL, 0);
> >+	if (IS_ERR(priv->pdev_i2c)) {
> >+		err = PTR_ERR(priv->pdev_i2c);
> >+		goto fail_alloc;
> >+	};
> >+
> >+	for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) {
> >+		mlxplat_topo[i].pdev = platform_device_register_resndata(dev,
> >+						mlxplat_topo[i].name, i, NULL,
> >+						0, &mlxplat_mux_data[i],
> >+						sizeof(mlxplat_mux_data[i]));
> >+		if (IS_ERR(mlxplat_topo[i].pdev)) {
> >+			err = PTR_ERR(mlxplat_topo[i].pdev);
> >+			goto fail_platform_mux_register;
> >+		}
> >+	}
> >+
> >+	return err;
> >+
> >+fail_platform_mux_register:
> >+	for (j = i; j > 0 ; j--)
> >+		platform_device_unregister(mlxplat_topo[j].pdev);
> >+	platform_device_unregister(priv->pdev_i2c);
> >+fail_alloc:
> >+	platform_device_del(mlxplat_dev);
> >+fail_platform_device_add:
> >+	platform_device_put(mlxplat_dev);
> >+
> >+	return err;
> >+}
> >+
> >+static void __exit mlxplat_exit(void)
> >+{
> >+	int i;
> >+	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
> >+
> >+	for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
> >+		platform_device_unregister(mlxplat_topo[i].pdev);
> >+
> >+	platform_device_unregister(priv->pdev_i2c);
> >+	mlxplat_lpc_config_clean(priv);
> >+	platform_device_del(mlxplat_dev);
> >+	platform_device_put(mlxplat_dev);
> >+}
> >+
> >+module_init(mlxplat_init);
> >+module_exit(mlxplat_exit);
> >+
> >+MODULE_AUTHOR("Vadim Pasternak (vadimp@mellanox.com)");
> >+MODULE_DESCRIPTION("Mellanox platform driver"); MODULE_LICENSE("GPL
> >+v2"); MODULE_ALIAS("platform:mlx-platform");
> 
> This is a horrifically uninformative piece of code.  What does this driver do?
> Why is it needed?  What makes it a "platform" in the sense of the way the term
> is used in the Linux kernel?  How does it affect the overall system, and in what
> sense does it diverge from a standard x86?

Hi Peter,

Thank for your reply.
This driver provides support for the number of Mellanox systems (switches), based on I7, Celeron, ATOM processors and equipped with Mellanox ASICs for Eth 100G/50G/40G/25G support.
It configures relevant PCI ranges and create basic infrastructure for switch.

If the location in arch/x86/platform is not correct for such code, maybe you could suggest another appropriate location?

Thanks,
Vadim.

> --
> Sent from my Android device with K-9 Mail. Please excuse brevity and
> formatting.

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  6:29 [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform vadimp
  2016-09-12  4:41 ` H. Peter Anvin
@ 2016-09-12  5:34 ` Guenter Roeck
  2016-09-12  6:11   ` Greg KH
  2016-09-12  6:12 ` Greg KH
  2 siblings, 1 reply; 39+ messages in thread
From: Guenter Roeck @ 2016-09-12  5:34 UTC (permalink / raw)
  To: vadimp, tglx
  Cc: mingo, hpa, davem, geert, akpm, gregkh, kvalo, mchehab, x86,
	linux-kernel, platform-driver-x86, jiri

On 09/11/2016 11:29 PM, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
>
> Enable system support for the Mellanox Technologies platform, 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.
>
> The Kconfig currently controlling compilation of this code is:
> arch/x86/platform:config MLX_PLATFORM
> arch/x86/platform:      tristate "Mellanox Technologies platform support"
>
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
>  MAINTAINERS                               |   7 +
>  arch/x86/Kconfig                          |  23 ++
>  arch/x86/platform/Makefile                |   1 +
>  arch/x86/platform/mellanox/Makefile       |   1 +
>  arch/x86/platform/mellanox/mlx-platform.c | 501 ++++++++++++++++++++++++++++++
>  5 files changed, 533 insertions(+)
>  create mode 100644 arch/x86/platform/mellanox/Makefile
>  create mode 100644 arch/x86/platform/mellanox/mlx-platform.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4705c94..8a675de 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7664,6 +7664,13 @@ W:	http://www.mellanox.com
>  Q:	http://patchwork.ozlabs.org/project/netdev/list/
>  F:	drivers/net/ethernet/mellanox/mlxsw/
>
> +MELLANOX PLATFORM DRIVER
> +M:      Vadim Pasternak <vadimp@mellanox.com>
> +L:      platform-driver-x86@vger.kernel.org
> +S:      Supported
> +W:      http://www.mellanox.com
> +F:      arch/x86/platform/mellanox/mlx-platform.c
> +
>  SOFT-ROCE DRIVER (rxe)
>  M:	Moni Shoua <monis@mellanox.com>
>  L:	linux-rdma@vger.kernel.org
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c580d8c..cc7efdd9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2631,6 +2631,29 @@ config TS5500
>
>  endif # X86_32
>
> +config MLX_PLATFORM
> +	tristate "Mellanox Technologies platform support"
> +	depends on X86_64
> +	depends on PCI
> +	depends on DMI
> +	depends on I2C_MLXCPLD
> +	depends on I2C_MUX_REG
> +	select HWMON
> +	select PMBUS
> +	select LM75
> +	select NEW_LEDS
> +	select LEDS_CLASS
> +	select LEDS_TRIGGERS
> +	select LEDS_TRIGGER_TIMER
> +	select LEDS_MLXCPLD
> +	---help---
> +	  This option enables system support for the Mellanox Technologies
> +	  platform.
> +
> +	  Say Y here if you are building a kernel for Mellanox system.
> +
> +	  Otherwise, say N.
> +
>  config AMD_NB
>  	def_bool y
>  	depends on CPU_SUP_AMD && PCI
> diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
> index 184842e..3c3c19e 100644
> --- a/arch/x86/platform/Makefile
> +++ b/arch/x86/platform/Makefile
> @@ -8,6 +8,7 @@ obj-y	+= iris/
>  obj-y	+= intel/
>  obj-y	+= intel-mid/
>  obj-y	+= intel-quark/
> +obj-y	+= mellanox/
>  obj-y	+= olpc/
>  obj-y	+= scx200/
>  obj-y	+= sfi/
> diff --git a/arch/x86/platform/mellanox/Makefile b/arch/x86/platform/mellanox/Makefile
> new file mode 100644
> index 0000000..f43c931
> --- /dev/null
> +++ b/arch/x86/platform/mellanox/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
> diff --git a/arch/x86/platform/mellanox/mlx-platform.c b/arch/x86/platform/mellanox/mlx-platform.c
> new file mode 100644
> index 0000000..02afa89
> --- /dev/null
> +++ b/arch/x86/platform/mellanox/mlx-platform.c
> @@ -0,0 +1,501 @@
> +/*
> + * arch/x86/platform/mellanox/mlx-platform.c
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Vadim Pasternak <vadimp@mellanox.com>
> + *
> + * 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/acpi.h>
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/i2c-mux-reg.h>
> +#include <linux/pci.h>
> +#include <linux/slab.h>
> +#include <linux/version.h>
> +
> +#define MLX_PLAT_DEVICE_NAME		"mlxplat"
> +
> +/* LPC IFC in PCH defines */
> +#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR		0x2000
> +#define MLXPLAT_CPLD_LPC_REG_BASE_ADRR		0x2500
> +#define MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID	0
> +#define MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID	31
> +#define MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID	0
> +#define MLXPLAT_CPLD_LPC_QM67_DEV_ID		0x1c4f
> +#define MLXPLAT_CPLD_LPC_QM77_DEV_ID		0x1e55
> +#define MLXPLAT_CPLD_LPC_RNG_DEV_ID		0x1f38
> +#define MLXPLAT_CPLD_LPC_I2C_CH1_OFF		0xdb
> +#define MLXPLAT_CPLD_LPC_I2C_CH2_OFF		0xda
> +#define MLXPLAT_CPLD_LPC_PIO_OFFSET		0x10000UL
> +#define MLXPLAT_CPLD_LPC_REG1	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
> +				  MLXPLAT_CPLD_LPC_I2C_CH1_OFF) | \
> +				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
> +#define MLXPLAT_CPLD_LPC_REG2	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
> +				  MLXPLAT_CPLD_LPC_I2C_CH2_OFF) | \
> +				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
> +
> +/* Use generic decode range 4 for CPLD LPC */
> +#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGE4	0x90
> +#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE	0x84
> +#define MLXPLAT_CPLD_LPC_RNG_CTRL		0x84
> +#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES	4
> +#define MLXPLAT_CPLD_LPC_I2C_RANGE		2
> +#define MLXPLAT_CPLD_LPC_RANGE			3
> +#define MLXPLAT_CPLD_LPC_CLKS_EN		0
> +#define MLXPLAT_CPLD_LPC_IO_RANGE		0x100
> +
> +/* Start channel numbers */
> +#define MLXPLAT_CPLD_CH1			2
> +#define MLXPLAT_CPLD_CH2			10
> +
> +/* mlxplat_priv - board private data
> + * @lpc_reg - register space
> + * @dev_id - platform device id
> + * @lpc_i2c_res- lpc cpld i2c resource space
> + * @lpc_cpld_res - lpc cpld register resource space
> + * @pdev - platform device
> + */
> +struct mlxplat_priv {
> +	u32 lpc_reg[MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES];
> +	u16 dev_id;
> +	struct resource *lpc_i2c_res;
> +	struct resource *lpc_cpld_res;
> +	struct platform_device *pdev;
> +	struct platform_device *pdev_i2c;
> +};
> +
> +/* Regions for LPC I2C controller and LPC base register space */
> +static struct resource mlxplat_lpc_resources[] = {
> +	[0] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_I2C_BASE_ADRR,
> +			       MLXPLAT_CPLD_LPC_IO_RANGE,
> +			       "mlxplat_cpld_lpc_i2c_ctrl", IORESOURCE_IO),
> +	[1] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_REG_BASE_ADRR,
> +			       MLXPLAT_CPLD_LPC_IO_RANGE,
> +			       "mlxplat_cpld_lpc_regs",
> +			       IORESOURCE_IO),
> +};
> +
> +/* Platfform channels */
> +static int mlxplat_channels[][8] = {
> +	{
> +		MLXPLAT_CPLD_CH1, MLXPLAT_CPLD_CH1 + 1, MLXPLAT_CPLD_CH1 + 2,
> +		MLXPLAT_CPLD_CH1 + 3, MLXPLAT_CPLD_CH1 + 4, MLXPLAT_CPLD_CH1 +
> +		5, MLXPLAT_CPLD_CH1 + 6, MLXPLAT_CPLD_CH1 + 7
> +	},
> +	{
> +		MLXPLAT_CPLD_CH2, MLXPLAT_CPLD_CH2 + 1, MLXPLAT_CPLD_CH2 + 2,
> +		MLXPLAT_CPLD_CH2 + 3, MLXPLAT_CPLD_CH2 + 4, MLXPLAT_CPLD_CH2 +
> +		+ 5, MLXPLAT_CPLD_CH2 + 6, MLXPLAT_CPLD_CH2 + 7
> +	},
> +};
> +
> +/* Platform mux data */
> +struct i2c_mux_reg_platform_data mlxplat_mux_data[] = {
> +	{
> +		.parent = 1,
> +		.base_nr = MLXPLAT_CPLD_CH1,
> +		.write_only = 1,
> +		.values = mlxplat_channels[0],
> +		.n_values = ARRAY_SIZE(mlxplat_channels[0]),
> +		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG1,
> +		.reg_size = 1,
> +		.idle_in_use = 1,
> +	},
> +	{
> +		.parent = 1,
> +		.base_nr = MLXPLAT_CPLD_CH2,
> +		.write_only = 1,
> +		.values = mlxplat_channels[1],
> +		.n_values = ARRAY_SIZE(mlxplat_channels[1]),
> +		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG2,
> +		.reg_size = 1,
> +		.idle_in_use = 1,
> +	},
> +
> +};
> +
> +/* mlxplat_topology - platform entry data:
> + * @pdev - platform device
> + * @name - platform device name
> + */
> +struct mlxplat_topology {
> +	struct platform_device *pdev;
> +	const char *name;
> +};
> +
> +/* Platform topology */
> +struct mlxplat_topology mlxplat_topo[] = {
> +	{
> +		.name = "i2c-mux-reg",
> +	},
> +	{
> +		.name = "i2c-mux-reg",
> +	},
> +};
> +
> +struct platform_device *mlxplat_dev;
> +
> +static int mlxplat_lpc_i2c_dec_range_config(struct mlxplat_priv *priv,
> +					    struct pci_dev *pdev, u8 range,
> +					    u16 base_addr)
> +{
> +	u16 rng_reg;
> +	u32 val;
> +	int err;
> +
> +	if (range >= MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES) {
> +		dev_err(&priv->pdev->dev, "Incorrect LPC decode range %d > %d\n",
> +			range, MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES);
> +		return -ERANGE;
> +	}
> +
> +	rng_reg = MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE + 4 * range;
> +	err = pci_read_config_dword(pdev, rng_reg, &val);
> +	if (err) {
> +		dev_err(&priv->pdev->dev, "Access to LPC_PCH config failed, err %d\n",
> +			err);
> +		return -EFAULT;
> +	}
> +	priv->lpc_reg[range] = val;
> +
> +	/* Clean all bits excepted reserved (reserved: 2, 16, 17 , 24 - 31). */
> +	val &= 0xff030002;
> +	/* Set bits 18 - 23 to allow decode range address mask, set bit 1 to
> +	 * enable decode range, clear bit 1,2 in base address.
> +	 */
> +	val |= 0xfc0001 | (base_addr & 0xfff3);
> +	err = pci_write_config_dword(pdev, rng_reg, val);
> +	if (err)
> +		dev_err(&priv->pdev->dev, "Config of LPC_PCH Generic Decode Range %d failed, err %d\n",
> +			range, err);
> +
> +	return err;
> +}
> +
> +static void mlxplat_lpc_dec_rng_config_clean(struct pci_dev *pdev, u32 val,
> +					     u8 range)
> +{
> +	/* Restore old value */
> +	if (pci_write_config_dword(pdev, (MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE +
> +				   range * 4), val))
> +		dev_err(&pdev->dev, "Deconfig of LPC_PCH Generic Decode Range %x failed\n",
> +			range);
> +
> +}
> +
> +static int mlxplat_lpc_request_region(struct mlxplat_priv *priv,
> +				      struct resource *res)
> +{
> +	resource_size_t size = resource_size(res);
> +
> +	if (!devm_request_region(&priv->pdev->dev, res->start, size,
> +				 res->name)) {
> +		devm_release_region(&priv->pdev->dev, res->start, size);
> +
> +		if (!devm_request_region(&priv->pdev->dev, res->start, size,
> +					 res->name)) {
> +			dev_err(&priv->pdev->dev, "Request ioregion 0x%llx len 0x%llx for %s fail\n",
> +				res->start, size, res->name);
> +			return -EIO;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int mlxplat_lpc_request_regions(struct mlxplat_priv *priv)
> +{
> +	int i;
> +	int err;
> +
> +	for (i = 0; i < ARRAY_SIZE(mlxplat_lpc_resources); i++) {
> +		err = mlxplat_lpc_request_region(priv,
> +						 &mlxplat_lpc_resources[i]);
> +		if (err)
> +			return err;
> +	}
> +
> +	priv->lpc_i2c_res = &mlxplat_lpc_resources[0];
> +	priv->lpc_cpld_res = &mlxplat_lpc_resources[1];
> +
> +	return 0;
> +}
> +
> +static int mlxplat_lpc_ivb_config(struct mlxplat_priv *priv,
> +				  struct pci_dev *pdev)
> +{
> +	int err;
> +
> +	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
> +					       MLXPLAT_CPLD_LPC_I2C_RANGE,
> +					       MLXPLAT_CPLD_LPC_I2C_BASE_ADRR);
> +	if (err) {
> +		dev_err(&priv->pdev->dev, "LPC decode range %d config failed, err %d\n",
> +			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
> +		pci_dev_put(pdev);
> +		return -EFAULT;
> +	}
> +
> +	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
> +					       MLXPLAT_CPLD_LPC_RANGE,
> +					       MLXPLAT_CPLD_LPC_REG_BASE_ADRR);
> +	if (err) {
> +		dev_err(&priv->pdev->dev, "LPC decode range %d config failed, err %d\n",
> +			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
> +		return -EFAULT;
> +	}
> +
> +	return err;
> +}
> +
> +static void mlxplat_lpc_ivb_config_clean(struct mlxplat_priv *priv,
> +					 struct pci_dev *pdev)
> +{
> +	mlxplat_lpc_dec_rng_config_clean(pdev,
> +				priv->lpc_reg[MLXPLAT_CPLD_LPC_RANGE],
> +				MLXPLAT_CPLD_LPC_RANGE);
> +	mlxplat_lpc_dec_rng_config_clean(pdev,
> +				priv->lpc_reg[MLXPLAT_CPLD_LPC_I2C_RANGE],
> +				MLXPLAT_CPLD_LPC_I2C_RANGE);
> +
> +}
> +
> +static int mlxplat_lpc_range_config(struct mlxplat_priv *priv,
> +				    struct pci_dev *pdev)
> +{
> +	u32 val, lpc_clks;
> +	int err;
> +
> +	err = pci_read_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL, &val);
> +	if (err) {
> +		dev_err(&priv->pdev->dev, "Access to LPC Ctrl reg failed, err %d\n",
> +			err);
> +		return -EFAULT;
> +	}
> +	lpc_clks = val & 0x3;
> +	if (lpc_clks != MLXPLAT_CPLD_LPC_CLKS_EN) {
> +		val &= 0xFFFFFFFC;
> +		err = pci_write_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL,
> +					     val);
> +		if (err) {
> +			dev_err(&priv->pdev->dev, "Config LPC CLKS CTRL failed, err %d\n",
> +				err);
> +			return -EFAULT;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +static int mlxplat_lpc_config(struct mlxplat_priv *priv)
> +{
> +	struct pci_dev *pdev = NULL;
> +	u16 dev_id;
> +	int err;
> +
> +	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> +				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> +				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> +

Kind of unusual way to initialize a PCI device. If this can't be implemented
as PCI driver, maybe it should be initialized using PCI quirks ?

> +	if (!pdev) {
> +		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d func:%d not found\n",
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> +		return -EFAULT;
> +	}
> +
> +	err = pci_read_config_word(pdev, 2, &dev_id);
> +	if (err) {
> +		dev_err(&priv->pdev->dev, "Access PCIe LPC interface failed, err %d\n",
> +			err);
> +		goto failure;
> +	}
> +
> +	switch (dev_id) {
> +	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
> +	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
> +		err = mlxplat_lpc_ivb_config(priv, pdev);
> +		break;
> +	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
> +		err = mlxplat_lpc_range_config(priv, pdev);
> +		break;
> +	default:
> +		err = -ENXIO;
> +		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d slot:%d func:%d\n",
> +			dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> +		goto failure;
> +	}
> +	priv->dev_id = dev_id;
> +
> +failure:
> +	pci_dev_put(pdev);
> +	return err;
> +}
> +
> +static int mlxplat_lpc_config_clean(struct mlxplat_priv *priv)
> +{
> +	struct pci_dev *pdev = NULL;
> +	int err = 0;
> +
> +	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> +				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> +				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> +	if (!pdev) {
> +		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d func:%d not found\n",
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> +		return -EFAULT;

WDAULT seems wrong.

> +	}
> +
> +	switch (priv->dev_id) {
> +	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
> +	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
> +		mlxplat_lpc_ivb_config_clean(priv, pdev);
> +		break;
> +	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
> +		break;
> +	default:
> +		err = -ENXIO;

ENODEV ?

> +		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d slot:%d func:%d\n",
> +			priv->dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> +			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
> +		break;
> +	}
> +
> +	pci_dev_put(pdev);
> +
> +	return err;
> +}
> +
> +static int __init mlxplat_init(void)
> +{
> +	struct mlxplat_priv *priv;
> +	struct device *dev;
> +	int i, j;
> +	int err;
> +

No platform detection ? I would expect that DMI or ACPI or maybe devicetree
would be used to detect if this is a supported Mellanox platform.

Guenter

> +	mlxplat_dev = platform_device_alloc(MLX_PLAT_DEVICE_NAME, -1);
> +	if (!mlxplat_dev) {
> +		pr_err("Alloc %s platform device failed\n",
> +			MLX_PLAT_DEVICE_NAME);
> +		return -ENOMEM;
> +	}
> +
> +	err = platform_device_add(mlxplat_dev);
> +	if (err) {
> +		pr_err("Add %s platform device failed (%d)\n",
> +			MLX_PLAT_DEVICE_NAME, err);
> +		goto fail_platform_device_add;
> +	}
> +	dev = &mlxplat_dev->dev;
> +
> +	priv = devm_kzalloc(dev, sizeof(struct mlxplat_priv), GFP_KERNEL);
> +	if (!priv) {
> +		err = -ENOMEM;
> +		dev_err(dev, "Failed to allocate mlxplat_priv\n");
> +		goto fail_alloc;
> +	}
> +	platform_set_drvdata(mlxplat_dev, priv);
> +	priv->pdev = mlxplat_dev;
> +
> +	err = mlxplat_lpc_config(priv);
> +	if (err) {
> +		dev_err(dev, "Failed to configure LPC interface\n");
> +		goto fail_alloc;
> +	}
> +
> +	err = mlxplat_lpc_request_regions(priv);
> +	if (err) {
> +		dev_err(dev, "Request ioregion failed (%d)\n", err);
> +		goto fail_alloc;
> +	}
> +
> +	priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", -1,
> +							 NULL, 0);
> +	if (IS_ERR(priv->pdev_i2c)) {
> +		err = PTR_ERR(priv->pdev_i2c);
> +		goto fail_alloc;
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) {
> +		mlxplat_topo[i].pdev = platform_device_register_resndata(dev,
> +						mlxplat_topo[i].name, i, NULL,
> +						0, &mlxplat_mux_data[i],
> +						sizeof(mlxplat_mux_data[i]));
> +		if (IS_ERR(mlxplat_topo[i].pdev)) {
> +			err = PTR_ERR(mlxplat_topo[i].pdev);
> +			goto fail_platform_mux_register;
> +		}
> +	}
> +
> +	return err;
> +
> +fail_platform_mux_register:
> +	for (j = i; j > 0 ; j--)
> +		platform_device_unregister(mlxplat_topo[j].pdev);
> +	platform_device_unregister(priv->pdev_i2c);
> +fail_alloc:
> +	platform_device_del(mlxplat_dev);
> +fail_platform_device_add:
> +	platform_device_put(mlxplat_dev);
> +
> +	return err;
> +}
> +
> +static void __exit mlxplat_exit(void)
> +{
> +	int i;
> +	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
> +
> +	for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
> +		platform_device_unregister(mlxplat_topo[i].pdev);
> +
> +	platform_device_unregister(priv->pdev_i2c);
> +	mlxplat_lpc_config_clean(priv);
> +	platform_device_del(mlxplat_dev);
> +	platform_device_put(mlxplat_dev);
> +}
> +
> +module_init(mlxplat_init);
> +module_exit(mlxplat_exit);
> +
> +MODULE_AUTHOR("Vadim Pasternak (vadimp@mellanox.com)");
> +MODULE_DESCRIPTION("Mellanox platform driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:mlx-platform");
>

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  5:34 ` Guenter Roeck
@ 2016-09-12  6:11   ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  6:11 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: vadimp, tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab,
	x86, linux-kernel, platform-driver-x86, jiri

On Sun, Sep 11, 2016 at 10:34:27PM -0700, Guenter Roeck wrote:
> > +static int mlxplat_lpc_config(struct mlxplat_priv *priv)
> > +{
> > +	struct pci_dev *pdev = NULL;
> > +	u16 dev_id;
> > +	int err;
> > +
> > +	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
> > +				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
> > +				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
> > +
> 
> Kind of unusual way to initialize a PCI device. If this can't be implemented
> as PCI driver, maybe it should be initialized using PCI quirks ?

That's a _very old_ way of writing a pci driver, I thought we had gotten
rid of all of that crud.

This needs to be a "normal" PCI driver, no need for it to be a platform
driver at all from what I can tell.

thanks,

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  6:29 [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform vadimp
  2016-09-12  4:41 ` H. Peter Anvin
  2016-09-12  5:34 ` Guenter Roeck
@ 2016-09-12  6:12 ` Greg KH
  2016-09-12  6:44     ` Vadim Pasternak
  2 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2016-09-12  6:12 UTC (permalink / raw)
  To: vadimp
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> Enable system support for the Mellanox Technologies platform, 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.

What does "system support" mean?

Why can't this just be a "normal" PCI driver, as you are just accessing
a PCI device and doing something with it, seems odd to claim it is a
"platform" driver.

thanks,

greg k-h

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

* [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  6:29 vadimp
  2016-09-12  4:41 ` H. Peter Anvin
                   ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: vadimp @ 2016-09-12  6:29 UTC (permalink / raw)
  To: tglx
  Cc: mingo, hpa, davem, geert, akpm, gregkh, kvalo, mchehab, linux,
	x86, linux-kernel, platform-driver-x86, jiri, Vadim Pasternak

From: Vadim Pasternak <vadimp@mellanox.com>

Enable system support for the Mellanox Technologies platform, 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.

The Kconfig currently controlling compilation of this code is:
arch/x86/platform:config MLX_PLATFORM
arch/x86/platform:      tristate "Mellanox Technologies platform support"

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 MAINTAINERS                               |   7 +
 arch/x86/Kconfig                          |  23 ++
 arch/x86/platform/Makefile                |   1 +
 arch/x86/platform/mellanox/Makefile       |   1 +
 arch/x86/platform/mellanox/mlx-platform.c | 501 ++++++++++++++++++++++++++++++
 5 files changed, 533 insertions(+)
 create mode 100644 arch/x86/platform/mellanox/Makefile
 create mode 100644 arch/x86/platform/mellanox/mlx-platform.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4705c94..8a675de 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7664,6 +7664,13 @@ W:	http://www.mellanox.com
 Q:	http://patchwork.ozlabs.org/project/netdev/list/
 F:	drivers/net/ethernet/mellanox/mlxsw/
 
+MELLANOX PLATFORM DRIVER
+M:      Vadim Pasternak <vadimp@mellanox.com>
+L:      platform-driver-x86@vger.kernel.org
+S:      Supported
+W:      http://www.mellanox.com
+F:      arch/x86/platform/mellanox/mlx-platform.c
+
 SOFT-ROCE DRIVER (rxe)
 M:	Moni Shoua <monis@mellanox.com>
 L:	linux-rdma@vger.kernel.org
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c580d8c..cc7efdd9 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2631,6 +2631,29 @@ config TS5500
 
 endif # X86_32
 
+config MLX_PLATFORM
+	tristate "Mellanox Technologies platform support"
+	depends on X86_64
+	depends on PCI
+	depends on DMI
+	depends on I2C_MLXCPLD
+	depends on I2C_MUX_REG
+	select HWMON
+	select PMBUS
+	select LM75
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_TIMER
+	select LEDS_MLXCPLD
+	---help---
+	  This option enables system support for the Mellanox Technologies
+	  platform.
+
+	  Say Y here if you are building a kernel for Mellanox system.
+
+	  Otherwise, say N.
+
 config AMD_NB
 	def_bool y
 	depends on CPU_SUP_AMD && PCI
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 184842e..3c3c19e 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -8,6 +8,7 @@ obj-y	+= iris/
 obj-y	+= intel/
 obj-y	+= intel-mid/
 obj-y	+= intel-quark/
+obj-y	+= mellanox/
 obj-y	+= olpc/
 obj-y	+= scx200/
 obj-y	+= sfi/
diff --git a/arch/x86/platform/mellanox/Makefile b/arch/x86/platform/mellanox/Makefile
new file mode 100644
index 0000000..f43c931
--- /dev/null
+++ b/arch/x86/platform/mellanox/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
diff --git a/arch/x86/platform/mellanox/mlx-platform.c b/arch/x86/platform/mellanox/mlx-platform.c
new file mode 100644
index 0000000..02afa89
--- /dev/null
+++ b/arch/x86/platform/mellanox/mlx-platform.c
@@ -0,0 +1,501 @@
+/*
+ * arch/x86/platform/mellanox/mlx-platform.c
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Vadim Pasternak <vadimp@mellanox.com>
+ *
+ * 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/acpi.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/i2c-mux-reg.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/version.h>
+
+#define MLX_PLAT_DEVICE_NAME		"mlxplat"
+
+/* LPC IFC in PCH defines */
+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR		0x2000
+#define MLXPLAT_CPLD_LPC_REG_BASE_ADRR		0x2500
+#define MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID	0
+#define MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID	31
+#define MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID	0
+#define MLXPLAT_CPLD_LPC_QM67_DEV_ID		0x1c4f
+#define MLXPLAT_CPLD_LPC_QM77_DEV_ID		0x1e55
+#define MLXPLAT_CPLD_LPC_RNG_DEV_ID		0x1f38
+#define MLXPLAT_CPLD_LPC_I2C_CH1_OFF		0xdb
+#define MLXPLAT_CPLD_LPC_I2C_CH2_OFF		0xda
+#define MLXPLAT_CPLD_LPC_PIO_OFFSET		0x10000UL
+#define MLXPLAT_CPLD_LPC_REG1	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
+				  MLXPLAT_CPLD_LPC_I2C_CH1_OFF) | \
+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
+#define MLXPLAT_CPLD_LPC_REG2	((MLXPLAT_CPLD_LPC_REG_BASE_ADRR + \
+				  MLXPLAT_CPLD_LPC_I2C_CH2_OFF) | \
+				  MLXPLAT_CPLD_LPC_PIO_OFFSET)
+
+/* Use generic decode range 4 for CPLD LPC */
+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGE4	0x90
+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE	0x84
+#define MLXPLAT_CPLD_LPC_RNG_CTRL		0x84
+#define MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES	4
+#define MLXPLAT_CPLD_LPC_I2C_RANGE		2
+#define MLXPLAT_CPLD_LPC_RANGE			3
+#define MLXPLAT_CPLD_LPC_CLKS_EN		0
+#define MLXPLAT_CPLD_LPC_IO_RANGE		0x100
+
+/* Start channel numbers */
+#define MLXPLAT_CPLD_CH1			2
+#define MLXPLAT_CPLD_CH2			10
+
+/* mlxplat_priv - board private data
+ * @lpc_reg - register space
+ * @dev_id - platform device id
+ * @lpc_i2c_res- lpc cpld i2c resource space
+ * @lpc_cpld_res - lpc cpld register resource space
+ * @pdev - platform device
+ */
+struct mlxplat_priv {
+	u32 lpc_reg[MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES];
+	u16 dev_id;
+	struct resource *lpc_i2c_res;
+	struct resource *lpc_cpld_res;
+	struct platform_device *pdev;
+	struct platform_device *pdev_i2c;
+};
+
+/* Regions for LPC I2C controller and LPC base register space */
+static struct resource mlxplat_lpc_resources[] = {
+	[0] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_I2C_BASE_ADRR,
+			       MLXPLAT_CPLD_LPC_IO_RANGE,
+			       "mlxplat_cpld_lpc_i2c_ctrl", IORESOURCE_IO),
+	[1] = DEFINE_RES_NAMED(MLXPLAT_CPLD_LPC_REG_BASE_ADRR,
+			       MLXPLAT_CPLD_LPC_IO_RANGE,
+			       "mlxplat_cpld_lpc_regs",
+			       IORESOURCE_IO),
+};
+
+/* Platfform channels */
+static int mlxplat_channels[][8] = {
+	{
+		MLXPLAT_CPLD_CH1, MLXPLAT_CPLD_CH1 + 1, MLXPLAT_CPLD_CH1 + 2,
+		MLXPLAT_CPLD_CH1 + 3, MLXPLAT_CPLD_CH1 + 4, MLXPLAT_CPLD_CH1 +
+		5, MLXPLAT_CPLD_CH1 + 6, MLXPLAT_CPLD_CH1 + 7
+	},
+	{
+		MLXPLAT_CPLD_CH2, MLXPLAT_CPLD_CH2 + 1, MLXPLAT_CPLD_CH2 + 2,
+		MLXPLAT_CPLD_CH2 + 3, MLXPLAT_CPLD_CH2 + 4, MLXPLAT_CPLD_CH2 +
+		+ 5, MLXPLAT_CPLD_CH2 + 6, MLXPLAT_CPLD_CH2 + 7
+	},
+};
+
+/* Platform mux data */
+struct i2c_mux_reg_platform_data mlxplat_mux_data[] = {
+	{
+		.parent = 1,
+		.base_nr = MLXPLAT_CPLD_CH1,
+		.write_only = 1,
+		.values = mlxplat_channels[0],
+		.n_values = ARRAY_SIZE(mlxplat_channels[0]),
+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG1,
+		.reg_size = 1,
+		.idle_in_use = 1,
+	},
+	{
+		.parent = 1,
+		.base_nr = MLXPLAT_CPLD_CH2,
+		.write_only = 1,
+		.values = mlxplat_channels[1],
+		.n_values = ARRAY_SIZE(mlxplat_channels[1]),
+		.reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG2,
+		.reg_size = 1,
+		.idle_in_use = 1,
+	},
+
+};
+
+/* mlxplat_topology - platform entry data:
+ * @pdev - platform device
+ * @name - platform device name
+ */
+struct mlxplat_topology {
+	struct platform_device *pdev;
+	const char *name;
+};
+
+/* Platform topology */
+struct mlxplat_topology mlxplat_topo[] = {
+	{
+		.name = "i2c-mux-reg",
+	},
+	{
+		.name = "i2c-mux-reg",
+	},
+};
+
+struct platform_device *mlxplat_dev;
+
+static int mlxplat_lpc_i2c_dec_range_config(struct mlxplat_priv *priv,
+					    struct pci_dev *pdev, u8 range,
+					    u16 base_addr)
+{
+	u16 rng_reg;
+	u32 val;
+	int err;
+
+	if (range >= MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES) {
+		dev_err(&priv->pdev->dev, "Incorrect LPC decode range %d > %d\n",
+			range, MLXPLAT_CPLD_LPC_PCH_GEN_DEC_RANGES);
+		return -ERANGE;
+	}
+
+	rng_reg = MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE + 4 * range;
+	err = pci_read_config_dword(pdev, rng_reg, &val);
+	if (err) {
+		dev_err(&priv->pdev->dev, "Access to LPC_PCH config failed, err %d\n",
+			err);
+		return -EFAULT;
+	}
+	priv->lpc_reg[range] = val;
+
+	/* Clean all bits excepted reserved (reserved: 2, 16, 17 , 24 - 31). */
+	val &= 0xff030002;
+	/* Set bits 18 - 23 to allow decode range address mask, set bit 1 to
+	 * enable decode range, clear bit 1,2 in base address.
+	 */
+	val |= 0xfc0001 | (base_addr & 0xfff3);
+	err = pci_write_config_dword(pdev, rng_reg, val);
+	if (err)
+		dev_err(&priv->pdev->dev, "Config of LPC_PCH Generic Decode Range %d failed, err %d\n",
+			range, err);
+
+	return err;
+}
+
+static void mlxplat_lpc_dec_rng_config_clean(struct pci_dev *pdev, u32 val,
+					     u8 range)
+{
+	/* Restore old value */
+	if (pci_write_config_dword(pdev, (MLXPLAT_CPLD_LPC_PCH_GEN_DEC_BASE +
+				   range * 4), val))
+		dev_err(&pdev->dev, "Deconfig of LPC_PCH Generic Decode Range %x failed\n",
+			range);
+
+}
+
+static int mlxplat_lpc_request_region(struct mlxplat_priv *priv,
+				      struct resource *res)
+{
+	resource_size_t size = resource_size(res);
+
+	if (!devm_request_region(&priv->pdev->dev, res->start, size,
+				 res->name)) {
+		devm_release_region(&priv->pdev->dev, res->start, size);
+
+		if (!devm_request_region(&priv->pdev->dev, res->start, size,
+					 res->name)) {
+			dev_err(&priv->pdev->dev, "Request ioregion 0x%llx len 0x%llx for %s fail\n",
+				res->start, size, res->name);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+static int mlxplat_lpc_request_regions(struct mlxplat_priv *priv)
+{
+	int i;
+	int err;
+
+	for (i = 0; i < ARRAY_SIZE(mlxplat_lpc_resources); i++) {
+		err = mlxplat_lpc_request_region(priv,
+						 &mlxplat_lpc_resources[i]);
+		if (err)
+			return err;
+	}
+
+	priv->lpc_i2c_res = &mlxplat_lpc_resources[0];
+	priv->lpc_cpld_res = &mlxplat_lpc_resources[1];
+
+	return 0;
+}
+
+static int mlxplat_lpc_ivb_config(struct mlxplat_priv *priv,
+				  struct pci_dev *pdev)
+{
+	int err;
+
+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
+					       MLXPLAT_CPLD_LPC_I2C_RANGE,
+					       MLXPLAT_CPLD_LPC_I2C_BASE_ADRR);
+	if (err) {
+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed, err %d\n",
+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
+		pci_dev_put(pdev);
+		return -EFAULT;
+	}
+
+	err = mlxplat_lpc_i2c_dec_range_config(priv, pdev,
+					       MLXPLAT_CPLD_LPC_RANGE,
+					       MLXPLAT_CPLD_LPC_REG_BASE_ADRR);
+	if (err) {
+		dev_err(&priv->pdev->dev, "LPC decode range %d config failed, err %d\n",
+			MLXPLAT_CPLD_LPC_I2C_RANGE, err);
+		return -EFAULT;
+	}
+
+	return err;
+}
+
+static void mlxplat_lpc_ivb_config_clean(struct mlxplat_priv *priv,
+					 struct pci_dev *pdev)
+{
+	mlxplat_lpc_dec_rng_config_clean(pdev,
+				priv->lpc_reg[MLXPLAT_CPLD_LPC_RANGE],
+				MLXPLAT_CPLD_LPC_RANGE);
+	mlxplat_lpc_dec_rng_config_clean(pdev,
+				priv->lpc_reg[MLXPLAT_CPLD_LPC_I2C_RANGE],
+				MLXPLAT_CPLD_LPC_I2C_RANGE);
+
+}
+
+static int mlxplat_lpc_range_config(struct mlxplat_priv *priv,
+				    struct pci_dev *pdev)
+{
+	u32 val, lpc_clks;
+	int err;
+
+	err = pci_read_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL, &val);
+	if (err) {
+		dev_err(&priv->pdev->dev, "Access to LPC Ctrl reg failed, err %d\n",
+			err);
+		return -EFAULT;
+	}
+	lpc_clks = val & 0x3;
+	if (lpc_clks != MLXPLAT_CPLD_LPC_CLKS_EN) {
+		val &= 0xFFFFFFFC;
+		err = pci_write_config_dword(pdev, MLXPLAT_CPLD_LPC_RNG_CTRL,
+					     val);
+		if (err) {
+			dev_err(&priv->pdev->dev, "Config LPC CLKS CTRL failed, err %d\n",
+				err);
+			return -EFAULT;
+		}
+	}
+
+	return err;
+}
+
+static int mlxplat_lpc_config(struct mlxplat_priv *priv)
+{
+	struct pci_dev *pdev = NULL;
+	u16 dev_id;
+	int err;
+
+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
+				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
+
+	if (!pdev) {
+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d func:%d not found\n",
+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
+		return -EFAULT;
+	}
+
+	err = pci_read_config_word(pdev, 2, &dev_id);
+	if (err) {
+		dev_err(&priv->pdev->dev, "Access PCIe LPC interface failed, err %d\n",
+			err);
+		goto failure;
+	}
+
+	switch (dev_id) {
+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
+		err = mlxplat_lpc_ivb_config(priv, pdev);
+		break;
+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
+		err = mlxplat_lpc_range_config(priv, pdev);
+		break;
+	default:
+		err = -ENXIO;
+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d slot:%d func:%d\n",
+			dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
+		goto failure;
+	}
+	priv->dev_id = dev_id;
+
+failure:
+	pci_dev_put(pdev);
+	return err;
+}
+
+static int mlxplat_lpc_config_clean(struct mlxplat_priv *priv)
+{
+	struct pci_dev *pdev = NULL;
+	int err = 0;
+
+	pdev = pci_get_bus_and_slot(MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
+				PCI_DEVFN(MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
+				MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID));
+	if (!pdev) {
+		dev_err(&priv->pdev->dev, "LPC controller bus:%d slot:%d func:%d not found\n",
+			MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
+		return -EFAULT;
+	}
+
+	switch (priv->dev_id) {
+	case MLXPLAT_CPLD_LPC_QM67_DEV_ID:
+	case MLXPLAT_CPLD_LPC_QM77_DEV_ID:
+		mlxplat_lpc_ivb_config_clean(priv, pdev);
+		break;
+	case MLXPLAT_CPLD_LPC_RNG_DEV_ID:
+		break;
+	default:
+		err = -ENXIO;
+		dev_err(&priv->pdev->dev, "Unsupported DevId 0x%x bus:%d slot:%d func:%d\n",
+			priv->dev_id, MLXPLAT_CPLD_LPC_CTRL_IFC_BUS_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_SLOT_ID,
+			MLXPLAT_CPLD_LPC_CTRL_IFC_FUNC_ID);
+		break;
+	}
+
+	pci_dev_put(pdev);
+
+	return err;
+}
+
+static int __init mlxplat_init(void)
+{
+	struct mlxplat_priv *priv;
+	struct device *dev;
+	int i, j;
+	int err;
+
+	mlxplat_dev = platform_device_alloc(MLX_PLAT_DEVICE_NAME, -1);
+	if (!mlxplat_dev) {
+		pr_err("Alloc %s platform device failed\n",
+			MLX_PLAT_DEVICE_NAME);
+		return -ENOMEM;
+	}
+
+	err = platform_device_add(mlxplat_dev);
+	if (err) {
+		pr_err("Add %s platform device failed (%d)\n",
+			MLX_PLAT_DEVICE_NAME, err);
+		goto fail_platform_device_add;
+	}
+	dev = &mlxplat_dev->dev;
+
+	priv = devm_kzalloc(dev, sizeof(struct mlxplat_priv), GFP_KERNEL);
+	if (!priv) {
+		err = -ENOMEM;
+		dev_err(dev, "Failed to allocate mlxplat_priv\n");
+		goto fail_alloc;
+	}
+	platform_set_drvdata(mlxplat_dev, priv);
+	priv->pdev = mlxplat_dev;
+
+	err = mlxplat_lpc_config(priv);
+	if (err) {
+		dev_err(dev, "Failed to configure LPC interface\n");
+		goto fail_alloc;
+	}
+
+	err = mlxplat_lpc_request_regions(priv);
+	if (err) {
+		dev_err(dev, "Request ioregion failed (%d)\n", err);
+		goto fail_alloc;
+	}
+
+	priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", -1,
+							 NULL, 0);
+	if (IS_ERR(priv->pdev_i2c)) {
+		err = PTR_ERR(priv->pdev_i2c);
+		goto fail_alloc;
+	};
+
+	for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) {
+		mlxplat_topo[i].pdev = platform_device_register_resndata(dev,
+						mlxplat_topo[i].name, i, NULL,
+						0, &mlxplat_mux_data[i],
+						sizeof(mlxplat_mux_data[i]));
+		if (IS_ERR(mlxplat_topo[i].pdev)) {
+			err = PTR_ERR(mlxplat_topo[i].pdev);
+			goto fail_platform_mux_register;
+		}
+	}
+
+	return err;
+
+fail_platform_mux_register:
+	for (j = i; j > 0 ; j--)
+		platform_device_unregister(mlxplat_topo[j].pdev);
+	platform_device_unregister(priv->pdev_i2c);
+fail_alloc:
+	platform_device_del(mlxplat_dev);
+fail_platform_device_add:
+	platform_device_put(mlxplat_dev);
+
+	return err;
+}
+
+static void __exit mlxplat_exit(void)
+{
+	int i;
+	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
+
+	for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
+		platform_device_unregister(mlxplat_topo[i].pdev);
+
+	platform_device_unregister(priv->pdev_i2c);
+	mlxplat_lpc_config_clean(priv);
+	platform_device_del(mlxplat_dev);
+	platform_device_put(mlxplat_dev);
+}
+
+module_init(mlxplat_init);
+module_exit(mlxplat_exit);
+
+MODULE_AUTHOR("Vadim Pasternak (vadimp@mellanox.com)");
+MODULE_DESCRIPTION("Mellanox platform driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mlx-platform");
-- 
2.1.4

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  6:12 ` Greg KH
@ 2016-09-12  6:44     ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  6:44 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 9:13 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > From: Vadim Pasternak <vadimp@mellanox.com>
> >
> > Enable system support for the Mellanox Technologies platform, 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.
> 
> What does "system support" mean?
> 
> Why can't this just be a "normal" PCI driver, as you are just accessing a PCI
> device and doing something with it, seems odd to claim it is a "platform" driver.
> 

This driver also activates probes to create i2c platform driver and muxes.
For ARM and PPC based systems I can activate such stuff through dts. To be honest I don't know what is the right way to do such things for x86 systems.
If I will move PCI related stuff to separate driver, could you suggest some right location for that?

For example, could I have the code like in f.e. in arch/x86/platform/ts5500/ts5500.c as a platform initialization code? 

Thanks,
Vadim.

> thanks,
> 
> greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  6:44     ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  6:44 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 9:13 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > From: Vadim Pasternak <vadimp@mellanox.com>
> >
> > Enable system support for the Mellanox Technologies platform, 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.
> 
> What does "system support" mean?
> 
> Why can't this just be a "normal" PCI driver, as you are just accessing a PCI
> device and doing something with it, seems odd to claim it is a "platform" driver.
> 

This driver also activates probes to create i2c platform driver and muxes.
For ARM and PPC based systems I can activate such stuff through dts. To be honest I don't know what is the right way to do such things for x86 systems.
If I will move PCI related stuff to separate driver, could you suggest some right location for that?

For example, could I have the code like in f.e. in arch/x86/platform/ts5500/ts5500.c as a platform initialization code? 

Thanks,
Vadim.

> thanks,
> 
> greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  6:44     ` Vadim Pasternak
@ 2016-09-12  7:04       ` Greg KH
  -1 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  7:04 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 9:13 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > > From: Vadim Pasternak <vadimp@mellanox.com>
> > >
> > > Enable system support for the Mellanox Technologies platform, 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.
> > 
> > What does "system support" mean?
> > 
> > Why can't this just be a "normal" PCI driver, as you are just accessing a PCI
> > device and doing something with it, seems odd to claim it is a "platform" driver.
> > 
> 
> This driver also activates probes to create i2c platform driver and muxes.

And how does it do that?  Through the PCI device?

> For ARM and PPC based systems I can activate such stuff through dts.
> To be honest I don't know what is the right way to do such things for
> x86 systems.

How is it found in a x86 system, in ACPI?

> If I will move PCI related stuff to separate driver, could you suggest
> some right location for that?

Depends on what it does.

> For example, could I have the code like in f.e. in
> arch/x86/platform/ts5500/ts5500.c as a platform initialization code? 

How does it talk to the hardware for this?

thanks,

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  7:04       ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  7:04 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 9:13 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > > From: Vadim Pasternak <vadimp@mellanox.com>
> > >
> > > Enable system support for the Mellanox Technologies platform, 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.
> > 
> > What does "system support" mean?
> > 
> > Why can't this just be a "normal" PCI driver, as you are just accessing a PCI
> > device and doing something with it, seems odd to claim it is a "platform" driver.
> > 
> 
> This driver also activates probes to create i2c platform driver and muxes.

And how does it do that?  Through the PCI device?

> For ARM and PPC based systems I can activate such stuff through dts.
> To be honest I don't know what is the right way to do such things for
> x86 systems.

How is it found in a x86 system, in ACPI?

> If I will move PCI related stuff to separate driver, could you suggest
> some right location for that?

Depends on what it does.

> For example, could I have the code like in f.e. in
> arch/x86/platform/ts5500/ts5500.c as a platform initialization code? 

How does it talk to the hardware for this?

thanks,

greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  7:04       ` Greg KH
@ 2016-09-12  7:23         ` Vadim Pasternak
  -1 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  7:23 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 10:05 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 9:13 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > >
> > > > Enable system support for the Mellanox Technologies platform,
> > > > 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.
> > >
> > > What does "system support" mean?
> > >
> > > Why can't this just be a "normal" PCI driver, as you are just
> > > accessing a PCI device and doing something with it, seems odd to claim it is a
> "platform" driver.
> > >
> >
> > This driver also activates probes to create i2c platform driver and muxes.
> 
> And how does it do that?  Through the PCI device?
> 
> > For ARM and PPC based systems I can activate such stuff through dts.
> > To be honest I don't know what is the right way to do such things for
> > x86 systems.
> 
> How is it found in a x86 system, in ACPI?
> 

We have no support in ACPI.
Currently it could be found through DMI.

> > If I will move PCI related stuff to separate driver, could you suggest
> > some right location for that?
> 
> Depends on what it does.

It creates platform i2c driver (Mellanox controller), like:
platform_device_register_simple("i2c_mlxcpld", -1,
                                                         NULL, 0);
And two mux platform device instances, like:
platform_device_register_resndata(dev,
                                                " i2c-mux-reg", i, NULL,
                                                0, &mlxplat_mux_data[i],
                                                sizeof(mlxplat_mux_data[i]));

> 
> > For example, could I have the code like in f.e. in
> > arch/x86/platform/ts5500/ts5500.c as a platform initialization code?
> 
> How does it talk to the hardware for this?
> 
It defines several platform devices, like:
static struct platform_device ts5500_dio1_pdev = {
        .name = "ts5500-dio1",
        .id = -1,
        .resource = ts5500_dio1_resource,
        .num_resources = 1,
};
And registers them in __init, like:
platform_device_register(&ts5500_dio1_pdev)


> thanks,
> 
> greg k-h

Thanks,
Vadim.

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  7:23         ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  7:23 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 10:05 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 9:13 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > >
> > > > Enable system support for the Mellanox Technologies platform,
> > > > 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.
> > >
> > > What does "system support" mean?
> > >
> > > Why can't this just be a "normal" PCI driver, as you are just
> > > accessing a PCI device and doing something with it, seems odd to claim it is a
> "platform" driver.
> > >
> >
> > This driver also activates probes to create i2c platform driver and muxes.
> 
> And how does it do that?  Through the PCI device?
> 
> > For ARM and PPC based systems I can activate such stuff through dts.
> > To be honest I don't know what is the right way to do such things for
> > x86 systems.
> 
> How is it found in a x86 system, in ACPI?
> 

We have no support in ACPI.
Currently it could be found through DMI.

> > If I will move PCI related stuff to separate driver, could you suggest
> > some right location for that?
> 
> Depends on what it does.

It creates platform i2c driver (Mellanox controller), like:
platform_device_register_simple("i2c_mlxcpld", -1,
                                                         NULL, 0);
And two mux platform device instances, like:
platform_device_register_resndata(dev,
                                                " i2c-mux-reg", i, NULL,
                                                0, &mlxplat_mux_data[i],
                                                sizeof(mlxplat_mux_data[i]));

> 
> > For example, could I have the code like in f.e. in
> > arch/x86/platform/ts5500/ts5500.c as a platform initialization code?
> 
> How does it talk to the hardware for this?
> 
It defines several platform devices, like:
static struct platform_device ts5500_dio1_pdev = {
        .name = "ts5500-dio1",
        .id = -1,
        .resource = ts5500_dio1_resource,
        .num_resources = 1,
};
And registers them in __init, like:
platform_device_register(&ts5500_dio1_pdev)


> thanks,
> 
> greg k-h

Thanks,
Vadim.

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  7:23         ` Vadim Pasternak
@ 2016-09-12  7:40           ` Greg KH
  -1 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  7:40 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 10:05 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > > Mellanox systems platform
> > > >
> > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > >
> > > > > Enable system support for the Mellanox Technologies platform,
> > > > > 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.
> > > >
> > > > What does "system support" mean?
> > > >
> > > > Why can't this just be a "normal" PCI driver, as you are just
> > > > accessing a PCI device and doing something with it, seems odd to claim it is a
> > "platform" driver.
> > > >
> > >
> > > This driver also activates probes to create i2c platform driver and muxes.
> > 
> > And how does it do that?  Through the PCI device?
> > 
> > > For ARM and PPC based systems I can activate such stuff through dts.
> > > To be honest I don't know what is the right way to do such things for
> > > x86 systems.
> > 
> > How is it found in a x86 system, in ACPI?
> > 
> 
> We have no support in ACPI.
> Currently it could be found through DMI.
> 
> > > If I will move PCI related stuff to separate driver, could you suggest
> > > some right location for that?
> > 
> > Depends on what it does.
> 
> It creates platform i2c driver (Mellanox controller), like:
> platform_device_register_simple("i2c_mlxcpld", -1,
>                                                          NULL, 0);
> And two mux platform device instances, like:
> platform_device_register_resndata(dev,
>                                                 " i2c-mux-reg", i, NULL,
>                                                 0, &mlxplat_mux_data[i],
>                                                 sizeof(mlxplat_mux_data[i]));

Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?

Ok, a DMI driver for these makes sense in the platform directory to me,
but you have to convince the maintainer of this subsystem :)

And rip all of that PCI stuff out, that belongs in a PCI driver.

thanks,

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  7:40           ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  7:40 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 10:05 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > > Mellanox systems platform
> > > >
> > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com wrote:
> > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > >
> > > > > Enable system support for the Mellanox Technologies platform,
> > > > > 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.
> > > >
> > > > What does "system support" mean?
> > > >
> > > > Why can't this just be a "normal" PCI driver, as you are just
> > > > accessing a PCI device and doing something with it, seems odd to claim it is a
> > "platform" driver.
> > > >
> > >
> > > This driver also activates probes to create i2c platform driver and muxes.
> > 
> > And how does it do that?  Through the PCI device?
> > 
> > > For ARM and PPC based systems I can activate such stuff through dts.
> > > To be honest I don't know what is the right way to do such things for
> > > x86 systems.
> > 
> > How is it found in a x86 system, in ACPI?
> > 
> 
> We have no support in ACPI.
> Currently it could be found through DMI.
> 
> > > If I will move PCI related stuff to separate driver, could you suggest
> > > some right location for that?
> > 
> > Depends on what it does.
> 
> It creates platform i2c driver (Mellanox controller), like:
> platform_device_register_simple("i2c_mlxcpld", -1,
>                                                          NULL, 0);
> And two mux platform device instances, like:
> platform_device_register_resndata(dev,
>                                                 " i2c-mux-reg", i, NULL,
>                                                 0, &mlxplat_mux_data[i],
>                                                 sizeof(mlxplat_mux_data[i]));

Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?

Ok, a DMI driver for these makes sense in the platform directory to me,
but you have to convince the maintainer of this subsystem :)

And rip all of that PCI stuff out, that belongs in a PCI driver.

thanks,

greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  7:40           ` Greg KH
@ 2016-09-12  7:48             ` Vadim Pasternak
  -1 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  7:48 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 10:41 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 10:05 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > for Mellanox systems platform
> > > > >
> > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com
> wrote:
> > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > >
> > > > > > Enable system support for the Mellanox Technologies platform,
> > > > > > 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.
> > > > >
> > > > > What does "system support" mean?
> > > > >
> > > > > Why can't this just be a "normal" PCI driver, as you are just
> > > > > accessing a PCI device and doing something with it, seems odd to
> > > > > claim it is a
> > > "platform" driver.
> > > > >
> > > >
> > > > This driver also activates probes to create i2c platform driver and muxes.
> > >
> > > And how does it do that?  Through the PCI device?
> > >
> > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > To be honest I don't know what is the right way to do such things
> > > > for
> > > > x86 systems.
> > >
> > > How is it found in a x86 system, in ACPI?
> > >
> >
> > We have no support in ACPI.
> > Currently it could be found through DMI.
> >
> > > > If I will move PCI related stuff to separate driver, could you
> > > > suggest some right location for that?
> > >
> > > Depends on what it does.
> >
> > It creates platform i2c driver (Mellanox controller), like:
> > platform_device_register_simple("i2c_mlxcpld", -1,
> >                                                          NULL, 0); And
> > two mux platform device instances, like:
> > platform_device_register_resndata(dev,
> >                                                 " i2c-mux-reg", i, NULL,
> >                                                 0, &mlxplat_mux_data[i],
> >
> > sizeof(mlxplat_mux_data[i]));
> 
> Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?

Actually this is 2015-2016.
Many switches use i2c virtual buses infrastructure for placing slow devices for system monitoring and control.

> 
> Ok, a DMI driver for these makes sense in the platform directory to me, but you
> have to convince the maintainer of this subsystem :)
> 
> And rip all of that PCI stuff out, that belongs in a PCI driver.
> 
Thank you very much for all your inputs.

> thanks,
> 
> greg k-h

Thanks,

Vadim.

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  7:48             ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  7:48 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 10:41 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 10:05 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > for Mellanox systems platform
> > > > >
> > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com
> wrote:
> > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > >
> > > > > > Enable system support for the Mellanox Technologies platform,
> > > > > > 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.
> > > > >
> > > > > What does "system support" mean?
> > > > >
> > > > > Why can't this just be a "normal" PCI driver, as you are just
> > > > > accessing a PCI device and doing something with it, seems odd to
> > > > > claim it is a
> > > "platform" driver.
> > > > >
> > > >
> > > > This driver also activates probes to create i2c platform driver and muxes.
> > >
> > > And how does it do that?  Through the PCI device?
> > >
> > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > To be honest I don't know what is the right way to do such things
> > > > for
> > > > x86 systems.
> > >
> > > How is it found in a x86 system, in ACPI?
> > >
> >
> > We have no support in ACPI.
> > Currently it could be found through DMI.
> >
> > > > If I will move PCI related stuff to separate driver, could you
> > > > suggest some right location for that?
> > >
> > > Depends on what it does.
> >
> > It creates platform i2c driver (Mellanox controller), like:
> > platform_device_register_simple("i2c_mlxcpld", -1,
> >                                                          NULL, 0); And
> > two mux platform device instances, like:
> > platform_device_register_resndata(dev,
> >                                                 " i2c-mux-reg", i, NULL,
> >                                                 0, &mlxplat_mux_data[i],
> >
> > sizeof(mlxplat_mux_data[i]));
> 
> Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?

Actually this is 2015-2016.
Many switches use i2c virtual buses infrastructure for placing slow devices for system monitoring and control.

> 
> Ok, a DMI driver for these makes sense in the platform directory to me, but you
> have to convince the maintainer of this subsystem :)
> 
> And rip all of that PCI stuff out, that belongs in a PCI driver.
> 
Thank you very much for all your inputs.

> thanks,
> 
> greg k-h

Thanks,

Vadim.

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  7:48             ` Vadim Pasternak
@ 2016-09-12  8:17               ` Greg KH
  -1 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  8:17 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 10:41 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > > Mellanox systems platform
> > > >
> > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > linux-kernel@vger.kernel.org;
> > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > > for Mellanox systems platform
> > > > > >
> > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com
> > wrote:
> > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > >
> > > > > > > Enable system support for the Mellanox Technologies platform,
> > > > > > > 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.
> > > > > >
> > > > > > What does "system support" mean?
> > > > > >
> > > > > > Why can't this just be a "normal" PCI driver, as you are just
> > > > > > accessing a PCI device and doing something with it, seems odd to
> > > > > > claim it is a
> > > > "platform" driver.
> > > > > >
> > > > >
> > > > > This driver also activates probes to create i2c platform driver and muxes.
> > > >
> > > > And how does it do that?  Through the PCI device?
> > > >
> > > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > > To be honest I don't know what is the right way to do such things
> > > > > for
> > > > > x86 systems.
> > > >
> > > > How is it found in a x86 system, in ACPI?
> > > >
> > >
> > > We have no support in ACPI.
> > > Currently it could be found through DMI.
> > >
> > > > > If I will move PCI related stuff to separate driver, could you
> > > > > suggest some right location for that?
> > > >
> > > > Depends on what it does.
> > >
> > > It creates platform i2c driver (Mellanox controller), like:
> > > platform_device_register_simple("i2c_mlxcpld", -1,
> > >                                                          NULL, 0); And
> > > two mux platform device instances, like:
> > > platform_device_register_resndata(dev,
> > >                                                 " i2c-mux-reg", i, NULL,
> > >                                                 0, &mlxplat_mux_data[i],
> > >
> > > sizeof(mlxplat_mux_data[i]));
> > 
> > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> 
> Actually this is 2015-2016.
> Many switches use i2c virtual buses infrastructure for placing slow
> devices for system monitoring and control.

Yes, but that i2c controller is on what type of bus?  A non-discoverable
one?  That's what I'm complaining about...

thanks,

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  8:17               ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  8:17 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 10:41 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > > Mellanox systems platform
> > > >
> > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > linux-kernel@vger.kernel.org;
> > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > > for Mellanox systems platform
> > > > > >
> > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000, vadimp@mellanox.com
> > wrote:
> > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > >
> > > > > > > Enable system support for the Mellanox Technologies platform,
> > > > > > > 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.
> > > > > >
> > > > > > What does "system support" mean?
> > > > > >
> > > > > > Why can't this just be a "normal" PCI driver, as you are just
> > > > > > accessing a PCI device and doing something with it, seems odd to
> > > > > > claim it is a
> > > > "platform" driver.
> > > > > >
> > > > >
> > > > > This driver also activates probes to create i2c platform driver and muxes.
> > > >
> > > > And how does it do that?  Through the PCI device?
> > > >
> > > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > > To be honest I don't know what is the right way to do such things
> > > > > for
> > > > > x86 systems.
> > > >
> > > > How is it found in a x86 system, in ACPI?
> > > >
> > >
> > > We have no support in ACPI.
> > > Currently it could be found through DMI.
> > >
> > > > > If I will move PCI related stuff to separate driver, could you
> > > > > suggest some right location for that?
> > > >
> > > > Depends on what it does.
> > >
> > > It creates platform i2c driver (Mellanox controller), like:
> > > platform_device_register_simple("i2c_mlxcpld", -1,
> > >                                                          NULL, 0); And
> > > two mux platform device instances, like:
> > > platform_device_register_resndata(dev,
> > >                                                 " i2c-mux-reg", i, NULL,
> > >                                                 0, &mlxplat_mux_data[i],
> > >
> > > sizeof(mlxplat_mux_data[i]));
> > 
> > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> 
> Actually this is 2015-2016.
> Many switches use i2c virtual buses infrastructure for placing slow
> devices for system monitoring and control.

Yes, but that i2c controller is on what type of bus?  A non-discoverable
one?  That's what I'm complaining about...

thanks,

greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  8:17               ` Greg KH
@ 2016-09-12  8:21                 ` Vadim Pasternak
  -1 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  8:21 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 11:17 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 10:41 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > for Mellanox systems platform
> > > > >
> > > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > support for Mellanox systems platform
> > > > > > >
> > > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000,
> > > > > > > vadimp@mellanox.com
> > > wrote:
> > > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > >
> > > > > > > > Enable system support for the Mellanox Technologies
> > > > > > > > platform, 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.
> > > > > > >
> > > > > > > What does "system support" mean?
> > > > > > >
> > > > > > > Why can't this just be a "normal" PCI driver, as you are
> > > > > > > just accessing a PCI device and doing something with it,
> > > > > > > seems odd to claim it is a
> > > > > "platform" driver.
> > > > > > >
> > > > > >
> > > > > > This driver also activates probes to create i2c platform driver and
> muxes.
> > > > >
> > > > > And how does it do that?  Through the PCI device?
> > > > >
> > > > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > > > To be honest I don't know what is the right way to do such
> > > > > > things for
> > > > > > x86 systems.
> > > > >
> > > > > How is it found in a x86 system, in ACPI?
> > > > >
> > > >
> > > > We have no support in ACPI.
> > > > Currently it could be found through DMI.
> > > >
> > > > > > If I will move PCI related stuff to separate driver, could you
> > > > > > suggest some right location for that?
> > > > >
> > > > > Depends on what it does.
> > > >
> > > > It creates platform i2c driver (Mellanox controller), like:
> > > > platform_device_register_simple("i2c_mlxcpld", -1,
> > > >                                                          NULL, 0);
> > > > And two mux platform device instances, like:
> > > > platform_device_register_resndata(dev,
> > > >                                                 " i2c-mux-reg", i, NULL,
> > > >                                                 0,
> > > > &mlxplat_mux_data[i],
> > > >
> > > > sizeof(mlxplat_mux_data[i]));
> > >
> > > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> >
> > Actually this is 2015-2016.
> > Many switches use i2c virtual buses infrastructure for placing slow
> > devices for system monitoring and control.
> 
> Yes, but that i2c controller is on what type of bus?  A non-discoverable one?
> That's what I'm complaining about...

This is LPC to I2C bridge.
Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.

> 
> thanks,
> 
> greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  8:21                 ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  8:21 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 11:17 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 10:41 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > for Mellanox systems platform
> > > > >
> > > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > support for Mellanox systems platform
> > > > > > >
> > > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000,
> > > > > > > vadimp@mellanox.com
> > > wrote:
> > > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > >
> > > > > > > > Enable system support for the Mellanox Technologies
> > > > > > > > platform, 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.
> > > > > > >
> > > > > > > What does "system support" mean?
> > > > > > >
> > > > > > > Why can't this just be a "normal" PCI driver, as you are
> > > > > > > just accessing a PCI device and doing something with it,
> > > > > > > seems odd to claim it is a
> > > > > "platform" driver.
> > > > > > >
> > > > > >
> > > > > > This driver also activates probes to create i2c platform driver and
> muxes.
> > > > >
> > > > > And how does it do that?  Through the PCI device?
> > > > >
> > > > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > > > To be honest I don't know what is the right way to do such
> > > > > > things for
> > > > > > x86 systems.
> > > > >
> > > > > How is it found in a x86 system, in ACPI?
> > > > >
> > > >
> > > > We have no support in ACPI.
> > > > Currently it could be found through DMI.
> > > >
> > > > > > If I will move PCI related stuff to separate driver, could you
> > > > > > suggest some right location for that?
> > > > >
> > > > > Depends on what it does.
> > > >
> > > > It creates platform i2c driver (Mellanox controller), like:
> > > > platform_device_register_simple("i2c_mlxcpld", -1,
> > > >                                                          NULL, 0);
> > > > And two mux platform device instances, like:
> > > > platform_device_register_resndata(dev,
> > > >                                                 " i2c-mux-reg", i, NULL,
> > > >                                                 0,
> > > > &mlxplat_mux_data[i],
> > > >
> > > > sizeof(mlxplat_mux_data[i]));
> > >
> > > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> >
> > Actually this is 2015-2016.
> > Many switches use i2c virtual buses infrastructure for placing slow
> > devices for system monitoring and control.
> 
> Yes, but that i2c controller is on what type of bus?  A non-discoverable one?
> That's what I'm complaining about...

This is LPC to I2C bridge.
Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.

> 
> thanks,
> 
> greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  8:21                 ` Vadim Pasternak
@ 2016-09-12  8:34                   ` Greg KH
  -1 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  8:34 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 08:21:28AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 11:17 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Monday, September 12, 2016 10:41 AM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > > Mellanox systems platform
> > > >
> > > > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > linux-kernel@vger.kernel.org;
> > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > > for Mellanox systems platform
> > > > > >
> > > > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > > support for Mellanox systems platform
> > > > > > > >
> > > > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000,
> > > > > > > > vadimp@mellanox.com
> > > > wrote:
> > > > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > >
> > > > > > > > > Enable system support for the Mellanox Technologies
> > > > > > > > > platform, 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.
> > > > > > > >
> > > > > > > > What does "system support" mean?
> > > > > > > >
> > > > > > > > Why can't this just be a "normal" PCI driver, as you are
> > > > > > > > just accessing a PCI device and doing something with it,
> > > > > > > > seems odd to claim it is a
> > > > > > "platform" driver.
> > > > > > > >
> > > > > > >
> > > > > > > This driver also activates probes to create i2c platform driver and
> > muxes.
> > > > > >
> > > > > > And how does it do that?  Through the PCI device?
> > > > > >
> > > > > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > > > > To be honest I don't know what is the right way to do such
> > > > > > > things for
> > > > > > > x86 systems.
> > > > > >
> > > > > > How is it found in a x86 system, in ACPI?
> > > > > >
> > > > >
> > > > > We have no support in ACPI.
> > > > > Currently it could be found through DMI.
> > > > >
> > > > > > > If I will move PCI related stuff to separate driver, could you
> > > > > > > suggest some right location for that?
> > > > > >
> > > > > > Depends on what it does.
> > > > >
> > > > > It creates platform i2c driver (Mellanox controller), like:
> > > > > platform_device_register_simple("i2c_mlxcpld", -1,
> > > > >                                                          NULL, 0);
> > > > > And two mux platform device instances, like:
> > > > > platform_device_register_resndata(dev,
> > > > >                                                 " i2c-mux-reg", i, NULL,
> > > > >                                                 0,
> > > > > &mlxplat_mux_data[i],
> > > > >
> > > > > sizeof(mlxplat_mux_data[i]));
> > > >
> > > > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> > >
> > > Actually this is 2015-2016.
> > > Many switches use i2c virtual buses infrastructure for placing slow
> > > devices for system monitoring and control.
> > 
> > Yes, but that i2c controller is on what type of bus?  A non-discoverable one?
> > That's what I'm complaining about...
> 
> This is LPC to I2C bridge.

"LPC"?

> Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.

And how is the CPLD attached to the CPU?

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  8:34                   ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12  8:34 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 08:21:28AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 12, 2016 11:17 AM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Monday, September 12, 2016 10:41 AM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > > Mellanox systems platform
> > > >
> > > > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > linux-kernel@vger.kernel.org;
> > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > > for Mellanox systems platform
> > > > > >
> > > > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > > support for Mellanox systems platform
> > > > > > > >
> > > > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000,
> > > > > > > > vadimp@mellanox.com
> > > > wrote:
> > > > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > >
> > > > > > > > > Enable system support for the Mellanox Technologies
> > > > > > > > > platform, 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.
> > > > > > > >
> > > > > > > > What does "system support" mean?
> > > > > > > >
> > > > > > > > Why can't this just be a "normal" PCI driver, as you are
> > > > > > > > just accessing a PCI device and doing something with it,
> > > > > > > > seems odd to claim it is a
> > > > > > "platform" driver.
> > > > > > > >
> > > > > > >
> > > > > > > This driver also activates probes to create i2c platform driver and
> > muxes.
> > > > > >
> > > > > > And how does it do that?  Through the PCI device?
> > > > > >
> > > > > > > For ARM and PPC based systems I can activate such stuff through dts.
> > > > > > > To be honest I don't know what is the right way to do such
> > > > > > > things for
> > > > > > > x86 systems.
> > > > > >
> > > > > > How is it found in a x86 system, in ACPI?
> > > > > >
> > > > >
> > > > > We have no support in ACPI.
> > > > > Currently it could be found through DMI.
> > > > >
> > > > > > > If I will move PCI related stuff to separate driver, could you
> > > > > > > suggest some right location for that?
> > > > > >
> > > > > > Depends on what it does.
> > > > >
> > > > > It creates platform i2c driver (Mellanox controller), like:
> > > > > platform_device_register_simple("i2c_mlxcpld", -1,
> > > > >                                                          NULL, 0);
> > > > > And two mux platform device instances, like:
> > > > > platform_device_register_resndata(dev,
> > > > >                                                 " i2c-mux-reg", i, NULL,
> > > > >                                                 0,
> > > > > &mlxplat_mux_data[i],
> > > > >
> > > > > sizeof(mlxplat_mux_data[i]));
> > > >
> > > > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> > >
> > > Actually this is 2015-2016.
> > > Many switches use i2c virtual buses infrastructure for placing slow
> > > devices for system monitoring and control.
> > 
> > Yes, but that i2c controller is on what type of bus?  A non-discoverable one?
> > That's what I'm complaining about...
> 
> This is LPC to I2C bridge.

"LPC"?

> Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.

And how is the CPLD attached to the CPU?

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  8:34                   ` Greg KH
@ 2016-09-12  8:44                     ` Vadim Pasternak
  -1 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  8:44 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 11:35 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 08:21:28AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 11:17 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Monday, September 12, 2016 10:41 AM
> > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > for Mellanox systems platform
> > > > >
> > > > > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > support for Mellanox systems platform
> > > > > > >
> > > > > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > > > support for Mellanox systems platform
> > > > > > > > >
> > > > > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000,
> > > > > > > > > vadimp@mellanox.com
> > > > > wrote:
> > > > > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > > >
> > > > > > > > > > Enable system support for the Mellanox Technologies
> > > > > > > > > > platform, 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.
> > > > > > > > >
> > > > > > > > > What does "system support" mean?
> > > > > > > > >
> > > > > > > > > Why can't this just be a "normal" PCI driver, as you are
> > > > > > > > > just accessing a PCI device and doing something with it,
> > > > > > > > > seems odd to claim it is a
> > > > > > > "platform" driver.
> > > > > > > > >
> > > > > > > >
> > > > > > > > This driver also activates probes to create i2c platform
> > > > > > > > driver and
> > > muxes.
> > > > > > >
> > > > > > > And how does it do that?  Through the PCI device?
> > > > > > >
> > > > > > > > For ARM and PPC based systems I can activate such stuff through
> dts.
> > > > > > > > To be honest I don't know what is the right way to do such
> > > > > > > > things for
> > > > > > > > x86 systems.
> > > > > > >
> > > > > > > How is it found in a x86 system, in ACPI?
> > > > > > >
> > > > > >
> > > > > > We have no support in ACPI.
> > > > > > Currently it could be found through DMI.
> > > > > >
> > > > > > > > If I will move PCI related stuff to separate driver, could
> > > > > > > > you suggest some right location for that?
> > > > > > >
> > > > > > > Depends on what it does.
> > > > > >
> > > > > > It creates platform i2c driver (Mellanox controller), like:
> > > > > > platform_device_register_simple("i2c_mlxcpld", -1,
> > > > > >                                                          NULL,
> > > > > > 0); And two mux platform device instances, like:
> > > > > > platform_device_register_resndata(dev,
> > > > > >                                                 " i2c-mux-reg", i, NULL,
> > > > > >                                                 0,
> > > > > > &mlxplat_mux_data[i],
> > > > > >
> > > > > > sizeof(mlxplat_mux_data[i]));
> > > > >
> > > > > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> > > >
> > > > Actually this is 2015-2016.
> > > > Many switches use i2c virtual buses infrastructure for placing
> > > > slow devices for system monitoring and control.
> > >
> > > Yes, but that i2c controller is on what type of bus?  A non-discoverable one?
> > > That's what I'm complaining about...
> >
> > This is LPC to I2C bridge.
> 
> "LPC"?
> 
> > Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.
> 
> And how is the CPLD attached to the CPU?

This is how it connected on Celeron, I7, ATOM based systems:

*--------*
| I2CLPC | i2c physical
| bridge | bus                          *---------*
| logic  |---------------------> * mux reg *
| in CPLD|                                *---------*
*--------*
    |
    |
<-------->
i2c cntrl
reg space
    | 
    |
    *------------- LPC bus ----------------- CPU

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  8:44                     ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-12  8:44 UTC (permalink / raw)
  To: Greg KH
  Cc: tglx, mingo, hpa, davem, geert, akpm, kvalo, mchehab, linux, x86,
	linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 12, 2016 11:35 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; geert@linux-m68k.org; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Mon, Sep 12, 2016 at 08:21:28AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 12, 2016 11:17 AM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > davem@davemloft.net; geert@linux-m68k.org;
> > > akpm@linux-foundation.org; kvalo@codeaurora.org; mchehab@kernel.org;
> > > linux@roeck-us.net; x86@kernel.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > > On Mon, Sep 12, 2016 at 07:48:24AM +0000, Vadim Pasternak wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Monday, September 12, 2016 10:41 AM
> > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support
> > > > > for Mellanox systems platform
> > > > >
> > > > > On Mon, Sep 12, 2016 at 07:23:59AM +0000, Vadim Pasternak wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > Sent: Monday, September 12, 2016 10:05 AM
> > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > support for Mellanox systems platform
> > > > > > >
> > > > > > > On Mon, Sep 12, 2016 at 06:44:03AM +0000, Vadim Pasternak wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > > > > > Sent: Monday, September 12, 2016 9:13 AM
> > > > > > > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > > Cc: tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> > > > > > > > > davem@davemloft.net; geert@linux-m68k.org;
> > > > > > > > > akpm@linux-foundation.org; kvalo@codeaurora.org;
> > > > > > > > > mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org;
> > > > > > > > > linux-kernel@vger.kernel.org;
> > > > > > > > > platform-driver- x86@vger.kernel.org; jiri@resnulli.us
> > > > > > > > > Subject: Re: [patch v1] x86/platform/mellanox: introduce
> > > > > > > > > support for Mellanox systems platform
> > > > > > > > >
> > > > > > > > > On Mon, Sep 12, 2016 at 06:29:58AM +0000,
> > > > > > > > > vadimp@mellanox.com
> > > > > wrote:
> > > > > > > > > > From: Vadim Pasternak <vadimp@mellanox.com>
> > > > > > > > > >
> > > > > > > > > > Enable system support for the Mellanox Technologies
> > > > > > > > > > platform, 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.
> > > > > > > > >
> > > > > > > > > What does "system support" mean?
> > > > > > > > >
> > > > > > > > > Why can't this just be a "normal" PCI driver, as you are
> > > > > > > > > just accessing a PCI device and doing something with it,
> > > > > > > > > seems odd to claim it is a
> > > > > > > "platform" driver.
> > > > > > > > >
> > > > > > > >
> > > > > > > > This driver also activates probes to create i2c platform
> > > > > > > > driver and
> > > muxes.
> > > > > > >
> > > > > > > And how does it do that?  Through the PCI device?
> > > > > > >
> > > > > > > > For ARM and PPC based systems I can activate such stuff through
> dts.
> > > > > > > > To be honest I don't know what is the right way to do such
> > > > > > > > things for
> > > > > > > > x86 systems.
> > > > > > >
> > > > > > > How is it found in a x86 system, in ACPI?
> > > > > > >
> > > > > >
> > > > > > We have no support in ACPI.
> > > > > > Currently it could be found through DMI.
> > > > > >
> > > > > > > > If I will move PCI related stuff to separate driver, could
> > > > > > > > you suggest some right location for that?
> > > > > > >
> > > > > > > Depends on what it does.
> > > > > >
> > > > > > It creates platform i2c driver (Mellanox controller), like:
> > > > > > platform_device_register_simple("i2c_mlxcpld", -1,
> > > > > >                                                          NULL,
> > > > > > 0); And two mux platform device instances, like:
> > > > > > platform_device_register_resndata(dev,
> > > > > >                                                 " i2c-mux-reg", i, NULL,
> > > > > >                                                 0,
> > > > > > &mlxplat_mux_data[i],
> > > > > >
> > > > > > sizeof(mlxplat_mux_data[i]));
> > > > >
> > > > > Ugh.  Really?  These aren't on a real bus?  What is this, the 1990's?
> > > >
> > > > Actually this is 2015-2016.
> > > > Many switches use i2c virtual buses infrastructure for placing
> > > > slow devices for system monitoring and control.
> > >
> > > Yes, but that i2c controller is on what type of bus?  A non-discoverable one?
> > > That's what I'm complaining about...
> >
> > This is LPC to I2C bridge.
> 
> "LPC"?
> 
> > Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.
> 
> And how is the CPLD attached to the CPU?

This is how it connected on Celeron, I7, ATOM based systems:

*--------*
| I2CLPC | i2c physical
| bridge | bus                          *---------*
| logic  |---------------------> * mux reg *
| in CPLD|                                *---------*
*--------*
    |
    |
<-------->
i2c cntrl
reg space
    | 
    |
    *------------- LPC bus ----------------- CPU

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  8:34                   ` Greg KH
@ 2016-09-12  9:14                     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 39+ messages in thread
From: Geert Uytterhoeven @ 2016-09-12  9:14 UTC (permalink / raw)
  To: Greg KH
  Cc: Vadim Pasternak, tglx, mingo, hpa, davem, akpm, kvalo, mchehab,
	linux, x86, linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> This is LPC to I2C bridge.
>
> "LPC"?

https://en.wikipedia.org/wiki/Low_Pin_Count

"Modern ISA"

>> Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.
>
> And how is the CPLD attached to the CPU?

Via LPC, I guess.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12  9:14                     ` Geert Uytterhoeven
  0 siblings, 0 replies; 39+ messages in thread
From: Geert Uytterhoeven @ 2016-09-12  9:14 UTC (permalink / raw)
  To: Greg KH
  Cc: Vadim Pasternak, tglx, mingo, hpa, davem, akpm, kvalo, mchehab,
	linux, x86, linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> This is LPC to I2C bridge.
>
> "LPC"?

https://en.wikipedia.org/wiki/Low_Pin_Count

"Modern ISA"

>> Controller logic is implemented in Lattice CPLD. CPLD itself is attached to LPC.
>
> And how is the CPLD attached to the CPU?

Via LPC, I guess.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12  9:14                     ` Geert Uytterhoeven
@ 2016-09-12 10:21                       ` Greg KH
  -1 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12 10:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Vadim Pasternak, tglx, mingo, hpa, davem, akpm, kvalo, mchehab,
	linux, x86, linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> This is LPC to I2C bridge.
> >
> > "LPC"?
> 
> https://en.wikipedia.org/wiki/Low_Pin_Count
> 
> "Modern ISA"

So my original point stands, 1990's technology being used for brand new
devices today, ugh :(

Someone needs to go kick some hardware designers...

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12 10:21                       ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-12 10:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Vadim Pasternak, tglx, mingo, hpa, davem, akpm, kvalo, mchehab,
	linux, x86, linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> This is LPC to I2C bridge.
> >
> > "LPC"?
> 
> https://en.wikipedia.org/wiki/Low_Pin_Count
> 
> "Modern ISA"

So my original point stands, 1990's technology being used for brand new
devices today, ugh :(

Someone needs to go kick some hardware designers...

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12 10:21                       ` Greg KH
@ 2016-09-12 10:55                         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 39+ messages in thread
From: Geert Uytterhoeven @ 2016-09-12 10:55 UTC (permalink / raw)
  To: Greg KH
  Cc: Vadim Pasternak, tglx, mingo, hpa, davem, akpm, kvalo, mchehab,
	linux, x86, linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 12:21 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
>> On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> >> This is LPC to I2C bridge.
>> >
>> > "LPC"?
>>
>> https://en.wikipedia.org/wiki/Low_Pin_Count
>>
>> "Modern ISA"
>
> So my original point stands, 1990's technology being used for brand new
> devices today, ugh :(
>
> Someone needs to go kick some hardware designers...

Non-discoverable buses are everywhere these days.

If only we had some machine-readable language to describe hardware topology.
Oh wait...

http://lmgtfy.com/?q=return+of+the+non-discoverable+buses

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12 10:55                         ` Geert Uytterhoeven
  0 siblings, 0 replies; 39+ messages in thread
From: Geert Uytterhoeven @ 2016-09-12 10:55 UTC (permalink / raw)
  To: Greg KH
  Cc: Vadim Pasternak, tglx, mingo, hpa, davem, akpm, kvalo, mchehab,
	linux, x86, linux-kernel, platform-driver-x86, jiri

On Mon, Sep 12, 2016 at 12:21 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
>> On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> >> This is LPC to I2C bridge.
>> >
>> > "LPC"?
>>
>> https://en.wikipedia.org/wiki/Low_Pin_Count
>>
>> "Modern ISA"
>
> So my original point stands, 1990's technology being used for brand new
> devices today, ugh :(
>
> Someone needs to go kick some hardware designers...

Non-discoverable buses are everywhere these days.

If only we had some machine-readable language to describe hardware topology.
Oh wait...

http://lmgtfy.com/?q=return+of+the+non-discoverable+buses

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12 10:21                       ` Greg KH
@ 2016-09-12 11:00                         ` Ingo Molnar
  -1 siblings, 0 replies; 39+ messages in thread
From: Ingo Molnar @ 2016-09-12 11:00 UTC (permalink / raw)
  To: Greg KH
  Cc: Geert Uytterhoeven, Vadim Pasternak, tglx, mingo, hpa, davem,
	akpm, kvalo, mchehab, linux, x86, linux-kernel,
	platform-driver-x86, jiri


* Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > >> This is LPC to I2C bridge.
> > >
> > > "LPC"?
> > 
> > https://en.wikipedia.org/wiki/Low_Pin_Count
> > 
> > "Modern ISA"
> 
> So my original point stands, 1990's technology being used for brand new
> devices today, ugh :(
> 
> Someone needs to go kick some hardware designers...

In their defense, "this is a carbon copy of published 1990s technology" is a 
pretty good starting point for a defendant, in patent litigation.

Thanks,

	Ingo

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-12 11:00                         ` Ingo Molnar
  0 siblings, 0 replies; 39+ messages in thread
From: Ingo Molnar @ 2016-09-12 11:00 UTC (permalink / raw)
  To: Greg KH
  Cc: Geert Uytterhoeven, Vadim Pasternak, tglx, mingo, hpa, davem,
	akpm, kvalo, mchehab, linux, x86, linux-kernel,
	platform-driver-x86, jiri


* Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > >> This is LPC to I2C bridge.
> > >
> > > "LPC"?
> > 
> > https://en.wikipedia.org/wiki/Low_Pin_Count
> > 
> > "Modern ISA"
> 
> So my original point stands, 1990's technology being used for brand new
> devices today, ugh :(
> 
> Someone needs to go kick some hardware designers...

In their defense, "this is a carbon copy of published 1990s technology" is a 
pretty good starting point for a defendant, in patent litigation.

Thanks,

	Ingo

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-12 11:00                         ` Ingo Molnar
@ 2016-09-13  7:27                           ` Vadim Pasternak
  -1 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-13  7:27 UTC (permalink / raw)
  To: Ingo Molnar, Greg KH
  Cc: Geert Uytterhoeven, tglx, mingo, hpa, davem, akpm, kvalo,
	mchehab, linux, x86, linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> Molnar
> Sent: Monday, September 12, 2016 2:01 PM
> To: Greg KH <gregkh@linuxfoundation.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Vadim Pasternak
> <vadimp@mellanox.com>; tglx@linutronix.de; mingo@redhat.com;
> hpa@zytor.com; davem@davemloft.net; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> 
> * Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org>
> wrote:
> > > >> This is LPC to I2C bridge.
> > > >
> > > > "LPC"?
> > >
> > > https://en.wikipedia.org/wiki/Low_Pin_Count
> > >
> > > "Modern ISA"
> >
> > So my original point stands, 1990's technology being used for brand
> > new devices today, ugh :(
> >
> > Someone needs to go kick some hardware designers...
> 
> In their defense, "this is a carbon copy of published 1990s technology" is a pretty
> good starting point for a defendant, in patent litigation.
> 

I understood your comments regarding undiscoverable busses.
But we use LPC on all our x86 based systems.
I have to activate all platform related stuff from some place and we don't support ACPI.

Do you think it would be OK, if I'll remove all PCI related code, make use of DMI and leave only platform activation code?
If yes, I'll re-work this driver and re-send it for your review.

Thanks,
Vadim.




> Thanks,
> 
> 	Ingo

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-13  7:27                           ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-13  7:27 UTC (permalink / raw)
  To: Ingo Molnar, Greg KH
  Cc: Geert Uytterhoeven, tglx, mingo, hpa, davem, akpm, kvalo,
	mchehab, linux, x86, linux-kernel, platform-driver-x86, jiri



> -----Original Message-----
> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> Molnar
> Sent: Monday, September 12, 2016 2:01 PM
> To: Greg KH <gregkh@linuxfoundation.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Vadim Pasternak
> <vadimp@mellanox.com>; tglx@linutronix.de; mingo@redhat.com;
> hpa@zytor.com; davem@davemloft.net; akpm@linux-foundation.org;
> kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> 
> * Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org>
> wrote:
> > > >> This is LPC to I2C bridge.
> > > >
> > > > "LPC"?
> > >
> > > https://en.wikipedia.org/wiki/Low_Pin_Count
> > >
> > > "Modern ISA"
> >
> > So my original point stands, 1990's technology being used for brand
> > new devices today, ugh :(
> >
> > Someone needs to go kick some hardware designers...
> 
> In their defense, "this is a carbon copy of published 1990s technology" is a pretty
> good starting point for a defendant, in patent litigation.
> 

I understood your comments regarding undiscoverable busses.
But we use LPC on all our x86 based systems.
I have to activate all platform related stuff from some place and we don't support ACPI.

Do you think it would be OK, if I'll remove all PCI related code, make use of DMI and leave only platform activation code?
If yes, I'll re-work this driver and re-send it for your review.

Thanks,
Vadim.




> Thanks,
> 
> 	Ingo

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-13  7:27                           ` Vadim Pasternak
@ 2016-09-13  8:12                             ` Greg KH
  -1 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-13  8:12 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: Ingo Molnar, Geert Uytterhoeven, tglx, mingo, hpa, davem, akpm,
	kvalo, mchehab, linux, x86, linux-kernel, platform-driver-x86,
	jiri

On Tue, Sep 13, 2016 at 07:27:20AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> > Molnar
> > Sent: Monday, September 12, 2016 2:01 PM
> > To: Greg KH <gregkh@linuxfoundation.org>
> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Vadim Pasternak
> > <vadimp@mellanox.com>; tglx@linutronix.de; mingo@redhat.com;
> > hpa@zytor.com; davem@davemloft.net; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > 
> > * Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > > > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org>
> > wrote:
> > > > >> This is LPC to I2C bridge.
> > > > >
> > > > > "LPC"?
> > > >
> > > > https://en.wikipedia.org/wiki/Low_Pin_Count
> > > >
> > > > "Modern ISA"
> > >
> > > So my original point stands, 1990's technology being used for brand
> > > new devices today, ugh :(
> > >
> > > Someone needs to go kick some hardware designers...
> > 
> > In their defense, "this is a carbon copy of published 1990s technology" is a pretty
> > good starting point for a defendant, in patent litigation.
> > 
> 
> I understood your comments regarding undiscoverable busses.
> But we use LPC on all our x86 based systems.
> I have to activate all platform related stuff from some place and we
> don't support ACPI.

x86 that doesn't support ACPI?  That's sad :(

> Do you think it would be OK, if I'll remove all PCI related code, make
> use of DMI and leave only platform activation code?
> If yes, I'll re-work this driver and re-send it for your review.

Yes, that sounds like a good start, let's see how the code looks after
doing that.

thanks,

greg k-h

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

* Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-13  8:12                             ` Greg KH
  0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2016-09-13  8:12 UTC (permalink / raw)
  To: Vadim Pasternak
  Cc: Ingo Molnar, Geert Uytterhoeven, tglx, mingo, hpa, davem, akpm,
	kvalo, mchehab, linux, x86, linux-kernel, platform-driver-x86,
	jiri

On Tue, Sep 13, 2016 at 07:27:20AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> > Molnar
> > Sent: Monday, September 12, 2016 2:01 PM
> > To: Greg KH <gregkh@linuxfoundation.org>
> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Vadim Pasternak
> > <vadimp@mellanox.com>; tglx@linutronix.de; mingo@redhat.com;
> > hpa@zytor.com; davem@davemloft.net; akpm@linux-foundation.org;
> > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; jiri@resnulli.us
> > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> > systems platform
> > 
> > 
> > * Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > > > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org>
> > wrote:
> > > > >> This is LPC to I2C bridge.
> > > > >
> > > > > "LPC"?
> > > >
> > > > https://en.wikipedia.org/wiki/Low_Pin_Count
> > > >
> > > > "Modern ISA"
> > >
> > > So my original point stands, 1990's technology being used for brand
> > > new devices today, ugh :(
> > >
> > > Someone needs to go kick some hardware designers...
> > 
> > In their defense, "this is a carbon copy of published 1990s technology" is a pretty
> > good starting point for a defendant, in patent litigation.
> > 
> 
> I understood your comments regarding undiscoverable busses.
> But we use LPC on all our x86 based systems.
> I have to activate all platform related stuff from some place and we
> don't support ACPI.

x86 that doesn't support ACPI?  That's sad :(

> Do you think it would be OK, if I'll remove all PCI related code, make
> use of DMI and leave only platform activation code?
> If yes, I'll re-work this driver and re-send it for your review.

Yes, that sounds like a good start, let's see how the code looks after
doing that.

thanks,

greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
  2016-09-13  8:12                             ` Greg KH
@ 2016-09-13  8:18                               ` Vadim Pasternak
  -1 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-13  8:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Ingo Molnar, Geert Uytterhoeven, tglx, mingo, hpa, davem, akpm,
	kvalo, mchehab, linux, x86, linux-kernel, platform-driver-x86,
	jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, September 13, 2016 11:12 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: Ingo Molnar <mingo@kernel.org>; Geert Uytterhoeven <geert@linux-
> m68k.org>; tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; akpm@linux-foundation.org; kvalo@codeaurora.org;
> mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org; linux-
> kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Tue, Sep 13, 2016 at 07:27:20AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of
> > > Ingo Molnar
> > > Sent: Monday, September 12, 2016 2:01 PM
> > > To: Greg KH <gregkh@linuxfoundation.org>
> > > Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Vadim Pasternak
> > > <vadimp@mellanox.com>; tglx@linutronix.de; mingo@redhat.com;
> > > hpa@zytor.com; davem@davemloft.net; akpm@linux-foundation.org;
> > > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > > x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > >
> > > * Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > > On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > > > > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH
> > > > > <gregkh@linuxfoundation.org>
> > > wrote:
> > > > > >> This is LPC to I2C bridge.
> > > > > >
> > > > > > "LPC"?
> > > > >
> > > > > https://en.wikipedia.org/wiki/Low_Pin_Count
> > > > >
> > > > > "Modern ISA"
> > > >
> > > > So my original point stands, 1990's technology being used for
> > > > brand new devices today, ugh :(
> > > >
> > > > Someone needs to go kick some hardware designers...
> > >
> > > In their defense, "this is a carbon copy of published 1990s
> > > technology" is a pretty good starting point for a defendant, in patent
> litigation.
> > >
> >
> > I understood your comments regarding undiscoverable busses.
> > But we use LPC on all our x86 based systems.
> > I have to activate all platform related stuff from some place and we
> > don't support ACPI.
> 
> x86 that doesn't support ACPI?  That's sad :(

Yes.
But we should make a decision for the coming platform - to provide ACPI support for board specific stuff, or equip the new systems with BMC (so we'll introduce all board related stuff through DTS).
But currently this is our reality. Me also sad.

> 
> > Do you think it would be OK, if I'll remove all PCI related code, make
> > use of DMI and leave only platform activation code?
> > If yes, I'll re-work this driver and re-send it for your review.
> 
> Yes, that sounds like a good start, let's see how the code looks after doing that.

Sure. I'll do it.
Thank you very much for your comments.

Thanks,

Vadim
> 
> thanks,
> 
> greg k-h

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

* RE: [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform
@ 2016-09-13  8:18                               ` Vadim Pasternak
  0 siblings, 0 replies; 39+ messages in thread
From: Vadim Pasternak @ 2016-09-13  8:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Ingo Molnar, Geert Uytterhoeven, tglx, mingo, hpa, davem, akpm,
	kvalo, mchehab, linux, x86, linux-kernel, platform-driver-x86,
	jiri



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, September 13, 2016 11:12 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: Ingo Molnar <mingo@kernel.org>; Geert Uytterhoeven <geert@linux-
> m68k.org>; tglx@linutronix.de; mingo@redhat.com; hpa@zytor.com;
> davem@davemloft.net; akpm@linux-foundation.org; kvalo@codeaurora.org;
> mchehab@kernel.org; linux@roeck-us.net; x86@kernel.org; linux-
> kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [patch v1] x86/platform/mellanox: introduce support for Mellanox
> systems platform
> 
> On Tue, Sep 13, 2016 at 07:27:20AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of
> > > Ingo Molnar
> > > Sent: Monday, September 12, 2016 2:01 PM
> > > To: Greg KH <gregkh@linuxfoundation.org>
> > > Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Vadim Pasternak
> > > <vadimp@mellanox.com>; tglx@linutronix.de; mingo@redhat.com;
> > > hpa@zytor.com; davem@davemloft.net; akpm@linux-foundation.org;
> > > kvalo@codeaurora.org; mchehab@kernel.org; linux@roeck-us.net;
> > > x86@kernel.org; linux-kernel@vger.kernel.org; platform-driver-
> > > x86@vger.kernel.org; jiri@resnulli.us
> > > Subject: Re: [patch v1] x86/platform/mellanox: introduce support for
> > > Mellanox systems platform
> > >
> > >
> > > * Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > > On Mon, Sep 12, 2016 at 11:14:26AM +0200, Geert Uytterhoeven wrote:
> > > > > On Mon, Sep 12, 2016 at 10:34 AM, Greg KH
> > > > > <gregkh@linuxfoundation.org>
> > > wrote:
> > > > > >> This is LPC to I2C bridge.
> > > > > >
> > > > > > "LPC"?
> > > > >
> > > > > https://en.wikipedia.org/wiki/Low_Pin_Count
> > > > >
> > > > > "Modern ISA"
> > > >
> > > > So my original point stands, 1990's technology being used for
> > > > brand new devices today, ugh :(
> > > >
> > > > Someone needs to go kick some hardware designers...
> > >
> > > In their defense, "this is a carbon copy of published 1990s
> > > technology" is a pretty good starting point for a defendant, in patent
> litigation.
> > >
> >
> > I understood your comments regarding undiscoverable busses.
> > But we use LPC on all our x86 based systems.
> > I have to activate all platform related stuff from some place and we
> > don't support ACPI.
> 
> x86 that doesn't support ACPI?  That's sad :(

Yes.
But we should make a decision for the coming platform - to provide ACPI support for board specific stuff, or equip the new systems with BMC (so we'll introduce all board related stuff through DTS).
But currently this is our reality. Me also sad.

> 
> > Do you think it would be OK, if I'll remove all PCI related code, make
> > use of DMI and leave only platform activation code?
> > If yes, I'll re-work this driver and re-send it for your review.
> 
> Yes, that sounds like a good start, let's see how the code looks after doing that.

Sure. I'll do it.
Thank you very much for your comments.

Thanks,

Vadim
> 
> thanks,
> 
> greg k-h

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

end of thread, other threads:[~2016-09-13 13:59 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-12  6:29 [patch v1] x86/platform/mellanox: introduce support for Mellanox systems platform vadimp
2016-09-12  4:41 ` H. Peter Anvin
2016-09-12  4:52   ` Vadim Pasternak
2016-09-12  4:52     ` Vadim Pasternak
2016-09-12  5:34 ` Guenter Roeck
2016-09-12  6:11   ` Greg KH
2016-09-12  6:12 ` Greg KH
2016-09-12  6:44   ` Vadim Pasternak
2016-09-12  6:44     ` Vadim Pasternak
2016-09-12  7:04     ` Greg KH
2016-09-12  7:04       ` Greg KH
2016-09-12  7:23       ` Vadim Pasternak
2016-09-12  7:23         ` Vadim Pasternak
2016-09-12  7:40         ` Greg KH
2016-09-12  7:40           ` Greg KH
2016-09-12  7:48           ` Vadim Pasternak
2016-09-12  7:48             ` Vadim Pasternak
2016-09-12  8:17             ` Greg KH
2016-09-12  8:17               ` Greg KH
2016-09-12  8:21               ` Vadim Pasternak
2016-09-12  8:21                 ` Vadim Pasternak
2016-09-12  8:34                 ` Greg KH
2016-09-12  8:34                   ` Greg KH
2016-09-12  8:44                   ` Vadim Pasternak
2016-09-12  8:44                     ` Vadim Pasternak
2016-09-12  9:14                   ` Geert Uytterhoeven
2016-09-12  9:14                     ` Geert Uytterhoeven
2016-09-12 10:21                     ` Greg KH
2016-09-12 10:21                       ` Greg KH
2016-09-12 10:55                       ` Geert Uytterhoeven
2016-09-12 10:55                         ` Geert Uytterhoeven
2016-09-12 11:00                       ` Ingo Molnar
2016-09-12 11:00                         ` Ingo Molnar
2016-09-13  7:27                         ` Vadim Pasternak
2016-09-13  7:27                           ` Vadim Pasternak
2016-09-13  8:12                           ` Greg KH
2016-09-13  8:12                             ` Greg KH
2016-09-13  8:18                             ` Vadim Pasternak
2016-09-13  8:18                               ` Vadim Pasternak

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.