lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <ba7ed44fdcfe0a3a80024f0ecdfa4e5255cc48c6.camel@irl.hu> Date: Tue, 05 Dec 2023 02:11:01 +0100 From: Gergo Koteles <soyer@....hu> To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>, Shenghao Ding <shenghao-ding@...com>, Kevin Lu <kevin-lu@...com>, Baojun Xu <baojun.xu@...com>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org> Cc: linux-kernel@...r.kernel.org, alsa-devel@...a-project.org Subject: Re: [PATCH 1/2] ASoc: tas2563: DSP Firmware loading support Hi Pierre-Louis, Thank you for your review. On Mon, 2023-12-04 at 18:05 -0600, Pierre-Louis Bossart wrote: > > > > sound/soc/codecs/tas2562.c | 2 +- > > are tas2562 and tas2563 (from commit subject) the same? > No, tas2563 is register compatible with tas2562. > > > > +#include <linux/slab.h> > > +#include <linux/delay.h> > > nit-pick: alphabetical order? > Good idea, I'll fix it and the others in the next version. > > + > > +#include <sound/tas2562.h> > > +#include <sound/tas25xx-dsp.h> > > + > > + > > +static void tas25xx_process_fw_delay(struct tas25xx_cmd *cmd) > > +{ > > + mdelay(cpu_to_be16(cmd->hdr.length)); > > is this the length of the header, or the duration of the delay? > > Someone will get it wrong with this naming... > I think this is because of the format. If cmd_type is TAS25XX_CMD_DELAY, then hdr.length is the length of the delay. At least I think so, based on https://lore.kernel.org/lkml/6d6aaed3-dac8-e1ec-436c-9b04273df2b3@ti.com/T/ https://github.com/torvalds/linux/blob/bee0e7762ad2c6025b9f5245c040fcc36ef2bde8/sound/soc/codecs/tas2781-fmwlib.c#L769 But I don't see any TAS25XX_CMD_DELAY command in the firmware I'm using. > > > > + prog_num = cpu_to_be32(fw_data->hdr->num_programs); > > + config_num = cpu_to_be32(fw_data->hdr->num_configs); > > + dev_info(dev, "Firmware loaded: programs %d, configs %d\n", > > + prog_num, config_num); > > + For me: release_firmware(fw) is missing from here > > + return fw_data; > > + > > +err_prog: > > + devm_kfree(dev, fw_data->prog_data); > > +err_hdr: > > + devm_kfree(dev, fw_data->hdr); > > +err_data: > > + devm_kfree(dev, fw_data); > > +err_fw: > > + release_firmware(fw); > > + > > + return NULL; > > +} > > Regards, Gergo
Powered by blists - more mailing lists