[PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

The core part is misleading since its only purpose to serve Crystal Cove PMIC,
although for couple of different platforms. Merge core part into crc one.

Advantages among others are:
- speed up a compilation and build
- decreasing the code base
- reducing noise in the namespace by making some data static and const

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/Makefile              |   3 +-
 drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
 drivers/mfd/intel_soc_pmic_core.h |  25 -----
 drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
 4 files changed, 157 insertions(+), 193 deletions(-)
 delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
 delete mode 100644 drivers/mfd/intel_soc_pmic_core.h

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 858cacf659d6..61db669f864c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -236,8 +236,7 @@ obj-$(CONFIG_MFD_RT4831)	+= rt4831.o
 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
-obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
+obj-$(CONFIG_INTEL_SOC_PMIC)		+= intel_soc_pmic_crc.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_CHTDC_TI)	+= intel_soc_pmic_chtdc_ti.o
diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c
deleted file mode 100644
index 5e8c94e008ed..000000000000
--- a/drivers/mfd/intel_soc_pmic_core.c
+++ /dev/null
@@ -1,160 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Intel SoC PMIC MFD Driver
- *
- * Copyright (C) 2013, 2014 Intel Corporation. All rights reserved.
- *
- * Author: Yang, Bin <bin.yang@intel.com>
- * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
- */
-
-#include <linux/acpi.h>
-#include <linux/i2c.h>
-#include <linux/interrupt.h>
-#include <linux/module.h>
-#include <linux/mfd/core.h>
-#include <linux/mfd/intel_soc_pmic.h>
-#include <linux/platform_data/x86/soc.h>
-#include <linux/pwm.h>
-#include <linux/regmap.h>
-
-#include "intel_soc_pmic_core.h"
-
-/* PWM consumed by the Intel GFX */
-static struct pwm_lookup crc_pwm_lookup[] = {
-	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
-};
-
-static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
-				    const struct i2c_device_id *i2c_id)
-{
-	struct device *dev = &i2c->dev;
-	struct intel_soc_pmic_config *config;
-	struct intel_soc_pmic *pmic;
-	int ret;
-
-	if (soc_intel_is_byt())
-		config = &intel_soc_pmic_config_byt_crc;
-	else
-		config = &intel_soc_pmic_config_cht_crc;
-
-	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
-	if (!pmic)
-		return -ENOMEM;
-
-	dev_set_drvdata(dev, pmic);
-
-	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
-	if (IS_ERR(pmic->regmap))
-		return PTR_ERR(pmic->regmap);
-
-	pmic->irq = i2c->irq;
-
-	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
-				  config->irq_flags | IRQF_ONESHOT,
-				  0, config->irq_chip,
-				  &pmic->irq_chip_data);
-	if (ret)
-		return ret;
-
-	ret = enable_irq_wake(pmic->irq);
-	if (ret)
-		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
-
-	/* Add lookup table for crc-pwm */
-	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
-
-	/* To distuingish this domain from the GPIO/charger's irqchip domains */
-	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
-				    DOMAIN_BUS_NEXUS);
-
-	ret = mfd_add_devices(dev, -1, config->cell_dev,
-			      config->n_cell_devs, NULL, 0,
-			      regmap_irq_get_domain(pmic->irq_chip_data));
-	if (ret)
-		goto err_del_irq_chip;
-
-	return 0;
-
-err_del_irq_chip:
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-	return ret;
-}
-
-static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
-{
-	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
-
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-
-	/* remove crc-pwm lookup table */
-	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
-
-	mfd_remove_devices(&i2c->dev);
-
-	return 0;
-}
-
-static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
-{
-	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
-
-	disable_irq(pmic->irq);
-
-	return;
-}
-
-#if defined(CONFIG_PM_SLEEP)
-static int intel_soc_pmic_suspend(struct device *dev)
-{
-	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
-
-	disable_irq(pmic->irq);
-
-	return 0;
-}
-
-static int intel_soc_pmic_resume(struct device *dev)
-{
-	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
-
-	enable_irq(pmic->irq);
-
-	return 0;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
-			 intel_soc_pmic_resume);
-
-static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
-
-#if defined(CONFIG_ACPI)
-static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
-	{ "INT33FD" },
-	{ },
-};
-MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
-#endif
-
-static struct i2c_driver intel_soc_pmic_i2c_driver = {
-	.driver = {
-		.name = "intel_soc_pmic_i2c",
-		.pm = &intel_soc_pmic_pm_ops,
-		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
-	},
-	.probe = intel_soc_pmic_i2c_probe,
-	.remove = intel_soc_pmic_i2c_remove,
-	.id_table = intel_soc_pmic_i2c_id,
-	.shutdown = intel_soc_pmic_shutdown,
-};
-
-module_i2c_driver(intel_soc_pmic_i2c_driver);
-
-MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Yang, Bin <bin.yang@intel.com>");
-MODULE_AUTHOR("Zhu, Lejun <lejun.zhu@linux.intel.com>");
diff --git a/drivers/mfd/intel_soc_pmic_core.h b/drivers/mfd/intel_soc_pmic_core.h
deleted file mode 100644
index d490685845eb..000000000000
--- a/drivers/mfd/intel_soc_pmic_core.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Intel SoC PMIC MFD Driver
- *
- * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
- *
- * Author: Yang, Bin <bin.yang@intel.com>
- * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
- */
-
-#ifndef __INTEL_SOC_PMIC_CORE_H__
-#define __INTEL_SOC_PMIC_CORE_H__
-
-struct intel_soc_pmic_config {
-	unsigned long irq_flags;
-	struct mfd_cell *cell_dev;
-	int n_cell_devs;
-	const struct regmap_config *regmap_config;
-	const struct regmap_irq_chip *irq_chip;
-};
-
-extern struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc;
-extern struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc;
-
-#endif	/* __INTEL_SOC_PMIC_CORE_H__ */
diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index 5bb0367bd974..b5974dfcc603 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -2,18 +2,21 @@
 /*
  * Device access for Crystal Cove PMIC
  *
- * Copyright (C) 2013, 2014 Intel Corporation. All rights reserved.
+ * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
  *
  * Author: Yang, Bin <bin.yang@intel.com>
  * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
  */
 
+#include <linux/acpi.h>
+#include <linux/i2c.h>
 #include <linux/interrupt.h>
-#include <linux/regmap.h>
+#include <linux/module.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/intel_soc_pmic.h>
-
-#include "intel_soc_pmic_core.h"
+#include <linux/platform_data/x86/soc.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
 
 #define CRYSTAL_COVE_MAX_REGISTER	0xC6
 
@@ -132,7 +135,20 @@ static const struct regmap_irq_chip crystal_cove_irq_chip = {
 	.mask_base = CRYSTAL_COVE_REG_MIRQLVL1,
 };
 
-struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
+/* PWM consumed by the Intel GFX */
+static struct pwm_lookup crc_pwm_lookup[] = {
+	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
+};
+
+struct intel_soc_pmic_config {
+	unsigned long irq_flags;
+	struct mfd_cell *cell_dev;
+	int n_cell_devs;
+	const struct regmap_config *regmap_config;
+	const struct regmap_irq_chip *irq_chip;
+};
+
+static const struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
 	.irq_flags = IRQF_TRIGGER_RISING,
 	.cell_dev = crystal_cove_byt_dev,
 	.n_cell_devs = ARRAY_SIZE(crystal_cove_byt_dev),
@@ -140,10 +156,144 @@ struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
 	.irq_chip = &crystal_cove_irq_chip,
 };
 
-struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
+static const struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
 	.irq_flags = IRQF_TRIGGER_RISING,
 	.cell_dev = crystal_cove_cht_dev,
 	.n_cell_devs = ARRAY_SIZE(crystal_cove_cht_dev),
 	.regmap_config = &crystal_cove_regmap_config,
 	.irq_chip = &crystal_cove_irq_chip,
 };
