[alsa-devel] [PATCH v2 2/2] ASoC: adau17x1: Implemented safeload support
Danny Smith
danny.smith at axis.com
Fri Aug 17 10:06:22 CEST 2018
On 08/13/2018 12:44 PM, Lars-Peter Clausen wrote:
> On 08/13/2018 09:33 AM, Robert Rosengren wrote:
>> From: Danny Smith <dannys at axis.com>
>>
>> Safeload support has been implemented which is used
>> when updating for instance filter parameters using
>> alsa controls. Without safeload support audio can
>> become distorted during update.
>>
>> Signed-off-by: Danny Smith <dannys at axis.com>
>> Signed-off-by: Robert Rosengren <robertr at axis.com>
> Hi,
>
> Thanks for implementing this. Some comments inline.
>
> [...]
>> +static int adau17x1_safeload(struct sigmadsp *sigmadsp, unsigned int addr,
>> + const uint8_t bytes[], size_t len)
>> +{
>> + uint8_t buf[4];
>> + unsigned int i;
>> + int ret;
>> +
>> + /* target address is offset by 1 */
>> + unsigned int addr_offset = addr - 1;
>> +
>> + /* write safeload addresses */
>> + for (i = 0; i < len / 4; i++) {
>> + memcpy(buf, bytes + i * 4, 4);
> Is this memcpy really required or could we just write bytes directly in a
> single regmap_raw_write()? I believe the adau17x1 will auto increment the
> register addresses.
>
> And I suppose we also need to handle the case when len is not a multiple of 4.
A single regmap_raw_write should work, forgot about the addresses auto
incrementing.
What would be the proper way of handling the case where len is not a
multiple of 4?
>
>> + ret = regmap_raw_write(sigmadsp->control_data,
>> + ADAU17X1_SAFELOAD_DATA(i), buf, 4);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + /* write target address */
>> + buf[0] = (addr_offset >> 24) & 0xff;
>> + buf[1] = (addr_offset >> 16) & 0xff;
>> + buf[2] = (addr_offset >> 8) & 0xff;
>> + buf[3] = (addr_offset) & 0xff;
> This could be slightly simplified using
>
> put_unaligend_be32(addr_offset, buf);
>
>> + ret = regmap_raw_write(sigmadsp->control_data,
>> + ADAU17X1_SAFELOAD_TARGET_ADDRESS, buf, 4);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* write nbr of words to trigger address */
>> + buf[0] = 0x00;
>> + buf[1] = 0x00;
>> + buf[2] = 0x00;
>> + buf[3] = i & 0xff;
> Same here I guess
>
>> + ret = regmap_raw_write(sigmadsp->control_data,
>> + ADAU17X1_SAFELOAD_TRIGGER, buf, 4);
>> +
>> + return ret;
>> +}
More information about the Alsa-devel
mailing list