[PATCH 1/2] ASoC: codecs: aw88261: fix changing sample rate and bit width

[PATCH 1/2] ASoC: codecs: aw88261: fix changing sample rate and bit width

[Val Packett]

The aw88261 driver only worked with 32-bit 48kHz streams so far due to
the lack of a proper PLL initialization sequence. Fix by selecting all
the necessary PLL settings based on what was passed to us in the
hw_params DAI callback. This replaces the strange downstream routine
that tries two divider modes in sequence.

Signed-off-by: Val Packett <val@packett.cool>
---

This driver is used for the speakers on devices like fairphone-fp5 and
motorola-dubai, but until now only with non-upstreamable hacks (see [1])
to force the use of S32_LE because it "did not initialize with the format
limited to S16_LE".

It seems like the downstream driver did not actually handle this properly
either, so I presume similar hacks are done there too. There are two
different versions I've found (in the same tree even): the "newer" more
complex one [2] sends hw_params straight to /dev/null and has this bizzare
I-don't-know-what-I'm-doing "mode1/mode2" routine that was copied into the
upstream driver, while the other simpler one [3] does indeed have an
hw_params callback that configures registers based on the params..

Except if anyone tried to just copy it, it wouldn't actually fix support
for bit widths other than 32. The crucial thing it missed was the I2SBCK
field, which is what's actually responsible for setting the "physical"
frame length! Also during testing I've discovered that the value of the
registers was being reset during aw88261_dev_pwd, so touching registers
directly in the hw_params handler would not work.

With this patch, the aw88261 speakers in my motorola-dubai do work with
the S16_LE format, avoiding the need for sm8250.c hacks.

[1]: https://github.com/sc7280-mainline/linux/commit/b9c78cb306ff069eee65213c67b7e7fba40e6221
[2]: https://github.com/LineageOS/android_kernel_motorola_sm7325/tree/lineage-23.2/techpack/audio/asoc/codecs/aw88261
[3]: https://github.com/LineageOS/android_kernel_motorola_sm7325/blob/lineage-23.2/techpack/audio/asoc/codecs/aw882xx/aw882xx.c

Thanks,
~val

---
 sound/soc/codecs/aw88261.c | 174 ++++++++++++++++++++++++-------------
 sound/soc/codecs/aw88261.h |  78 ++++++++++++++++-
 2 files changed, 189 insertions(+), 63 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index a6805d5405cd..3c7d3e3865eb 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -13,6 +13,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
+#include <sound/pcm_params.h>
 #include "aw88261.h"
 #include "aw88395/aw88395_data_type.h"
 #include "aw88395/aw88395_device.h"
@@ -158,7 +159,7 @@ static int aw88261_dev_get_iis_status(struct aw_device *aw_dev)
 	return ret;
 }
 
-static int aw88261_dev_check_mode1_pll(struct aw_device *aw_dev)
+static int aw88261_dev_check_pll(struct aw_device *aw_dev)
 {
 	int ret, i;
 
@@ -175,71 +176,95 @@ static int aw88261_dev_check_mode1_pll(struct aw_device *aw_dev)
 	return -EPERM;
 }
 
