[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8ab556fc-2bbe-486a-ac19-05226134e66c@opensource.cirrus.com>
Date: Mon, 28 Apr 2025 18:34:31 +0100
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: Stefan Binding <sbinding@...nsource.cirrus.com>,
Mark Brown <broonie@...nel.org>
Cc: linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
patches@...nsource.cirrus.com
Subject: Re: [PATCH v1 2/5] ASoC: cs35l56: Add struct to index firmware
registers
On 7/4/25 16:16, Stefan Binding wrote:
> Firmware based registers may be different addresses across different
> device ids and revision ids. Create a structure to store and access
> these addresses.
>
> Signed-off-by: Stefan Binding <sbinding@...nsource.cirrus.com>
> ---
> include/sound/cs35l56.h | 11 ++++++++
> sound/pci/hda/cs35l56_hda.c | 5 ++--
> sound/pci/hda/cs35l56_hda_i2c.c | 3 +++
> sound/pci/hda/cs35l56_hda_spi.c | 3 +++
> sound/soc/codecs/cs35l56-i2c.c | 1 +
> sound/soc/codecs/cs35l56-sdw.c | 1 +
> sound/soc/codecs/cs35l56-shared.c | 42 ++++++++++++++++++++++++-------
> sound/soc/codecs/cs35l56-spi.c | 3 +++
> sound/soc/codecs/cs35l56.c | 5 ++--
> 9 files changed, 61 insertions(+), 13 deletions(-)
>
> diff --git a/include/sound/cs35l56.h b/include/sound/cs35l56.h
> index 5d653a3491d0..d712cb79652b 100644
> --- a/include/sound/cs35l56.h
> +++ b/include/sound/cs35l56.h
> @@ -267,6 +267,14 @@ struct cs35l56_spi_payload {
> } __packed;
> static_assert(sizeof(struct cs35l56_spi_payload) == 10);
>
> +struct cs35l56_fw_reg {
> + unsigned int fw_ver;
> + unsigned int halo_state;
> + unsigned int pm_cur_stat;
> + unsigned int prot_sts;
> + unsigned int transducer_actual_ps;
> +};
> +
> struct cs35l56_base {
> struct device *dev;
> struct regmap *regmap;
> @@ -283,6 +291,7 @@ struct cs35l56_base {
> struct cirrus_amp_cal_data cal_data;
> struct gpio_desc *reset_gpio;
> struct cs35l56_spi_payload *spi_payload_buf;
> + const struct cs35l56_fw_reg *fw_reg;
> };
>
> static inline bool cs35l56_is_otp_register(unsigned int reg)
> @@ -311,6 +320,8 @@ extern const struct regmap_config cs35l56_regmap_i2c;
> extern const struct regmap_config cs35l56_regmap_spi;
> extern const struct regmap_config cs35l56_regmap_sdw;
>
> +extern const struct cs35l56_fw_reg cs35l56_fw_reg;
> +
> extern const struct cirrus_amp_cal_controls cs35l56_calibration_controls;
>
> extern const char * const cs35l56_tx_input_texts[CS35L56_NUM_INPUT_SRC];
> diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
> index 4ef7878e8fd4..f6257fe45b9f 100644
> --- a/sound/pci/hda/cs35l56_hda.c
> +++ b/sound/pci/hda/cs35l56_hda.c
> @@ -68,7 +68,7 @@ static void cs35l56_hda_play(struct cs35l56_hda *cs35l56)
> if (ret == 0) {
> /* Wait for firmware to enter PS0 power state */
> ret = regmap_read_poll_timeout(cs35l56->base.regmap,
> - CS35L56_TRANSDUCER_ACTUAL_PS,
> + cs35l56->base.fw_reg->transducer_actual_ps,
> val, (val == CS35L56_PS0),
> CS35L56_PS0_POLL_US,
> CS35L56_PS0_TIMEOUT_US);
> @@ -667,7 +667,8 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
>
> regcache_sync(cs35l56->base.regmap);
>
> - regmap_clear_bits(cs35l56->base.regmap, CS35L56_PROTECTION_STATUS,
> + regmap_clear_bits(cs35l56->base.regmap,
> + cs35l56->base.fw_reg->prot_sts,
> CS35L56_FIRMWARE_MISSING);
> cs35l56->base.fw_patched = true;
>
> diff --git a/sound/pci/hda/cs35l56_hda_i2c.c b/sound/pci/hda/cs35l56_hda_i2c.c
> index c7b836613149..d10209e4eddd 100644
> --- a/sound/pci/hda/cs35l56_hda_i2c.c
> +++ b/sound/pci/hda/cs35l56_hda_i2c.c
> @@ -26,6 +26,9 @@ static int cs35l56_hda_i2c_probe(struct i2c_client *clt)
> #ifdef CS35L56_WAKE_HOLD_TIME_US
> cs35l56->base.can_hibernate = true;
> #endif
> +
> + cs35l56->base.fw_reg = &cs35l56_fw_reg;
> +
> cs35l56->base.regmap = devm_regmap_init_i2c(clt, &cs35l56_regmap_i2c);
> if (IS_ERR(cs35l56->base.regmap)) {
> ret = PTR_ERR(cs35l56->base.regmap);
> diff --git a/sound/pci/hda/cs35l56_hda_spi.c b/sound/pci/hda/cs35l56_hda_spi.c
> index 903578466905..f57533d3d728 100644
> --- a/sound/pci/hda/cs35l56_hda_spi.c
> +++ b/sound/pci/hda/cs35l56_hda_spi.c
> @@ -29,6 +29,9 @@ static int cs35l56_hda_spi_probe(struct spi_device *spi)
> #ifdef CS35L56_WAKE_HOLD_TIME_US
> cs35l56->base.can_hibernate = true;
> #endif
> +
> + cs35l56->base.fw_reg = &cs35l56_fw_reg;
> +
> cs35l56->base.regmap = devm_regmap_init_spi(spi, &cs35l56_regmap_spi);
> if (IS_ERR(cs35l56->base.regmap)) {
> ret = PTR_ERR(cs35l56->base.regmap);
> diff --git a/sound/soc/codecs/cs35l56-i2c.c b/sound/soc/codecs/cs35l56-i2c.c
> index 5962914e2180..38c391d11c78 100644
> --- a/sound/soc/codecs/cs35l56-i2c.c
> +++ b/sound/soc/codecs/cs35l56-i2c.c
> @@ -35,6 +35,7 @@ static int cs35l56_i2c_probe(struct i2c_client *client)
> switch (id) {
> case 0x3556:
> regmap_config = &cs35l56_regmap_i2c;
> + cs35l56->base.fw_reg = &cs35l56_fw_reg;
> break;
> default:
> return -ENODEV;
> diff --git a/sound/soc/codecs/cs35l56-sdw.c b/sound/soc/codecs/cs35l56-sdw.c
> index d178357e1196..2e0422b41385 100644
> --- a/sound/soc/codecs/cs35l56-sdw.c
> +++ b/sound/soc/codecs/cs35l56-sdw.c
> @@ -526,6 +526,7 @@ static int cs35l56_sdw_probe(struct sdw_slave *peripheral, const struct sdw_devi
> case 0x3556:
> case 0x3557:
> regmap_config = &cs35l56_regmap_sdw;
> + cs35l56->base.fw_reg = &cs35l56_fw_reg;
> break;
> default:
> return -ENODEV;
> diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
> index e28bfefa72f3..bc8f9379bc74 100644
> --- a/sound/soc/codecs/cs35l56-shared.c
> +++ b/sound/soc/codecs/cs35l56-shared.c
> @@ -253,7 +253,8 @@ int cs35l56_firmware_shutdown(struct cs35l56_base *cs35l56_base)
> if (ret)
> return ret;
>
> - ret = regmap_read_poll_timeout(cs35l56_base->regmap, CS35L56_DSP1_PM_CUR_STATE,
> + ret = regmap_read_poll_timeout(cs35l56_base->regmap,
> + cs35l56_base->fw_reg->pm_cur_stat,
> val, (val == CS35L56_HALO_STATE_SHUTDOWN),
> CS35L56_HALO_STATE_POLL_US,
> CS35L56_HALO_STATE_TIMEOUT_US);
> @@ -278,7 +279,9 @@ int cs35l56_wait_for_firmware_boot(struct cs35l56_base *cs35l56_base)
> CS35L56_HALO_STATE_POLL_US,
> CS35L56_HALO_STATE_TIMEOUT_US,
> false,
> - cs35l56_base->regmap, CS35L56_DSP1_HALO_STATE, &val);
> + cs35l56_base->regmap,
> + cs35l56_base->fw_reg->halo_state,
> + &val);
>
> if (poll_ret) {
> dev_err(cs35l56_base->dev, "Firmware boot timed out(%d): HALO_STATE=%#x\n",
> @@ -395,9 +398,17 @@ void cs35l56_system_reset(struct cs35l56_base *cs35l56_base, bool is_soundwire)
> return;
> }
>
> - regmap_multi_reg_write_bypassed(cs35l56_base->regmap,
> - cs35l56_system_reset_seq,
> - ARRAY_SIZE(cs35l56_system_reset_seq));
> + switch (cs35l56_base->type) {
> + case 0x54:
> + case 0x56:
> + case 0x57:
> + regmap_multi_reg_write_bypassed(cs35l56_base->regmap,
> + cs35l56_system_reset_seq,
> + ARRAY_SIZE(cs35l56_system_reset_seq));
> + break;
> + default:
> + break;
> + }
>
> /* On SoundWire the registers won't be accessible until it re-enumerates. */
> if (is_soundwire)
> @@ -514,7 +525,9 @@ int cs35l56_is_fw_reload_needed(struct cs35l56_base *cs35l56_base)
> return ret;
> }
>
> - ret = regmap_read(cs35l56_base->regmap, CS35L56_PROTECTION_STATUS, &val);
> + ret = regmap_read(cs35l56_base->regmap,
> + cs35l56_base->fw_reg->prot_sts,
> + &val);
> if (ret)
> dev_err(cs35l56_base->dev, "Failed to read PROTECTION_STATUS: %d\n", ret);
> else
> @@ -562,7 +575,7 @@ int cs35l56_runtime_suspend_common(struct cs35l56_base *cs35l56_base)
>
> /* Firmware must have entered a power-save state */
> ret = regmap_read_poll_timeout(cs35l56_base->regmap,
> - CS35L56_TRANSDUCER_ACTUAL_PS,
> + cs35l56_base->fw_reg->transducer_actual_ps,
> val, (val >= CS35L56_PS3),
> CS35L56_PS3_POLL_US,
> CS35L56_PS3_TIMEOUT_US);
> @@ -752,7 +765,8 @@ int cs35l56_read_prot_status(struct cs35l56_base *cs35l56_base,
> unsigned int prot_status;
> int ret;
>
> - ret = regmap_read(cs35l56_base->regmap, CS35L56_PROTECTION_STATUS, &prot_status);
> + ret = regmap_read(cs35l56_base->regmap,
> + cs35l56_base->fw_reg->prot_sts, &prot_status);
> if (ret) {
> dev_err(cs35l56_base->dev, "Get PROTECTION_STATUS failed: %d\n", ret);
> return ret;
> @@ -760,7 +774,8 @@ int cs35l56_read_prot_status(struct cs35l56_base *cs35l56_base,
>
> *fw_missing = !!(prot_status & CS35L56_FIRMWARE_MISSING);
>
> - ret = regmap_read(cs35l56_base->regmap, CS35L56_DSP1_FW_VER, fw_version);
> + ret = regmap_read(cs35l56_base->regmap,
> + cs35l56_base->fw_reg->fw_ver, fw_version);
> if (ret) {
> dev_err(cs35l56_base->dev, "Get FW VER failed: %d\n", ret);
> return ret;
> @@ -1045,6 +1060,15 @@ const struct regmap_config cs35l56_regmap_sdw = {
> };
> EXPORT_SYMBOL_NS_GPL(cs35l56_regmap_sdw, "SND_SOC_CS35L56_SHARED");
>
> +const struct cs35l56_fw_reg cs35l56_fw_reg = {
> + .fw_ver = CS35L56_DSP1_FW_VER,
> + .halo_state = CS35L56_DSP1_HALO_STATE,
> + .pm_cur_stat = CS35L56_DSP1_PM_CUR_STATE,
> + .prot_sts = CS35L56_PROTECTION_STATUS,
> + .transducer_actual_ps = CS35L56_TRANSDUCER_ACTUAL_PS,
> +};
> +EXPORT_SYMBOL_NS_GPL(cs35l56_fw_reg, "SND_SOC_CS35L56_SHARED");
> +
> MODULE_DESCRIPTION("ASoC CS35L56 Shared");
> MODULE_AUTHOR("Richard Fitzgerald <rf@...nsource.cirrus.com>");
> MODULE_AUTHOR("Simon Trimmer <simont@...nsource.cirrus.com>");
> diff --git a/sound/soc/codecs/cs35l56-spi.c b/sound/soc/codecs/cs35l56-spi.c
> index ca6c03a8766d..c2ddee22cd23 100644
> --- a/sound/soc/codecs/cs35l56-spi.c
> +++ b/sound/soc/codecs/cs35l56-spi.c
> @@ -25,6 +25,9 @@ static int cs35l56_spi_probe(struct spi_device *spi)
> return -ENOMEM;
>
> spi_set_drvdata(spi, cs35l56);
> +
> + cs35l56->base.fw_reg = &cs35l56_fw_reg;
> +
> cs35l56->base.regmap = devm_regmap_init_spi(spi, regmap_config);
> if (IS_ERR(cs35l56->base.regmap)) {
> ret = PTR_ERR(cs35l56->base.regmap);
> diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
> index b3158a84b87a..c1d8bfb803b9 100644
> --- a/sound/soc/codecs/cs35l56.c
> +++ b/sound/soc/codecs/cs35l56.c
> @@ -174,7 +174,7 @@ static int cs35l56_play_event(struct snd_soc_dapm_widget *w,
> case SND_SOC_DAPM_POST_PMU:
> /* Wait for firmware to enter PS0 power state */
> ret = regmap_read_poll_timeout(cs35l56->base.regmap,
> - CS35L56_TRANSDUCER_ACTUAL_PS,
> + cs35l56->base.fw_reg->transducer_actual_ps,
> val, (val == CS35L56_PS0),
> CS35L56_PS0_POLL_US,
> CS35L56_PS0_TIMEOUT_US);
> @@ -760,7 +760,8 @@ static void cs35l56_patch(struct cs35l56_private *cs35l56, bool firmware_missing
> goto err_unlock;
> }
>
> - regmap_clear_bits(cs35l56->base.regmap, CS35L56_PROTECTION_STATUS,
> + regmap_clear_bits(cs35l56->base.regmap,
> + cs35l56->base.fw_reg->prot_sts,
> CS35L56_FIRMWARE_MISSING);
> cs35l56->base.fw_patched = true;
>
Reviewed-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
Powered by blists - more mailing lists