+
+static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
+				    const struct i2c_device_id *i2c_id)
+{
+	const struct intel_soc_pmic_config *config;
+	struct device *dev = &i2c->dev;
+	struct intel_soc_pmic *pmic;
+	int ret;
+
+	if (soc_intel_is_byt())
+		config = &intel_soc_pmic_config_byt_crc;
+	else
+		config = &intel_soc_pmic_config_cht_crc;
+
+	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
+	if (!pmic)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, pmic);
+
+	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
+	if (IS_ERR(pmic->regmap))
+		return PTR_ERR(pmic->regmap);
+
+	pmic->irq = i2c->irq;
+
+	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
+				  config->irq_flags | IRQF_ONESHOT,
+				  0, config->irq_chip,
+				  &pmic->irq_chip_data);
+	if (ret)
+		return ret;
+
+	ret = enable_irq_wake(pmic->irq);
+	if (ret)
+		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
+
+	/* Add lookup table for crc-pwm */
+	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
+
+	/* To distuingish this domain from the GPIO/charger's irqchip domains */
+	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
+				    DOMAIN_BUS_NEXUS);
+
+	ret = mfd_add_devices(dev, -1, config->cell_dev,
+			      config->n_cell_devs, NULL, 0,
+			      regmap_irq_get_domain(pmic->irq_chip_data));
+	if (ret)
+		goto err_del_irq_chip;
+
+	return 0;
+
+err_del_irq_chip:
+	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
+	return ret;
+}
+
+static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
+{
+	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
+
+	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
+
+	/* remove crc-pwm lookup table */
+	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
+
+	mfd_remove_devices(&i2c->dev);
+
+	return 0;
+}
+
+static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
+{
+	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
+
+	disable_irq(pmic->irq);
+
+	return;
+}
+
+#if defined(CONFIG_PM_SLEEP)
+static int intel_soc_pmic_suspend(struct device *dev)
+{
+	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
+
+	disable_irq(pmic->irq);
+
+	return 0;
+}
+
+static int intel_soc_pmic_resume(struct device *dev)
+{
+	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
+
+	enable_irq(pmic->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
+			 intel_soc_pmic_resume);
+
+static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
+
+#if defined(CONFIG_ACPI)
+static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
+	{ "INT33FD" },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
+#endif
+
+static struct i2c_driver intel_soc_pmic_i2c_driver = {
+	.driver = {
+		.name = "intel_soc_pmic_i2c",
+		.pm = &intel_soc_pmic_pm_ops,
+		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
+	},
+	.probe = intel_soc_pmic_i2c_probe,
+	.remove = intel_soc_pmic_i2c_remove,
+	.id_table = intel_soc_pmic_i2c_id,
+	.shutdown = intel_soc_pmic_shutdown,
+};
+
+module_i2c_driver(intel_soc_pmic_i2c_driver);
+
+MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Yang, Bin <bin.yang@intel.com>");
+MODULE_AUTHOR("Zhu, Lejun <lejun.zhu@linux.intel.com>");
-- 
2.35.1

[PATCH v1 2/9] mfd: intel_soc_pmic: Move non-Intel Makefile entries to their own group

[Andy Shevchenko]

It looks like a random position for couple of Makefile entries that are
disrupting Intel PMIC group. Move them to their own group.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 61db669f864c..30f5de657e88 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -172,6 +172,10 @@ obj-$(CONFIG_MFD_MAX8998)	+= max8998.o max8998-irq.o
 
 obj-$(CONFIG_MFD_MP2629)	+= mp2629.o
 
+obj-$(CONFIG_MFD_MT6360)	+= mt6360-core.o
+mt6397-objs			:= mt6397-core.o mt6397-irq.o mt6358-irq.o
+obj-$(CONFIG_MFD_MT6397)	+= mt6397.o
+
 pcf50633-objs			:= pcf50633-core.o pcf50633-irq.o
 obj-$(CONFIG_MFD_PCF50633)	+= pcf50633.o
 obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
@@ -240,9 +244,6 @@ obj-$(CONFIG_INTEL_SOC_PMIC)		+= intel_soc_pmic_crc.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_CHTDC_TI)	+= intel_soc_pmic_chtdc_ti.o
-obj-$(CONFIG_MFD_MT6360)	+= mt6360-core.o
-mt6397-objs			:= mt6397-core.o mt6397-irq.o mt6358-irq.o
-obj-$(CONFIG_MFD_MT6397)	+= mt6397.o
 obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD)	+= intel_soc_pmic_mrfld.o
 
 obj-$(CONFIG_MFD_ALTERA_A10SR)	+= altera-a10sr.o
-- 
2.35.1

[PATCH v1 3/9] mfd: intel_soc_pmic_crc: Use devm_regmap_add_irq_chip()

[Andy Shevchenko]

Use devm_regmap_add_irq_chip() to simplify the code.

While at it, replace -1 magic parameter by PLATFORM_DEVID_NONE when
calling mfd_add_devices().

Note, the mfd_add_devices() left in non-devm variant here due to
potentially increased churn while wrapping pwm_remove_table().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index b5974dfcc603..187a4ede1148 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -189,10 +189,9 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
 
 	pmic->irq = i2c->irq;
 
-	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
-				  config->irq_flags | IRQF_ONESHOT,
-				  0, config->irq_chip,
-				  &pmic->irq_chip_data);
+	ret = devm_regmap_add_irq_chip(dev, pmic->regmap, pmic->irq,
+				       config->irq_flags | IRQF_ONESHOT,
+				       0, config->irq_chip, &pmic->irq_chip_data);
 	if (ret)
 		return ret;
 
@@ -207,25 +206,12 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
 	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
 				    DOMAIN_BUS_NEXUS);
 
-	ret = mfd_add_devices(dev, -1, config->cell_dev,
-			      config->n_cell_devs, NULL, 0,
-			      regmap_irq_get_domain(pmic->irq_chip_data));
-	if (ret)
-		goto err_del_irq_chip;
-
-	return 0;
-
-err_del_irq_chip:
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-	return ret;
+	return mfd_add_devices(dev, PLATFORM_DEVID_NONE, config->cell_dev, config->n_cell_devs,
+			       NULL, 0, regmap_irq_get_domain(pmic->irq_chip_data));
 }
 
 static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
 {
-	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
-
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-
 	/* remove crc-pwm lookup table */
 	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
 
-- 
2.35.1

[PATCH v1 4/9] mfd: intel_soc_pmic_crc: Convert to use i2c_get/set_clientdata()

[Andy Shevchenko]

We have the specific helpers for I2C device to set and get its driver data.
Convert driver to use them instead of open coded variants.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index 187a4ede1148..165f686132f6 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -181,7 +181,7 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
 	if (!pmic)
 		return -ENOMEM;
 
-	dev_set_drvdata(dev, pmic);
+	i2c_set_clientdata(i2c, pmic);
 
 	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
 	if (IS_ERR(pmic->regmap))
@@ -222,7 +222,7 @@ static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
 
 static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
 {
-	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
+	struct intel_soc_pmic *pmic = i2c_get_clientdata(i2c);
 
 	disable_irq(pmic->irq);
 
-- 
2.35.1

[PATCH v1 5/9] mfd: intel_soc_pmic_crc: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr() etc

[Andy Shevchenko]

Letting the compiler remove these functions when the kernel is built
without CONFIG_PM_SLEEP support is simpler and less error prone than the
use of #ifdef based kernel configuration guards.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index 165f686132f6..c034e226a0bb 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -229,7 +229,6 @@ static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
 	return;
 }
 
-#if defined(CONFIG_PM_SLEEP)
 static int intel_soc_pmic_suspend(struct device *dev)
 {
 	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
@@ -247,10 +246,8 @@ static int intel_soc_pmic_resume(struct device *dev)
 
 	return 0;
 }
-#endif
 
-static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
-			 intel_soc_pmic_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(crystal_cove_pm_ops, intel_soc_pmic_suspend, intel_soc_pmic_resume);
 
 static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
 	{ }
@@ -268,7 +265,7 @@ MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
 static struct i2c_driver intel_soc_pmic_i2c_driver = {
 	.driver = {
 		.name = "intel_soc_pmic_i2c",
-		.pm = &intel_soc_pmic_pm_ops,
+		.pm = pm_sleep_ptr(&crystal_cove_pm_ops),
 		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
 	},
 	.probe = intel_soc_pmic_i2c_probe,
-- 
2.35.1

[PATCH v1 6/9] mfd: intel_soc_pmic_crc: Drop redundant ACPI_PTR() and ifdeffery

[Andy Shevchenko]

The driver depends on ACPI, ACPI_PTR() resolution is always the same.
Otherwise a compiler may produce a warning.

That said, the rule of thumb either ugly ifdeffery with ACPI_PTR or
none should be used in a driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/Kconfig              | 4 ++--
 drivers/mfd/intel_soc_pmic_crc.c | 6 ++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3b59456f5545..2bbfeeac6f8e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -588,8 +588,8 @@ config LPC_SCH
 
 config INTEL_SOC_PMIC
 	bool "Support for Crystal Cove PMIC"
-	depends on ACPI && HAS_IOMEM && I2C=y && GPIOLIB && COMMON_CLK
-	depends on X86 || COMPILE_TEST
+	depends on HAS_IOMEM && I2C=y && GPIOLIB && COMMON_CLK
+	depends on (X86 && ACPI) || COMPILE_TEST
 	depends on I2C_DESIGNWARE_PLATFORM=y
 	select MFD_CORE
 	select REGMAP_I2C
diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index c034e226a0bb..f1e5fdd846b3 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -8,9 +8,9 @@
  * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
  */
 
-#include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/intel_soc_pmic.h>
@@ -254,19 +254,17 @@ static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
 
-#if defined(CONFIG_ACPI)
 static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
 	{ "INT33FD" },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
-#endif
 
 static struct i2c_driver intel_soc_pmic_i2c_driver = {
 	.driver = {
 		.name = "intel_soc_pmic_i2c",
 		.pm = pm_sleep_ptr(&crystal_cove_pm_ops),
-		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
+		.acpi_match_table = intel_soc_pmic_acpi_match,
 	},
 	.probe = intel_soc_pmic_i2c_probe,
 	.remove = intel_soc_pmic_i2c_remove,
-- 
2.35.1

[PATCH v1 7/9] mfd: intel_soc_pmic_crc: Convert driver to use ->probe_new()

[Andy Shevchenko]

Use the ->probe_new() callback.

The driver does not use const struct i2c_device_id * argument,
so convert it to utilise the simplified I²C driver registration.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index f1e5fdd846b3..edadc07d473e 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -164,8 +164,7 @@ static const struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
 	.irq_chip = &crystal_cove_irq_chip,
 };
 
-static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
-				    const struct i2c_device_id *i2c_id)
+static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c)
 {
 	const struct intel_soc_pmic_config *config;
 	struct device *dev = &i2c->dev;
@@ -249,11 +248,6 @@ static int intel_soc_pmic_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(crystal_cove_pm_ops, intel_soc_pmic_suspend, intel_soc_pmic_resume);
 
-static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
-
 static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
 	{ "INT33FD" },
 	{ },
@@ -266,9 +260,8 @@ static struct i2c_driver intel_soc_pmic_i2c_driver = {
 		.pm = pm_sleep_ptr(&crystal_cove_pm_ops),
 		.acpi_match_table = intel_soc_pmic_acpi_match,
 	},
-	.probe = intel_soc_pmic_i2c_probe,
+	.probe_new = intel_soc_pmic_i2c_probe,
 	.remove = intel_soc_pmic_i2c_remove,
-	.id_table = intel_soc_pmic_i2c_id,
 	.shutdown = intel_soc_pmic_shutdown,
 };
 
