[PATCH RESEND 1/4] mfd: cs42l43: Prepare support for updated bios patch

[PATCH RESEND 1/4] mfd: cs42l43: Prepare support for updated bios patch

[Charles Keepax]

From: Maciej Strozek <mstrozek@opensource.cirrus.com>

Newer bios patch firmware versions now require use of the shadow register
interface, which was previously only required by the full firmware, update
the check accordingly.

Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/cs42l43.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index e5f17fc430e4..11a1b7f1e121 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -48,6 +48,7 @@
 
 #define CS42L43_MCU_SUPPORTED_REV		0x2105
 #define CS42L43_MCU_SHADOW_REGS_REQUIRED_REV	0x2200
+#define CS42L43_BIOS_SHADOW_REGS_REQUIRED_REV	0x1002
 #define CS42L43_MCU_SUPPORTED_BIOS_REV		0x0001
 
 #define CS42L43_VDDP_DELAY_US			50
@@ -773,7 +774,8 @@ static int cs42l43_mcu_update_step(struct cs42l43 *cs42l43)
 	 * Later versions of the firmwware require the driver to access some
 	 * features through a set of shadow registers.
 	 */
-	shadow = mcu_rev >= CS42L43_MCU_SHADOW_REGS_REQUIRED_REV;
+	shadow = (mcu_rev >= CS42L43_MCU_SHADOW_REGS_REQUIRED_REV) ||
+		 (bios_rev >= CS42L43_BIOS_SHADOW_REGS_REQUIRED_REV);
 
 	ret = regmap_read(cs42l43->regmap, CS42L43_BOOT_CONTROL, &secure_cfg);
 	if (ret) {
-- 
2.39.5

[PATCH RESEND 2/4] mfd: cs42l43: Use gpiod_set_raw for GPIO operations

[Charles Keepax]

The GPIO framework supports specifying if a GPIO is active low or
high and will invert accordingly. Whilst specifying this is part of
the normal GPIO definition flow on device tree systems, it is a DSD
extension under ACPI, that Windows doesn't really use. This means most
ACPI systems do not set the polarity of the pin.

The current cs42l43 driver assumes it is setting the level of the line
directly, which is actually the case on all current systems and likely
most future ones. However if the part was used in a device tree system
or an ACPI system that actually used the DSD extensions this would get
inverted, causing the driver to fail probe. As the driver always knows
the polarity of its own reset line, use the raw set API making the
intention to set the level directly clear and to avoid any such future
issues.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/cs42l43.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index 11a1b7f1e121..167d4060b230 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -984,7 +984,7 @@ static int cs42l43_power_up(struct cs42l43 *cs42l43)
 	/* vdd-p must be on for 50uS before any other supply */
 	usleep_range(CS42L43_VDDP_DELAY_US, 2 * CS42L43_VDDP_DELAY_US);
 
-	gpiod_set_value_cansleep(cs42l43->reset, 1);
+	gpiod_set_raw_value_cansleep(cs42l43->reset, 1);
 
 	ret = regulator_bulk_enable(CS42L43_N_SUPPLIES, cs42l43->core_supplies);
 	if (ret) {
@@ -1005,7 +1005,7 @@ static int cs42l43_power_up(struct cs42l43 *cs42l43)
 err_core_supplies:
 	regulator_bulk_disable(CS42L43_N_SUPPLIES, cs42l43->core_supplies);
 err_reset:
-	gpiod_set_value_cansleep(cs42l43->reset, 0);
+	gpiod_set_raw_value_cansleep(cs42l43->reset, 0);
 	regulator_disable(cs42l43->vdd_p);
 
 	return ret;
@@ -1027,7 +1027,7 @@ static int cs42l43_power_down(struct cs42l43 *cs42l43)
 		return ret;
 	}
 
-	gpiod_set_value_cansleep(cs42l43->reset, 0);
+	gpiod_set_raw_value_cansleep(cs42l43->reset, 0);
 
 	ret = regulator_disable(cs42l43->vdd_p);
 	if (ret) {
@@ -1052,11 +1052,13 @@ int cs42l43_dev_probe(struct cs42l43 *cs42l43)
 
 	regcache_cache_only(cs42l43->regmap, true);
 
-	cs42l43->reset = devm_gpiod_get_optional(cs42l43->dev, "reset", GPIOD_OUT_LOW);
+	cs42l43->reset = devm_gpiod_get_optional(cs42l43->dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(cs42l43->reset))
 		return dev_err_probe(cs42l43->dev, PTR_ERR(cs42l43->reset),
 				     "Failed to get reset\n");
 
+	gpiod_set_raw_value_cansleep(cs42l43->reset, 0);
+
 	cs42l43->vdd_p = devm_regulator_get(cs42l43->dev, "vdd-p");
 	if (IS_ERR(cs42l43->vdd_p))
 		return dev_err_probe(cs42l43->dev, PTR_ERR(cs42l43->vdd_p),
-- 
2.39.5

[PATCH 3/4] mfd: cs42l43: Increase the SoundWire attach timeout

[Charles Keepax]

Some SoundWire controllers take a very long time to fully power up. As
such, increase the timeout that the cs42l43 driver will wait for the
device to initially appear on the bus.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/cs42l43.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index 167d4060b230..9572c7fd419a 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -29,7 +29,7 @@
 
 #define CS42L43_RESET_DELAY_MS			20
 
-#define CS42L43_SDW_ATTACH_TIMEOUT_MS		500
+#define CS42L43_SDW_ATTACH_TIMEOUT_MS		5000
 #define CS42L43_SDW_DETACH_TIMEOUT_MS		100
 
 #define CS42L43_MCU_BOOT_STAGE1			1
-- 
2.39.5

[PATCH 4/4] mfd: cs42l43: Use devres for remove as well

[Charles Keepax]

Currently the device is powered down in the remove callback, however
all other clean up is done through devres. The problem here is the
MFD children are cleaned up through devres. As this happens after
the remove callback has run, this leads to the incorrect ordering
where the child remove functions run after the device has been powered
down. Put the power down into devres as well such that everything runs
in the expected order.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/cs42l43-i2c.c |  8 --------
 drivers/mfd/cs42l43-sdw.c | 10 ----------
 drivers/mfd/cs42l43.c     | 21 +++++++++++++--------
 drivers/mfd/cs42l43.h     |  1 -
 4 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/mfd/cs42l43-i2c.c b/drivers/mfd/cs42l43-i2c.c
index c9e4ea76149a..1e6422cdf012 100644
--- a/drivers/mfd/cs42l43-i2c.c
+++ b/drivers/mfd/cs42l43-i2c.c
@@ -56,13 +56,6 @@ static int cs42l43_i2c_probe(struct i2c_client *i2c)
 	return cs42l43_dev_probe(cs42l43);
 }
 
-static void cs42l43_i2c_remove(struct i2c_client *i2c)
-{
-	struct cs42l43 *cs42l43 = dev_get_drvdata(&i2c->dev);
-
-	cs42l43_dev_remove(cs42l43);
-}
-
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id cs42l43_of_match[] = {
 	{ .compatible = "cirrus,cs42l43", },
@@ -88,7 +81,6 @@ static struct i2c_driver cs42l43_i2c_driver = {
 	},
 
 	.probe		= cs42l43_i2c_probe,
-	.remove		= cs42l43_i2c_remove,
 };
 module_i2c_driver(cs42l43_i2c_driver);
 
diff --git a/drivers/mfd/cs42l43-sdw.c b/drivers/mfd/cs42l43-sdw.c
index 65f7b1d78248..6af8465b2099 100644
--- a/drivers/mfd/cs42l43-sdw.c
+++ b/drivers/mfd/cs42l43-sdw.c
@@ -187,15 +187,6 @@ static int cs42l43_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *
 	return cs42l43_dev_probe(cs42l43);
 }
 
-static int cs42l43_sdw_remove(struct sdw_slave *sdw)
-{
-	struct cs42l43 *cs42l43 = dev_get_drvdata(&sdw->dev);
-
-	cs42l43_dev_remove(cs42l43);
-
-	return 0;
-}
-
 static const struct sdw_device_id cs42l43_sdw_id[] = {
 	SDW_SLAVE_ENTRY(0x01FA, 0x4243, 0),
 	{}
@@ -209,7 +200,6 @@ static struct sdw_driver cs42l43_sdw_driver = {
 	},
 
 	.probe		= cs42l43_sdw_probe,
-	.remove		= cs42l43_sdw_remove,
 	.id_table	= cs42l43_sdw_id,
 	.ops		= &cs42l43_sdw_ops,
 };
diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index 9572c7fd419a..beb63c4efd21 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -1038,6 +1038,15 @@ static int cs42l43_power_down(struct cs42l43 *cs42l43)
 	return 0;
 }
 
