Hi Anthony,
the patch is good; please post a v2 with the following minor
improvements:
On 06/28/19 18:16, Anthony PERARD wrote:
> In XenBusDxe, the XenBusAddDevice() opens the gXenIoProtocolGuid on
> behalf of child controllers. It is never closed and prevent from
(1) s/prevent/prevents us/
> uninstalling the protocol.
>
> Close it were we stop all the childs in XenBusDxe->Stop().
(2) s/were/where/
(3) s/childs/children/ -- applies to the subject line as well
>
> Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
> ---
> OvmfPkg/XenBusDxe/XenBusDxe.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c
> index 0e63707f50..fac6f3a09d 100644
> --- a/OvmfPkg/XenBusDxe/XenBusDxe.c
> +++ b/OvmfPkg/XenBusDxe/XenBusDxe.c
> @@ -453,6 +453,11 @@ XenBusDxeDriverBindingStop (
> continue;
> }
>
> + Status = gBS->CloseProtocol (Dev->ControllerHandle, &gXenIoProtocolGuid,
> + Dev->This->DriverBindingHandle,
> + ChildData->Handle);
> + ASSERT_EFI_ERROR (Status);
> +
(4) The indentation of function call arguments is inconsistent in this
driver. Still, if it's not a lot of trouble, please correct the
indentation here. Please pick one of the two below:
(4a)
Status = gBS->CloseProtocol (
Dev->ControllerHandle,
&gXenIoProtocolGuid,
Dev->This->DriverBindingHandle,
ChildData->Handle
);
(4b)
Status = gBS->CloseProtocol (Dev->ControllerHandle, &gXenIoProtocolGuid,
Dev->This->DriverBindingHandle, ChildData->Handle);
With those updates, please add, to v2:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
(5) Side remark (no need to do anything about it in the scope of this
patch): I think the DisconnectController() call in
XenBusDxeDriverBindingStop() is superfluous. That kind of disconnection
is not the job of EFI_DRIVER_BINDING_PROTOCOL.Stop().
Thanks
Laszlo
> Status = gBS->UninstallMultipleProtocolInterfaces (
> ChildData->Handle,
> &gEfiDevicePathProtocolGuid, ChildData->DevicePath,
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43049): https://edk2.groups.io/g/devel/message/43049
Mute This Topic: https://groups.io/mt/32243460/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
