On 06/12/2022 14:46, Jan Beulich wrote:
>
>
> 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.
I'm totally fine to add an extra sentence.
>
>> 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)?
I said that it is ok to change Xen and not Linux because this file already
diverged,
so we cannot assume that future backports will apply cleanly. If we change a
file
that did not diverge, then we are required to modify the origin first and then
do the backport.
>
> Jan
~Michal