-- 
2.35.1

[PATCH v1 8/9] mfd: intel_soc_pmic_crc: Replace intel_soc_pmic with crystal_cove

[Andy Shevchenko]

To reflect the point that this driver is only for one type of the PMICs,
replace intel_soc_pmic with crystal_cove (avoid using crc for possible
namespace collisions with CRC library APIs).

Note, also rename the driver name since we don't expect any user
that enumerates by it, only ACPI known so far.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 42 ++++++++++++++++----------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index edadc07d473e..bb3cc1a808f0 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -140,7 +140,7 @@ static struct pwm_lookup crc_pwm_lookup[] = {
 	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
 };
 
-struct intel_soc_pmic_config {
+struct crystal_cove_config {
 	unsigned long irq_flags;
 	struct mfd_cell *cell_dev;
 	int n_cell_devs;
@@ -148,7 +148,7 @@ struct intel_soc_pmic_config {
 	const struct regmap_irq_chip *irq_chip;
 };
 
-static const struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
+static const struct crystal_cove_config crystal_cove_config_byt_crc = {
 	.irq_flags = IRQF_TRIGGER_RISING,
 	.cell_dev = crystal_cove_byt_dev,
 	.n_cell_devs = ARRAY_SIZE(crystal_cove_byt_dev),
@@ -156,7 +156,7 @@ static const struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
 	.irq_chip = &crystal_cove_irq_chip,
 };
 
-static const struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
+static const struct crystal_cove_config crystal_cove_config_cht_crc = {
 	.irq_flags = IRQF_TRIGGER_RISING,
 	.cell_dev = crystal_cove_cht_dev,
 	.n_cell_devs = ARRAY_SIZE(crystal_cove_cht_dev),
@@ -164,17 +164,17 @@ static const struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
 	.irq_chip = &crystal_cove_irq_chip,
 };
 
-static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c)
+static int crystal_cove_i2c_probe(struct i2c_client *i2c)
 {
-	const struct intel_soc_pmic_config *config;
+	const struct crystal_cove_config *config;
 	struct device *dev = &i2c->dev;
 	struct intel_soc_pmic *pmic;
 	int ret;
 
 	if (soc_intel_is_byt())
-		config = &intel_soc_pmic_config_byt_crc;
+		config = &crystal_cove_config_byt_crc;
 	else
-		config = &intel_soc_pmic_config_cht_crc;
+		config = &crystal_cove_config_cht_crc;
 
 	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
 	if (!pmic)
@@ -209,7 +209,7 @@ static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c)
 			       NULL, 0, regmap_irq_get_domain(pmic->irq_chip_data));
 }
 
-static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
+static int crystal_cove_i2c_remove(struct i2c_client *i2c)
 {
 	/* remove crc-pwm lookup table */
 	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
@@ -219,7 +219,7 @@ static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }
 
-static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
+static void crystal_cove_shutdown(struct i2c_client *i2c)
 {
 	struct intel_soc_pmic *pmic = i2c_get_clientdata(i2c);
 
@@ -228,7 +228,7 @@ static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
 	return;
 }
 
-static int intel_soc_pmic_suspend(struct device *dev)
+static int crystal_cove_suspend(struct device *dev)
 {
 	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
 
@@ -237,7 +237,7 @@ static int intel_soc_pmic_suspend(struct device *dev)
 	return 0;
 }
 
-static int intel_soc_pmic_resume(struct device *dev)
+static int crystal_cove_resume(struct device *dev)
 {
 	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
 
@@ -246,26 +246,26 @@ static int intel_soc_pmic_resume(struct device *dev)
 	return 0;
 }
 
-static DEFINE_SIMPLE_DEV_PM_OPS(crystal_cove_pm_ops, intel_soc_pmic_suspend, intel_soc_pmic_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(crystal_cove_pm_ops, crystal_cove_suspend, crystal_cove_resume);
 
-static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
+static const struct acpi_device_id crystal_cove_acpi_match[] = {
 	{ "INT33FD" },
 	{ },
 };
-MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
+MODULE_DEVICE_TABLE(acpi, crystal_cove_acpi_match);
 
-static struct i2c_driver intel_soc_pmic_i2c_driver = {
+static struct i2c_driver crystal_cove_i2c_driver = {
 	.driver = {
-		.name = "intel_soc_pmic_i2c",
+		.name = "crystal_cove_i2c",
 		.pm = pm_sleep_ptr(&crystal_cove_pm_ops),
-		.acpi_match_table = intel_soc_pmic_acpi_match,
+		.acpi_match_table = crystal_cove_acpi_match,
 	},
-	.probe_new = intel_soc_pmic_i2c_probe,
-	.remove = intel_soc_pmic_i2c_remove,
-	.shutdown = intel_soc_pmic_shutdown,
+	.probe_new = crystal_cove_i2c_probe,
+	.remove = crystal_cove_i2c_remove,
+	.shutdown = crystal_cove_shutdown,
 };
 
-module_i2c_driver(intel_soc_pmic_i2c_driver);
+module_i2c_driver(crystal_cove_i2c_driver);
 
 MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
 MODULE_LICENSE("GPL v2");
-- 
2.35.1

[PATCH v1 9/9] mfd: intel_soc_pmic_crc: Update the copyright year

[Andy Shevchenko]

Update the copyright year to be 2012-2014, 2022.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index bb3cc1a808f0..0e5aae7ae9ad 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -2,7 +2,7 @@
 /*
  * Device access for Crystal Cove PMIC
  *
- * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
+ * Copyright (C) 2012-2014, 2022 Intel Corporation. All rights reserved.
  *
  * Author: Yang, Bin <bin.yang@intel.com>
  * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
-- 
2.35.1

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Hans de Goede]

Hi,

Thank you for this series, cleaning this up is long overdue.

The entire series looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

for the series.

Regards,

Hans

p.s.

Maybe do a follow up renaming CONFIG_INTEL_SOC_PMIC to
CONFIG_INTEL_SOC_PMIC_CRC and also change the module name
from intel-soc-pmic to intel-soc-pmic-crc ?

I don't expect any userspace dependencies on the module-name
(there are no module parameters) so this should be fine.




On 6/16/22 20:25, Andy Shevchenko wrote:
> The core part is misleading since its only purpose to serve Crystal Cove PMIC,
> although for couple of different platforms. Merge core part into crc one.
> 
> Advantages among others are:
> - speed up a compilation and build
> - decreasing the code base
> - reducing noise in the namespace by making some data static and const
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Makefile              |   3 +-
>  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
>  drivers/mfd/intel_soc_pmic_core.h |  25 -----
>  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
>  4 files changed, 157 insertions(+), 193 deletions(-)
>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> 
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 858cacf659d6..61db669f864c 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -236,8 +236,7 @@ obj-$(CONFIG_MFD_RT4831)	+= rt4831.o
>  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
> -obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
> +obj-$(CONFIG_INTEL_SOC_PMIC)		+= intel_soc_pmic_crc.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_CHTDC_TI)	+= intel_soc_pmic_chtdc_ti.o
> diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c
> deleted file mode 100644
> index 5e8c94e008ed..000000000000
> --- a/drivers/mfd/intel_soc_pmic_core.c
> +++ /dev/null
> @@ -1,160 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Intel SoC PMIC MFD Driver
> - *
> - * Copyright (C) 2013, 2014 Intel Corporation. All rights reserved.
> - *
> - * Author: Yang, Bin <bin.yang@intel.com>
> - * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
> - */
> -
> -#include <linux/acpi.h>
> -#include <linux/i2c.h>
> -#include <linux/interrupt.h>
> -#include <linux/module.h>
> -#include <linux/mfd/core.h>
> -#include <linux/mfd/intel_soc_pmic.h>
> -#include <linux/platform_data/x86/soc.h>
> -#include <linux/pwm.h>
> -#include <linux/regmap.h>
> -
> -#include "intel_soc_pmic_core.h"
> -
> -/* PWM consumed by the Intel GFX */
> -static struct pwm_lookup crc_pwm_lookup[] = {
> -	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
> -};
> -
> -static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
> -				    const struct i2c_device_id *i2c_id)
> -{
> -	struct device *dev = &i2c->dev;
> -	struct intel_soc_pmic_config *config;
> -	struct intel_soc_pmic *pmic;
> -	int ret;
> -
> -	if (soc_intel_is_byt())
> -		config = &intel_soc_pmic_config_byt_crc;
> -	else
> -		config = &intel_soc_pmic_config_cht_crc;
> -
> -	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
> -	if (!pmic)
> -		return -ENOMEM;
> -
> -	dev_set_drvdata(dev, pmic);
> -
> -	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
> -	if (IS_ERR(pmic->regmap))
> -		return PTR_ERR(pmic->regmap);
> -
> -	pmic->irq = i2c->irq;
> -
> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
> -				  config->irq_flags | IRQF_ONESHOT,
> -				  0, config->irq_chip,
> -				  &pmic->irq_chip_data);
> -	if (ret)
> -		return ret;
> -
> -	ret = enable_irq_wake(pmic->irq);
> -	if (ret)
> -		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
> -
> -	/* Add lookup table for crc-pwm */
> -	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> -
> -	/* To distuingish this domain from the GPIO/charger's irqchip domains */
> -	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
> -				    DOMAIN_BUS_NEXUS);
> -
> -	ret = mfd_add_devices(dev, -1, config->cell_dev,
> -			      config->n_cell_devs, NULL, 0,
> -			      regmap_irq_get_domain(pmic->irq_chip_data));
> -	if (ret)
> -		goto err_del_irq_chip;
> -
> -	return 0;
> -
> -err_del_irq_chip:
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> -	return ret;
> -}
> -
> -static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> -
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> -
> -	/* remove crc-pwm lookup table */
> -	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> -
> -	mfd_remove_devices(&i2c->dev);
> -
> -	return 0;
> -}
> -
> -static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> -
> -	disable_irq(pmic->irq);
> -
> -	return;
> -}
> -
> -#if defined(CONFIG_PM_SLEEP)
> -static int intel_soc_pmic_suspend(struct device *dev)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> -
> -	disable_irq(pmic->irq);
> -
> -	return 0;
> -}
> -
> -static int intel_soc_pmic_resume(struct device *dev)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> -
> -	enable_irq(pmic->irq);
> -
> -	return 0;
> -}
> -#endif
> -
> -static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
> -			 intel_soc_pmic_resume);
> -
> -static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
> -
> -#if defined(CONFIG_ACPI)
> -static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
> -	{ "INT33FD" },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
> -#endif
> -
> -static struct i2c_driver intel_soc_pmic_i2c_driver = {
> -	.driver = {
> -		.name = "intel_soc_pmic_i2c",
> -		.pm = &intel_soc_pmic_pm_ops,
> -		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
> -	},
> -	.probe = intel_soc_pmic_i2c_probe,
> -	.remove = intel_soc_pmic_i2c_remove,
> -	.id_table = intel_soc_pmic_i2c_id,
> -	.shutdown = intel_soc_pmic_shutdown,
> -};
> -
> -module_i2c_driver(intel_soc_pmic_i2c_driver);
> -
> -MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
> -MODULE_LICENSE("GPL v2");
> -MODULE_AUTHOR("Yang, Bin <bin.yang@intel.com>");
> -MODULE_AUTHOR("Zhu, Lejun <lejun.zhu@linux.intel.com>");
> diff --git a/drivers/mfd/intel_soc_pmic_core.h b/drivers/mfd/intel_soc_pmic_core.h
> deleted file mode 100644
> index d490685845eb..000000000000
> --- a/drivers/mfd/intel_soc_pmic_core.h
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * Intel SoC PMIC MFD Driver
> - *
> - * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
> - *
> - * Author: Yang, Bin <bin.yang@intel.com>
> - * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
> - */
> -
> -#ifndef __INTEL_SOC_PMIC_CORE_H__
> -#define __INTEL_SOC_PMIC_CORE_H__
> -
> -struct intel_soc_pmic_config {
> -	unsigned long irq_flags;
> -	struct mfd_cell *cell_dev;
> -	int n_cell_devs;
> -	const struct regmap_config *regmap_config;
> -	const struct regmap_irq_chip *irq_chip;
> -};
> -
> -extern struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc;
> -extern struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc;
> -
> -#endif	/* __INTEL_SOC_PMIC_CORE_H__ */
> diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
> index 5bb0367bd974..b5974dfcc603 100644
> --- a/drivers/mfd/intel_soc_pmic_crc.c
> +++ b/drivers/mfd/intel_soc_pmic_crc.c
> @@ -2,18 +2,21 @@
>  /*
>   * Device access for Crystal Cove PMIC
>   *
> - * Copyright (C) 2013, 2014 Intel Corporation. All rights reserved.
> + * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
>   *
>   * Author: Yang, Bin <bin.yang@intel.com>
>   * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
>   */
>  
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
>  #include <linux/interrupt.h>
> -#include <linux/regmap.h>
> +#include <linux/module.h>
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/intel_soc_pmic.h>
> -
> -#include "intel_soc_pmic_core.h"
> +#include <linux/platform_data/x86/soc.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
>  
>  #define CRYSTAL_COVE_MAX_REGISTER	0xC6
>  
> @@ -132,7 +135,20 @@ static const struct regmap_irq_chip crystal_cove_irq_chip = {
>  	.mask_base = CRYSTAL_COVE_REG_MIRQLVL1,
>  };
>  
> -struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
> +/* PWM consumed by the Intel GFX */
> +static struct pwm_lookup crc_pwm_lookup[] = {
> +	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
> +};
> +
> +struct intel_soc_pmic_config {
> +	unsigned long irq_flags;
> +	struct mfd_cell *cell_dev;
> +	int n_cell_devs;
> +	const struct regmap_config *regmap_config;
> +	const struct regmap_irq_chip *irq_chip;
> +};
> +
> +static const struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
>  	.irq_flags = IRQF_TRIGGER_RISING,
>  	.cell_dev = crystal_cove_byt_dev,
>  	.n_cell_devs = ARRAY_SIZE(crystal_cove_byt_dev),
> @@ -140,10 +156,144 @@ struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
>  	.irq_chip = &crystal_cove_irq_chip,
>  };
>  
> -struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
> +static const struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
>  	.irq_flags = IRQF_TRIGGER_RISING,
>  	.cell_dev = crystal_cove_cht_dev,
>  	.n_cell_devs = ARRAY_SIZE(crystal_cove_cht_dev),
>  	.regmap_config = &crystal_cove_regmap_config,
>  	.irq_chip = &crystal_cove_irq_chip,
>  };
> +
> +static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
> +				    const struct i2c_device_id *i2c_id)
> +{
> +	const struct intel_soc_pmic_config *config;
> +	struct device *dev = &i2c->dev;
> +	struct intel_soc_pmic *pmic;
> +	int ret;
> +
> +	if (soc_intel_is_byt())
> +		config = &intel_soc_pmic_config_byt_crc;
> +	else
> +		config = &intel_soc_pmic_config_cht_crc;
> +
> +	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
> +	if (!pmic)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, pmic);
> +
> +	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
> +	if (IS_ERR(pmic->regmap))
> +		return PTR_ERR(pmic->regmap);
> +
> +	pmic->irq = i2c->irq;
> +
> +	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
> +				  config->irq_flags | IRQF_ONESHOT,
> +				  0, config->irq_chip,
> +				  &pmic->irq_chip_data);
> +	if (ret)
> +		return ret;
> +
> +	ret = enable_irq_wake(pmic->irq);
> +	if (ret)
> +		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
> +
> +	/* Add lookup table for crc-pwm */
> +	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> +
> +	/* To distuingish this domain from the GPIO/charger's irqchip domains */
> +	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
> +				    DOMAIN_BUS_NEXUS);
> +
> +	ret = mfd_add_devices(dev, -1, config->cell_dev,
> +			      config->n_cell_devs, NULL, 0,
> +			      regmap_irq_get_domain(pmic->irq_chip_data));
> +	if (ret)
> +		goto err_del_irq_chip;
> +
> +	return 0;
> +
> +err_del_irq_chip:
> +	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> +	return ret;
> +}
> +
> +static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> +
> +	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> +
> +	/* remove crc-pwm lookup table */
> +	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> +
> +	mfd_remove_devices(&i2c->dev);
> +
> +	return 0;
> +}
> +
> +static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> +
> +	disable_irq(pmic->irq);
> +
> +	return;
> +}
> +
> +#if defined(CONFIG_PM_SLEEP)
> +static int intel_soc_pmic_suspend(struct device *dev)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> +	disable_irq(pmic->irq);
> +
> +	return 0;
> +}
> +
> +static int intel_soc_pmic_resume(struct device *dev)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> +	enable_irq(pmic->irq);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
> +			 intel_soc_pmic_resume);
> +
> +static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
> +
> +#if defined(CONFIG_ACPI)
> +static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
> +	{ "INT33FD" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
> +#endif
> +
> +static struct i2c_driver intel_soc_pmic_i2c_driver = {
> +	.driver = {
> +		.name = "intel_soc_pmic_i2c",
> +		.pm = &intel_soc_pmic_pm_ops,
> +		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
> +	},
> +	.probe = intel_soc_pmic_i2c_probe,
> +	.remove = intel_soc_pmic_i2c_remove,
> +	.id_table = intel_soc_pmic_i2c_id,
> +	.shutdown = intel_soc_pmic_shutdown,
> +};
> +
> +module_i2c_driver(intel_soc_pmic_i2c_driver);
> +
> +MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Yang, Bin <bin.yang@intel.com>");
> +MODULE_AUTHOR("Zhu, Lejun <lejun.zhu@linux.intel.com>");

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Hans de Goede]

