[PATCH v2 01/11] mfd: intel_soc_pmic_bxtwc: Don't shadow error codes in show()/store()

[PATCH v2 01/11] mfd: intel_soc_pmic_bxtwc: Don't shadow error codes in show()/store()

[Andy Shevchenko]

kstrtox() along with regmap API can return different error codes based on
circumstances.

Don't shadow them when returning to the caller.

While at it, remove rather confusing message from addr_store().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: dropped a confusing message as well (Lee)
 drivers/mfd/intel_soc_pmic_bxtwc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index bc069c4daa60..bdc3153ee890 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -339,10 +339,12 @@ static ssize_t addr_show(struct device *dev,
 static ssize_t addr_store(struct device *dev,
 			  struct device_attribute *attr, const char *buf, size_t count)
 {
-	if (kstrtoul(buf, 0, &bxtwc_reg_addr)) {
-		dev_err(dev, "Invalid register address\n");
-		return -EINVAL;
-	}
+	int ret;
+
+	ret = kstrtoul(buf, 0, &bxtwc_reg_addr);
+	if (ret)
+		return ret;
+
 	return (ssize_t)count;
 }
 
@@ -354,9 +356,9 @@ static ssize_t val_show(struct device *dev,
 	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
 
 	ret = regmap_read(pmic->regmap, bxtwc_reg_addr, &val);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(dev, "Failed to read 0x%lx\n", bxtwc_reg_addr);
-		return -EIO;
+		return ret;
 	}
 
 	return sprintf(buf, "0x%02x\n", val);
@@ -377,7 +379,7 @@ static ssize_t val_store(struct device *dev,
 	if (ret) {
 		dev_err(dev, "Failed to write value 0x%02x to address 0x%lx",
 			val, bxtwc_reg_addr);
-		return -EIO;
+		return ret;
 	}
 	return count;
 }
-- 
2.35.1

[PATCH v2 02/11] mfd: intel_soc_pmic_bxtwc: Create sysfs attributes using core driver's facility

[Andy Shevchenko]

Driver core takes care of sysfs attributes. Use this facility instead of
doing it explicitly in ->probe() and ->remove().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)
 drivers/mfd/intel_soc_pmic_bxtwc.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index bdc3153ee890..d43cd18f5189 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -396,6 +396,11 @@ static const struct attribute_group bxtwc_group = {
 	.attrs = bxtwc_attrs,
 };
 
+static const struct attribute_group *bxtwc_groups[] = {
+	&bxtwc_group,
+	NULL
+};
+
 static const struct regmap_config bxtwc_regmap_config = {
 	.reg_bits = 16,
 	.val_bits = 8,
@@ -555,12 +560,6 @@ static int bxtwc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = sysfs_create_group(&pdev->dev.kobj, &bxtwc_group);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to create sysfs group %d\n", ret);
-		return ret;
-	}
-
 	/*
 	 * There is known hw bug. Upon reset BIT 5 of register
 	 * BXTWC_CHGR_LVL1_IRQ is 0 which is the expected value. However,
@@ -568,15 +567,7 @@ static int bxtwc_probe(struct platform_device *pdev)
 	 * have the software workaround here to unmaksed it in order to let
 	 * charger interrutp work.
 	 */
-	regmap_update_bits(pmic->regmap, BXTWC_MIRQLVL1,
-				BXTWC_MIRQLVL1_MCHGR, 0);
-
-	return 0;
-}
-
-static int bxtwc_remove(struct platform_device *pdev)
-{
-	sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
+	regmap_update_bits(pmic->regmap, BXTWC_MIRQLVL1, BXTWC_MIRQLVL1_MCHGR, 0);
 
 	return 0;
 }
