Re: [Xen-devel] [PATCH v2 01/12] pci: introduce a devfn field to pci_sbdf_t

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.06.19 at 12:13, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Jun 06, 2019 at 04:09:31AM -0600, Jan Beulich wrote:
>> >>> On 06.06.19 at 11:50, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >>  -----Original Message-----
>> >> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf 
>> >> Of 
>> > Roger Pau Monne
>> >> Sent: 06 June 2019 10:02
>> >> To: xen-devel@xxxxxxxxxxxxxxxxxxxx 
>> >> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; 
>> >> Konrad 
>> > Rzeszutek Wilk
>> >> <konrad.wilk@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; 
>> >> Andrew 
>> > Cooper
>> >> <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Tim 
> (Xen.org) 
>> > <tim@xxxxxxx>; Julien
>> >> Grall <julien.grall@xxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Roger Pau 
>> >> Monne 
>> > <roger.pau@xxxxxxxxxx>
>> >> Subject: [Xen-devel] [PATCH v2 01/12] pci: introduce a devfn field to 
>> > pci_sbdf_t
>> >> 
>> >> This is equivalent to the current extfunc field in term of contents.
>> >> 
>> >> Switch the two current users of extfunc to use devfn instead for
>> >> correctness.
>> >> 
>> >> No functional change.
>> >> 
>> >> Requested-by: Jan Beulich <jbeulich@xxxxxxxx>
>> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> >> ---
>> >> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> >> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
>> >> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>> >> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>> >> Cc: Julien Grall <julien.grall@xxxxxxx>
>> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> >> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> >> Cc: Tim Deegan <tim@xxxxxxx>
>> >> Cc: Wei Liu <wl@xxxxxxx>
>> >> ---
>> >> Changes since v1:
>> >>  - New in this version.
>> >> ---
>> >> NB: Paul suggested to name the function field fn instead of func, so
>> >> that it would match the naming of the devfn field. Sadly the func
>> >> field cannot be aliased to another field using a union because it's a
>> >> bit field, so the only option is to rename func to fn.
>> > 
>> > Is that true? Can you not do something like...
>> > 
>> > union {
>> >   struct {
>> >     uint8_t func : 3,
>> >             dev  : 5;
>> >   };
>> >   struct {
>> >     uint8_t fn   : 3,
>> >             pad  : 5;
>> 
>> And the "pad" field here wouldn't really be necessary.
>> 
>> Is there a reason "func" needs to be kept? If so, is there a plan to
>> phase out its use? If so, perhaps fn and dev should be grouped
>> together, and func should become the (temporary) alias?
> 
> I think I can prepare a pre-patch to rename func to fn, the users of
> pci_sbdf_t are very limited at this point. If you agree with this I
> will add such a patch at the beginning of the series.

Well, I'm okay with either, as each has it's up and down sides:
"fn" is more consistent with "devfn", but "func" fits better with
PCI_FUNC() (which is already not really fitting with PCI_DEVFN(),
just like PCI_SLOT() isn't).

Therefore I wouldn't object to sticking to func, but since Paul
would prefer it to become fn, I'm also okay with that. Of course
just a single, consistently used name for the field as the final
result of the series would be very desirable.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel