[PATCH v4 0/3] ACPI: PMIC: a small refactoring

[PATCH v4 0/3] ACPI: PMIC: a small refactoring

[Andy Shevchenko]

Use sizeof(), dev_err()/dev_warn(), and regmap bulk read
where it makes sense.

In v4:
- dropped controversial changes (Hans)

Andy Shevchenko (3):
  ACPI: PMIC: Use sizeof() instead of hard coded value
  ACPI: PMIC: Convert pr_*() to dev_*() printing macros
  ACPI: PMIC: Replace open coded be16_to_cpu()

 drivers/acpi/pmic/intel_pmic_chtdc_ti.c | 13 ++++++++-----
 drivers/acpi/pmic/intel_pmic_chtwc.c    |  5 +++--
 drivers/acpi/pmic/intel_pmic_xpower.c   |  7 ++++---
 3 files changed, 15 insertions(+), 10 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v4 1/3] ACPI: PMIC: Use sizeof() instead of hard coded value

[Andy Shevchenko]

It's better to use sizeof() of a given buffer than spreading
a hard coded value.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/pmic/intel_pmic_chtdc_ti.c | 2 +-
 drivers/acpi/pmic/intel_pmic_xpower.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
index c84ef3d15181..35744a0307aa 100644
--- a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
+++ b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
@@ -87,7 +87,7 @@ static int chtdc_ti_pmic_get_raw_temp(struct regmap *regmap, int reg)
 {
 	u8 buf[2];
 
-	if (regmap_bulk_read(regmap, reg, buf, 2))
+	if (regmap_bulk_read(regmap, reg, buf, sizeof(buf)))
 		return -EIO;
 
 	/* stored in big-endian */
diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index 61bbe4c24d87..33c5e85294cd 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -255,7 +255,7 @@ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg)
 	if (ret)
 		return ret;
 
-	ret = regmap_bulk_read(regmap, AXP288_GP_ADC_H, buf, 2);
+	ret = regmap_bulk_read(regmap, AXP288_GP_ADC_H, buf, sizeof(buf));
 	if (ret == 0)
 		ret = (buf[0] << 4) + ((buf[1] >> 4) & 0x0f);
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v4 2/3] ACPI: PMIC: Convert pr_*() to dev_*() printing macros

[Andy Shevchenko]

Since we have a device pointer in the regmap, use it for
error messages.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/pmic/intel_pmic_chtwc.c  | 5 +++--
 drivers/acpi/pmic/intel_pmic_xpower.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c
index f2c42f4c79ca..25aa3e33b09a 100644
--- a/drivers/acpi/pmic/intel_pmic_chtwc.c
+++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
@@ -236,11 +236,12 @@ static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
 						   u32 reg_address,
 						   u32 value, u32 mask)
 {
+	struct device *dev = regmap_get_device(regmap);
 	u32 address;
 
 	if (i2c_client_address > 0xff || reg_address > 0xff) {
-		pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
-			__func__, i2c_client_address, reg_address);
+		dev_warn(dev, "warning addresses too big client 0x%x reg 0x%x\n",
+			 i2c_client_address, reg_address);
 		return -ERANGE;
 	}
 
diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
index 33c5e85294cd..43c5850b4bf3 100644
--- a/drivers/acpi/pmic/intel_pmic_xpower.c
+++ b/drivers/acpi/pmic/intel_pmic_xpower.c
@@ -274,11 +274,12 @@ static int intel_xpower_exec_mipi_pmic_seq_element(struct regmap *regmap,
 						   u16 i2c_address, u32 reg_address,
 						   u32 value, u32 mask)
 {
+	struct device *dev = regmap_get_device(regmap);
 	int ret;
 
 	if (i2c_address != 0x34) {
-		pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
-		       __func__, i2c_address, reg_address, value, mask);
+		dev_err(dev, "Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
+			i2c_address, reg_address, value, mask);
 		return -ENXIO;
 	}
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v4 3/3] ACPI: PMIC: Replace open coded be16_to_cpu()

[Andy Shevchenko]

It's easier to understand the nature of a data type when
it's written explicitly. With that, replace open coded
endianess conversion.

As a side effect it fixes the returned value of
intel_crc_pmic_update_aux() since ACPI PMIC core code
expects negative or zero and never uses positive one.

While at it, use macros from bits.h to reduce a room for mistake.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/pmic/intel_pmic_chtdc_ti.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
index 35744a0307aa..79f9df552524 100644
--- a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
+++ b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
@@ -8,12 +8,16 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/bits.h>
 #include <linux/init.h>
 #include <linux/mfd/intel_soc_pmic.h>
 #include <linux/platform_device.h>
+#include <asm/byteorder.h>
 #include "intel_pmic.h"
 
 /* registers stored in 16bit BE (high:low, total 10bit) */
+#define PMIC_REG_MASK		GENMASK(9, 0)
+
 #define CHTDC_TI_VBAT		0x54
 #define CHTDC_TI_DIETEMP	0x56
 #define CHTDC_TI_BPTHERM	0x58
@@ -73,7 +77,7 @@ static int chtdc_ti_pmic_get_power(struct regmap *regmap, int reg, int bit,
 	if (regmap_read(regmap, reg, &data))
 		return -EIO;
 
-	*value = data & 1;
+	*value = data & BIT(0);
 	return 0;
 }
 
@@ -85,13 +89,12 @@ static int chtdc_ti_pmic_update_power(struct regmap *regmap, int reg, int bit,
 
 static int chtdc_ti_pmic_get_raw_temp(struct regmap *regmap, int reg)
 {
-	u8 buf[2];
+	__be16 buf;
 
-	if (regmap_bulk_read(regmap, reg, buf, sizeof(buf)))
+	if (regmap_bulk_read(regmap, reg, &buf, sizeof(buf)))
 		return -EIO;
 
-	/* stored in big-endian */
-	return ((buf[0] & 0x03) << 8) | buf[1];
+	return be16_to_cpu(buf) & PMIC_REG_MASK;
 }
 
 static const struct intel_pmic_opregion_data chtdc_ti_pmic_opregion_data = {
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v4 0/3] ACPI: PMIC: a small refactoring

[Hans de Goede]

Hi,

On 6/6/24 10:54 PM, Andy Shevchenko wrote:
> Use sizeof(), dev_err()/dev_warn(), and regmap bulk read
> where it makes sense.
> 
> In v4:
> - dropped controversial changes (Hans)

Thanks, the whole series looks good to me:

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

for the series.

Regards,

Hans




> 
> Andy Shevchenko (3):
>   ACPI: PMIC: Use sizeof() instead of hard coded value
>   ACPI: PMIC: Convert pr_*() to dev_*() printing macros
>   ACPI: PMIC: Replace open coded be16_to_cpu()
> 
>  drivers/acpi/pmic/intel_pmic_chtdc_ti.c | 13 ++++++++-----
>  drivers/acpi/pmic/intel_pmic_chtwc.c    |  5 +++--
>  drivers/acpi/pmic/intel_pmic_xpower.c   |  7 ++++---
>  3 files changed, 15 insertions(+), 10 deletions(-)
> 

Re: [PATCH v4 0/3] ACPI: PMIC: a small refactoring

[Rafael J. Wysocki]

On Fri, Jun 7, 2024 at 11:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 6/6/24 10:54 PM, Andy Shevchenko wrote:
> > Use sizeof(), dev_err()/dev_warn(), and regmap bulk read
> > where it makes sense.
> >
> > In v4:
> > - dropped controversial changes (Hans)
>
> Thanks, the whole series looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> for the series.

All of the patches in the series applied as 6.11 material, thanks!