Re: kern/58027: ehci failed attach stops suspend/resume

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

>  	if (sc->sc_ih) {
>  		pci_intr_disestablish(sc->sc_pc, sc->sc_ih);
> +		pci_intr_release(sc->sc_pc, sc->sc_pihp, 1);
>  		sc->sc_ih = NULL;
>  	}

This part seems wrong -- I think it should be a separate branch:

 	if (sc->sc_ih) {
 		pci_intr_disestablish(sc->sc_pc, sc->sc_ih);
 		sc->sc_ih = NULL;
 	}
	if (sc->sc_pihp) {
		pci_intr_release(sc->sc_pc, sc->sc_pihp, 1);
		sc->sc_pihp = NULL;
	}

and then we can delete some more redundant code in ehci_pci_attach's
pci_intr_establish_xname error branch.

> @@ -262,6 +263,7 @@ ehci_pci_attach(device_t parent, device_
>  	int err = ehci_init(&sc->sc);
>  	if (err) {
>  		aprint_error_dev(self, "init failed, error=%d\n", err);
> +		ehci_release_ownership(&sc->sc, sc->sc_pc, sc->sc_tag);
>  		goto fail;
>  	}
> ...
>  	if (sc->sc.sc_size) {
> -		ehci_release_ownership(&sc->sc, sc->sc_pc, sc->sc_tag);
>  		bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
>  		sc->sc.sc_size = 0;
>  	}
> ...
> @@ -302,27 +317,17 @@ ehci_pci_detach(device_t self, int flags
> ...
> -	if (sc->sc.sc_size) {
>  		ehci_release_ownership(&sc->sc, sc->sc_pc, sc->sc_tag);
> -		bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
> -		sc->sc.sc_size = 0;
>  	}

Why did the ehci_release_ownership part move?  I think we should
generally have less logic in the `if (err) goto fail' branches
themselves, and more logic shared in the common cleanup path.