Re: [PATCH v2 01/15] pci: Introduce parse_pci_sbdf{_seg}()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/06/2026 5:50 pm, Andrew Cooper wrote:
> On 18/06/2026 3:50 pm, Teddy Astie wrote:
>> diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
>> index 084be3880c..1d06cb035b 100644
>> --- a/xen/drivers/pci/pci.c
>> +++ b/xen/drivers/pci/pci.c
>> @@ -202,3 +202,21 @@ const char *__init parse_pci_seg(const char *s, 
>> unsigned int *seg_p,
>>  
>>      return s;
>>  }
>> +
>> +const char *parse_pci_sbdf(const char *s, pci_sbdf_t *sbdf)
>> +{
>> +    unsigned int seg, bus, dev, func;
>> +    const char *out = parse_pci(s, &seg, &bus, &dev, &func);
>> +
>> +    *sbdf = PCI_SBDF(seg, bus, dev, func);
>> +    return out;
>> +}
>> +
>> +const char *parse_pci_sbdf_seg(const char *s, pci_sbdf_t *sbdf, bool 
>> *def_seg)
>> +{
>> +    unsigned int seg, bus, dev, func;
>> +    const char *out = parse_pci_seg(s, &seg, &bus, &dev, &func, def_seg);
>> +
>> +    *sbdf = PCI_SBDF(seg, bus, dev, func);
>> +    return out;
>> +}
> You correct it later in patch 13, but these want to be __init from this
> patch.
>
> Also, if you introduce these functions ahead of parse_pci{,_seg}()
> respectively, then the diff in patch 13 becomes far more simple.
>
> With that, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

P.S. this is what patch 13 looks like with the suggested change:

diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index 19f24e8fda8c..80b65bfe77d7 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -152,17 +152,6 @@ void pci_intx(const struct pci_dev *pdev, bool enable)
 }
 
 const char *__init parse_pci_sbdf(const char *s, pci_sbdf_t *sbdf)
-{
-    unsigned int seg, bus, dev, func;
-    const char *out = parse_pci(s, &seg, &bus, &dev, &func);
-
-    *sbdf = PCI_SBDF(seg, bus, dev, func);
-    return out;
-}
-
-const char *__init parse_pci(const char *s, unsigned int *seg_p,
-                             unsigned int *bus_p, unsigned int *dev_p,
-                             unsigned int *func_p)
 {
     bool def_seg;
 
@@ -170,17 +159,6 @@ const char *__init parse_pci(const char *s, unsigned int 
*seg_p,
 }
 
 const char *__init parse_pci_sbdf_seg(const char *s, pci_sbdf_t *sbdf, bool 
*def_seg)
-{
-    unsigned int seg, bus, dev, func;
-    const char *out = parse_pci_seg(s, &seg, &bus, &dev, &func, def_seg);
-
-    *sbdf = PCI_SBDF(seg, bus, dev, func);
-    return out;
-}
-
-const char *__init parse_pci_seg(const char *s, unsigned int *seg_p,
-                                 unsigned int *bus_p, unsigned int *dev_p,
-                                 unsigned int *func_p, bool *def_seg)
 {
     unsigned long seg = simple_strtoul(s, &s, 16), bus, dev, func;
 

~Andrew