@@ -616,12 +607,12 @@ MODULE_DEVICE_TABLE(acpi, bxtwc_acpi_ids);
 
 static struct platform_driver bxtwc_driver = {
 	.probe = bxtwc_probe,
-	.remove	= bxtwc_remove,
 	.shutdown = bxtwc_shutdown,
 	.driver	= {
 		.name	= "BXTWC PMIC",
 		.pm     = &bxtwc_pm_ops,
 		.acpi_match_table = ACPI_PTR(bxtwc_acpi_ids),
+		.dev_groups = bxtwc_groups,
 	},
 };
 
-- 
2.35.1

Re: [PATCH v2 02/11] mfd: intel_soc_pmic_bxtwc: Create sysfs attributes using core driver's facility

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> Driver core takes care of sysfs attributes. Use this facility instead of
> doing it explicitly in ->probe() and ->remove().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)

Applied, thanks.

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

[PATCH v2 03/11] mfd: intel_soc_pmic_bxtwc: Convert to use platform_get/set_drvdata()

[Andy Shevchenko]

We have the specific helpers for platform 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>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index d43cd18f5189..540bb31128c0 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -457,7 +457,7 @@ static int bxtwc_probe(struct platform_device *pdev)
 		return ret;
 	pmic->irq = ret;
 
-	dev_set_drvdata(&pdev->dev, pmic);
+	platform_set_drvdata(pdev, pmic);
 	pmic->dev = &pdev->dev;
 
 	pmic->scu = devm_intel_scu_ipc_dev_get(&pdev->dev);
@@ -574,7 +574,7 @@ static int bxtwc_probe(struct platform_device *pdev)
 
 static void bxtwc_shutdown(struct platform_device *pdev)
 {
-	struct intel_soc_pmic *pmic = dev_get_drvdata(&pdev->dev);
+	struct intel_soc_pmic *pmic = platform_get_drvdata(pdev);
 
 	disable_irq(pmic->irq);
 }
-- 
2.35.1

Re: [PATCH v2 03/11] mfd: intel_soc_pmic_bxtwc: Convert to use platform_get/set_drvdata()

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> We have the specific helpers for platform 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>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

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

[PATCH v2 04/11] mfd: intel_soc_pmic_bxtwc: Use dev_err_probe()

[Andy Shevchenko]

Simplify the mux error path a bit by using dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 86 +++++++++---------------------
 1 file changed, 26 insertions(+), 60 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 540bb31128c0..25ac2cf0ad37 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -417,12 +417,9 @@ static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
 	int irq;
 
 	irq = regmap_irq_get_virq(pdata, pirq);
-	if (irq < 0) {
-		dev_err(pmic->dev,
-			"Failed to get parent vIRQ(%d) for chip %s, ret:%d\n",
-			pirq, chip->name, irq);
-		return irq;
-	}
+	if (irq < 0)
+		return dev_err_probe(pmic->dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n",
+				     pirq, chip->name);
 
 	return devm_regmap_add_irq_chip(pmic->dev, pmic->regmap, irq, irq_flags,
 					0, chip, data);
@@ -430,6 +427,7 @@ static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
 
 static int bxtwc_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	int ret;
 	acpi_handle handle;
 	acpi_status status;
@@ -438,15 +436,10 @@ static int bxtwc_probe(struct platform_device *pdev)
 
 	handle = ACPI_HANDLE(&pdev->dev);
 	status = acpi_evaluate_integer(handle, "_HRV", NULL, &hrv);
-	if (ACPI_FAILURE(status)) {
-		dev_err(&pdev->dev, "Failed to get PMIC hardware revision\n");
-		return -ENODEV;
-	}
-	if (hrv != BROXTON_PMIC_WC_HRV) {
-		dev_err(&pdev->dev, "Invalid PMIC hardware revision: %llu\n",
-			hrv);
-		return -ENODEV;
-	}
+	if (ACPI_FAILURE(status))
+		return dev_err_probe(dev, -ENODEV, "Failed to get PMIC hardware revision\n");
+	if (hrv != BROXTON_PMIC_WC_HRV)
+		return dev_err_probe(dev, -ENODEV, "Invalid PMIC hardware revision: %llu\n", hrv);
 
 	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
 	if (!pmic)