p.s.

I've added this commit to my local tree, so that the next time I boot
a device with a CRC PMIC it will get some testing.

On 6/16/22 20:25, Andy Shevchenko wrote:
> The core part is misleading since its only purpose to serve Crystal Cove PMIC,
> although for couple of different platforms. Merge core part into crc one.
> 
> Advantages among others are:
> - speed up a compilation and build
> - decreasing the code base
> - reducing noise in the namespace by making some data static and const
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Makefile              |   3 +-
>  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
>  drivers/mfd/intel_soc_pmic_core.h |  25 -----
>  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
>  4 files changed, 157 insertions(+), 193 deletions(-)
>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> 
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 858cacf659d6..61db669f864c 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -236,8 +236,7 @@ obj-$(CONFIG_MFD_RT4831)	+= rt4831.o
>  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
> -obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
> +obj-$(CONFIG_INTEL_SOC_PMIC)		+= intel_soc_pmic_crc.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_CHTDC_TI)	+= intel_soc_pmic_chtdc_ti.o
> diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c
> deleted file mode 100644
> index 5e8c94e008ed..000000000000
> --- a/drivers/mfd/intel_soc_pmic_core.c
> +++ /dev/null
> @@ -1,160 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Intel SoC PMIC MFD Driver
> - *
> - * Copyright (C) 2013, 2014 Intel Corporation. All rights reserved.
> - *
> - * Author: Yang, Bin <bin.yang@intel.com>
> - * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
> - */
> -
> -#include <linux/acpi.h>
> -#include <linux/i2c.h>
> -#include <linux/interrupt.h>
> -#include <linux/module.h>
> -#include <linux/mfd/core.h>
> -#include <linux/mfd/intel_soc_pmic.h>
> -#include <linux/platform_data/x86/soc.h>
> -#include <linux/pwm.h>
> -#include <linux/regmap.h>
> -
> -#include "intel_soc_pmic_core.h"
> -
> -/* PWM consumed by the Intel GFX */
> -static struct pwm_lookup crc_pwm_lookup[] = {
> -	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
> -};
> -
> -static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
> -				    const struct i2c_device_id *i2c_id)
> -{
> -	struct device *dev = &i2c->dev;
> -	struct intel_soc_pmic_config *config;
> -	struct intel_soc_pmic *pmic;
> -	int ret;
> -
> -	if (soc_intel_is_byt())
> -		config = &intel_soc_pmic_config_byt_crc;
> -	else
> -		config = &intel_soc_pmic_config_cht_crc;
> -
> -	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
> -	if (!pmic)
> -		return -ENOMEM;
> -
> -	dev_set_drvdata(dev, pmic);
> -
> -	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
> -	if (IS_ERR(pmic->regmap))
> -		return PTR_ERR(pmic->regmap);
> -
> -	pmic->irq = i2c->irq;
> -
> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
> -				  config->irq_flags | IRQF_ONESHOT,
> -				  0, config->irq_chip,
> -				  &pmic->irq_chip_data);
> -	if (ret)
> -		return ret;
> -
> -	ret = enable_irq_wake(pmic->irq);
> -	if (ret)
> -		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
> -
> -	/* Add lookup table for crc-pwm */
> -	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> -
> -	/* To distuingish this domain from the GPIO/charger's irqchip domains */
> -	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
> -				    DOMAIN_BUS_NEXUS);
> -
> -	ret = mfd_add_devices(dev, -1, config->cell_dev,
> -			      config->n_cell_devs, NULL, 0,
> -			      regmap_irq_get_domain(pmic->irq_chip_data));
> -	if (ret)
> -		goto err_del_irq_chip;
> -
> -	return 0;
> -
> -err_del_irq_chip:
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> -	return ret;
> -}
> -
> -static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> -
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> -
> -	/* remove crc-pwm lookup table */
> -	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> -
> -	mfd_remove_devices(&i2c->dev);
> -
> -	return 0;
> -}
> -
> -static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> -
> -	disable_irq(pmic->irq);
> -
> -	return;
> -}
> -
> -#if defined(CONFIG_PM_SLEEP)
> -static int intel_soc_pmic_suspend(struct device *dev)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> -
> -	disable_irq(pmic->irq);
> -
> -	return 0;
> -}
> -
> -static int intel_soc_pmic_resume(struct device *dev)
> -{
> -	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> -
> -	enable_irq(pmic->irq);
> -
> -	return 0;
> -}
> -#endif
> -
> -static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
> -			 intel_soc_pmic_resume);
> -
> -static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
> -
> -#if defined(CONFIG_ACPI)
> -static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
> -	{ "INT33FD" },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
> -#endif
> -
> -static struct i2c_driver intel_soc_pmic_i2c_driver = {
> -	.driver = {
> -		.name = "intel_soc_pmic_i2c",
> -		.pm = &intel_soc_pmic_pm_ops,
> -		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
> -	},
> -	.probe = intel_soc_pmic_i2c_probe,
> -	.remove = intel_soc_pmic_i2c_remove,
> -	.id_table = intel_soc_pmic_i2c_id,
> -	.shutdown = intel_soc_pmic_shutdown,
> -};
> -
> -module_i2c_driver(intel_soc_pmic_i2c_driver);
> -
> -MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
> -MODULE_LICENSE("GPL v2");
> -MODULE_AUTHOR("Yang, Bin <bin.yang@intel.com>");
> -MODULE_AUTHOR("Zhu, Lejun <lejun.zhu@linux.intel.com>");
> diff --git a/drivers/mfd/intel_soc_pmic_core.h b/drivers/mfd/intel_soc_pmic_core.h
> deleted file mode 100644
> index d490685845eb..000000000000
> --- a/drivers/mfd/intel_soc_pmic_core.h
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * Intel SoC PMIC MFD Driver
> - *
> - * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
> - *
> - * Author: Yang, Bin <bin.yang@intel.com>
> - * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
> - */
> -
> -#ifndef __INTEL_SOC_PMIC_CORE_H__
> -#define __INTEL_SOC_PMIC_CORE_H__
> -
> -struct intel_soc_pmic_config {
> -	unsigned long irq_flags;
> -	struct mfd_cell *cell_dev;
> -	int n_cell_devs;
> -	const struct regmap_config *regmap_config;
> -	const struct regmap_irq_chip *irq_chip;
> -};
> -
> -extern struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc;
> -extern struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc;
> -
> -#endif	/* __INTEL_SOC_PMIC_CORE_H__ */
> diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
> index 5bb0367bd974..b5974dfcc603 100644
> --- a/drivers/mfd/intel_soc_pmic_crc.c
> +++ b/drivers/mfd/intel_soc_pmic_crc.c
> @@ -2,18 +2,21 @@
>  /*
>   * Device access for Crystal Cove PMIC
>   *
> - * Copyright (C) 2013, 2014 Intel Corporation. All rights reserved.
> + * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
>   *
>   * Author: Yang, Bin <bin.yang@intel.com>
>   * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>
>   */
>  
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
>  #include <linux/interrupt.h>
> -#include <linux/regmap.h>
> +#include <linux/module.h>
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/intel_soc_pmic.h>
> -
> -#include "intel_soc_pmic_core.h"
> +#include <linux/platform_data/x86/soc.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
>  
>  #define CRYSTAL_COVE_MAX_REGISTER	0xC6
>  
> @@ -132,7 +135,20 @@ static const struct regmap_irq_chip crystal_cove_irq_chip = {
>  	.mask_base = CRYSTAL_COVE_REG_MIRQLVL1,
>  };
>  
> -struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
> +/* PWM consumed by the Intel GFX */
> +static struct pwm_lookup crc_pwm_lookup[] = {
> +	PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
> +};
> +
> +struct intel_soc_pmic_config {
> +	unsigned long irq_flags;
> +	struct mfd_cell *cell_dev;
> +	int n_cell_devs;
> +	const struct regmap_config *regmap_config;
> +	const struct regmap_irq_chip *irq_chip;
> +};
> +
> +static const struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
>  	.irq_flags = IRQF_TRIGGER_RISING,
>  	.cell_dev = crystal_cove_byt_dev,
>  	.n_cell_devs = ARRAY_SIZE(crystal_cove_byt_dev),
> @@ -140,10 +156,144 @@ struct intel_soc_pmic_config intel_soc_pmic_config_byt_crc = {
>  	.irq_chip = &crystal_cove_irq_chip,
>  };
>  
> -struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
> +static const struct intel_soc_pmic_config intel_soc_pmic_config_cht_crc = {
>  	.irq_flags = IRQF_TRIGGER_RISING,
>  	.cell_dev = crystal_cove_cht_dev,
>  	.n_cell_devs = ARRAY_SIZE(crystal_cove_cht_dev),
>  	.regmap_config = &crystal_cove_regmap_config,
>  	.irq_chip = &crystal_cove_irq_chip,
>  };
> +
> +static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
> +				    const struct i2c_device_id *i2c_id)
> +{
> +	const struct intel_soc_pmic_config *config;
> +	struct device *dev = &i2c->dev;
> +	struct intel_soc_pmic *pmic;
> +	int ret;
> +
> +	if (soc_intel_is_byt())
> +		config = &intel_soc_pmic_config_byt_crc;
> +	else
> +		config = &intel_soc_pmic_config_cht_crc;
> +
> +	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
> +	if (!pmic)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, pmic);
> +
> +	pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config);
> +	if (IS_ERR(pmic->regmap))
> +		return PTR_ERR(pmic->regmap);
> +
> +	pmic->irq = i2c->irq;
> +
> +	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
> +				  config->irq_flags | IRQF_ONESHOT,
> +				  0, config->irq_chip,
> +				  &pmic->irq_chip_data);
> +	if (ret)
> +		return ret;
> +
> +	ret = enable_irq_wake(pmic->irq);
> +	if (ret)
> +		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
> +
> +	/* Add lookup table for crc-pwm */
> +	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> +
> +	/* To distuingish this domain from the GPIO/charger's irqchip domains */
> +	irq_domain_update_bus_token(regmap_irq_get_domain(pmic->irq_chip_data),
> +				    DOMAIN_BUS_NEXUS);
> +
> +	ret = mfd_add_devices(dev, -1, config->cell_dev,
> +			      config->n_cell_devs, NULL, 0,
> +			      regmap_irq_get_domain(pmic->irq_chip_data));
> +	if (ret)
> +		goto err_del_irq_chip;
> +
> +	return 0;
> +
> +err_del_irq_chip:
> +	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> +	return ret;
> +}
> +
> +static int intel_soc_pmic_i2c_remove(struct i2c_client *i2c)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> +
> +	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> +
> +	/* remove crc-pwm lookup table */
> +	pwm_remove_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
> +
> +	mfd_remove_devices(&i2c->dev);
> +
> +	return 0;
> +}
> +
> +static void intel_soc_pmic_shutdown(struct i2c_client *i2c)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(&i2c->dev);
> +
> +	disable_irq(pmic->irq);
> +
> +	return;
> +}
> +
> +#if defined(CONFIG_PM_SLEEP)
> +static int intel_soc_pmic_suspend(struct device *dev)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> +	disable_irq(pmic->irq);
> +
> +	return 0;
> +}
> +
> +static int intel_soc_pmic_resume(struct device *dev)
> +{
> +	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> +	enable_irq(pmic->irq);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(intel_soc_pmic_pm_ops, intel_soc_pmic_suspend,
> +			 intel_soc_pmic_resume);
> +
> +static const struct i2c_device_id intel_soc_pmic_i2c_id[] = {
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id);
> +
> +#if defined(CONFIG_ACPI)
> +static const struct acpi_device_id intel_soc_pmic_acpi_match[] = {
> +	{ "INT33FD" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match);
> +#endif
> +
> +static struct i2c_driver intel_soc_pmic_i2c_driver = {
> +	.driver = {
> +		.name = "intel_soc_pmic_i2c",
> +		.pm = &intel_soc_pmic_pm_ops,
> +		.acpi_match_table = ACPI_PTR(intel_soc_pmic_acpi_match),
> +	},
> +	.probe = intel_soc_pmic_i2c_probe,
> +	.remove = intel_soc_pmic_i2c_remove,
> +	.id_table = intel_soc_pmic_i2c_id,
> +	.shutdown = intel_soc_pmic_shutdown,
> +};
> +
> +module_i2c_driver(intel_soc_pmic_i2c_driver);
> +
> +MODULE_DESCRIPTION("I2C driver for Intel SoC PMIC");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Yang, Bin <bin.yang@intel.com>");
> +MODULE_AUTHOR("Zhu, Lejun <lejun.zhu@linux.intel.com>");

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Mon, Jun 20, 2022 at 10:51:33AM +0200, Hans de Goede wrote:
> Hi,
> 
> Thank you for this series, cleaning this up is long overdue.
> 
> The entire series looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> for the series.

Thanks!

> Maybe do a follow up renaming CONFIG_INTEL_SOC_PMIC to
> CONFIG_INTEL_SOC_PMIC_CRC and also change the module name
> from intel-soc-pmic to intel-soc-pmic-crc ?

I thought about that, but decided to leave it for later since it will take a
lot of subsystems and better to be considered out of the scope of this series.

Do you agree?

> I don't expect any userspace dependencies on the module-name
> (there are no module parameters) so this should be fine.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Mon, Jun 20, 2022 at 11:04:38AM +0200, Hans de Goede wrote:
> p.s.
> 
> I've added this commit to my local tree, so that the next time I boot
> a device with a CRC PMIC it will get some testing.

Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Hans de Goede]

Hi,

On 6/20/22 11:42, Andy Shevchenko wrote:
> On Mon, Jun 20, 2022 at 10:51:33AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> Thank you for this series, cleaning this up is long overdue.
>>
>> The entire series looks good to me:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> for the series.
> 
> Thanks!
> 
>> Maybe do a follow up renaming CONFIG_INTEL_SOC_PMIC to
>> CONFIG_INTEL_SOC_PMIC_CRC and also change the module name
>> from intel-soc-pmic to intel-soc-pmic-crc ?
> 
> I thought about that, but decided to leave it for later since it will take a
> lot of subsystems and better to be considered out of the scope of this series.
> 
> Do you agree?

Sure we can do this later. Note that there are not that much dependencies
on INTEL_SOC_PMIC though. The ones which I could find are:

drivers/usb/typec/tcpm/Kconfig: config TYPEC_WCOVE
-which is wrong regardless of the rename and should just be dropped
 in a separate/unrelated patch. can you submit a patch for this ?

drivers/gpio/Kconfig: config GPIO_CRYSTAL_COVE
drivers/pwm/Kconfig: config PWM_CRC
drivers/acpi/pmic/Kconfig: config BYTCRC_PMIC_OPREGION &&
                           config CHTCRC_PMIC_OPREGION

So not that much, but still 3 subystems, so yeah probably best
to do that cleanup later,

Regards,

Hans


> 
>> I don't expect any userspace dependencies on the module-name
>> (there are no module parameters) so this should be fine.
> 

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Mon, Jun 20, 2022 at 12:00:35PM +0200, Hans de Goede wrote:
> On 6/20/22 11:42, Andy Shevchenko wrote:
> > On Mon, Jun 20, 2022 at 10:51:33AM +0200, Hans de Goede wrote:

...

> >> Maybe do a follow up renaming CONFIG_INTEL_SOC_PMIC to
> >> CONFIG_INTEL_SOC_PMIC_CRC and also change the module name
> >> from intel-soc-pmic to intel-soc-pmic-crc ?
> > 
> > I thought about that, but decided to leave it for later since it will take a
> > lot of subsystems and better to be considered out of the scope of this series.
> > 
> > Do you agree?
> 
> Sure we can do this later. Note that there are not that much dependencies
> on INTEL_SOC_PMIC though. The ones which I could find are:
> 
> drivers/usb/typec/tcpm/Kconfig: config TYPEC_WCOVE
> -which is wrong regardless of the rename and should just be dropped
>  in a separate/unrelated patch. can you submit a patch for this ?

Sure...

Done!

> drivers/gpio/Kconfig: config GPIO_CRYSTAL_COVE
> drivers/pwm/Kconfig: config PWM_CRC
> drivers/acpi/pmic/Kconfig: config BYTCRC_PMIC_OPREGION &&
>                            config CHTCRC_PMIC_OPREGION
> 
> So not that much, but still 3 subystems, so yeah probably best
> to do that cleanup later,

Yup, let's do it later with Type-C fix in mind, that will save us
doing unneeded churn.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> The core part is misleading since its only purpose to serve Crystal Cove PMIC,
> although for couple of different platforms. Merge core part into crc one.
> 
> Advantages among others are:
> - speed up a compilation and build
> - decreasing the code base
> - reducing noise in the namespace by making some data static and const
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Makefile              |   3 +-
>  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
>  drivers/mfd/intel_soc_pmic_core.h |  25 -----
>  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
>  4 files changed, 157 insertions(+), 193 deletions(-)
>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h

Can you submit this again with the -M flag please.

Trying to figure out what exactly changed is proving challenging.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 2/9] mfd: intel_soc_pmic: Move non-Intel Makefile entries to their own group

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> It looks like a random position for couple of Makefile entries that are
> disrupting Intel PMIC group. Move them to their own group.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Makefile | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 3/9] mfd: intel_soc_pmic_crc: Use devm_regmap_add_irq_chip()

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> Use devm_regmap_add_irq_chip() to simplify the code.
> 
> While at it, replace -1 magic parameter by PLATFORM_DEVID_NONE when
> calling mfd_add_devices().
> 
> Note, the mfd_add_devices() left in non-devm variant here due to
> potentially increased churn while wrapping pwm_remove_table().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 4/9] mfd: intel_soc_pmic_crc: Convert to use i2c_get/set_clientdata()

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> We have the specific helpers for I2C device to set and get its driver data.
> Convert driver to use them instead of open coded variants.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 5/9] mfd: intel_soc_pmic_crc: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr() etc

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less error prone than the
> use of #ifdef based kernel configuration guards.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 6/9] mfd: intel_soc_pmic_crc: Drop redundant ACPI_PTR() and ifdeffery

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> The driver depends on ACPI, ACPI_PTR() resolution is always the same.
> Otherwise a compiler may produce a warning.
> 
> That said, the rule of thumb either ugly ifdeffery with ACPI_PTR or
> none should be used in a driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/Kconfig              | 4 ++--
>  drivers/mfd/intel_soc_pmic_crc.c | 6 ++----
>  2 files changed, 4 insertions(+), 6 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 5/9] mfd: intel_soc_pmic_crc: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr() etc

