On 3/5/2024 2:26 PM, Shenghao Ding wrote:
The tas2783 is a smart audio amplifier with integrated MIPI SoundWire
interface (Version 1.2.1 compliant), I2C, and I2S/TDM interfaces designed
for portable applications. An on-chip DSP supports Texas Instruments
SmartAmp speaker protection algorithm. The integrated speaker voltage and
current sense provides for real-time monitoring of loudspeakers.
The ASoC component provides the majority of the functionality of the
device, all the audio functions.
Signed-off-by: Shenghao Ding <shenghao-ding@xxxxxx>
---
...
+
+static void tas2783_apply_calibv2(struct tasdevice_priv *tas_dev,
+ unsigned int *cali_data)
+{
+ const unsigned int arr_size = ARRAY_SIZE(tas2783_cali_reg);
+ struct regmap *map = tas_dev->regmap;
+ unsigned int dev_sum = cali_data[1], i, j, k;
+ u8 *cali_start;
+ u16 dev_info;
+ int ret;
+
+ if (!tas_dev->sdw_peripheral) {
+ dev_err(tas_dev->dev, "%s: peripheral doesn't exist.\n",
+ __func__);
+ return;
+ }
+
+ dev_info = tas_dev->sdw_peripheral->bus->link_id |
+ tas_dev->sdw_peripheral->id.unique_id << 16;
+
+ /*
+ * The area saving tas2783 calibrated data is specified by its
+ * unique_id offset. cali_start is the first address of current
+ * tas2783's calibrated data.
+ */
+ cali_start = (u8 *)&cali_data[3];
+ for (i = 0; i < dev_sum; i++) {
+ k = i * (arr_size + 1) + 3;
+ if (dev_info != cali_data[k]) {
+ for (j = 0; j < arr_size; j++) {
+ k = 4 * (k + 1 + j);
+ ret = regmap_bulk_write(map,
+ tas2783_cali_reg[j],
+ &cali_start[k], 4);
+ if (ret) {
+ dev_err(tas_dev->dev,
+ "Cali failed %x:%d\n",
+ tas2783_cali_reg[j], ret);
+ break;
+ }
+ }
+ break;
+ }
+ }
This seems a bit hard to read, any chance to do some reordering to make
it more readable?
+}
+
...
+
+static int tasdevice_mute(struct snd_soc_dai *dai, int mute,
+ int direction)
+{
+ struct snd_soc_component *component = dai->component;
+ struct tasdevice_priv *tas_dev =
+ snd_soc_component_get_drvdata(component);
+ struct regmap *map = tas_dev->regmap;
+ int ret;
+
+ dev_dbg(tas_dev->dev, "%s: %d.\n", __func__, mute);
+
+ if (mute) {
+ if (direction == SNDRV_PCM_STREAM_CAPTURE) {
+ ret = regmap_update_bits(map, TAS2873_REG_PWR_CTRL,
+ TAS2783_REG_AEF_MASK,
+ TAS2783_REG_AEF_INACTIVE);
+ if (ret)
+ dev_err(tas_dev->dev,
+ "%s: Disable AEF failed.\n", __func__);
+ } else {
+ /* FU23 mute (0x40400108) */
+ ret = regmap_write(map,
+ SDW_SDCA_CTL(TAS2783_FUNC_TYPE_SMART_AMP,
+ TAS2783_SDCA_ENT_FU23,
+ TAS2783_SDCA_CTL_FU_MUTE, 0), 1);
+ if (ret) {
+ dev_err(tas_dev->dev,
+ "%s: FU23 mute failed.\n", __func__);
+ goto out;
+ }
+ /*
+ * Both playback and echo data will be shutdown in
+ * playback stream.
+ */
+ ret = regmap_update_bits(map, TAS2873_REG_PWR_CTRL,
+ TAS2783_REG_PWR_MODE_MASK |
+ TAS2783_REG_AEF_MASK,
+ TAS2783_REG_PWR_MODE_ACTIVE |
+ TAS2783_REG_PWR_MODE_SW_PWD);
+ if (ret) {
+ dev_err(tas_dev->dev,
+ "%s: PWR&AEF shutdown failed.\n",
+ __func__);
+ goto out;
+ }
+ tas_dev->pstream = false;
+ }
+ } else {
+ /* FU23 Unmute, 0x40400108. */
+ if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
+ ret = regmap_write(map,
+ SDW_SDCA_CTL(TAS2783_FUNC_TYPE_SMART_AMP,
+ TAS2783_SDCA_ENT_FU23,
+ TAS2783_SDCA_CTL_FU_MUTE, 0), 0);
+ if (ret) {
+ dev_err(tas_dev->dev,
+ "%s: FU23 Unmute failed.\n", __func__);
+ goto out;
+ }
+ ret = regmap_update_bits(map, TAS2873_REG_PWR_CTRL,
+ TAS2783_REG_PWR_MODE_MASK,
+ TAS2783_REG_PWR_MODE_ACTIVE);
+ if (ret) {
+ dev_err(tas_dev->dev,
+ "%s: PWR Unmute failed.\n", __func__);
+ goto out;
+ }
+ tas_dev->pstream = true;
+ } else {
+ /* Capture stream is the echo ref data for voice.
+ * Without playback, it can't be active.
+ */
+ if (tas_dev->pstream == true) {
+ ret = regmap_update_bits(map,
+ TAS2873_REG_PWR_CTRL,
+ TAS2783_REG_AEF_MASK,
+ TAS2783_REG_AEF_ACTIVE);
+ if (ret) {
+ dev_err(tas_dev->dev,
+ "%s: AEF enable failed.\n",
+ __func__);
+ goto out;
+ }
+ } else {
+ dev_err(tas_dev->dev,
+ "%s: No playback, no AEF!", __func__);
+ ret = -EINVAL;
+ }
+ }
+ }
+out:
+ if (ret)
+ dev_err(tas_dev->dev, "Mute or unmute %d failed %d.\n",
+ mute, ret);
+
+ return ret;
+}
Above function seem to be bit long, which also causes a lot of
indentation, perhaps split it into mute and unmute helpers?
...
+
+static int tasdevice_read_prop(struct sdw_slave *slave)
+{
+ struct sdw_slave_prop *prop = &slave->prop;
+ struct sdw_dpn_prop *dpn;
+ unsigned long addr;
+ int nval, i, j;
+ u32 bit;
+
+ prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
+ prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY;
+
+ prop->paging_support = true;
+
+ /* first we need to allocate memory for set bits in port lists */
+ prop->source_ports = BIT(2); /* BITMAP: 00000100 */
+ prop->sink_ports = BIT(1); /* BITMAP: 00000010 */
+
+ nval = hweight32(prop->source_ports);
+ prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
+ sizeof(*prop->src_dpn_prop), GFP_KERNEL);
+ if (!prop->src_dpn_prop)
+ return -ENOMEM;
+
+ i = 0;
+ dpn = prop->src_dpn_prop;
+ addr = prop->source_ports;
+ for_each_set_bit(bit, &addr, 32) {
+ dpn[i].num = bit;
+ dpn[i].type = SDW_DPN_FULL;
+ dpn[i].simple_ch_prep_sm = true;
+ dpn[i].ch_prep_timeout = 10;
+ i++;
+ }
+
+ /* do this again for sink now */
+ nval = hweight32(prop->sink_ports);
+ prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
+ sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
+ if (!prop->sink_dpn_prop)
+ return -ENOMEM;
+
+ j = 0;
No need for separate j variable, you can reuse i here.
+ dpn = prop->sink_dpn_prop;
+ addr = prop->sink_ports;
+ for_each_set_bit(bit, &addr, 32) {
+ dpn[j].num = bit;
+ dpn[j].type = SDW_DPN_FULL;
+ dpn[j].simple_ch_prep_sm = true;
+ dpn[j].ch_prep_timeout = 10;
+ j++;
+ }
+
+ /* set the timeout values */
+ prop->clk_stop_timeout = 20;
+
+ return 0;
+}
+
...
+
+struct tasdevice_priv {
+ struct snd_soc_component *component;
Apart from being assigned this field seems to be unused.
+ struct sdw_slave *sdw_peripheral;
+ enum sdw_slave_status status;
This one seems to be only used in tasdevice_update_status()? Does it
really need to be kept in struct?
+ struct sdw_bus_params params;
Unused?
+ struct regmap *regmap;
+ struct device *dev;
+ unsigned char dspfw_binaryname[TAS2783_DSPFW_FILENAME_LEN];
This one also seems weird, it is mainly needed when loading FW and could
be local to tasdevice_comp_probe(), although there is one dev_warn which
uses it outside of it, but pretty sure it could be dropped.
+ unsigned char dev_name[32];
Another unused field.
+ unsigned int chip_id;
Another one that only seems to be assigned.
+ bool pstream;
+ bool hw_init;
+ bool first_hw_init;
+};
+
+#endif /*__TAS2783_H__ */
