Re: [alsa-devel] [PATCH 2/2 v2] ALSA: hda - Deferred probing with request_firmware_nowait()

On 08/09/2012 03:36 PM, Takashi Iwai wrote:

+/* callback from request_firmware_nowait() */ +static void azx_firmware_cb(const struct firmware *fw, void *context) +{

• struct snd_card *card = context;
• struct azx *chip = card->private_data;
• struct pci_dev *pci = chip->pci;
• if (!fw) {

• snd_printk(KERN_ERR SFX "Cannot load firmware, aborting\n");
  

• goto error;
  

• }

Another thing, aren't you missing a

chip->fw = fw;

here?

• if (!chip->disabled) {

• /* continue probing */
  

• if (azx_probe_continue(chip))
  

• 	goto error;
  

• }
• return; /* OK */
• error:
• snd_card_free(card);
• pci_set_drvdata(pci, NULL);

+}

• static int __devinit azx_probe(struct pci_dev *pci, const struct pci_device_id *pci_id) { static int dev; struct snd_card *card; struct azx *chip;

• bool probe_deferred; int err;

  if (dev >= SNDRV_CARDS)

@@ -3182,18 +3211,22 @@ static int __devinit azx_probe(struct pci_dev *pci, if (err < 0) goto out_free; card->private_data = chip;

Right, this patch has the race removed, as the variable is no longer set in azx_firmware_cb. To be picky, it's slightly confusing that probe_deferred is also used for signalling a disabled chip. Maybe "probe_now" (inverted) would have been even better, like this:

• probe_deferred = chip->disabled;

probe_now = !chip->disabled;

#ifdef CONFIG_SND_HDA_PATCH_LOADER if (patch[dev] && *patch[dev]) {

(maybe we should not ask for firmware for a disabled chip either)

snd_printk(KERN_ERR SFX "Applying patch firmware '%s'\n",
	   patch[dev]);

• err = request_firmware(&chip->fw, patch[dev], &pci->dev);
  

• err = request_firmware_nowait(THIS_MODULE, true, patch[dev],
  

• 			      &pci->dev, GFP_KERNEL, card,
  

• 			      azx_firmware_cb);
  

  if (err < 0) goto out_free;

• probe_deferred = true;
  

probe_now = false;

} #endif /* CONFIG_SND_HDA_PATCH_LOADER */

• if (!chip->disabled) {

• if (!probe_deferred) {

if (probe_now) {

err = azx_probe_continue(chip);
if (err < 0)
	goto out_free;