@@ -466,40 +459,31 @@ static int bxtwc_probe(struct platform_device *pdev)
 
 	pmic->regmap = devm_regmap_init(&pdev->dev, NULL, pmic,
 					&bxtwc_regmap_config);
-	if (IS_ERR(pmic->regmap)) {
-		ret = PTR_ERR(pmic->regmap);
-		dev_err(&pdev->dev, "Failed to initialise regmap: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(pmic->regmap))
+		return dev_err_probe(dev, PTR_ERR(pmic->regmap), "Failed to initialise regmap\n");
 
 	ret = devm_regmap_add_irq_chip(&pdev->dev, pmic->regmap, pmic->irq,
 				       IRQF_ONESHOT | IRQF_SHARED,
 				       0, &bxtwc_regmap_irq_chip,
 				       &pmic->irq_chip_data);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add IRQ chip\n");
 
 	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
 					 BXTWC_PWRBTN_LVL1_IRQ,
 					 IRQF_ONESHOT,
 					 &bxtwc_regmap_irq_chip_pwrbtn,
 					 &pmic->irq_chip_data_pwrbtn);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add PWRBTN IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add PWRBTN IRQ chip\n");
 
 	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
 					 BXTWC_TMU_LVL1_IRQ,
 					 IRQF_ONESHOT,
 					 &bxtwc_regmap_irq_chip_tmu,
 					 &pmic->irq_chip_data_tmu);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add TMU IRQ chip\n");
 
 	/* Add chained IRQ handler for BCU IRQs */
 	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
@@ -507,12 +491,8 @@ static int bxtwc_probe(struct platform_device *pdev)
 					 IRQF_ONESHOT,
 					 &bxtwc_regmap_irq_chip_bcu,
 					 &pmic->irq_chip_data_bcu);
-
-
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add BUC IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add BUC IRQ chip\n");
 
 	/* Add chained IRQ handler for ADC IRQs */
 	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
@@ -520,12 +500,8 @@ static int bxtwc_probe(struct platform_device *pdev)
 					 IRQF_ONESHOT,
 					 &bxtwc_regmap_irq_chip_adc,
 					 &pmic->irq_chip_data_adc);
-
-
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add ADC IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add ADC IRQ chip\n");
 
 	/* Add chained IRQ handler for CHGR IRQs */
 	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
@@ -533,12 +509,8 @@ static int bxtwc_probe(struct platform_device *pdev)
 					 IRQF_ONESHOT,
 					 &bxtwc_regmap_irq_chip_chgr,
 					 &pmic->irq_chip_data_chgr);
-
-
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add CHGR IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add CHGR IRQ chip\n");
 
 	/* Add chained IRQ handler for CRIT IRQs */
 	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
@@ -546,19 +518,13 @@ static int bxtwc_probe(struct platform_device *pdev)
 					 IRQF_ONESHOT,
 					 &bxtwc_regmap_irq_chip_crit,
 					 &pmic->irq_chip_data_crit);
