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@ti.com
...
+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__ */