From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757548AbcISGCs (ORCPT ); Mon, 19 Sep 2016 02:02:48 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:54142 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750845AbcISGCj (ORCPT ); Mon, 19 Sep 2016 02:02:39 -0400 Subject: Re: [patch v2] x86/platform/mellanox: introduce support for Mellanox systems platform To: vadimp@mellanox.com, tglx@linutronix.de References: <1473933306-124919-1-git-send-email-vadimp@mellanox.com> Cc: mingo@redhat.com, hpa@zytor.com, davem@davemloft.net, geert@linux-m68k.org, akpm@linux-foundation.org, gregkh@linuxfoundation.org, kvalo@codeaurora.org, mchehab@kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org, jiri@resnulli.us From: Guenter Roeck Message-ID: <4497bb80-42e3-ca02-5f41-9c66186ace99@roeck-us.net> Date: Sun, 18 Sep 2016 23:02:33 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1473933306-124919-1-git-send-email-vadimp@mellanox.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/15/2016 02:55 AM, vadimp@mellanox.com wrote: > From: Vadim Pasternak > > 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 > --- > v1->v2: > Comments pointed out by Greg: > - kick out all PCI related code; > --- > MAINTAINERS | 7 + > arch/x86/Kconfig | 25 +++ > arch/x86/platform/Makefile | 1 + > arch/x86/platform/mellanox/Makefile | 1 + > arch/x86/platform/mellanox/mlx-platform.c | 275 ++++++++++++++++++++++++++++++ > 5 files changed, 309 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 > +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 > L: linux-rdma@vger.kernel.org > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index c580d8c..1346441 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -550,6 +550,31 @@ config X86_INTEL_QUARK > Say Y here if you have a Quark based system such as the Arduino > compatible Intel Galileo. > > +config MLX_PLATFORM > + tristate "Mellanox Technologies platform support" > + depends on X86_64 > + depends on X86_EXTENDED_PLATFORM > + depends on PCI No longer needed. > + depends on DMI > + depends on I2C_MLXCPLD > + depends on I2C_MUX_REG > + select HWMON > + select PMBUS > + select SENSORS_PMBUS > + select LM75 > + select NEW_LEDS > + select LEDS_CLASS > + select LEDS_TRIGGERS > + select LEDS_TRIGGER_TIMER > + select LEDS_MLXCPLD Kind of unusual to select drivers not directly related to this one like this. > + ---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 X86_INTEL_LPSS > bool "Intel Low Power Subsystem Support" > depends on X86 && ACPI > 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..d29da68 > --- /dev/null > +++ b/arch/x86/platform/mellanox/mlx-platform.c > @@ -0,0 +1,275 @@ > +/* > + * arch/x86/platform/mellanox/mlx-platform.c > + * Copyright (c) 2016 Mellanox Technologies. All rights reserved. > + * Copyright (c) 2016 Vadim Pasternak > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MLX_PLAT_DEVICE_NAME "mlxplat" > + > +/* LPC bus IO offsets */ > +#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR 0x2000 > +#define MLXPLAT_CPLD_LPC_REG_BASE_ADRR 0x2500 > +#define MLXPLAT_CPLD_LPC_IO_RANGE 0x100 > +#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) > + > +/* Start channel numbers */ > +#define MLXPLAT_CPLD_CH1 2 > +#define MLXPLAT_CPLD_CH2 10 > + > +/* Number of LPC attached MUX platform devices */ > +#define MLXPLAT_CPLD_LPC_MUX_DEVS 2 > + > +/* mlxplat_priv - platform private data > + * @pdev_i2c - i2c controller platform device > + * @pdev_mux - array of mux platform devices > + */ > +struct mlxplat_priv { > + struct platform_device *pdev_i2c; > + struct platform_device *pdev_mux[MLXPLAT_CPLD_LPC_MUX_DEVS]; > +}; > + > +/* 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), > +}; > + > +/* Platform default channels */ > +static int mlxplat_default_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 + Odd line split. > + + 5, MLXPLAT_CPLD_CH2 + 6, MLXPLAT_CPLD_CH2 + 7 > + }, > +}; > + > +/* Platform channels for MSN21xx system family */ > +static int mlxplat_msn21xx_channels[][8] = { > + { > + 1, 2, 3, 4, 5, 6, 7, 8 > + }, > + { > + 1, 2, 3, 4, 5, 6, 7, 8 > + }, Seems to be a waste. A single dimensional array should be sufficient. > +}; > + > +/* Platform mux data */ > +struct i2c_mux_reg_platform_data mlxplat_mux_data[] = { Why global ? > + { > + .parent = 1, > + .base_nr = MLXPLAT_CPLD_CH1, > + .write_only = 1, > + .reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG1, > + .reg_size = 1, > + .idle_in_use = 1, > + }, > + { > + .parent = 1, > + .base_nr = MLXPLAT_CPLD_CH2, > + .write_only = 1, > + .reg = (void __iomem *)MLXPLAT_CPLD_LPC_REG2, > + .reg_size = 1, > + .idle_in_use = 1, > + }, > + > +}; > + > +struct platform_device *mlxplat_dev; > + Is this global on purpose ? If so, why ? > +static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) { > + mlxplat_mux_data[i].values = mlxplat_default_channels[i]; > + mlxplat_mux_data[i].n_values = > + ARRAY_SIZE(mlxplat_default_channels[i]); > + } > + > + return 1; > +}; > + > +static int __init mlxplat_dmi_msn21xx_matched(const struct dmi_system_id *dmi) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) { > + mlxplat_mux_data[i].values = mlxplat_msn21xx_channels[i], > + mlxplat_mux_data[i].n_values = > + ARRAY_SIZE(mlxplat_msn21xx_channels[i]); > + } > + > + return 1; > +}; > + > +static struct dmi_system_id mlxplat_dmi_table[] __initdata = { > + { > + .callback = mlxplat_dmi_default_matched, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "Mellanox Technologies"), > + DMI_MATCH(DMI_PRODUCT_NAME, "MSN24"), > + }, > + }, > + { > + .callback = mlxplat_dmi_default_matched, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "Mellanox Technologies"), > + DMI_MATCH(DMI_PRODUCT_NAME, "MSN27"), > + }, > + }, > + { > + .callback = mlxplat_dmi_default_matched, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "Mellanox Technologies"), > + DMI_MATCH(DMI_PRODUCT_NAME, "MSB"), > + }, > + }, > + { > + .callback = mlxplat_dmi_default_matched, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "Mellanox Technologies"), > + DMI_MATCH(DMI_PRODUCT_NAME, "MSX"), > + }, > + }, > + { > + .callback = mlxplat_dmi_msn21xx_matched, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "Mellanox Technologies"), > + DMI_MATCH(DMI_PRODUCT_NAME, "MSN21"), > + }, > + }, > + { } > +}; > + > +static int __init mlxplat_init(void) > +{ > + struct mlxplat_priv *priv; > + int i; > + int err; > + > + if (!dmi_check_system(mlxplat_dmi_table)) > + return -ENODEV; > + > + mlxplat_dev = platform_device_register_simple(MLX_PLAT_DEVICE_NAME, -1, > + mlxplat_lpc_resources, > + ARRAY_SIZE(mlxplat_lpc_resources)); > + > + if (!mlxplat_dev) { > + pr_err("Alloc %s platform device failed\n", > + MLX_PLAT_DEVICE_NAME); Error messages for memory allocations are unnecessary. > + return -ENOMEM; > + } > + > + priv = devm_kzalloc(&mlxplat_dev->dev, sizeof(struct mlxplat_priv), > + GFP_KERNEL); > + if (!priv) { > + err = -ENOMEM; > + dev_err(&mlxplat_dev->dev, "Failed to allocate mlxplat_priv\n"); Same here. > + goto fail_alloc; > + } > + platform_set_drvdata(mlxplat_dev, priv); > + > + 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++) { > + priv->pdev_mux[i] = platform_device_register_resndata( > + &mlxplat_dev->dev, > + "i2c-mux-reg", i, NULL, > + 0, &mlxplat_mux_data[i], > + sizeof(mlxplat_mux_data[i])); > + if (IS_ERR(priv->pdev_mux[i])) { > + err = PTR_ERR(priv->pdev_mux[i]); > + goto fail_platform_mux_register; > + } > + } > + > + return err; return 0; > + > +fail_platform_mux_register: > + for (i--; i > 0 ; i--) >= 0 ? > + platform_device_unregister(priv->pdev_mux[i]); > + platform_device_unregister(priv->pdev_i2c); > +fail_alloc: > + platform_device_unregister(mlxplat_dev); > + > + return err; > +} > + > +static void __exit mlxplat_exit(void) > +{ > + struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev); > + int i; > + > + for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--) > + platform_device_unregister(priv->pdev_mux[i]); > + > + platform_device_unregister(priv->pdev_i2c); > + platform_device_unregister(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"); > Should this possibly be a dmi module alias ?