-
-
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add CRIT IRQ chip\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add CRIT IRQ chip\n");
 
 	ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
 				   ARRAY_SIZE(bxt_wc_dev), NULL, 0, NULL);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to add devices\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add devices\n");
 
 	/*
 	 * There is known hw bug. Upon reset BIT 5 of register
-- 
2.35.1

Re: [PATCH v2 04/11] mfd: intel_soc_pmic_bxtwc: Use dev_err_probe()

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> Simplify the mux error path a bit by using dev_err_probe().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 86 +++++++++---------------------
>  1 file changed, 26 insertions(+), 60 deletions(-)

Applied, thanks.

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

[PATCH v2 05/11] mfd: intel_soc_pmic_bxtwc: Extend use of temporary variable for struct device

[Andy Shevchenko]

Extend use of temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 25ac2cf0ad37..bcecb3d75d5a 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -429,19 +429,17 @@ static int bxtwc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	int ret;
-	acpi_handle handle;
 	acpi_status status;
 	unsigned long long hrv;
 	struct intel_soc_pmic *pmic;
 
-	handle = ACPI_HANDLE(&pdev->dev);
-	status = acpi_evaluate_integer(handle, "_HRV", NULL, &hrv);
+	status = acpi_evaluate_integer(ACPI_HANDLE(dev), "_HRV", NULL, &hrv);
 	if (ACPI_FAILURE(status))
 		return dev_err_probe(dev, -ENODEV, "Failed to get PMIC hardware revision\n");
 	if (hrv != BROXTON_PMIC_WC_HRV)
 		return dev_err_probe(dev, -ENODEV, "Invalid PMIC hardware revision: %llu\n", hrv);
 
-	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
+	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
 	if (!pmic)
 		return -ENOMEM;
 
@@ -451,18 +449,17 @@ static int bxtwc_probe(struct platform_device *pdev)
 	pmic->irq = ret;
 
 	platform_set_drvdata(pdev, pmic);
-	pmic->dev = &pdev->dev;
+	pmic->dev = dev;
 
-	pmic->scu = devm_intel_scu_ipc_dev_get(&pdev->dev);
+	pmic->scu = devm_intel_scu_ipc_dev_get(dev);
 	if (!pmic->scu)
 		return -EPROBE_DEFER;
 
-	pmic->regmap = devm_regmap_init(&pdev->dev, NULL, pmic,
-					&bxtwc_regmap_config);
+	pmic->regmap = devm_regmap_init(dev, NULL, pmic, &bxtwc_regmap_config);
 	if (IS_ERR(pmic->regmap))
 		return dev_err_probe(dev, PTR_ERR(pmic->regmap), "Failed to initialise regmap\n");
 
-	ret = devm_regmap_add_irq_chip(&pdev->dev, pmic->regmap, pmic->irq,
+	ret = devm_regmap_add_irq_chip(dev, pmic->regmap, pmic->irq,
 				       IRQF_ONESHOT | IRQF_SHARED,
 				       0, &bxtwc_regmap_irq_chip,
 				       &pmic->irq_chip_data);
@@ -521,8 +518,8 @@ static int bxtwc_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add CRIT IRQ chip\n");
 
-	ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
-				   ARRAY_SIZE(bxt_wc_dev), NULL, 0, NULL);
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, bxt_wc_dev, ARRAY_SIZE(bxt_wc_dev),
+				   NULL, 0, NULL);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add devices\n");
 
-- 
2.35.1

Re: [PATCH v2 05/11] mfd: intel_soc_pmic_bxtwc: Extend use of temporary variable for struct device

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> Extend use of temporary variable for struct device to make code neater.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)

Applied, thanks.

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

[PATCH v2 06/11] mfd: intel_soc_pmic_bxtwc: 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>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index bcecb3d75d5a..7c3ce440c826 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -542,7 +542,6 @@ static void bxtwc_shutdown(struct platform_device *pdev)
 	disable_irq(pmic->irq);
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int bxtwc_suspend(struct device *dev)
 {
 	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
@@ -559,8 +558,8 @@ static int bxtwc_resume(struct device *dev)
 	enable_irq(pmic->irq);
 	return 0;
 }
-#endif
-static SIMPLE_DEV_PM_OPS(bxtwc_pm_ops, bxtwc_suspend, bxtwc_resume);
+
+static DEFINE_SIMPLE_DEV_PM_OPS(bxtwc_pm_ops, bxtwc_suspend, bxtwc_resume);
 
 static const struct acpi_device_id bxtwc_acpi_ids[] = {
 	{ "INT34D3", },
@@ -573,7 +572,7 @@ static struct platform_driver bxtwc_driver = {
 	.shutdown = bxtwc_shutdown,
 	.driver	= {
 		.name	= "BXTWC PMIC",
-		.pm     = &bxtwc_pm_ops,
+		.pm     = pm_sleep_ptr(&bxtwc_pm_ops),
 		.acpi_match_table = ACPI_PTR(bxtwc_acpi_ids),
 		.dev_groups = bxtwc_groups,
 	},
-- 
2.35.1

Re: [PATCH v2 06/11] mfd: intel_soc_pmic_bxtwc: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr() etc

[Lee Jones]

On Wed, 29 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>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Applied, thanks.

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

[PATCH v2 07/11] mfd: intel_soc_pmic_bxtwc: Drop redundant ACPI_PTR()

[Andy Shevchenko]

The driver depends on ACPI (via MFD_INTEL_PMC_BXT), 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>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 7c3ce440c826..f79ae0ddc495 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -573,7 +573,7 @@ static struct platform_driver bxtwc_driver = {
 	.driver	= {
 		.name	= "BXTWC PMIC",
 		.pm     = pm_sleep_ptr(&bxtwc_pm_ops),
-		.acpi_match_table = ACPI_PTR(bxtwc_acpi_ids),
+		.acpi_match_table = bxtwc_acpi_ids,
 		.dev_groups = bxtwc_groups,
 	},
 };
-- 
2.35.1

Re: [PATCH v2 07/11] mfd: intel_soc_pmic_bxtwc: Drop redundant ACPI_PTR()

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> The driver depends on ACPI (via MFD_INTEL_PMC_BXT), 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>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

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

[PATCH v2 08/11] mfd: intel_soc_pmic_bxtwc: Use bits.h macros for all masks

[Andy Shevchenko]

Currently we are using BIT(), but GENMASK(). Make use of the latter one
as well (far less error-prone, far more concise).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index f79ae0ddc495..5ac623eda1c1 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
@@ -18,9 +19,9 @@
 #include <asm/intel_scu_ipc.h>
 
 /* PMIC device registers */
