Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965238AbbHKPIM (ORCPT ); Tue, 11 Aug 2015 11:08:12 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:33511 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965118AbbHKPIK (ORCPT ); Tue, 11 Aug 2015 11:08:10 -0400 Date: Tue, 11 Aug 2015 16:08:03 +0100 From: Lee Jones To: Qipeng Zha Cc: linux-kernel@vger.kernel.org, sameo@linux.intel.com, fei.yang@intel.com, samuel.ortiz@intel.com Subject: Re: [PATCH v4] mfd: add Intel Broxton Whiskey Cove PMIC driver Message-ID: <20150811150803.GN18282@x1> References: <1439308419-130845-1-git-send-email-qipeng.zha@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1439308419-130845-1-git-send-email-qipeng.zha@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 21251 Lines: 785 On Tue, 11 Aug 2015, Qipeng Zha wrote: > From: "qipeng.zha" Can you set your name properly in Git please? I believe it should read "Qipeng Zha", as it does in your mailer. > Add MFD core driver for Intel Broxton Whiskey Cove PMIC, > which is specially accessed by hardware IPC, not a generic > I2C device > > Signed-off-by: qipeng.zha > --- > change in v4 > add compile dependency to PMC IPC driver in Makefile, or will > use NULL stubs defined in PMC IPC header file; > > change in v3 > implement ipc regmap r/w callback in pmic driver, since there dropped > before patch of implement virtual ipc support in regmap core; > update some function's comment; > --- > drivers/mfd/Makefile | 1 + > drivers/mfd/intel_soc_pmic_bxtwc.c | 529 +++++++++++++++++++++++++++++++++++++ > include/linux/mfd/intel_bxtwc.h | 69 +++++ > include/linux/mfd/intel_soc_pmic.h | 3 + > 4 files changed, 602 insertions(+) > create mode 100644 drivers/mfd/intel_soc_pmic_bxtwc.c > create mode 100644 include/linux/mfd/intel_bxtwc.h > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 0e5cfeb..8fc894e 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -183,5 +183,6 @@ obj-$(CONFIG_MFD_RT5033) += rt5033.o > obj-$(CONFIG_MFD_SKY81452) += sky81452.o > > intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o > +intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o You want to build this for all Intel products? > obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o > obj-$(CONFIG_MFD_MT6397) += mt6397-core.o > diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c > new file mode 100644 > index 0000000..7ebbcb2 > --- /dev/null > +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c > @@ -0,0 +1,529 @@ > +/* > + * intel_soc_pmic_bxtwc.c: MFD core driver for Intel Broxton Whiskey Cove PMIC > + * > + * Copyright (C) 2015 Intel Corporation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * Superfluous '\n'. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Alphabetical please. > +/* PMIC device registers */ > +#define REG_ADDR_MASK 0xFF00 > +#define REG_ADDR_SHIFT 8 > +#define REG_OFFSET_MASK 0xFF > + > +/* Interrupt Status Registers */ > +#define BXTWC_IRQLVL1 0x4E02 > +#define BXTWC_PWRBTNIRQ 0x4E03 > + > +#define BXTWC_THRM0IRQ 0x4E04 > +#define BXTWC_THRM1IRQ 0x4E05 > +#define BXTWC_THRM2IRQ 0x4E06 > +#define BXTWC_BCUIRQ 0x4E07 > +#define BXTWC_ADCIRQ 0x4E08 > +#define BXTWC_CHGR0IRQ 0x4E09 > +#define BXTWC_CHGR1IRQ 0x4E0A > +#define BXTWC_GPIOIRQ0 0x4E0B > +#define BXTWC_GPIOIRQ1 0x4E0C > +#define BXTWC_CRITIRQ 0x4E0D > + > +/* Interrupt MASK Registers */ > +#define BXTWC_MIRQLVL1 0x4E0E > +#define BXTWC_MPWRTNIRQ 0x4E0F > + > +#define BXTWC_MTHRM0IRQ 0x4E12 > +#define BXTWC_MTHRM1IRQ 0x4E13 > +#define BXTWC_MTHRM2IRQ 0x4E14 > +#define BXTWC_MBCUIRQ 0x4E15 > +#define BXTWC_MADCIRQ 0x4E16 > +#define BXTWC_MCHGR0IRQ 0x4E17 > +#define BXTWC_MCHGR1IRQ 0x4E18 > +#define BXTWC_MGPIO0IRQ 0x4E19 > +#define BXTWC_MGPIO1IRQ 0x4E1A > +#define BXTWC_MCRITIRQ 0x4E1B > + > +/* Whiskey Cove PMIC share same ACPI ID between different platforms */ > +#define BROXTON_PMIC_WC_HRV 4 > + > +/* Manage in two irq chips since mask registers are not consecutive*/ s/irq/IRQ/ Fix whitespace issue. > +enum bxtwc_irqs { > + /* Level 1 */ > + BXTWC_PWRBTN_LVL1_IRQ = 0, > + BXTWC_TMU_LVL1_IRQ, > + BXTWC_THRM_LVL1_IRQ, > + BXTWC_BCU_LVL1_IRQ, > + BXTWC_ADC_LVL1_IRQ, > + BXTWC_CHGR_LVL1_IRQ, > + BXTWC_GPIO_LVL1_IRQ, > + BXTWC_CRIT_LVL1_IRQ, > + > + /* Level 2 */ > + BXTWC_PWRBTN_IRQ, > +}; > + > +enum bxtwc_irqs_level2 { > + /* Level 2 */ > + BXTWC_THRM0_IRQ = 0, > + BXTWC_THRM1_IRQ, > + BXTWC_THRM2_IRQ, > + BXTWC_BCU_IRQ, > + BXTWC_ADC_IRQ, > + BXTWC_CHGR0_IRQ, > + BXTWC_CHGR1_IRQ, > + BXTWC_GPIO0_IRQ, > + BXTWC_GPIO1_IRQ, > + BXTWC_CRIT_IRQ, > +}; > + > +#define INIT_REGMAP_IRQ(_irq, _off, _mask) \ > + [_irq] = { .reg_offset = (_off), .mask = (_mask) } If this is useful to you, it's likely to be useful to others. Please place this in include submit this as a separate patch. > +static const struct regmap_irq bxtwc_regmap_irqs[] = { > + INIT_REGMAP_IRQ(BXTWC_PWRBTN_LVL1_IRQ, 0, BIT(0)), > + INIT_REGMAP_IRQ(BXTWC_TMU_LVL1_IRQ, 0, BIT(1)), > + INIT_REGMAP_IRQ(BXTWC_THRM_LVL1_IRQ, 0, BIT(2)), > + INIT_REGMAP_IRQ(BXTWC_BCU_LVL1_IRQ, 0, BIT(3)), > + INIT_REGMAP_IRQ(BXTWC_ADC_LVL1_IRQ, 0, BIT(4)), > + INIT_REGMAP_IRQ(BXTWC_CHGR_LVL1_IRQ, 0, BIT(5)), > + INIT_REGMAP_IRQ(BXTWC_GPIO_LVL1_IRQ, 0, BIT(6)), > + INIT_REGMAP_IRQ(BXTWC_CRIT_LVL1_IRQ, 0, BIT(7)), > + INIT_REGMAP_IRQ(BXTWC_PWRBTN_IRQ, 1, 0x03), > +}; > + > +static const struct regmap_irq bxtwc_regmap_irqs_level2[] = { > + INIT_REGMAP_IRQ(BXTWC_THRM0_IRQ, 0, 0xff), > + INIT_REGMAP_IRQ(BXTWC_THRM1_IRQ, 1, 0xbf), > + INIT_REGMAP_IRQ(BXTWC_THRM2_IRQ, 2, 0xff), > + INIT_REGMAP_IRQ(BXTWC_BCU_IRQ, 3, 0x1f), > + INIT_REGMAP_IRQ(BXTWC_ADC_IRQ, 4, 0xff), > + INIT_REGMAP_IRQ(BXTWC_CHGR0_IRQ, 5, 0x1f), > + INIT_REGMAP_IRQ(BXTWC_CHGR1_IRQ, 6, 0x1f), > + INIT_REGMAP_IRQ(BXTWC_GPIO0_IRQ, 7, 0xff), > + INIT_REGMAP_IRQ(BXTWC_GPIO1_IRQ, 8, 0x3f), > + INIT_REGMAP_IRQ(BXTWC_CRIT_IRQ, 9, 0x03), > +}; > + > +static struct regmap_irq_chip bxtwc_regmap_irq_chip = { > + .name = "bxtwc_irq_chip", > + .status_base = BXTWC_IRQLVL1, > + .mask_base = BXTWC_MIRQLVL1, > + .irqs = bxtwc_regmap_irqs, > + .num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs), > + .num_regs = 2, > +}; > + > +static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = { > + .name = "bxtwc_irq_chip_level2", > + .status_base = BXTWC_THRM0IRQ, > + .mask_base = BXTWC_MTHRM0IRQ, > + .irqs = bxtwc_regmap_irqs_level2, > + .num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2), > + .num_regs = 10, > +}; > + > +static struct resource gpio_resources[] = { > + { > + .name = "GPIO0", > + .start = BXTWC_GPIO0_IRQ, > + .end = BXTWC_GPIO0_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > + { > + .name = "GPIO1", > + .start = BXTWC_GPIO1_IRQ, > + .end = BXTWC_GPIO1_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +static struct resource adc_resources[] = { > + { > + .name = "ADC", > + .start = BXTWC_ADC_IRQ, > + .end = BXTWC_ADC_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +static struct resource charger_resources[] = { > + { > + .name = "CHARGER", > + .start = BXTWC_CHGR0_IRQ, > + .end = BXTWC_CHGR0_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > + { > + .name = "CHARGER1", > + .start = BXTWC_CHGR1_IRQ, > + .end = BXTWC_CHGR1_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +static struct resource thermal_resources[] = { > + { > + .start = BXTWC_THRM0_IRQ, > + .end = BXTWC_THRM0_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > + { > + .start = BXTWC_THRM1_IRQ, > + .end = BXTWC_THRM1_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > + { > + .start = BXTWC_THRM2_IRQ, > + .end = BXTWC_THRM2_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +static struct resource bcu_resources[] = { > + { > + .name = "BCU", > + .start = BXTWC_BCU_IRQ, > + .end = BXTWC_BCU_IRQ, > + .flags = IORESOURCE_IRQ, > + }, > +}; Please use the DEFINE_RES_* macros for all of the above. > +static struct mfd_cell bxt_wc_dev[] = { > + { > + .name = "bxt_wcove_gpadc", > + .num_resources = ARRAY_SIZE(adc_resources), > + .resources = adc_resources, > + }, > + { > + .name = "bxt_wcove_thermal", > + .num_resources = ARRAY_SIZE(thermal_resources), > + .resources = thermal_resources, > + }, > + { > + .name = "bxt_wcove_ext_charger", > + .num_resources = ARRAY_SIZE(charger_resources), > + .resources = charger_resources, > + }, > + { > + .name = "bxt_wcove_bcu", > + .num_resources = ARRAY_SIZE(bcu_resources), > + .resources = bcu_resources, > + }, > + { > + .name = "bxt_wcove_gpio", > + .num_resources = ARRAY_SIZE(gpio_resources), > + .resources = gpio_resources, > + }, > + { > + .name = "bxt_wcove_sw_fuel_gauge", > + .num_resources = 0, > + .resources = NULL, Remove these two lines. > + }, > + { > + .name = "bxt_wcove_sw_fuel_gauge_ha", > + .num_resources = 0, > + .resources = NULL, As above. > + }, > +}; > + > +static int regmap_ipc_byte_reg_read(void *context, unsigned int reg, > + unsigned int *val) > +{ > + int ret; > + int i2c_addr; > + u8 ipc_in[2]; > + u8 ipc_out[4]; > + struct intel_soc_pmic *pmic = context; > + > + if (reg & REG_ADDR_MASK) > + i2c_addr = (reg & REG_ADDR_MASK) >> REG_ADDR_SHIFT; > + else { > + i2c_addr = pmic->default_i2c_addr; > + if (!i2c_addr) { > + dev_err(pmic->dev, "Wrong register addr to read\n"); "I2C address not set"? > + return -EINVAL; > + } > + } > + reg &= REG_OFFSET_MASK; > + > + ipc_in[0] = reg; > + ipc_in[1] = i2c_addr; > + ret = intel_pmc_ipc_command(PMC_IPC_PMIC_ACCESS, > + PMC_IPC_PMIC_ACCESS_READ, > + ipc_in, sizeof(ipc_in), (u32 *)ipc_out, 1); > + if (ret) { > + dev_err(pmic->dev, "Err: ipc read pmic\n"); "Failed to read from PMIC"? > + return ret; > + } > + *val = ipc_out[0]; > + return 0; > +} > + > +static int regmap_ipc_byte_reg_write(void *context, unsigned int reg, > + unsigned int val) > +{ > + int ret; > + int i2c_addr; > + u8 ipc_in[3]; > + struct intel_soc_pmic *pmic = context; > + > + if (reg & REG_ADDR_MASK) > + i2c_addr = (reg & REG_ADDR_MASK) >> REG_ADDR_SHIFT; > + else { > + i2c_addr = pmic->default_i2c_addr; > + if (!i2c_addr) { > + dev_err(pmic->dev, "Wrong register addr to write\n"); "I2C address not set"? > + return -EINVAL; > + } > + } > + reg &= REG_OFFSET_MASK; > + > + ipc_in[0] = reg; > + ipc_in[1] = i2c_addr; > + ipc_in[2] = val; > + ret = intel_pmc_ipc_command(PMC_IPC_PMIC_ACCESS, > + PMC_IPC_PMIC_ACCESS_WRITE, > + ipc_in, sizeof(ipc_in), NULL, 0); > + if (ret) { > + dev_err(pmic->dev, "Err: ipc write pmic\n"); "Failed to write to PMIC"? > + return ret; > + } > + return 0; > +} > + > +/* sysfs interfaces to r/w PMIC registers, required by initial script */ You really want to be able to do that from userspace? > +static unsigned long bxtwc_reg_addr; > +static ssize_t bxtwc_reg_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "0x%lx\n", bxtwc_reg_addr); > +} > + > +static ssize_t bxtwc_reg_store(struct device *dev, > + struct device_attribute *attr, const char *buf, size_t count) > +{ > + if (kstrtoul(buf, 0, &bxtwc_reg_addr)) { > + dev_err(dev, "Invalid register addr\n"); Why don't you write "address" in full? It's much more professional in error messages. > + return -EINVAL; > + } > + return (ssize_t)count; > +} > + > +static ssize_t bxtwc_val_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + int ret; > + unsigned int val; > + struct intel_soc_pmic *pmic = dev_get_drvdata(dev); > + > + ret = regmap_read(pmic->regmap, bxtwc_reg_addr, &val); > + if (ret < 0) { > + dev_err(dev, "Fail to read 0x%lx\n", bxtwc_reg_addr); > + return -EIO; > + } > + > + return sprintf(buf, "0x%02x\n", val); > +} > + > +static ssize_t bxtwc_val_store(struct device *dev, > + struct device_attribute *attr, const char *buf, size_t count) > +{ > + int ret; > + unsigned int val; > + struct intel_soc_pmic *pmic = dev_get_drvdata(dev); > + > + if (kstrtoul(buf, 0, (unsigned long *)&val)) { > + dev_err(dev, "Invalid value\n"); "value" is pretty vague. I'm sure you can do better than that. > + return -EINVAL; > + } > + > + ret = regmap_write(pmic->regmap, bxtwc_reg_addr, val); > + if (ret) { > + dev_err(dev, "Write err: 0x%lx=0x%02x\n", bxtwc_reg_addr, val); "Failed to write value 0x%02x to address 0x%lx", val, bxtwc_reg_addr ? > + return -EIO; > + } > + return count; > +} > + > +static DEVICE_ATTR(addr, S_IWUSR | S_IRUSR, bxtwc_reg_show, bxtwc_reg_store); > +static DEVICE_ATTR(val, S_IWUSR | S_IRUSR, bxtwc_val_show, bxtwc_val_store); > +static struct attribute *bxtwc_attrs[] = { > + &dev_attr_addr.attr, > + &dev_attr_val.attr, > + NULL > +}; > + > +static const struct attribute_group bxtwc_group = { > + .attrs = bxtwc_attrs, > +}; > + > +static const struct regmap_config bxtwc_regmap_config = { > + .reg_bits = 16, > + .val_bits = 8, > + .reg_write = regmap_ipc_byte_reg_write, > + .reg_read = regmap_ipc_byte_reg_read, > +}; > + > +static int bxtwc_probe(struct platform_device *pdev) > +{ > + struct intel_soc_pmic *pmic; > + unsigned long long hrv; > + acpi_handle handle; > + acpi_status status; > + int ret; > + > + handle = ACPI_HANDLE(&pdev->dev); > + status = acpi_evaluate_integer(handle, "_HRV", NULL, &hrv); > + if (ACPI_FAILURE(status)) { > + dev_err(&pdev->dev, "ACPI error when get _HRV\n"); I'm not even sure what this means. Please find a better way to portray what just happened. > + return -ENODEV; > + } > + if (hrv != BROXTON_PMIC_WC_HRV) { > + dev_err(&pdev->dev, "Wrong _HRV %llu\n", hrv); > + return -ENODEV; > + } You have to think to yourself, what will a user make of this? I would think, unless you know this code, it's pretty undecipherable. > + pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL); > + if (!pmic) > + return -ENOMEM; > + > + pmic->irq = platform_get_irq(pdev, 0); > + if (pmic->irq < 0) { > + dev_err(&pdev->dev, "No irq resource?\n"); "Invalid IRQ"? > + return -ENOENT; If irq < 0, then it's already an error code, so just return that, rather than making up your own. > + } > + > + dev_set_drvdata(&pdev->dev, pmic); > + pmic->dev = &pdev->dev; > + pmic->default_i2c_addr = BXTWC_DEVICE1_ADDR; Can this be anything different? If not, get rid of the variable and just use the define. If it can, how can it? > + pmic->regmap = devm_regmap_init(&pdev->dev, NULL, pmic, > + &bxtwc_regmap_config); > + if (IS_ERR(pmic->regmap)) { > + ret = PTR_ERR(pmic->regmap); > + dev_err(&pdev->dev, "Fail to init pmic ipc regmap: %d\n", ret); "Failed to initialise regmap". > + return ret; > + } > + > + ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, > + IRQF_ONESHOT | IRQF_SHARED, > + 0, &bxtwc_regmap_irq_chip, > + &pmic->irq_chip_data); > + if (ret) { > + dev_err(&pdev->dev, "Fail to add irq chip\n"); "Failed to add IRQ chip" > + return ret; > + } > + > + ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, > + IRQF_ONESHOT | IRQF_SHARED, > + 0, &bxtwc_regmap_irq_chip_level2, > + &pmic->irq_chip_data_level2); > + if (ret) { > + dev_err(&pdev->dev, "Fail to add irq chip level2\n"); "Failed to add secondary IRQ chip" > + goto err_irq_chip_level2; > + } > + > + ret = mfd_add_devices(&pdev->dev, -1, bxt_wc_dev, Don't use magic numbers, there are defines these. > + ARRAY_SIZE(bxt_wc_dev), NULL, 0, > + NULL); > + if (ret) { > + dev_err(&pdev->dev, "Fail to addr irq chip level2\n"); Eh? "Failed to add devices"? > + goto err_mfd; > + } > + > + ret = sysfs_create_group(&pdev->dev.kobj, &bxtwc_group); > + if (ret) { > + dev_err(&pdev->dev, "Fail to create sysfs group %d\n", ret); > + goto err_sysfs; > + } > + > + return 0; > + > +err_sysfs: > + mfd_remove_devices(&pdev->dev); > +err_mfd: > + regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2); > +err_irq_chip_level2: > + regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data); > + > + return ret; > +} > + > +static int bxtwc_remove(struct platform_device *pdev) > +{ > + struct intel_soc_pmic *pmic = dev_get_drvdata(&pdev->dev); > + > + sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group); > + mfd_remove_devices(&pdev->dev); > + regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data); > + regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2); I'd prefer to see a '\n' here. > + return 0; > +} > + > +static void bxtwc_shutdown(struct platform_device *pdev) > +{ > + struct intel_soc_pmic *pmic = dev_get_drvdata(&pdev->dev); > + > + disable_irq(pmic->irq); > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int bxtwc_suspend(struct device *dev) > +{ > + struct intel_soc_pmic *pmic = dev_get_drvdata(dev); > + > + disable_irq(pmic->irq); ... and here. > + return 0; > +} > + > +static int bxtwc_resume(struct device *dev) > +{ > + struct intel_soc_pmic *pmic = dev_get_drvdata(dev); > + > + enable_irq(pmic->irq); ... and here. > + return 0; > +} > +#endif > +static SIMPLE_DEV_PM_OPS(bxtwc_pm_ops, bxtwc_suspend, bxtwc_resume); > + > +#ifdef CONFIG_ACPI > +static const struct acpi_device_id bxtwc_acpi_ids[] = { > + { "INT34D3", }, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, pmic_acpi_ids); > +#endif Can this driver even operate without ACPI? > +static struct platform_driver bxtwc_driver = { > + .probe = bxtwc_probe, > + .remove = bxtwc_remove, > + .shutdown = bxtwc_shutdown, > + .driver = { > + .name = "bxtwc", If this is a PMIC driver, it would be good to reflect that in the name. After all, this is what's going to identify the part in error messages and the like. > + .pm = &bxtwc_pm_ops, > + .acpi_match_table = ACPI_PTR(bxtwc_acpi_ids), > + }, > +}; > + > +module_platform_driver(bxtwc_driver); > + > +MODULE_LICENSE("GPL V2"); > +MODULE_AUTHOR("qipeng zha"); Capitalise your name and fix the whitespace issue. > diff --git a/include/linux/mfd/intel_bxtwc.h b/include/linux/mfd/intel_bxtwc.h > new file mode 100644 > index 0000000..1a0ee9d > --- /dev/null > +++ b/include/linux/mfd/intel_bxtwc.h > @@ -0,0 +1,69 @@ > +/* > + * intel_bxtwc.h - Header file for Intel Broxton Whiskey Cove PMIC > + * > + * Copyright (C) 2015 Intel Corporation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > + > +#ifndef __INTEL_BXTWC_H__ > +#define __INTEL_BXTWC_H__ > + > +/* BXT WC devices */ > +#define BXTWC_DEVICE1_ADDR 0x4E > +#define BXTWC_DEVICE2_ADDR 0x4F > +#define BXTWC_DEVICE3_ADDR 0x5E > + > +/* device1 Registers */ > +#define BXTWC_CHIPID 0x4E00 > +#define BXTWC_CHIPVER 0x4E01 > + > +#define BXTWC_SCHGRIRQ0_ADDR 0x5E1A > +#define BXTWC_CHGRCTRL0_ADDR 0x5E16 > +#define BXTWC_CHGRCTRL1_ADDR 0x5E17 > +#define BXTWC_CHGRCTRL2_ADDR 0x5E18 > +#define BXTWC_CHGRSTATUS_ADDR 0x5E19 > +#define BXTWC_THRMBATZONE_ADDR 0x4F22 > + > +#define BXTWC_USBPATH_ADDR 0x5E19 > +#define BXTWC_USBPHYCTRL_ADDR 0x5E07 > +#define BXTWC_USBIDCTRL_ADDR 0x5E05 > +#define BXTWC_USBIDEN_MASK 0x01 > +#define BXTWC_USBIDSTAT_ADDR 0x00FF > +#define BXTWC_USBSRCDETSTATUS_ADDR 0x5E29 > + > +#define BXTWC_DBGUSBBC1_ADDR 0x5FE0 > +#define BXTWC_DBGUSBBC2_ADDR 0x5FE1 > +#define BXTWC_DBGUSBBCSTAT_ADDR 0x5FE2 > + > +#define BXTWC_WAKESRC_ADDR 0x4E22 > +#define BXTWC_WAKESRC2_ADDR 0x4EE5 > +#define BXTWC_CHRTTADDR_ADDR 0x5E22 > +#define BXTWC_CHRTTDATA_ADDR 0x5E23 > + > +#define BXTWC_STHRMIRQ0_ADDR 0x4F19 > +#define WC_MTHRMIRQ1_ADDR 0x4E12 > +#define WC_STHRMIRQ1_ADDR 0x4F1A > +#define WC_STHRMIRQ2_ADDR 0x4F1B > + > +#define BXTWC_THRMZN0H_ADDR 0x4F44 > +#define BXTWC_THRMZN0L_ADDR 0x4F45 > +#define BXTWC_THRMZN1H_ADDR 0x4F46 > +#define BXTWC_THRMZN1L_ADDR 0x4F47 > +#define BXTWC_THRMZN2H_ADDR 0x4F48 > +#define BXTWC_THRMZN2L_ADDR 0x4F49 > +#define BXTWC_THRMZN3H_ADDR 0x4F4A > +#define BXTWC_THRMZN3L_ADDR 0x4F4B > +#define BXTWC_THRMZN4H_ADDR 0x4F4C > +#define BXTWC_THRMZN4L_ADDR 0x4F4D > + > +#endif > diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h > index abcbfcf..35e7904 100644 > --- a/include/linux/mfd/intel_soc_pmic.h > +++ b/include/linux/mfd/intel_soc_pmic.h > @@ -25,6 +25,9 @@ struct intel_soc_pmic { > int irq; > struct regmap *regmap; > struct regmap_irq_chip_data *irq_chip_data; > + struct regmap_irq_chip_data *irq_chip_data_level2; > + struct device *dev; > + int default_i2c_addr; > }; > > #endif /* __INTEL_SOC_PMIC_H__ */ -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/