+static void cs42l43_dev_remove(void *data)
+{
+	struct cs42l43 *cs42l43 = data;
+
+	cancel_work_sync(&cs42l43->boot_work);
+
+	cs42l43_power_down(cs42l43);
+}
+
 int cs42l43_dev_probe(struct cs42l43 *cs42l43)
 {
 	int i, ret;
@@ -1084,6 +1093,10 @@ int cs42l43_dev_probe(struct cs42l43 *cs42l43)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(cs42l43->dev, cs42l43_dev_remove, cs42l43);
+	if (ret)
+		return ret;
+
 	pm_runtime_set_autosuspend_delay(cs42l43->dev, CS42L43_AUTOSUSPEND_TIME_MS);
 	pm_runtime_use_autosuspend(cs42l43->dev);
 	pm_runtime_set_active(cs42l43->dev);
@@ -1102,14 +1115,6 @@ int cs42l43_dev_probe(struct cs42l43 *cs42l43)
 }
 EXPORT_SYMBOL_NS_GPL(cs42l43_dev_probe, MFD_CS42L43);
 
-void cs42l43_dev_remove(struct cs42l43 *cs42l43)
-{
-	cancel_work_sync(&cs42l43->boot_work);
-
-	cs42l43_power_down(cs42l43);
-}
-EXPORT_SYMBOL_NS_GPL(cs42l43_dev_remove, MFD_CS42L43);
-
 static int cs42l43_suspend(struct device *dev)
 {
 	struct cs42l43 *cs42l43 = dev_get_drvdata(dev);
diff --git a/drivers/mfd/cs42l43.h b/drivers/mfd/cs42l43.h
index 8d1b1b0f5a47..f3da783930f5 100644
--- a/drivers/mfd/cs42l43.h
+++ b/drivers/mfd/cs42l43.h
@@ -25,6 +25,5 @@ bool cs42l43_precious_register(struct device *dev, unsigned int reg);
 bool cs42l43_volatile_register(struct device *dev, unsigned int reg);
 
 int cs42l43_dev_probe(struct cs42l43 *cs42l43);
-void cs42l43_dev_remove(struct cs42l43 *cs42l43);
 
 #endif /* CS42L43_CORE_INT_H */
-- 
2.39.5

Re: [PATCH RESEND 1/4] mfd: cs42l43: Prepare support for updated bios patch

[Lee Jones]

On Thu, 05 Dec 2024 11:58:19 +0000, Charles Keepax wrote:
> Newer bios patch firmware versions now require use of the shadow register
> interface, which was previously only required by the full firmware, update
> the check accordingly.
> 
> 

Applied, thanks!

[1/4] mfd: cs42l43: Prepare support for updated bios patch
      commit: 049d5ca76637d305650e68332ecf5cfb18d74b0b
[2/4] mfd: cs42l43: Use gpiod_set_raw for GPIO operations
      commit: 5fb65a3749286b8fb1c8c0b1ccc6ed7caedf3632
[3/4] mfd: cs42l43: Increase the SoundWire attach timeout
      commit: 7c28a3909ccd45ab4d560ef1e57fbc0890fbf6e9
[4/4] mfd: cs42l43: Use devres for remove as well
      commit: c5bb88eac10f5a69b2c281edaed39f25e096e43a

--
Lee Jones [李琼斯]