Re: [PATCH 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC

2017-08-22 Thread Takashi Iwai
On Tue, 22 Aug 2017 11:37:45 +0200,
Andy Shevchenko wrote:
> 
> On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> > This patch adds the MFD driver for Dollar Cove TI PMIC (ACPI INT33F5)
> > that is found on some Intel Cherry Trail devices.
> > The driver is based on the original work by Intel, found at:
> >   https://github.com/01org/ProductionKernelQuilts
> > 
> > This is a minimal version for adding the basic resources.  Currently,
> > only ACPI PMIC opregion and the external power-button are used.
> 
> Overall looks good. Few comments below.
> 
> I hope Hans de Goede had been involved somehow to this. He did enormous
> work to examine all this PMIC/ACPI/I2C stuff on CHT platforms.

Sure, he was of a great help, although he didn't touch directly about
these codes.

> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
> Alphabetical order?

OK, will rearrange.
Also, gpio stuff was removed from this version, so it's superfluous.
Will rip off.


> > +static const struct i2c_device_id dc_ti_i2c_id[] = {
> > +   { }
> > +};
> 
> No need anymore. Moreover, you already are using ->probe_new().

OK.


> > +static struct i2c_driver dc_ti_i2c_driver = {
> > +   .driver = {
> > +   .name = KBUILD_MODNAME,
> > +   .pm = &dc_ti_pm_ops,
> > +   .acpi_match_table = ACPI_PTR(dc_ti_acpi_ids),
> 
> ACPI_PTR is redundant here, driver is solely for ACPI case.

Right.


thanks,

Takashi


Re: [PATCH 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC

2017-08-22 Thread Andy Shevchenko
On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote:
> This patch adds the MFD driver for Dollar Cove TI PMIC (ACPI INT33F5)
> that is found on some Intel Cherry Trail devices.
> The driver is based on the original work by Intel, found at:
>   https://github.com/01org/ProductionKernelQuilts
> 
> This is a minimal version for adding the basic resources.  Currently,
> only ACPI PMIC opregion and the external power-button are used.

Overall looks good. Few comments below.

I hope Hans de Goede had been involved somehow to this. He did enormous
work to examine all this PMIC/ACPI/I2C stuff on CHT platforms.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Alphabetical order?

> +static const struct i2c_device_id dc_ti_i2c_id[] = {
> + { }
> +};

No need anymore. Moreover, you already are using ->probe_new().


> +static struct i2c_driver dc_ti_i2c_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .pm = &dc_ti_pm_ops,
> + .acpi_match_table = ACPI_PTR(dc_ti_acpi_ids),

ACPI_PTR is redundant here, driver is solely for ACPI case.

> + },
> + .probe_new = dc_ti_probe,
> + .shutdown = dc_ti_shutdown,
> + .id_table = dc_ti_i2c_id,
> +};

-- 
Andy Shevchenko 
Intel Finland Oy


[PATCH 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC

2017-08-21 Thread Takashi Iwai
This patch adds the MFD driver for Dollar Cove TI PMIC (ACPI INT33F5)
that is found on some Intel Cherry Trail devices.
The driver is based on the original work by Intel, found at:
  https://github.com/01org/ProductionKernelQuilts

This is a minimal version for adding the basic resources.  Currently,
only ACPI PMIC opregion and the external power-button are used.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=193891
Signed-off-by: Takashi Iwai 
---
 drivers/mfd/Kconfig|  13 +++
 drivers/mfd/Makefile   |   1 +
 drivers/mfd/intel_soc_pmic_dc_ti.c | 189 +
 3 files changed, 203 insertions(+)
 create mode 100644 drivers/mfd/intel_soc_pmic_dc_ti.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 94ad2c1c3d90..28c12bb50c9f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -496,6 +496,19 @@ config INTEL_SOC_PMIC_CHTWC
  available before any devices using it are probed. This option also
  causes the designware-i2c driver to be builtin for the same reason.
 
+config INTEL_SOC_PMIC_DC_TI
+   tristate "Support for Dollar Cove TI PMIC"
+   depends on GPIOLIB
+   depends on I2C
+   depends on ACPI
+   depends on X86
+   select MFD_CORE
+   select REGMAP_I2C
+   select REGMAP_IRQ
+   help
+ Select this option for supporting Dollar Cove TI PMIC device that is
+ found on some Intel Cherry Trail systems.
+
 config MFD_INTEL_LPSS
tristate
select COMMON_CLK
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 080793b3fd0e..16c4fe519d30 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -216,6 +216,7 @@ intel-soc-pmic-objs := intel_soc_pmic_core.o 
intel_soc_pmic_crc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)   += intel-soc-pmic.o
 obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o
+obj-$(CONFIG_INTEL_SOC_PMIC_DC_TI) += intel_soc_pmic_dc_ti.o
 obj-$(CONFIG_MFD_MT6397)   += mt6397-core.o
 
 obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
diff --git a/drivers/mfd/intel_soc_pmic_dc_ti.c 
b/drivers/mfd/intel_soc_pmic_dc_ti.c
new file mode 100644
index ..f1c2ac310bf0
--- /dev/null
+++ b/drivers/mfd/intel_soc_pmic_dc_ti.c
@@ -0,0 +1,189 @@
+/*
+ * Device access for Dollar Cove TI PMIC
+ * Copyright (c) 2014, Intel Corporation.
+ *   Author: Ramakrishna Pallala 
+ *
+ * Cleanup and forward-ported
+ *   Copyright (c) 2017 Takashi Iwai 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DC_TI_IRQLVL1  0x01
+#define DC_TI_MASK_IRQLVL1 0x02
+
+/* Level 1 IRQs */
+enum {
+   DC_TI_PWRBTN = 0,   /* power button */
+   DC_TI_DIETMPWARN,   /* thermal */
+   DC_TI_ADCCMPL,  /* ADC */
+   /* no irq 3 */
+   DC_TI_VBATLOW = 4,  /* battery */
+   DC_TI_VBUSDET,  /* power source */
+   /* no irq 6 */
+   DC_TI_CCEOCAL = 7,  /* battery */
+};
+
+static struct resource power_button_resources[] = {
+   DEFINE_RES_IRQ(DC_TI_PWRBTN),
+};
+
+static struct resource thermal_resources[] = {
+   DEFINE_RES_IRQ(DC_TI_DIETMPWARN),
+};
+
+static struct resource adc_resources[] = {
+   DEFINE_RES_IRQ(DC_TI_ADCCMPL),
+};
+
+static struct resource pwrsrc_resources[] = {
+   DEFINE_RES_IRQ(DC_TI_VBUSDET),
+};
+
+static struct resource battery_resources[] = {
+   DEFINE_RES_IRQ(DC_TI_VBATLOW),
+   DEFINE_RES_IRQ(DC_TI_CCEOCAL),
+};
+
+static struct mfd_cell dc_ti_dev[] = {
+   {
+   .name = "dc_ti_pwrbtn",
+   .num_resources = ARRAY_SIZE(power_button_resources),
+   .resources = power_button_resources,
+   },
+   {
+   .name = "dc_ti_adc",
+   .num_resources = ARRAY_SIZE(adc_resources),
+   .resources = adc_resources,
+   },
+   {
+   .name = "dc_ti_thermal",
+   .num_resources = ARRAY_SIZE(thermal_resources),
+   .resources = thermal_resources,
+   },
+   {
+   .name = "dc_ti_pwrsrc",
+   .num_resources = ARRAY_SIZE(pwrsrc_resources),
+   .resources = pwrsrc_resources,
+   },
+   {
+   .name = "dc_ti_battery",
+   .num_resources = ARRAY_SIZE(battery_resources),
+   .resources = battery_resources,
+   },
+   {
+   .name = "dc_ti_region",
+   },
+};
+
+static const struct regmap_config dc_ti_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = 128,
+   .cache_type = REGCACHE_NONE,
+};
+
+static const struct regmap_irq dc_ti_irqs[] = {
+   REGMAP_IRQ_REG(DC_TI_PWRBTN, 0, BIT(DC_TI_PWRBTN)),
+   REGMAP_IRQ_REG(DC_TI_DIETMPWARN, 0, BIT(DC_TI_DIETMPWARN)),
+   REGMAP_IRQ_REG(DC_TI_ADCCMPL, 0, BIT(DC_TI_ADCCMPL)),
+   REGMAP_IRQ_REG