On 06.12.2022 14:05, Michal Orzel wrote: > On 06/12/2022 13:42, Jan Beulich wrote: >> On 06.12.2022 11:59, Michal Orzel wrote: >>> --- a/xen/include/xen/pci_regs.h >>> +++ b/xen/include/xen/pci_regs.h >>> @@ -246,13 +246,13 @@ >>> #define PCI_PM_CTRL_STATE_MASK 0x0003 /* Current power state (D0 to >>> D3) */ >>> #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ >>> #define PCI_PM_CTRL_PME_ENABLE 0x0100 /* PME pin enable */ >>> -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ >>> -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ >>> +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ >>> +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ >>> #define PCI_PM_CTRL_PME_STATUS 0x8000 /* PME pin status */ >>> -#define PCI_PM_PPB_EXTENSIONS 6 /* PPB support extensions >>> (??) */ >>> -#define PCI_PM_PPB_B2_B3 0x40 /* Stop clock when in D3hot (??) */ >>> -#define PCI_PM_BPCC_ENABLE 0x80 /* Bus power/clock control enable >>> (??) */ >>> -#define PCI_PM_DATA_REGISTER 7 /* (??) */ >>> +#define PCI_PM_PPB_EXTENSIONS 6 /* PPB support extensions (?) >>> */ >>> +#define PCI_PM_PPB_B2_B3 0x40 /* Stop clock when in D3hot (?) */ >>> +#define PCI_PM_BPCC_ENABLE 0x80 /* Bus power/clock control enable (?) >>> */ >>> +#define PCI_PM_DATA_REGISTER 7 /* (?) */ >>> #define PCI_PM_SIZEOF 8 >> >> We've inherited all of these from Linux, and I notice Linux still has it >> this same way. Ideally Linux would change first, but I'd be okay with a >> sentence added to the description clarifying that we knowingly accept >> the divergence. > This file already diverged and we are not in sync with Linux as well.
Of course - that's the case for the majority of files we've taken from somewhere. But a Linux patch dropping the (??) parts of the comment (perhaps once whoever had put them there convinced themselves that the names of the constants and/or the comments are valid / applicable) would then no longer apply cleanly if we wanted to carry it over. Hence my request for amending the description. > Also there is no functional change being made by this patch so it is ok > to change Xen and not Linux in this case (which has quite a lot of trigraphs > all over the place). Based on what criteria are you saying this is ok (unconditionally)? Jan