-static int aw88261_dev_check_mode2_pll(struct aw_device *aw_dev)
-{
-	unsigned int reg_val;
-	int ret, i;
-
-	ret = regmap_read(aw_dev->regmap, AW88261_PLLCTRL1_REG, &reg_val);
-	if (ret)
-		return ret;
-
-	reg_val &= (~AW88261_CCO_MUX_MASK);
-	if (reg_val == AW88261_CCO_MUX_DIVIDED_VALUE) {
-		dev_dbg(aw_dev->dev, "CCO_MUX is already divider");
-		return -EPERM;
-	}
-
-	/* change mode2 */
-	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
-			~AW88261_CCO_MUX_MASK, AW88261_CCO_MUX_DIVIDED_VALUE);
-	if (ret)
-		return ret;
-
-	for (i = 0; i < AW88261_DEV_SYSST_CHECK_MAX; i++) {
-		ret = aw88261_dev_get_iis_status(aw_dev);
-		if (ret) {
-			dev_err(aw_dev->dev, "mode2 iis signal check error");
-			usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
-		} else {
-			break;
-		}
-	}
-
-	/* change mode1 */
-	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
-			~AW88261_CCO_MUX_MASK, AW88261_CCO_MUX_BYPASS_VALUE);
-	if (ret == 0) {
-		usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
-		for (i = 0; i < AW88261_DEV_SYSST_CHECK_MAX; i++) {
-			ret = aw88261_dev_check_mode1_pll(aw_dev);
-			if (ret) {
-				dev_err(aw_dev->dev, "mode2 switch to mode1, iis signal check error");
-				usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
-			} else {
-				break;
-			}
-		}
-	}
-
-	return ret;
-}
-
-static int aw88261_dev_check_syspll(struct aw_device *aw_dev)
+static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 {
+	struct aw_device *aw_dev = aw88261->aw_pa;
+	uint32_t sr_value, fs_value, cco_mux_value, bck_value;
 	int ret;
 
-	ret = aw88261_dev_check_mode1_pll(aw_dev);
-	if (ret) {
-		dev_dbg(aw_dev->dev, "mode1 check iis failed try switch to mode2 check");
-		ret = aw88261_dev_check_mode2_pll(aw_dev);
-		if (ret) {
-			dev_err(aw_dev->dev, "mode2 check iis failed");
-			return ret;
-		}
+	switch (aw88261->sample_rate) {
+	case 8000:
+		sr_value = AW88261_I2SSR_8KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 16000:
+		sr_value = AW88261_I2SSR_16KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 32000:
+		sr_value = AW88261_I2SSR_32KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 44100:
+		sr_value = AW88261_I2SSR_44P1KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	case 48000:
+		sr_value = AW88261_I2SSR_48KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	case 96000:
+		sr_value = AW88261_I2SSR_96KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	case 192000:
+		sr_value = AW88261_I2SSR_192KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	default:
+		dev_err(aw_dev->dev, "unsupported sample rate %d\n",
+			aw88261->sample_rate);
+		return -EINVAL;
 	}
 
-	return ret;
+	switch (aw88261->bit_width) {
+	case 16:
+		fs_value = AW88261_I2SFS_16_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_32FS_VALUE;
+		break;
+	case 20:
+		fs_value = AW88261_I2SFS_20_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_48FS_VALUE;
+		break;
+	case 24:
+		fs_value = AW88261_I2SFS_24_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_48FS_VALUE;
+		break;
+	case 32:
+		fs_value = AW88261_I2SFS_32_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_64FS_VALUE;
+		break;
+	default:
+		dev_err(aw_dev->dev, "unsupported bit width %d\n",
+			aw88261->bit_width);
+		return -EINVAL;
+	}
+
+	/* PLL divider must be used for 8/16/32 kHz modes */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
+			~AW88261_CCO_MUX_MASK, cco_mux_value);
+	if (ret)
+		return ret;
+
+	/* The word clock (WCK) defines the beginning of a frame */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SSR_MASK, sr_value);
+	if (ret)
+		return ret;
+
+	/* The bit clock (BCK) defines the length of a frame */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SBCK_MASK, bck_value);
+	if (ret)
+		return ret;
+
+	/* The logical frame size is the width of data for 1 slot */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SFS_MASK, fs_value);
+	if (ret)
+		return ret;
+
+	return aw88261_dev_check_pll(aw_dev);
 }
 
 static int aw88261_dev_check_sysst(struct aw_device *aw_dev)
@@ -558,7 +583,7 @@ static int aw88261_dev_start(struct aw88261 *aw88261)
 	aw88261_dev_pwd(aw_dev, false);
 	usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
 
-	ret = aw88261_dev_check_syspll(aw_dev);
+	ret = aw88261_dev_configure_syspll(aw88261);
 	if (ret) {
 		dev_err(aw_dev->dev, "pll check failed cannot start");
 		goto pll_check_fail;
@@ -712,6 +737,26 @@ static void aw88261_start(struct aw88261 *aw88261, bool sync_start)
 			AW88261_START_WORK_DELAY_MS);
 }
 
