snd_hda: Fix cleanup regression in hdac_detach()

[christos]

Summary

bus_generic_detach() does not clean up properly and if we unload and
reload, we get an additional duplicate hdacc for each reload, along with
duplicate PCM devices.

Fixes: 11a9117871e6 ("Use bus_generic_detach to detach and delete child devices during detach")
Sponsored by: The FreeBSD Foundation
MFC after: 1 day

[markj]

The only real difference I can see is that this change causes child devices to be detached in reverse order. Does the order matter for some reason?

[christos]

In D50378#1150042, @markj wrote:

The only real difference I can see is that this change causes child devices to be detached in reverse order. Does the order matter for some reason?

Correct me if I'm wrong, but unlike bus_generic_detach() which just calls device_detach(), device_delete_children() seems to be calling device_delete_child(), which in turns calls device_detach(), and then does additional cleanup. In any case, this change does fix the bug. That's also how the rest of the HDA layers (hdacc, hdaa) cleanup.

[markj]

In D50378#1150084, @christos wrote:

In D50378#1150042, @markj wrote:

The only real difference I can see is that this change causes child devices to be detached in reverse order. Does the order matter for some reason?

Correct me if I'm wrong, but unlike bus_generic_detach() which just calls device_detach(), device_delete_children() seems to be calling device_delete_child(), which in turns calls device_detach(), and then does additional cleanup. In any case, this change does fix the bug.

bus_generic_detach() calls device_detach() on each child, then calls device_delete_children(). With this patch we're just calling device_delete_children().

I believe you that the change fixes a problem, but it's not clear to me at least why it fixes it, so it's hard to say that the change is right. Maybe someone more familiar with newbus can chime in. Failing that, it'd be helpful to know what exactly is failing during detach such that the driver isn't cleaning up properly. Is one of hdacc_detach() or hdaa_detach() returning an error?

That's also how the rest of the HDA layers (hdacc, hdaa) cleanup.

I don't quite follow: hdacc_detach() and hdaa_detach() both call bus_generic_detach().

[christos]

I don't quite follow: hdacc_detach() and hdaa_detach() both call bus_generic_detach().

Scratch that... I hadn't rebased my work branch for a while and so I cherrypicked only 11a9117871e6 and missed the rest of the series. I now did a proper rebase and the bug does not exist. Sorry about the noise.