It's starting to look good, but the use of the dev_num is conceptually broken. The only way to reliably identify a device is the combination of link_id and unique_id. The information is present for both ACPI and DT systems. See comments below. -Pierre
+static void tas2783_apply_calib(struct tasdevice_priv *tas_dev,
- unsigned int *cali_data)
+{
- struct regmap *map = tas_dev->regmap;
- u8 *cali_start;
- u16 dev_num;
- int ret;
- if (!tas_dev->sdw_peripheral) {
dev_err(tas_dev->dev, "%s: peripheral doesn't exist.\n",__func__);return;- }
- dev_num = clamp(tas_dev->sdw_peripheral->dev_num, 1, 4) - 1;
Not following what restrictions you are trying to enforce on the device number. That's a value selected by the manager. You absolutely cannot assume the value is between 1 and 4, the max value is 11.
- /*
* The area saving tas2783 calibrated data is specified by its* dev_num. cali_start is the first address of current tas2783's* calibrated data.*/- cali_start = (u8 *)(cali_data + dev_num * sizeof(tas2783_cali_reg));
- for (int i = 0; i < ARRAY_SIZE(tas2783_cali_reg); i++) {
ret = regmap_bulk_write(map, tas2783_cali_reg[i],&cali_start[4 * i], 4);if (ret) {dev_err(tas_dev->dev, "Cali failed %x:%d\n",tas2783_cali_reg[i], ret);break;}- }
+}
+/*
- Load the calibration data, including speaker impedance, f0, etc.
- Calibration is done by the manufacturer in the factory. The calibration
- data are used by the algorithm for calculating the speaker temperature,
- speaker membrane excursion and f0 in real time during playback.
- The DSP will work with default data values if calibrated data are
- missing or are invalid.
- Layout of calibrated Data in UEFI:
- Calibrated Data of Dev 0 (20 bytes)
- Calibrated Data of Dev 1 (20 bytes)
- Calibrated Data of Dev 2 (20 bytes)
- Calibrated Data of Dev 3 (20 bytes)
You will have a hard-time matching those device indices with the dev_num, which depends on the enumeration order and the bus allocation.
The only stable board-specific value is to use a combination of link_id and unique_id (possibly controller id as well).
- CRC (4 bytes)
- */
+static int tasdevice_comp_probe(struct snd_soc_component *comp) +{
- struct tasdevice_priv *tas_dev = snd_soc_component_get_drvdata(comp);
- acpi_handle handle = ACPI_HANDLE(tas_dev->dev);
- const struct firmware *fw_entry = NULL;
- const char *sub = NULL;
- int ret, value_sdw;
- if (handle) {
sub = acpi_get_subsystem_id(handle);if (IS_ERR(sub))sub = NULL;- }
- tas_dev->component = comp;
- /*
* Each tas2783 in the system has its own dspfw.*/- if (comp->name_prefix) {
/** name_prefix.bin stores the dsp firmware including speaker* protection algorithm, audio acoustic algorithm, speaker* characters and algorithm params, it must be copied into* firmware folder.*/scnprintf(tas_dev->dspfw_binaryname,TAS2783_DSPFW_FILENAME_LEN, "%s-tas2783.bin",comp->name_prefix);- } else {
/* Compatible with the previous naming rule */if (sub) {/** subsystem_id-link_id[0,1,...,N]-dev_num[1,...,4].bin stores* the dsp firmware including speaker protection algorithm,* audio acoustic algorithm, speaker characters and algorithm* params, it must be copied into firmware folder.
no the dev_num cannot be used. It's only used for host-device communication and cannot be used to identify a device position.
The link_id+unique_unique is the only way to go.
But in addition you want want to consider a platform-specific prefix which contains the OEM name or device SKU.
*/scnprintf(tas_dev->dspfw_binaryname,TAS2783_DSPFW_FILENAME_LEN,"%s-%d-%d.bin", sub,tas_dev->sdw_peripheral->bus->link_id,tas_dev->sdw_peripheral->dev_num);} else {/** tas2783-link_id[0,1,...,N]-dev_num[1,...,4].bin stores* the dsp firmware including speaker protection algorithm,* audio acoustic algorithm, speaker characters and algorithm* params, it must be copied into firmware folder.*/scnprintf(tas_dev->dspfw_binaryname,TAS2783_DSPFW_FILENAME_LEN,"tas2783-%d-%d.bin",tas_dev->sdw_peripheral->bus->link_id,tas_dev->sdw_peripheral->dev_num);}- }
- ret = request_firmware(&fw_entry, tas_dev->dspfw_binaryname,
tas_dev->dev);- if (ret) {
dev_err(tas_dev->dev,"%s: request_firmware %x open status: %d.\n", __func__,tas_dev->sdw_peripheral->id.unique_id, ret);goto out;- }
- tasdevice_dspfw_ready(fw_entry, tas_dev);
- /* Select left/right channel based on device number. */
- value_sdw = 0x1a;
- value_sdw |= (tas_dev->sdw_peripheral->dev_num & BIT(0)) << 4;
- dev_dbg(tas_dev->dev, "%s: dev_num = %u", __func__,
tas_dev->sdw_peripheral->dev_num);- regmap_write(tas_dev->regmap, TAS2783_REG_TDM_RX_CFG, value_sdw);
- if (ret != 0)
dev_warn(tas_dev->dev, "%s: L/R setting failed: %d.\n",__func__, ret);+out:
- if (fw_entry)
release_firmware(fw_entry);- return 0;
+}
+static int tasdevice_io_init(struct device *dev,
- struct sdw_slave *slave)
+{
- struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
- int ret;
- regcache_cache_only(tas_priv->regmap, false);
- if (tas_priv->first_hw_init) {
regcache_cache_bypass(tas_priv->regmap, true);- } else {
- /*
* PM runtime is only enabled when a Slave reports as Attached* Update count of parent 'active' children*/
weird indentation for comments in multiple places.
pm_runtime_set_active(&slave->dev);- }
- /* sw reset */
- ret = regmap_write(tas_priv->regmap, TAS2873_REG_SWRESET,
TAS2873_REG_SWRESET_RESET);- if (ret) {
dev_err(tas_priv->dev, "Reset failed.\n");goto out;- }
- if (tas_priv->first_hw_init) {
regcache_cache_bypass(tas_priv->regmap, false);regcache_mark_dirty(tas_priv->regmap);- }
- tas_priv->first_hw_init = true;
- tas_priv->hw_init = true;
+out:
- return ret;
+}