On Wed, 9 Aug 2023, Sui Jingfeng wrote:
> From: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
>
Changelog body is missing.
> Fixes: 934f992c763a ("drm/i915: Recognise non-VGA display devices")
> Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
> ---
> drivers/pci/vgaarb.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index 811510253553..a6b8c0def35d 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -964,7 +964,7 @@ EXPORT_SYMBOL(vga_set_legacy_decoding);
> *
> * To unregister just call vga_client_unregister().
> *
> - * Returns: 0 on success, -1 on failure
> + * Returns: 0 on success, -ENODEV on failure
So this is the true substance of this change??
It doesn't warrant Fixes tag which requires a real problem to fix. An
incorrect comment is not enough.
I think the shortlog is a bit misleading as is because it doesn't in any
way indicate the problem is only in a comment.
> */
> int vga_client_register(struct pci_dev *pdev,
> unsigned int (*set_decode)(struct pci_dev *pdev, bool decode))
> @@ -975,16 +975,13 @@ int vga_client_register(struct pci_dev *pdev,
>
> spin_lock_irqsave(&vga_lock, flags);
> vgadev = vgadev_find(pdev);
> - if (!vgadev)
> - goto bail;
> -
> - vgadev->set_decode = set_decode;
> - ret = 0;
> -
> -bail:
> + if (vgadev) {
> + vgadev->set_decode = set_decode;
> + ret = 0;
> + }
> spin_unlock_irqrestore(&vga_lock, flags);
> - return ret;
>
> + return ret;
No logic changes in this at all? I don't think it belongs to the same
patch. I'm not sure if the new logic is improvement anyway. I'd prefer to
initialize ret = 0 instead:
int ret = 0;
...
if (!vgadev) {
err = -ENODEV;
goto unlock;
}
...
unlock:
...
--
i.
