Messages in this thread | | | Date | Wed, 9 Aug 2023 16:52:09 +0300 (EEST) | From | Ilpo Järvinen <> | Subject | Re: [PATCH v2 07/11] PCI/VGA: vga_client_register() return -ENODEV on failure, not -1 |
| |
On Wed, 9 Aug 2023, Sui Jingfeng wrote:
> From: Sui Jingfeng <suijingfeng@loongson.cn> >
Changelog body is missing.
> Fixes: 934f992c763a ("drm/i915: Recognise non-VGA display devices") > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > --- > 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.
| |