[Lee Jones]

On Mon, 27 Jun 2022, Lee Jones wrote:

> On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> 
> > Letting the compiler remove these functions when the kernel is built
> > without CONFIG_PM_SLEEP support is simpler and less error prone than the
> > use of #ifdef based kernel configuration guards.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/intel_soc_pmic_crc.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> Applied, thanks.

Sorry, wrong key combo - long day:

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 6/9] mfd: intel_soc_pmic_crc: Drop redundant ACPI_PTR() and ifdeffery

[Lee Jones]

On Mon, 27 Jun 2022, Lee Jones wrote:

> On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> 
> > The driver depends on ACPI, ACPI_PTR() resolution is always the same.
> > Otherwise a compiler may produce a warning.
> > 
> > That said, the rule of thumb either ugly ifdeffery with ACPI_PTR or
> > none should be used in a driver.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/Kconfig              | 4 ++--
> >  drivers/mfd/intel_soc_pmic_crc.c | 6 ++----
> >  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> Applied, thanks.

Sorry, wrong key combo - long day:                             
                                                               
For my own reference (apply this as-is to your sign-off block):
                                                               
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>           

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 7/9] mfd: intel_soc_pmic_crc: Convert driver to use ->probe_new()

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> Use the ->probe_new() callback.
> 
> The driver does not use const struct i2c_device_id * argument,
> so convert it to utilise the simplified I²C driver registration.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 8/9] mfd: intel_soc_pmic_crc: Replace intel_soc_pmic with crystal_cove

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> To reflect the point that this driver is only for one type of the PMICs,
> replace intel_soc_pmic with crystal_cove (avoid using crc for possible
> namespace collisions with CRC library APIs).
> 
> Note, also rename the driver name since we don't expect any user
> that enumerates by it, only ACPI known so far.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 42 ++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 21 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 9/9] mfd: intel_soc_pmic_crc: Update the copyright year

