[alsa-devel] [PATCH] fm801: move to pcim_* and devm_* functions

[Takashi Iwai]

At Wed, 7 Jan 2015 15:27:35 +0200,
Andy Shevchenko wrote:
> 
> On Wed, Jan 7, 2015 at 3:12 PM, Takashi Iwai <tiwai at suse.de> wrote:
> > At Wed, 7 Jan 2015 14:20:26 +0200,
> > Andy Shevchenko wrote:
> >>
> >> On Wed, Jan 7, 2015 at 1:27 PM, Takashi Iwai <tiwai at suse.de> wrote:
> >> > At Wed, 7 Jan 2015 12:54:40 +0200,
> >> > Andy Shevchenko wrote:
> >> >>
> >> >> On Wed, Jan 7, 2015 at 9:11 AM, Takashi Iwai <tiwai at suse.de> wrote:
> >> >> > At Wed,  7 Jan 2015 01:40:17 +0200,
> >> >> > Andy Shevchenko wrote:
> >> >> >>
> >> >> >> The managed functions allow to get ->probe() and ->remove() simplier.
> >> >> >> This patch converts code to use managed functions.
> >> >> >
> >> >> > This doesn't work well because of the order of release calls in
> >> >> > device_release().  devres_release_all() is called at first before
> >> >> > anything else.  Thus the card's free callback would be called after
> >> >> > it, and snd_fm801_free() touches the hardware before disabling.
> >> >> >
> >> >>
> >> >> Hmm… I didn't get what make those calls. Sound core? The driver is a
> >> >> normal PCI driver, so resources will be freed *after* ->remove() of
> >> >> driver was called.
> >> >> Please, elaborate.
> >> >
> >> > Ah, OK, this seems working, as this is the managed *pci* device that
> >> > is a parent of the card device, thus it shall be released after the
> >> > children.
> >> >
> >> > But any reason not to use pcim_iomap_regions_request_all()?
> >>
> >> Yes. I was doubt to put a comment about this.
> >
> > Well, pcim_iomap_regions_request_all() can just request regions
> > without actually iomapping via passing 0 to mask.  But, now looking at
> > the code again, I couldn't find the place releasing the non-mmio
> > regions.  Hmm... it might be a leak?
> 
> It's in snd_fm801_free().
> pci_release_regions(chip->pci);

I meant the release of regions allocated in
pcim_iomap_regions_request_all().  The function requests all regions,
not only for MMIO ones:

int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
				   const char *name)
{
	int request_mask = ((1 << 6) - 1) & ~mask;
	int rc;

	rc = pci_request_selected_regions(pdev, request_mask, name);
	if (rc)
		return rc;

	rc = pcim_iomap_regions(pdev, mask, name);
	if (rc)
		pci_release_selected_regions(pdev, request_mask);
	return rc;
}

The regions allocated via pcim_iomap_regions() are released via
pcim_iomap_release() properly.  But what about the former one, the
regions allocated via pci_request_selected_regions()...?


Takashi