mbox series

[v2,00/79] ALSA: More devres usages

Message ID 20210715075941.23332-1-tiwai@suse.de
Headers show
Series ALSA: More devres usages | expand

Message

Takashi Iwai July 15, 2021, 7:58 a.m. UTC
Hi,

this is a v2 patch series for adapting more devres usages in ALSA
drivers, a resurrect of my early RFC patch with the fix and applying
to more drivers.  The main purpose is merely the code cleanup over the
tree by replacing the resource allocations with devres helpers as much
as possible.

The only major change is the addition of devres-supported card object
handling snd_devm_card_new() and the page allocation helper
snd_devm_alloc_pages().  Those allow us to get rid of the remove
callback completely in many drivers.  In the end, the series resulted
in a significant amount of code reduction.

Now ISA drivers and a few other drivers have been covered as
well as a couple of missing PCI drivers and fixes.


Takashi

===

v1->v2:
* Rearrange patch order
* Use size_t in snd_devm_card_new()
* Move card->releasing check into devres release
* Most of ISA drivers are included
* A few platform drivers included
* Fixed missing initializations, etc in a few drivers.

Takashi Iwai (79):
  ALSA: core: Add device-managed page allocator helper
  ALSA: core: Add managed card creation
  ALSA: core: Add device-managed request_dma()
  ALSA: doc: Add device-managed resource section
  ALSA: intel8x0: Allocate resources with device-managed APIs
  ALSA: atiixp: Allocate resources with device-managed APIs
  ALSA: hda: Allocate resources with device-managed APIs
  ALSA: ad1889: Allocate resources with device-managed APIs
  ALSA: als300: Allocate resources with device-managed APIs
  ALSA: als4000: Allocate resources with device-managed APIs
  ALSA: azt3328: Allocate resources with device-managed APIs
  ALSA: bt87x: Allocate resources with device-managed APIs
  ALSA: cmipci: Allocate resources with device-managed APIs
  ALSA: cs4281: Allocate resources with device-managed APIs
  ALSA: cs5530: Allocate resources with device-managed APIs
  ALSA: ens137x: Allocate resources with device-managed APIs
  ALSA: es1938: Allocate resources with device-managed APIs
  ALSA: es1968: Allocate resources with device-managed APIs
  ALSA: fm801: Allocate resources with device-managed APIs
  ALSA: maestro3: Allocate resources with device-managed APIs
  ALSA: rme32: Allocate resources with device-managed APIs
  ALSA: rme96: Allocate resources with device-managed APIs
  ALSA: sis7019: Allocate resources with device-managed APIs
  ALSA: sonicvibes: Allocate resources with device-managed APIs
  ALSA: via82xx: Allocate resources with device-managed APIs
  ALSA: ali5451: Allocate resources with device-managed APIs
  ALSA: au88x0: Allocate resources with device-managed APIs
  ALSA: aw2: Allocate resources with device-managed APIs
  ALSA: ca0106: Allocate resources with device-managed APIs
  ALSA: cs46xx: Allocate resources with device-managed APIs
  ALSA: cs5535audio: Allocate resources with device-managed APIs
  ALSA: echoaudio: Allocate resources with device-managed APIs
  ALSA: emu10k1: Allocate resources with device-managed APIs
  ALSA: emu10k1x: Allocate resources with device-managed APIs
  ALSA: ice1712: Allocate resources with device-managed APIs
  ALSA: ice1724: Allocate resources with device-managed APIs
  ALSA: ali5451: Allocate resources with device-managed APIs
  ALSA: ice1724: Allocate resources with device-managed APIs
  ALSA: korg1212: Allocate resources with device-managed APIs
  ALSA: lola: Allocate resources with device-managed APIs
  ALSA: lx6464es: Allocate resources with device-managed APIs
  ALSA: nm256: Allocate resources with device-managed APIs
  ALSA: oxygen: Allocate resources with device-managed APIs
  ALSA: riptide: Allocate resources with device-managed APIs
  ALSA: hdsp: Allocate resources with device-managed APIs
  ALSA: hdspm: Allocate resources with device-managed APIs
  ALSA: rme9652: Allocate resources with device-managed APIs
  ALSA: trident: Allocate resources with device-managed APIs
  ALSA: vx: Manage vx_core object with devres
  ALSA: vx222: Allocate resources with device-managed APIs
  ALSA: ymfpci: Allocate resources with device-managed APIs
  ALSA: ad1816a: Allocate resources with device-managed APIs
  ALSA: wss: Allocate resources with device-managed APIs
  ALSA: sb: Allocate resources with device-managed APIs
  ALSA: ad1848: Allocate resources with device-managed APIs
  ALSA: adlib: Allocate resources with device-managed APIs
  ALSA: als100: Allocate resources with device-managed APIs
  ALSA: azt2320: Allocate resources with device-managed APIs
  ALSA: cmi8328: Allocate resources with device-managed APIs
  ALSA: cmi8330: Allocate resources with device-managed APIs
  ALSA: cs423x: Allocate resources with device-managed APIs
  ALSA: es1688: Allocate resources with device-managed APIs
  ALSA: es18xx: Allocate resources with device-managed APIs
  ALSA: galaxy: Allocate resources with device-managed APIs
  ALSA: gus: Allocate resources with device-managed APIs
  ALSA: msnd: Allocate resources with device-managed APIs
  ALSA: opti9xx: Allocate resources with device-managed APIs
  ALSA: opl3sa2: Allocate resources with device-managed APIs
  ALSA: sc6000: Allocate resources with device-managed APIs
  ALSA: sscape: Allocate resources with device-managed APIs
  ALSA: wavefront: Allocate resources with device-managed APIs
  ALSA: x86: Allocate resources with device-managed APIs
  ALSA: virmidi: Allocate resources with device-managed APIs
  ALSA: mtpav: Allocate resources with device-managed APIs
  ALSA: serial-u16550: Allocate resources with device-managed APIs
  ALSA: mpu401: Allocate resources with device-managed APIs
  ALSA: aloop: Allocate resources with device-managed APIs
  ALSA: dummy: Allocate resources with device-managed APIs
  ALSA: pcsp: Allocate resources with device-managed APIs

 .../kernel-api/writing-an-alsa-driver.rst     |  33 +++
 include/sound/core.h                          |   6 +
 include/sound/emu10k1.h                       |   6 +-
 include/sound/emu8000.h                       |   3 -
 include/sound/es1688.h                        |   1 -
 include/sound/memalloc.h                      |   4 +
 sound/core/init.c                             |  99 +++++++-
 sound/core/isadma.c                           |  38 ++++
 sound/core/memalloc.c                         |  46 ++++
 sound/drivers/aloop.c                         |  26 +--
 sound/drivers/dummy.c                         |  24 +-
 sound/drivers/mpu401/mpu401.c                 |  34 +--
 sound/drivers/mtpav.c                         |  30 +--
 sound/drivers/pcsp/pcsp.c                     |  49 ++--
 sound/drivers/pcsp/pcsp_input.c               |  14 +-
 sound/drivers/pcsp/pcsp_input.h               |   1 -
 sound/drivers/serial-u16550.c                 |  57 +----
 sound/drivers/virmidi.c                       |  21 +-
 sound/drivers/vx/vx_core.c                    |  12 +-
 sound/isa/ad1816a/ad1816a.c                   |  41 +---
 sound/isa/ad1816a/ad1816a_lib.c               |  49 +---
 sound/isa/ad1848/ad1848.c                     |  19 +-
 sound/isa/adlib.c                             |  28 +--
 sound/isa/als100.c                            |  41 +---
 sound/isa/azt2320.c                           |  49 +---
 sound/isa/cmi8328.c                           |  31 +--
 sound/isa/cmi8330.c                           |  27 +--
 sound/isa/cs423x/cs4231.c                     |  21 +-
 sound/isa/cs423x/cs4236.c                     |  52 +----
 sound/isa/cs423x/cs4236_lib.c                 |   2 -
 sound/isa/es1688/es1688.c                     |  33 +--
 sound/isa/es1688/es1688_lib.c                 |  29 +--
 sound/isa/es18xx.c                            | 112 ++--------
 sound/isa/galaxy/galaxy.c                     |  82 +++----
 sound/isa/gus/gus_main.c                      |  44 ++--
 sound/isa/gus/gusclassic.c                    |  28 +--
 sound/isa/gus/gusextreme.c                    |  39 ++--
 sound/isa/gus/gusmax.c                        |  65 ++----
 sound/isa/gus/interwave.c                     |  53 +----
 sound/isa/msnd/msnd_pinnacle.c                | 119 +++-------
 sound/isa/opl3sa2.c                           |  60 +----
 sound/isa/opti9xx/miro.c                      |  76 ++-----
 sound/isa/opti9xx/opti92x-ad1848.c            |  80 ++-----
 sound/isa/sb/emu8000.c                        |  48 +---
 sound/isa/sb/jazz16.c                         |  39 +---
 sound/isa/sb/sb16.c                           |  42 +---
 sound/isa/sb/sb8.c                            |  48 ++--
 sound/isa/sb/sb_common.c                      |  64 ++----
 sound/isa/sc6000.c                            |  73 +++---
 sound/isa/sscape.c                            |  92 ++------
 sound/isa/wavefront/wavefront.c               |  46 +---
 sound/isa/wss/wss_lib.c                       |  67 +-----
 sound/pci/ad1889.c                            | 144 +++---------
 sound/pci/ali5451/ali5451.c                   |  90 ++------
 sound/pci/als300.c                            |  79 ++-----
 sound/pci/als4000.c                           |  59 ++---
 sound/pci/atiixp.c                            |  92 ++------
 sound/pci/atiixp_modem.c                      |  92 ++------
 sound/pci/au88x0/au88x0.c                     | 134 +++--------
 sound/pci/aw2/aw2-alsa.c                      | 102 ++-------
 sound/pci/azt3328.c                           | 124 +++-------
 sound/pci/bt87x.c                             |  98 +++-----
 sound/pci/ca0106/ca0106.h                     |   3 +-
 sound/pci/ca0106/ca0106_main.c                | 114 +++-------
 sound/pci/cmipci.c                            | 104 +++------
 sound/pci/cs4281.c                            | 112 ++--------
 sound/pci/cs46xx/cs46xx.c                     |  51 ++---
 sound/pci/cs46xx/cs46xx.h                     |   4 +-
 sound/pci/cs46xx/cs46xx_lib.c                 | 111 ++-------
 sound/pci/cs5530.c                            |  86 ++-----
 sound/pci/cs5535audio/cs5535audio.c           |  94 ++------
 sound/pci/cs5535audio/cs5535audio_olpc.c      |   7 +-
 sound/pci/echoaudio/echoaudio.c               | 168 ++++----------
 sound/pci/echoaudio/echoaudio.h               |   2 +-
 sound/pci/emu10k1/emu10k1.c                   |  53 ++---
 sound/pci/emu10k1/emu10k1_main.c              | 102 +++------
 sound/pci/emu10k1/emu10k1x.c                  | 128 +++--------
 sound/pci/emu10k1/p16v.c                      |  22 +-
 sound/pci/ens1370.c                           | 115 +++-------
 sound/pci/es1938.c                            |  97 ++------
 sound/pci/es1968.c                            | 112 ++--------
 sound/pci/fm801.c                             | 103 ++-------
 sound/pci/hda/hda_controller.h                |   1 -
 sound/pci/hda/hda_intel.c                     |  26 +--
 sound/pci/ice1712/ice1712.c                   | 133 +++--------
 sound/pci/ice1712/ice1724.c                   | 129 +++--------
 sound/pci/intel8x0.c                          | 140 ++++--------
 sound/pci/intel8x0m.c                         | 139 ++++--------
 sound/pci/korg1212/korg1212.c                 | 211 +++++-------------
 sound/pci/lola/lola.c                         | 127 +++--------
 sound/pci/lola/lola.h                         |   5 +-
 sound/pci/lola/lola_pcm.c                     |  20 +-
 sound/pci/lx6464es/lx6464es.c                 | 112 ++--------
 sound/pci/maestro3.c                          | 106 ++-------
 sound/pci/nm256/nm256.c                       | 130 +++--------
 sound/pci/oxygen/oxygen.c                     |   1 -
 sound/pci/oxygen/oxygen.h                     |   1 -
 sound/pci/oxygen/oxygen_lib.c                 |  66 ++----
 sound/pci/oxygen/se6x.c                       |   1 -
 sound/pci/oxygen/virtuoso.c                   |   1 -
 sound/pci/riptide/riptide.c                   |  89 ++------
 sound/pci/rme32.c                             |  49 +---
 sound/pci/rme96.c                             |  57 ++---
 sound/pci/rme9652/hdsp.c                      |  89 +++-----
 sound/pci/rme9652/hdspm.c                     |  64 +-----
 sound/pci/rme9652/rme9652.c                   |  85 ++-----
 sound/pci/sis7019.c                           |  87 ++------
 sound/pci/sonicvibes.c                        | 117 +++-------
 sound/pci/trident/trident.c                   |  39 +---
 sound/pci/trident/trident.h                   |   7 +-
 sound/pci/trident/trident_main.c              |  90 +++-----
 sound/pci/trident/trident_memory.c            |   8 +-
 sound/pci/via82xx.c                           | 116 +++-------
 sound/pci/via82xx_modem.c                     |  88 ++------
 sound/pci/vx222/vx222.c                       |  69 +-----
 sound/pci/ymfpci/ymfpci.c                     |  66 +++---
 sound/pci/ymfpci/ymfpci.h                     |   8 +-
 sound/pci/ymfpci/ymfpci_main.c                | 149 ++++---------
 sound/pcmcia/vx/vxpocket.c                    |  22 --
 sound/x86/intel_hdmi_audio.c                  |  57 ++---
 120 files changed, 1997 insertions(+), 5513 deletions(-)

