On Tue, 20 Jul 2021 21:45:17 +0200, Nathan Chancellor wrote:
On Thu, Jul 15, 2021 at 09:58:31AM +0200, Takashi Iwai wrote:
This patch converts the resource management in PCI als300 driver with devres as a clean up. Each manual resource management is converted with the corresponding devres helper, and the card object release is managed now via card->private_free instead of a lowlevel snd_device.
This should give no user-visible functional changes.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/als300.c | 79 ++++++++++------------------------------------ 1 file changed, 17 insertions(+), 62 deletions(-)
diff --git a/sound/pci/als300.c b/sound/pci/als300.c index 668008fc21f7..9c94072572a5 100644 --- a/sound/pci/als300.c +++ b/sound/pci/als300.c @@ -163,21 +163,11 @@ static void snd_als300_set_irq_flag(struct snd_als300 *chip, int cmd) snd_als300_gcr_write(chip->port, MISC_CONTROL, tmp); }
-static int snd_als300_free(struct snd_als300 *chip) +static void snd_als300_free(struct snd_card *card) {
- snd_als300_set_irq_flag(chip, IRQ_DISABLE);
- if (chip->irq >= 0)
free_irq(chip->irq, chip);- pci_release_regions(chip->pci);
- pci_disable_device(chip->pci);
- kfree(chip);
- return 0;
-}
- struct snd_als300 *chip = card->private_data;
-static int snd_als300_dev_free(struct snd_device *device) -{
- struct snd_als300 *chip = device->device_data;
- return snd_als300_free(chip);
- snd_als300_set_irq_flag(chip, IRQ_DISABLE);
}
static irqreturn_t snd_als300_interrupt(int irq, void *dev_id) @@ -248,11 +238,6 @@ static irqreturn_t snd_als300plus_interrupt(int irq, void *dev_id) return IRQ_HANDLED; }
-static void snd_als300_remove(struct pci_dev *pci) -{
- snd_card_free(pci_get_drvdata(pci));
-}
static unsigned short snd_als300_ac97_read(struct snd_ac97 *ac97, unsigned short reg) { @@ -610,35 +595,22 @@ static void snd_als300_init(struct snd_als300 *chip) }
static int snd_als300_create(struct snd_card *card,
struct pci_dev *pci, int chip_type,struct snd_als300 **rchip)
struct pci_dev *pci, int chip_type){
- struct snd_als300 *chip;
- struct snd_als300 *chip = card->private_data; void *irq_handler; int err;
- static const struct snd_device_ops ops = {
.dev_free = snd_als300_dev_free,- };
- *rchip = NULL;
- err = pci_enable_device(pci);
err = pcim_enable_device(pci); if (err < 0) return err;
if (dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(28))) { dev_err(card->dev, "error setting 28bit DMA mask\n");
pci_disable_device(pci);return -ENXIO; } pci_set_master(pci);
chip = kzalloc(sizeof(*chip), GFP_KERNEL);
if (chip == NULL) {
pci_disable_device(pci);return -ENOMEM;}
chip->card = card; chip->pci = pci; chip->irq = -1;
@@ -646,11 +618,9 @@ static int snd_als300_create(struct snd_card *card, spin_lock_init(&chip->reg_lock);
err = pci_request_regions(pci, "ALS300");
- if (err < 0) {
kfree(chip);pci_disable_device(pci);
- if (err < 0) return err;
- }
chip->port = pci_resource_start(pci, 0);
if (chip->chip_type == DEVICE_ALS300_PLUS)
@@ -658,38 +628,29 @@ static int snd_als300_create(struct snd_card *card, else irq_handler = snd_als300_interrupt;
- if (request_irq(pci->irq, irq_handler, IRQF_SHARED,
KBUILD_MODNAME, chip)) {
- if (devm_request_irq(&pci->dev, pci->irq, irq_handler, IRQF_SHARED,
KBUILD_MODNAME, chip)) {
snd_als300_free(chip);
card->private_free = snd_als300_free;
snd_als300_init(chip);
err = snd_als300_ac97(chip); if (err < 0) { dev_err(card->dev, "Could not create ac97\n");
snd_als300_free(chip);return err; }
err = snd_als300_new_pcm(chip); if (err < 0) { dev_err(card->dev, "Could not create PCM\n");
snd_als300_free(chip);return err;}
err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
if (err < 0) {
snd_als300_free(chip);return err; }
*rchip = chip; return 0;
}
@@ -737,20 +698,16 @@ static int snd_als300_probe(struct pci_dev *pci, return -ENOENT; }
- err = snd_card_new(&pci->dev, index[dev], id[dev], THIS_MODULE,
0, &card);
err = snd_devm_card_new(&pci->dev, index[dev], id[dev], THIS_MODULE,
sizeof(*chip), &card);if (err < 0) return err;
chip_type = pci_id->driver_data;
- err = snd_als300_create(card, pci, chip_type, &chip);
- if (err < 0) {
snd_card_free(card);
- err = snd_als300_create(card, pci, chip_type);
- if (err < 0) return err;
}
card->private_data = chip;
strcpy(card->driver, "ALS300"); if (chip->chip_type == DEVICE_ALS300_PLUS)
clang warns:
sound/pci/als300.c:713:6: error: variable 'chip' is uninitialized when used here [-Werror,-Wuninitialized] if (chip->chip_type == DEVICE_ALS300_PLUS) ^~~~ sound/pci/als300.c:691:25: note: initialize the variable 'chip' to silence this warning struct snd_als300 *chip; ^ = NULL 1 error generated.
Thanks, it was another silly mistake. I'll queue the fix patch below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: als300: Fix missing chip initialization
The recent code refactoring missed the initialization of the chip variable as its allocation was moved to card->private_data. Let's fix it.
Fixes: 21a9314cf93b ("ALSA: als300: Allocate resources with device-managed APIs") Reported-by: Nathan Chancellor nathan@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/als300.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/pci/als300.c b/sound/pci/als300.c index 9c94072572a5..b86565dcdbe4 100644 --- a/sound/pci/als300.c +++ b/sound/pci/als300.c @@ -702,6 +702,7 @@ static int snd_als300_probe(struct pci_dev *pci, sizeof(*chip), &card); if (err < 0) return err; + chip = card->private_data;
chip_type = pci_id->driver_data;