-#define REG_ADDR_MASK		0xFF00
+#define REG_ADDR_MASK		GENMASK(15, 8)
 #define REG_ADDR_SHIFT		8
-#define REG_OFFSET_MASK		0xFF
+#define REG_OFFSET_MASK		GENMASK(7, 0)
 
 /* Interrupt Status Registers */
 #define BXTWC_IRQLVL1		0x4E02
@@ -112,29 +113,29 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_pwrbtn[] = {
-	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 0, 0x01),
+	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 0, BIT(0)),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
-	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, GENMASK(4, 0)),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
-	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
+	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, GENMASK(7, 0)),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
-	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, 0x20),
-	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
+	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, GENMASK(4, 0)),
+	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, GENMASK(4, 0)),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
-	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
+	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, GENMASK(2, 1)),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
-	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
+	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, GENMASK(1, 0)),
 };
 
 static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
-- 
2.35.1

Re: [PATCH v2 08/11] mfd: intel_soc_pmic_bxtwc: Use bits.h macros for all masks

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> Currently we are using BIT(), but GENMASK(). Make use of the latter one
> as well (far less error-prone, far more concise).
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)

Applied, thanks.

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

[PATCH v2 09/11] mfd: intel_soc_pmic_bxtwc: Use sysfs_emit() instead of sprintf()

[Andy Shevchenko]