[Lee Jones]

On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> Update the copyright year to be 2012-2014, 2022.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_crc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
> index bb3cc1a808f0..0e5aae7ae9ad 100644
> --- a/drivers/mfd/intel_soc_pmic_crc.c
> +++ b/drivers/mfd/intel_soc_pmic_crc.c
> @@ -2,7 +2,7 @@
>  /*
>   * Device access for Crystal Cove PMIC
>   *
> - * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
> + * Copyright (C) 2012-2014, 2022 Intel Corporation. All rights reserved.

Same question as before.

What happens between these dates?

>   * Author: Yang, Bin <bin.yang@intel.com>
>   * Author: Zhu, Lejun <lejun.zhu@linux.intel.com>

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 9/9] mfd: intel_soc_pmic_crc: Update the copyright year

[Andy Shevchenko]

On Mon, Jun 27, 2022 at 02:46:21PM +0100, Lee Jones wrote:
> On Thu, 16 Jun 2022, Andy Shevchenko wrote:

> > - * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
> > + * Copyright (C) 2012-2014, 2022 Intel Corporation. All rights reserved.
> 
> Same question as before.
> 
> What happens between these dates?

Same answer as before. I believe nothing (which may justify Copyright update)
happened.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
> On Thu, 16 Jun 2022, Andy Shevchenko wrote:

...

> >  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
> >  drivers/mfd/intel_soc_pmic_core.h |  25 -----
> >  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
> >  4 files changed, 157 insertions(+), 193 deletions(-)
> >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
> >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> 
> Can you submit this again with the -M flag please.

This is done with this flag. Basically for the last several years I do my
submissions with that flag.

Anything else I can do?

> Trying to figure out what exactly changed is proving challenging.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Mon, Jun 20, 2022 at 11:04:38AM +0200, Hans de Goede wrote:
> p.s.
> 
> I've added this commit to my local tree, so that the next time I boot
> a device with a CRC PMIC it will get some testing.

Have you had a chance to boot such device?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Hans de Goede]

Hi,

On 6/29/22 01:06, Andy Shevchenko wrote:
> On Mon, Jun 20, 2022 at 11:04:38AM +0200, Hans de Goede wrote:
>> p.s.
>>
>> I've added this commit to my local tree, so that the next time I boot
>> a device with a CRC PMIC it will get some testing.
> 
> Have you had a chance to boot such device?

Not yet, but I've just put a new kernel on one of these devices
and everything still works fine AFAICT:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Hans de Goede]

On 6/29/22 15:18, Hans de Goede wrote:
> Hi,
> 
> On 6/29/22 01:06, Andy Shevchenko wrote:
>> On Mon, Jun 20, 2022 at 11:04:38AM +0200, Hans de Goede wrote:
>>> p.s.
>>>
>>> I've added this commit to my local tree, so that the next time I boot
>>> a device with a CRC PMIC it will get some testing.
>>
>> Have you had a chance to boot such device?
> 
> Not yet, but I've just put a new kernel on one of these devices
> and everything still works fine AFAICT:
> 
> Tested-by: Hans de Goede <hdegoede@redhat.com>

p.s.

This is for the entire series.

Regards,

Hans

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Tue, 28 Jun 2022, Andy Shevchenko wrote:

> On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
> > On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> 
> ...
> 
> > >  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
> > >  drivers/mfd/intel_soc_pmic_core.h |  25 -----
> > >  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
> > >  4 files changed, 157 insertions(+), 193 deletions(-)
> > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
> > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> > 
> > Can you submit this again with the -M flag please.
> 
> This is done with this flag. Basically for the last several years I do my
> submissions with that flag.

Odd.  I thought -M only showed diff for the changes.

> Anything else I can do?
> 
> > Trying to figure out what exactly changed is proving challenging.
> 

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 28 Jun 2022, Andy Shevchenko wrote:> > On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
> > > On Thu, 16 Jun 2022, Andy Shevchenko wrote:

...

> > > >  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
> > > >  drivers/mfd/intel_soc_pmic_core.h |  25 -----
> > > >  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
> > > >  4 files changed, 157 insertions(+), 193 deletions(-)
> > > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
> > > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> > >
> > > Can you submit this again with the -M flag please.
> >
> > This is done with this flag. Basically for the last several years I do my
> > submissions with that flag.
>
> Odd.  I thought -M only showed diff for the changes.

It's exactly what happens here in this patch. What did I miss?

Note here is not renaming, but merging contents of one file (actually
two files) into another. What you are talking about is probably -D,
but AFAIR Git (at least that time) can't catch up deleted files from
the mbox format. That's why I do not use -D for submissions.

> > Anything else I can do?
> >
> > > Trying to figure out what exactly changed is proving challenging.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Wed, Jun 29, 2022 at 03:19:17PM +0200, Hans de Goede wrote:
> On 6/29/22 15:18, Hans de Goede wrote:
> > On 6/29/22 01:06, Andy Shevchenko wrote:
> >> On Mon, Jun 20, 2022 at 11:04:38AM +0200, Hans de Goede wrote:
> >>> p.s.
> >>>
> >>> I've added this commit to my local tree, so that the next time I boot
> >>> a device with a CRC PMIC it will get some testing.
> >>
> >> Have you had a chance to boot such device?
> > 
> > Not yet, but I've just put a new kernel on one of these devices
> > and everything still works fine AFAICT:
> > 
> > Tested-by: Hans de Goede <hdegoede@redhat.com>
> 
> p.s.
> 
> This is for the entire series.

Thanks, I applied it locally in case I would need to send another version with
some additional flags for `git format-patch`. Otherwise I hope Lee can catch
up it from here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 28 Jun 2022, Andy Shevchenko wrote:> > On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
> > > > On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> 
> ...
> 
> > > > >  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
> > > > >  drivers/mfd/intel_soc_pmic_core.h |  25 -----
> > > > >  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
> > > > >  4 files changed, 157 insertions(+), 193 deletions(-)
> > > > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
> > > > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> > > >
> > > > Can you submit this again with the -M flag please.
> > >
> > > This is done with this flag. Basically for the last several years I do my
> > > submissions with that flag.
> >
> > Odd.  I thought -M only showed diff for the changes.
> 
> It's exactly what happens here in this patch. What did I miss?
> 
> Note here is not renaming, but merging contents of one file (actually
> two files) into another. What you are talking about is probably -D,
> but AFAIR Git (at least that time) can't catch up deleted files from
> the mbox format. That's why I do not use -D for submissions.

Ah yes, that's probably it then.

From a quick look at the diff (I missed the 2 "--" at the end), it
looked like this was a rename.  In which case -M won't do anything
useful here.  I'll have to brain grep the differences instead.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Thu, Jun 30, 2022 at 10:01 AM Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 29 Jun 2022, Andy Shevchenko wrote:
> > On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > On Tue, 28 Jun 2022, Andy Shevchenko wrote:> > On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
> > > > > On Thu, 16 Jun 2022, Andy Shevchenko wrote:

...

> > > > > >  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
> > > > > >  drivers/mfd/intel_soc_pmic_core.h |  25 -----
> > > > > >  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
> > > > > >  4 files changed, 157 insertions(+), 193 deletions(-)
> > > > > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
> > > > > >  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> > > > >
> > > > > Can you submit this again with the -M flag please.
> > > >
> > > > This is done with this flag. Basically for the last several years I do my
> > > > submissions with that flag.
> > >
> > > Odd.  I thought -M only showed diff for the changes.
> >
> > It's exactly what happens here in this patch. What did I miss?
> >
> > Note here is not renaming, but merging contents of one file (actually
> > two files) into another. What you are talking about is probably -D,
> > but AFAIR Git (at least that time) can't catch up deleted files from
> > the mbox format. That's why I do not use -D for submissions.
>
> Ah yes, that's probably it then.
>
> From a quick look at the diff (I missed the 2 "--" at the end), it
> looked like this was a rename.  In which case -M won't do anything
> useful here.  I'll have to brain grep the differences instead.

Please do, it will be good to have this double checked.

As I pointed out in the commit message the main difference in the code
(after massaging the header inclusion block) is the static const.
Without static we will have warning from the compiler, const is the
new thingy, besides the merge of the Copyright line.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Hans de Goede]

Hi,

On 6/30/22 10:07, Andy Shevchenko wrote:
> On Thu, Jun 30, 2022 at 10:01 AM Lee Jones <lee.jones@linaro.org> wrote:
>> On Wed, 29 Jun 2022, Andy Shevchenko wrote:
>>> On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <lee.jones@linaro.org> wrote:
>>>> On Tue, 28 Jun 2022, Andy Shevchenko wrote:> > On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
>>>>>> On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> 
> ...
> 
>>>>>>>  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
>>>>>>>  drivers/mfd/intel_soc_pmic_core.h |  25 -----
>>>>>>>  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
>>>>>>>  4 files changed, 157 insertions(+), 193 deletions(-)
>>>>>>>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
>>>>>>>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
>>>>>>
>>>>>> Can you submit this again with the -M flag please.
>>>>>
>>>>> This is done with this flag. Basically for the last several years I do my
>>>>> submissions with that flag.
>>>>
>>>> Odd.  I thought -M only showed diff for the changes.
>>>
>>> It's exactly what happens here in this patch. What did I miss?
>>>
>>> Note here is not renaming, but merging contents of one file (actually
>>> two files) into another. What you are talking about is probably -D,
>>> but AFAIR Git (at least that time) can't catch up deleted files from
>>> the mbox format. That's why I do not use -D for submissions.
>>
>> Ah yes, that's probably it then.
>>
>> From a quick look at the diff (I missed the 2 "--" at the end), it
>> looked like this was a rename.  In which case -M won't do anything
>> useful here.  I'll have to brain grep the differences instead.
> 
> Please do, it will be good to have this double checked.

Note that I already did a manual compare of the moved code blocks
to check that they were not changed before giving my Reviewed-by.

Regards,

Hans

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Thu, 30 Jun 2022, Hans de Goede wrote:

> Hi,
> 
> On 6/30/22 10:07, Andy Shevchenko wrote:
> > On Thu, Jun 30, 2022 at 10:01 AM Lee Jones <lee.jones@linaro.org> wrote:
> >> On Wed, 29 Jun 2022, Andy Shevchenko wrote:
> >>> On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <lee.jones@linaro.org> wrote:
> >>>> On Tue, 28 Jun 2022, Andy Shevchenko wrote:> > On Mon, Jun 27, 2022 at 02:08:39PM +0100, Lee Jones wrote:
> >>>>>> On Thu, 16 Jun 2022, Andy Shevchenko wrote:
> > 
> > ...
> > 
> >>>>>>>  drivers/mfd/intel_soc_pmic_core.c | 160 -----------------------------
> >>>>>>>  drivers/mfd/intel_soc_pmic_core.h |  25 -----
> >>>>>>>  drivers/mfd/intel_soc_pmic_crc.c  | 162 ++++++++++++++++++++++++++++--
> >>>>>>>  4 files changed, 157 insertions(+), 193 deletions(-)
> >>>>>>>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.c
> >>>>>>>  delete mode 100644 drivers/mfd/intel_soc_pmic_core.h
> >>>>>>
> >>>>>> Can you submit this again with the -M flag please.
> >>>>>
> >>>>> This is done with this flag. Basically for the last several years I do my
> >>>>> submissions with that flag.
> >>>>
> >>>> Odd.  I thought -M only showed diff for the changes.
> >>>
> >>> It's exactly what happens here in this patch. What did I miss?
> >>>
> >>> Note here is not renaming, but merging contents of one file (actually
> >>> two files) into another. What you are talking about is probably -D,
> >>> but AFAIR Git (at least that time) can't catch up deleted files from
> >>> the mbox format. That's why I do not use -D for submissions.
> >>
> >> Ah yes, that's probably it then.
> >>
> >> From a quick look at the diff (I missed the 2 "--" at the end), it
> >> looked like this was a rename.  In which case -M won't do anything
> >> useful here.  I'll have to brain grep the differences instead.
> > 
> > Please do, it will be good to have this double checked.
> 
> Note that I already did a manual compare of the moved code blocks
> to check that they were not changed before giving my Reviewed-by.

Super, thanks Hans.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Thu, Jun 30, 2022 at 09:45:53AM +0100, Lee Jones wrote:
> On Thu, 30 Jun 2022, Hans de Goede wrote:
> > On 6/30/22 10:07, Andy Shevchenko wrote:

...

> > Note that I already did a manual compare of the moved code blocks
> > to check that they were not changed before giving my Reviewed-by.
> 
> Super, thanks Hans.

Is it okay to be applied?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Fri, 15 Jul 2022, Andy Shevchenko wrote:

> On Thu, Jun 30, 2022 at 09:45:53AM +0100, Lee Jones wrote:
> > On Thu, 30 Jun 2022, Hans de Goede wrote:
> > > On 6/30/22 10:07, Andy Shevchenko wrote:
> 
> ...
> 
> > > Note that I already did a manual compare of the moved code blocks
> > > to check that they were not changed before giving my Reviewed-by.
> > 
> > Super, thanks Hans.
> 
> Is it okay to be applied?

Yes, I think so.

Can you collect all of the Acked-by/Tested-by tags you received
and submit a [RESEND] please?  Hans' TB which should be applied to all
patches springs to mind.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Mon, 18 Jul 2022, Lee Jones wrote:

> On Fri, 15 Jul 2022, Andy Shevchenko wrote:
> 
> > On Thu, Jun 30, 2022 at 09:45:53AM +0100, Lee Jones wrote:
> > > On Thu, 30 Jun 2022, Hans de Goede wrote:
> > > > On 6/30/22 10:07, Andy Shevchenko wrote:
> > 
> > ...
> > 
> > > > Note that I already did a manual compare of the moved code blocks
> > > > to check that they were not changed before giving my Reviewed-by.
> > > 
> > > Super, thanks Hans.
> > 
> > Is it okay to be applied?
> 
> Yes, I think so.
> 
> Can you collect all of the Acked-by/Tested-by tags you received
> and submit a [RESEND] please?  Hans' TB which should be applied to all
> patches springs to mind.

To save yourself some pain, leave off my Acks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Andy Shevchenko]

On Mon, Jul 18, 2022 at 09:12:55AM +0100, Lee Jones wrote:
> On Mon, 18 Jul 2022, Lee Jones wrote:
> > On Fri, 15 Jul 2022, Andy Shevchenko wrote:
> > > On Thu, Jun 30, 2022 at 09:45:53AM +0100, Lee Jones wrote:

...

> > > Is it okay to be applied?
> > 
> > Yes, I think so.
> > 
> > Can you collect all of the Acked-by/Tested-by tags you received
> > and submit a [RESEND] please?  Hans' TB which should be applied to all
> > patches springs to mind.
> 
> To save yourself some pain, leave off my Acks.

Actually it's less pain to leave your tags as they are now in the commit
messages.

I will resend it soon with your tags still in.

Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/9] mfd: intel_soc_pmic_crc: Merge Intel PMIC core to crc

[Lee Jones]

On Mon, 18 Jul 2022, Andy Shevchenko wrote:

> On Mon, Jul 18, 2022 at 09:12:55AM +0100, Lee Jones wrote:
> > On Mon, 18 Jul 2022, Lee Jones wrote:
> > > On Fri, 15 Jul 2022, Andy Shevchenko wrote:
> > > > On Thu, Jun 30, 2022 at 09:45:53AM +0100, Lee Jones wrote:
> 
> ...
> 
> > > > Is it okay to be applied?
> > > 
> > > Yes, I think so.
> > > 
> > > Can you collect all of the Acked-by/Tested-by tags you received
> > > and submit a [RESEND] please?  Hans' TB which should be applied to all
> > > patches springs to mind.
> > 
> > To save yourself some pain, leave off my Acks.
> 
> Actually it's less pain to leave your tags as they are now in the commit
> messages.
> 
> I will resend it soon with your tags still in.

Sure, whatever works.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog