2022-06-16 19:12:53

by Andy Shevchenko

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

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 <[email protected]>
---
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 <[email protected]>
- * Author: Zhu, Lejun <[email protected]>
- */
-
-#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 <[email protected]>");
-MODULE_AUTHOR("Zhu, Lejun <[email protected]>");
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 <[email protected]>
- * Author: Zhu, Lejun <[email protected]>
- */
-
-#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 <[email protected]>
* Author: Zhu, Lejun <[email protected]>
*/

+#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 <[email protected]>");
+MODULE_AUTHOR("Zhu, Lejun <[email protected]>");
--
2.35.1


2022-06-20 08:54:25

by Hans de Goede

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

Hi,

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

The entire series looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

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 <[email protected]>
> ---
> 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 <[email protected]>
> - * Author: Zhu, Lejun <[email protected]>
> - */
> -
> -#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 <[email protected]>");
> -MODULE_AUTHOR("Zhu, Lejun <[email protected]>");
> 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 <[email protected]>
> - * Author: Zhu, Lejun <[email protected]>
> - */
> -
> -#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 <[email protected]>
> * Author: Zhu, Lejun <[email protected]>
> */
>
> +#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 <[email protected]>");
> +MODULE_AUTHOR("Zhu, Lejun <[email protected]>");

2022-06-20 09:45:35

by Hans de Goede

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> - * Author: Zhu, Lejun <[email protected]>
> - */
> -
> -#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 <[email protected]>");
> -MODULE_AUTHOR("Zhu, Lejun <[email protected]>");
> 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 <[email protected]>
> - * Author: Zhu, Lejun <[email protected]>
> - */
> -
> -#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 <[email protected]>
> * Author: Zhu, Lejun <[email protected]>
> */
>
> +#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 <[email protected]>");
> +MODULE_AUTHOR("Zhu, Lejun <[email protected]>");

2022-06-20 09:53:12

by Andy Shevchenko

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

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 <[email protected]>
>
> 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


2022-06-20 09:53:20

by Andy Shevchenko

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

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


2022-06-20 10:35:51

by Hans de Goede

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

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 <[email protected]>
>>
>> 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.
>

2022-06-20 11:34:03

by Andy Shevchenko

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

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


2022-06-27 13:18:16

by Lee Jones

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

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 <[email protected]>
> ---
> 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

2022-06-28 10:00:00

by Andy Shevchenko

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

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


2022-06-28 23:36:47

by Andy Shevchenko

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

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


2022-06-29 13:45:31

by Hans de Goede

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

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 <[email protected]>

Regards,

Hans

2022-06-29 14:14:40

by Hans de Goede

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

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 <[email protected]>

p.s.

This is for the entire series.

Regards,

Hans

2022-06-29 15:19:42

by Lee Jones

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

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

2022-06-29 16:07:09

by Andy Shevchenko

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

On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <[email protected]> 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

2022-06-29 16:26:27

by Andy Shevchenko

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

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 <[email protected]>
>
> 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


2022-06-30 08:09:26

by Lee Jones

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

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <[email protected]> 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

2022-06-30 08:18:59

by Andy Shevchenko

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

On Thu, Jun 30, 2022 at 10:01 AM Lee Jones <[email protected]> wrote:
> On Wed, 29 Jun 2022, Andy Shevchenko wrote:
> > On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <[email protected]> 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

2022-06-30 08:57:35

by Hans de Goede

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

Hi,

On 6/30/22 10:07, Andy Shevchenko wrote:
> On Thu, Jun 30, 2022 at 10:01 AM Lee Jones <[email protected]> wrote:
>> On Wed, 29 Jun 2022, Andy Shevchenko wrote:
>>> On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <[email protected]> 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

2022-06-30 09:34:54

by Lee Jones

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

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 <[email protected]> wrote:
> >> On Wed, 29 Jun 2022, Andy Shevchenko wrote:
> >>> On Wed, Jun 29, 2022 at 4:36 PM Lee Jones <[email protected]> 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

2022-07-15 20:12:57

by Andy Shevchenko

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

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


2022-07-18 08:18:24

by Lee Jones

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

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

2022-07-18 08:19:27

by Lee Jones

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

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

2022-07-18 12:24:41

by Andy Shevchenko

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

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


2022-07-18 15:28:01

by Lee Jones

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

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