Comments

Nathan Chancellor July 20, 2021, 7:46 p.m. UTC | #1
On Thu, Jul 15, 2021 at 09:58:36AM +0200, Takashi Iwai wrote:
> This patch converts the resource management in PCI cs4281 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/cs4281.c | 112 ++++++++++-----------------------------------
>  1 file changed, 24 insertions(+), 88 deletions(-)
> 
> diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c
> index e122a168c148..f338caf98354 100644
> --- a/sound/pci/cs4281.c
> +++ b/sound/pci/cs4281.c
> @@ -1268,8 +1268,10 @@ static inline int snd_cs4281_create_gameport(struct cs4281 *chip) { return -ENOS
>  static inline void snd_cs4281_free_gameport(struct cs4281 *chip) { }
>  #endif /* IS_REACHABLE(CONFIG_GAMEPORT) */
>  
> -static int snd_cs4281_free(struct cs4281 *chip)
> +static void snd_cs4281_free(struct snd_card *card)
>  {
> +	struct cs4281 *chip = card->private_data;
> +
>  	snd_cs4281_free_gameport(chip);
>  
>  	/* Mask interrupts */
> @@ -1278,49 +1280,20 @@ static int snd_cs4281_free(struct cs4281 *chip)
>  	snd_cs4281_pokeBA0(chip, BA0_CLKCR1, 0);
>  	/* Sound System Power Management - Turn Everything OFF */
>  	snd_cs4281_pokeBA0(chip, BA0_SSPM, 0);
> -	/* PCI interface - D3 state */
> -	pci_set_power_state(chip->pci, PCI_D3hot);
> -
> -	if (chip->irq >= 0)
> -		free_irq(chip->irq, chip);
> -	iounmap(chip->ba0);
> -	iounmap(chip->ba1);
> -	pci_release_regions(chip->pci);
> -	pci_disable_device(chip->pci);
> -
> -	kfree(chip);
> -	return 0;
> -}
> -
> -static int snd_cs4281_dev_free(struct snd_device *device)
> -{
> -	struct cs4281 *chip = device->device_data;
> -	return snd_cs4281_free(chip);
>  }
>  
>  static int snd_cs4281_chip_init(struct cs4281 *chip); /* defined below */
>  
>  static int snd_cs4281_create(struct snd_card *card,
>  			     struct pci_dev *pci,
> -			     struct cs4281 **rchip,
>  			     int dual_codec)
>  {
>  	struct cs4281 *chip;
> -	unsigned int tmp;
>  	int err;
> -	static const struct snd_device_ops ops = {
> -		.dev_free =	snd_cs4281_dev_free,
> -	};
>  
> -	*rchip = NULL;
> -	err = pci_enable_device(pci);
> +	err = pcim_enable_device(pci);
>  	if (err < 0)
>  		return err;
> -	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> -	if (chip == NULL) {
> -		pci_disable_device(pci);
> -		return -ENOMEM;
> -	}
>  	spin_lock_init(&chip->reg_lock);
>  	chip->card = card;

clang warns:

sound/pci/cs4281.c:1298:2: error: variable 'chip' is uninitialized when used here [-Werror,-Wuninitialized]
        chip->card = card;
        ^~~~
sound/pci/cs4281.c:1291:21: note: initialize the variable 'chip' to silence this warning
        struct cs4281 *chip;
                           ^
                            = NULL
1 error generated.

>  	chip->pci = pci;
> @@ -1332,46 +1305,29 @@ static int snd_cs4281_create(struct snd_card *card,
>  	}
>  	chip->dual_codec = dual_codec;
>  
> -	err = pci_request_regions(pci, "CS4281");
> -	if (err < 0) {
> -		kfree(chip);
> -		pci_disable_device(pci);
> +	err = pcim_iomap_regions(pci, 0x03, "CS4281"); /* 2 BARs */
> +	if (err < 0)
>  		return err;
> -	}
>  	chip->ba0_addr = pci_resource_start(pci, 0);
>  	chip->ba1_addr = pci_resource_start(pci, 1);
>  
> -	chip->ba0 = pci_ioremap_bar(pci, 0);
> -	chip->ba1 = pci_ioremap_bar(pci, 1);
> -	if (!chip->ba0 || !chip->ba1) {
> -		snd_cs4281_free(chip);
> -		return -ENOMEM;
> -	}
> +	chip->ba0 = pcim_iomap_table(pci)[0];
> +	chip->ba1 = pcim_iomap_table(pci)[1];
>  	
> -	if (request_irq(pci->irq, snd_cs4281_interrupt, IRQF_SHARED,
> -			KBUILD_MODNAME, chip)) {
> +	if (devm_request_irq(&pci->dev, pci->irq, snd_cs4281_interrupt,
> +			     IRQF_SHARED, KBUILD_MODNAME, chip)) {
>  		dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
> -		snd_cs4281_free(chip);
>  		return -ENOMEM;
>  	}
>  	chip->irq = pci->irq;
>  	card->sync_irq = chip->irq;
> +	card->private_free = snd_cs4281_free;
>  
> -	tmp = snd_cs4281_chip_init(chip);
> -	if (tmp) {
> -		snd_cs4281_free(chip);
> -		return tmp;
> -	}
> -
> -	err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
> -	if (err < 0) {
> -		snd_cs4281_free(chip);
> +	err = snd_cs4281_chip_init(chip);
> +	if (err)
>  		return err;
> -	}
>  
>  	snd_cs4281_proc_init(chip);
> -
> -	*rchip = chip;
>  	return 0;
>  }
>  
> @@ -1887,46 +1843,34 @@ static int snd_cs4281_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 = card->private_data;
>  
> -	err = snd_cs4281_create(card, pci, &chip, dual_codec[dev]);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	err = snd_cs4281_create(card, pci, dual_codec[dev]);
> +	if (err < 0)
>  		return err;
> -	}
> -	card->private_data = chip;
>  
>  	err = snd_cs4281_mixer(chip);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	if (err < 0)
>  		return err;
> -	}
>  	err = snd_cs4281_pcm(chip, 0);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	if (err < 0)
>  		return err;
> -	}
>  	err = snd_cs4281_midi(chip, 0);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	if (err < 0)
>  		return err;
> -	}
>  	err = snd_opl3_new(card, OPL3_HW_OPL3_CS4281, &opl3);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	if (err < 0)
>  		return err;
> -	}
>  	opl3->private_data = chip;
>  	opl3->command = snd_cs4281_opl3_command;
>  	snd_opl3_init(opl3);
>  	err = snd_opl3_hwdep_new(opl3, 0, 1, NULL);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	if (err < 0)
>  		return err;
> -	}
>  	snd_cs4281_create_gameport(chip);
>  	strcpy(card->driver, "CS4281");
>  	strcpy(card->shortname, "Cirrus Logic CS4281");
> @@ -1936,21 +1880,14 @@ static int snd_cs4281_probe(struct pci_dev *pci,
>  		chip->irq);
>  
>  	err = snd_card_register(card);
> -	if (err < 0) {
> -		snd_card_free(card);
> +	if (err < 0)
>  		return err;
> -	}
>  
>  	pci_set_drvdata(pci, card);
>  	dev++;
>  	return 0;
>  }
>  
> -static void snd_cs4281_remove(struct pci_dev *pci)
> -{
> -	snd_card_free(pci_get_drvdata(pci));
> -}
> -
>  /*
>   * Power Management
>   */
> @@ -2054,7 +1991,6 @@ static struct pci_driver cs4281_driver = {
>  	.name = KBUILD_MODNAME,
>  	.id_table = snd_cs4281_ids,
>  	.probe = snd_cs4281_probe,
> -	.remove = snd_cs4281_remove,
>  	.driver = {
>  		.pm = CS4281_PM_OPS,
>  	},
> -- 
> 2.26.2