[v11,06/17] vpci/header: rework exit path in init_bars

Commit Message

Volodymyr Babchuk Dec. 2, 2023, 1:27 a.m. UTC

Introduce "fail" label in init_bars() function to have the centralized
error return path. This is the pre-requirement for the future changes
in this function.

This patch does not introduce functional changes.

Signed-off-by: Volodymyr Babchuk <[e-mail]>
Suggested-by: Roger Pau Monné <[e-mail]>
Acked-by: Roger Pau Monné <[e-mail]>
--
In v11:
- Do not remove empty line between "goto fail;" and "continue;"
In v10:
- Added Roger's A-b tag.
In v9:
- New in v9
---
 xen/drivers/vpci/header.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

Comments

Jan Beulich Dec. 4, 2023, 8:30 a.m. UTC | #1

On 02.12.2023 02:27, Volodymyr Babchuk wrote:
> Introduce "fail" label in init_bars() function to have the centralized
> error return path. This is the pre-requirement for the future changes
> in this function.
> 
> This patch does not introduce functional changes.
> 
> Signed-off-by: Volodymyr Babchuk <[e-mail]>
> Suggested-by: Roger Pau Monné <[e-mail]>
> Acked-by: Roger Pau Monné <[e-mail]>

Nit: Tags in chronological order please.

Jan

Volodymyr Babchuk Dec. 5, 2023, 12:53 a.m. UTC | #2

Hello Jan,

Jan Beulich <[e-mail]> writes:

> On 02.12.2023 02:27, Volodymyr Babchuk wrote:
>> Introduce "fail" label in init_bars() function to have the centralized
>> error return path. This is the pre-requirement for the future changes
>> in this function.
>> 
>> This patch does not introduce functional changes.
>> 
>> Signed-off-by: Volodymyr Babchuk <[e-mail]>
>> Suggested-by: Roger Pau Monné <[e-mail]>
>> Acked-by: Roger Pau Monné <[e-mail]>
>
> Nit: Tags in chronological order please.

Just to be clarify, it should be

Suggested-by
Signed-off-by
Acked-by

Correct?

Jan Beulich Dec. 5, 2023, 7:42 a.m. UTC | #3

On 05.12.2023 01:53, Volodymyr Babchuk wrote:
> Jan Beulich <[e-mail]> writes:
>> On 02.12.2023 02:27, Volodymyr Babchuk wrote:
>>> Introduce "fail" label in init_bars() function to have the centralized
>>> error return path. This is the pre-requirement for the future changes
>>> in this function.
>>>
>>> This patch does not introduce functional changes.
>>>
>>> Signed-off-by: Volodymyr Babchuk <[e-mail]>
>>> Suggested-by: Roger Pau Monné <[e-mail]>
>>> Acked-by: Roger Pau Monné <[e-mail]>
>>
>> Nit: Tags in chronological order please.
> 
> Just to be clarify, it should be
> 
> Suggested-by
> Signed-off-by
> Acked-by
> 
> Correct?

Yes.

Jan

Stewart Hildebrand Dec. 5, 2023, 3:58 p.m. UTC | #4

On 12/1/23 20:27, Volodymyr Babchuk wrote:
> Introduce "fail" label in init_bars() function to have the centralized

The name was correct at the time of submission, but since then, init_bars() was renamed to init_header() in staging

> error return path. This is the pre-requirement for the future changes
> in this function.
> 
> This patch does not introduce functional changes.
> 
> Signed-off-by: Volodymyr Babchuk <[e-mail]>
> Suggested-by: Roger Pau Monné <[e-mail]>
> Acked-by: Roger Pau Monné <[e-mail]>
> --

NIT: The scissors line should be tree dashes, not two

Patch

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index ec6c93eef6..e6a1d58c42 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -581,10 +581,7 @@  static int cf_check init_bars(struct pci_dev *pdev)
             rc = vpci_add_register(pdev->vpci, vpci_hw_read32, bar_write, reg,
                                    4, &bars[i]);
             if ( rc )
-            {
-                pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
-                return rc;
-            }
+                goto fail;
 
             continue;
         }
@@ -604,10 +601,7 @@  static int cf_check init_bars(struct pci_dev *pdev)
         rc = pci_size_mem_bar(pdev->sbdf, reg, &addr, &size,
                               (i == num_bars - 1) ? PCI_BAR_LAST : 0);
         if ( rc < 0 )
-        {
-            pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
-            return rc;
-        }
+            goto fail;
 
         if ( size == 0 )
         {
@@ -622,10 +616,7 @@  static int cf_check init_bars(struct pci_dev *pdev)
         rc = vpci_add_register(pdev->vpci, vpci_hw_read32, bar_write, reg, 4,
                                &bars[i]);
         if ( rc )
-        {
-            pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
-            return rc;
-        }
+            goto fail;
     }
 
     /* Check expansion ROM. */
@@ -647,6 +638,10 @@  static int cf_check init_bars(struct pci_dev *pdev)
     }
 
     return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, cmd, false) : 0;
+
+ fail:
+    pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
+    return rc;
 }
 REGISTER_VPCI_INIT(init_bars, VPCI_PRIORITY_MIDDLE);