sysfs_emit() is preferred over sprintf() when formatting the value to be
returned to user space in show() functions, because it knows about sysfs
buffer specifics and has sanity checks.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 5ac623eda1c1..db7bda114d2d 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -334,7 +334,7 @@ static unsigned long bxtwc_reg_addr;
 static ssize_t addr_show(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0x%lx\n", bxtwc_reg_addr);
+	return sysfs_emit(buf, "0x%lx\n", bxtwc_reg_addr);
 }
 
 static ssize_t addr_store(struct device *dev,
@@ -362,7 +362,7 @@ static ssize_t val_show(struct device *dev,
 		return ret;
 	}
 
-	return sprintf(buf, "0x%02x\n", val);
+	return sysfs_emit(buf, "0x%02x\n", val);
 }
 
 static ssize_t val_store(struct device *dev,
-- 
2.35.1

Re: [PATCH v2 09/11] mfd: intel_soc_pmic_bxtwc: Use sysfs_emit() instead of sprintf()

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> sysfs_emit() is preferred over sprintf() when formatting the value to be
> returned to user space in show() functions, because it knows about sysfs
> buffer specifics and has sanity checks.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

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

[PATCH v2 10/11] mfd: intel_soc_pmic_bxtwc: Drop unneeded casting

[Andy Shevchenko]

The casting from size_t to ssize_t is not needed in addr_store(),
drop it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
v2: added tag (Lee)

 drivers/mfd/intel_soc_pmic_bxtwc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index db7bda114d2d..1a80038c0e36 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -346,7 +346,7 @@ static ssize_t addr_store(struct device *dev,
 	if (ret)
 		return ret;
 
-	return (ssize_t)count;
+	return count;
 }
 
 static ssize_t val_show(struct device *dev,
-- 
2.35.1

Re: [PATCH v2 10/11] mfd: intel_soc_pmic_bxtwc: Drop unneeded casting

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> The casting from size_t to ssize_t is not needed in addr_store(),
> drop it.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
> v2: added tag (Lee)
> 
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

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

[PATCH v2 11/11] mfd: intel_soc_pmic_bxtwc: Fix spelling in the comment

[Andy Shevchenko]

There are a few of spelling issues in the comment, fix them.
While at it, fix indentation in the MODULE_AUTHOR() parameter
and update copyright years.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: more spelling and grammar fixes (Lee)
 drivers/mfd/intel_soc_pmic_bxtwc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 1a80038c0e36..8dac0d41f64f 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -2,7 +2,7 @@
 /*
  * MFD core driver for Intel Broxton Whiskey Cove PMIC
  *
- * Copyright (C) 2015 Intel Corporation. All rights reserved.
+ * Copyright (C) 2015-2017, 2022 Intel Corporation. All rights reserved.
  */
 
 #include <linux/acpi.h>
@@ -525,11 +525,11 @@ static int bxtwc_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret, "Failed to add devices\n");
 
 	/*
-	 * There is known hw bug. Upon reset BIT 5 of register
+	 * There is a known H/W bug. Upon reset, BIT 5 of register
 	 * BXTWC_CHGR_LVL1_IRQ is 0 which is the expected value. However,
 	 * later it's set to 1(masked) automatically by hardware. So we
-	 * have the software workaround here to unmaksed it in order to let
-	 * charger interrutp work.
+	 * place the software workaround here to unmask it again in order
+	 * to re-enable the charger interrupt.
 	 */
 	regmap_update_bits(pmic->regmap, BXTWC_MIRQLVL1, BXTWC_MIRQLVL1_MCHGR, 0);
 
@@ -582,4 +582,4 @@ static struct platform_driver bxtwc_driver = {
 module_platform_driver(bxtwc_driver);
 
 MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Qipeng Zha<qipeng.zha@intel.com>");
+MODULE_AUTHOR("Qipeng Zha <qipeng.zha@intel.com>");
-- 
2.35.1

Re: [PATCH v2 11/11] mfd: intel_soc_pmic_bxtwc: Fix spelling in the comment

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> There are a few of spelling issues in the comment, fix them.
> While at it, fix indentation in the MODULE_AUTHOR() parameter
> and update copyright years.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: more spelling and grammar fixes (Lee)
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 10 +++++-----
>  1 file changed, 5 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 v2 01/11] mfd: intel_soc_pmic_bxtwc: Don't shadow error codes in show()/store()

[Lee Jones]

On Wed, 29 Jun 2022, Andy Shevchenko wrote:

> kstrtox() along with regmap API can return different error codes based on
> circumstances.
> 
> Don't shadow them when returning to the caller.
> 
> While at it, remove rather confusing message from addr_store().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: dropped a confusing message as well (Lee)
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)

Applied, thanks.

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