Messages in this thread | | | Date | Mon, 24 Jan 2022 10:12:02 -0500 | Subject | Re: [PATCH v2 08/30] s390/pci: stash associated GISA designation | From | Matthew Rosato <> |
| |
On 1/24/22 9:08 AM, Pierre Morel wrote: > > > On 1/14/22 21:31, Matthew Rosato wrote: >> For passthrough devices, we will need to know the GISA designation of the >> guest if interpretation facilities are to be used. Setup to stash >> this in >> the zdev and set a default of 0 (no GISA designation) for now; a >> subsequent >> patch will set a valid GISA designation for passthrough devices. >> Also, extend mpcific routines to specify this stashed designation as part >> of the mpcific command. >> >> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> >> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Reviewed-by: Eric Farman <farman@linux.ibm.com> >> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> >> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >> --- >> arch/s390/include/asm/pci.h | 1 + >> arch/s390/include/asm/pci_clp.h | 3 ++- >> arch/s390/pci/pci.c | 6 ++++++ >> arch/s390/pci/pci_clp.c | 1 + >> arch/s390/pci/pci_irq.c | 5 +++++ >> 5 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h >> index 90824be5ce9a..2474b8d30f2a 100644 >> --- a/arch/s390/include/asm/pci.h >> +++ b/arch/s390/include/asm/pci.h >> @@ -123,6 +123,7 @@ struct zpci_dev { >> enum zpci_state state; >> u32 fid; /* function ID, used by sclp */ >> u32 fh; /* function handle, used by insn's */ >> + u32 gd; /* GISA designation for passthrough */ > > I already gave my R-B, and do not want to remove it, but wouldn't it be > possible to use more explicit names like gisa_designation instead of > just gd. > It would not change anything to the functionality but would facilitate > the maintenance? >
Honestly, I don't have a strong opinion on this one -- AFAICT struct zpci_dev has a fair mix of short names (fh) and explicit names (max_bus_speed).
It does require changes to this patch and various subsequent patches -- The changes are, as you say, not functional, so I think it's not a big deal?
I do think 'gisa_designation' is too verbose though -- How about just 'gisa', this is the same name used in the structure where we get this value from (gisa in struct sie_page2)
As long as nobody objects I will s/gd/gisa/ here and in struct clp_req_set_pci, retaining review tags.
| |