+static int aw88261_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params,
+	struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		return 0;
+
+	/* Only store the settings as the regs do get reset when starting */
+	aw88261->sample_rate = params_rate(params);
+	aw88261->bit_width = params_width(params);
+	return 0;
+}
+
+static const struct snd_soc_dai_ops aw88261_dai_ops = {
+	.hw_params = aw88261_hw_params,
+};
+
 static struct snd_soc_dai_driver aw88261_dai[] = {
 	{
 		.name = "aw88261-aif",
@@ -730,6 +775,7 @@ static struct snd_soc_dai_driver aw88261_dai[] = {
 			.rates = AW88261_RATES,
 			.formats = AW88261_FORMATS,
 		},
+		.ops = &aw88261_dai_ops,
 	},
 };
 
@@ -1249,6 +1295,10 @@ static int aw88261_i2c_probe(struct i2c_client *i2c)
 	if (!aw88261)
 		return -ENOMEM;
 
+	/* set defaults */
+	aw88261->sample_rate = 48000;
+	aw88261->bit_width = 32;
+
 	mutex_init(&aw88261->lock);
 
 	i2c_set_clientdata(i2c, aw88261);
diff --git a/sound/soc/codecs/aw88261.h b/sound/soc/codecs/aw88261.h
index 1fee589608d6..2100fddaa68f 100644
--- a/sound/soc/codecs/aw88261.h
+++ b/sound/soc/codecs/aw88261.h
@@ -264,7 +264,81 @@
 #define AW88261_I2STXEN_ENABLE_VALUE	\
 	(AW88261_I2STXEN_ENABLE << AW88261_I2STXEN_START_BIT)
 
