On 08/13/2018 09:33 AM, Robert Rosengren wrote: > From: Danny Smith <dannys@xxxxxxxx> > > 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@xxxxxxxx> > Signed-off-by: Robert Rosengren <robertr@xxxxxxxx> 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. > + 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; > +} _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel