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;
+}