-#define AW88261_CCO_MUX_START_BIT	(14)
+#define AW88261_I2SFS_START_BIT			(6)
+#define AW88261_I2SFS_BITS_LEN			(2)
+#define AW88261_I2SFS_MASK				\
+	(~(((1<<AW88261_I2SFS_BITS_LEN)-1)<<AW88261_I2SFS_START_BIT))
+
+#define AW88261_I2SFS_16_BITS			(0)
+#define AW88261_I2SFS_16_BITS_VALUE		\
+	(AW88261_I2SFS_16_BITS << AW88261_I2SFS_START_BIT)
+#define AW88261_I2SFS_20_BITS			(1)
+#define AW88261_I2SFS_20_BITS_VALUE		\
+	(AW88261_I2SFS_20_BITS << AW88261_I2SFS_START_BIT)
+#define AW88261_I2SFS_24_BITS			(2)
+#define AW88261_I2SFS_24_BITS_VALUE		\
+	(AW88261_I2SFS_24_BITS << AW88261_I2SFS_START_BIT)
+#define AW88261_I2SFS_32_BITS			(3)
+#define AW88261_I2SFS_32_BITS_VALUE		\
+	(AW88261_I2SFS_32_BITS << AW88261_I2SFS_START_BIT)
+
+#define AW88261_I2SBCK_START_BIT	(4)
+#define AW88261_I2SBCK_BITS_LEN	(2)
+#define AW88261_I2SBCK_MASK	\
+	(~(((1<<AW88261_I2SBCK_BITS_LEN)-1) << AW88261_I2SBCK_START_BIT))
+
+#define AW88261_I2SBCK_32FS	(0)
+#define AW88261_I2SBCK_32FS_VALUE	\
+	(AW88261_I2SBCK_32FS << AW88261_I2SBCK_START_BIT)
+
+#define AW88261_I2SBCK_48FS	(1)
+#define AW88261_I2SBCK_48FS_VALUE	\
+	(AW88261_I2SBCK_48FS << AW88261_I2SBCK_START_BIT)
+
+#define AW88261_I2SBCK_64FS	(2)
+#define AW88261_I2SBCK_64FS_VALUE	\
+	(AW88261_I2SBCK_64FS << AW88261_I2SBCK_START_BIT)
+
+#define AW88261_I2SSR_START_BIT			(0)
+#define AW88261_I2SSR_BITS_LEN			(4)
+#define AW88261_I2SSR_MASK				\
+	(~(((1<<AW88261_I2SSR_BITS_LEN)-1) << AW88261_I2SSR_START_BIT))
+
+#define AW88261_I2SSR_8KHZ				(0)
+#define AW88261_I2SSR_8KHZ_VALUE		\
+	(AW88261_I2SSR_8KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_11P025KHZ			(1)
+#define AW88261_I2SSR_11P025KHZ_VALUE	\
+	(AW88261_I2SSR_11P025KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_12KHZ				(2)
+#define AW88261_I2SSR_12KHZ_VALUE		\
+	(AW88261_I2SSR_12KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_16KHZ				(3)
+#define AW88261_I2SSR_16KHZ_VALUE		\
+	(AW88261_I2SSR_16KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_22P05KHZ			(4)
+#define AW88261_I2SSR_22P05KHZ_VALUE	\
+	(AW88261_I2SSR_22P05KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_24KHZ				(5)
+#define AW88261_I2SSR_24KHZ_VALUE		\
+	(AW88261_I2SSR_24KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_32KHZ				(6)
+#define AW88261_I2SSR_32KHZ_VALUE		\
+	(AW88261_I2SSR_32KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_44P1KHZ			(7)
+#define AW88261_I2SSR_44P1KHZ_VALUE	\
+	(AW88261_I2SSR_44P1KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_48KHZ				(8)
+#define AW88261_I2SSR_48KHZ_VALUE		\
+	(AW88261_I2SSR_48KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_96KHZ				(9)
+#define AW88261_I2SSR_96KHZ_VALUE		\
+	(AW88261_I2SSR_96KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_192KHZ			(10)
+#define AW88261_I2SSR_192KHZ_VALUE		\
+	(AW88261_I2SSR_192KHZ << AW88261_I2SSR_START_BIT)
+
+#define AW88261_CCO_MUX_START_BIT	(6)
 #define AW88261_CCO_MUX_BITS_LEN	(1)
 #define AW88261_CCO_MUX_MASK		\
 	(~(((1<<AW88261_CCO_MUX_BITS_LEN)-1) << AW88261_CCO_MUX_START_BIT))
@@ -450,6 +524,8 @@ struct aw88261 {
 	int frcset_en;
 	unsigned int mute_st;
 	unsigned int amppd_st;
+	int sample_rate;
+	int bit_width;
 
 	bool phase_sync;
 };
-- 
2.53.0

[PATCH 2/2] ASoC: codecs: aw88261: use correct registers for AW88258

[Val Packett]

The AW88258 has a slightly different register map, namely the PLL ones
are different and some extra boost ones are absent. Before the recent
hw_params fix, the PLL CCO_MUX accesses were actually always wrong as
the PLLCTRL1 address (0x54) was from the AW88261 but the CCO_MUX offset
(14) was from the AW88258. With said fix the AW88261 parameters were
always used, making it work on that chip. Make the accesses correct
for the AW88258 as well.

Signed-off-by: Val Packett <val@packett.cool>
---
This one is not tested (don't have the hardware), but since I've noticed
that the CCO_MUX was not even correct for any of the supported chips,
I've decided to try fixing it for the other one as well.

Code style wise this is kind of a really annoying situation but hopefully
this looks okay enough

~val
---
 sound/soc/codecs/aw88258.h | 34 ++++++++++++++++++++++++++++++++++
 sound/soc/codecs/aw88261.c | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 60 insertions(+), 11 deletions(-)
 create mode 100644 sound/soc/codecs/aw88258.h

diff --git a/sound/soc/codecs/aw88258.h b/sound/soc/codecs/aw88258.h
new file mode 100644
index 000000000000..7ae177667ae6
--- /dev/null
+++ b/sound/soc/codecs/aw88258.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// aw88258.h  --  AW88258 ALSA SoC Audio driver
+//
+// Copyright (c) 2023 awinic Technology CO., LTD
+//
+// Author: Jimmy Zhang <zhangjianming@awinic.com>
+// Author: Weidong Wang <wangweidong.a@awinic.com>
+//
+
+#ifndef __AW88258_H__
+#define __AW88258_H__
+
+/* This file contains definitions for registers that differ between
+ * the AW88261 (ID 0x2113) and the AW88258 (ID 0x1852). */
+
+#define AW88258_PLLCTRL1_REG		(0x66)
+#define AW88258_PLLCTRL2_REG		(0x67)
+#define AW88258_PLLCTRL3_REG		(0x68)
+
+#define AW88258_CCO_MUX_START_BIT	(14)
+#define AW88258_CCO_MUX_BITS_LEN	(1)
+#define AW88258_CCO_MUX_MASK		\
+	(~(((1<<AW88258_CCO_MUX_BITS_LEN)-1) << AW88258_CCO_MUX_START_BIT))
+
+#define AW88258_CCO_MUX_DIVIDED	(0)
+#define AW88258_CCO_MUX_DIVIDED_VALUE	\
+	(AW88258_CCO_MUX_DIVIDED << AW88258_CCO_MUX_START_BIT)
+
+#define AW88258_CCO_MUX_BYPASS		(1)
+#define AW88258_CCO_MUX_BYPASS_VALUE	\
+	(AW88258_CCO_MUX_BYPASS << AW88258_CCO_MUX_START_BIT)
+
+#endif
diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index 3c7d3e3865eb..172f38a4921c 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -14,6 +14,7 @@
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
+#include "aw88258.h"
 #include "aw88261.h"
 #include "aw88395/aw88395_data_type.h"
 #include "aw88395/aw88395_device.h"
@@ -179,37 +180,36 @@ static int aw88261_dev_check_pll(struct aw_device *aw_dev)
 static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 {
 	struct aw_device *aw_dev = aw88261->aw_pa;
-	uint32_t sr_value, fs_value, cco_mux_value, bck_value;
+	uint32_t sr_value, fs_value, bck_value;
+	bool bypass_divider = false;
 	int ret;
 
 	switch (aw88261->sample_rate) {
 	case 8000:
 		sr_value = AW88261_I2SSR_8KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
 		break;
 	case 16000:
 		sr_value = AW88261_I2SSR_16KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
 		break;
 	case 32000:
 		sr_value = AW88261_I2SSR_32KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		bypass_divider = true;
 		break;
 	case 44100:
 		sr_value = AW88261_I2SSR_44P1KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	case 48000:
 		sr_value = AW88261_I2SSR_48KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	case 96000:
 		sr_value = AW88261_I2SSR_96KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	case 192000:
 		sr_value = AW88261_I2SSR_192KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	default:
 		dev_err(aw_dev->dev, "unsupported sample rate %d\n",
@@ -241,8 +241,21 @@ static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 	}
 
 	/* PLL divider must be used for 8/16/32 kHz modes */
-	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
-			~AW88261_CCO_MUX_MASK, cco_mux_value);
+	switch (aw_dev->chip_id) {
+	case AW88261_CHIP_ID:
+		ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
+				~AW88261_CCO_MUX_MASK, bypass_divider ?
+				AW88261_CCO_MUX_BYPASS_VALUE : AW88261_CCO_MUX_DIVIDED);
+		break;
+	case AW88258_CHIP_ID:
+		ret = regmap_update_bits(aw_dev->regmap, AW88258_PLLCTRL1_REG,
+				~AW88258_CCO_MUX_MASK, bypass_divider ?
+				AW88258_CCO_MUX_BYPASS_VALUE : AW88258_CCO_MUX_DIVIDED);
+		break;
+	default:
+		WARN_ONCE(1, "missing PLL regs for chip %x", aw_dev->chip_id);
+		return -ENXIO;
+	}
 	if (ret)
 		return ret;
 
@@ -306,7 +319,9 @@ static void aw88261_dev_uls_hmute(struct aw_device *aw_dev, bool uls_hmute)
 
 static void aw88261_reg_force_set(struct aw88261 *aw88261)
 {
-	if (aw88261->frcset_en == AW88261_FRCSET_ENABLE) {
+	if (aw88261->aw_pa->chip_id == AW88258_CHIP_ID) {
+		dev_dbg(aw88261->aw_pa->dev, "force_set not supported on aw88258");
+	} else if (aw88261->frcset_en == AW88261_FRCSET_ENABLE) {
 		/* set FORCE_PWM */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL3_REG,
 				AW88261_FORCE_PWM_MASK, AW88261_FORCE_PWM_FORCEMINUS_PWM_VALUE);
-- 
2.53.0