On Sat, Feb 8, 2020 at 2:02 AM Jon Derrick <jonathan.derrick@xxxxxxxxx> wrote: > > Update the PCIe register behaviors and comments for PCIe v5.0. I think you may elaborate the changes here, like a summary. > Replace the specific bit definitions with BIT and GENMASK to make > updating easier in the future. ... > + * Device status register has bits 6 and [3:0] W1C, [5:4] RO, > + * the rest is reserved Perhaps it makes sense to add '.' (period) to the end of sentence. And the same for the rest comments. ... > - .w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | > - PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | > - PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16, > - .ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS | > - PCI_EXP_SLTSTA_EIS) << 16, > + .w1c = (BIT(8) | GENMASK(4, 0)) << 16, > + .ro = GENMASK(7, 5) << 16, I'm not sure the new one is better. Perhaps you need to add description for the new bits and, if you consider it's needed, add a picture of the bits in the comment, like XXXX rwX1 XXrX XwXX, where r = ro, w = rw, 1 = w1c, X = rsvd. But it's up to Bjorn and you, I have no strong opinion here. Same for the rest similar changes. ... > - .rw = (PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE | > - PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE | > - PCI_EXP_RTCTL_CRSSVE), > - .ro = PCI_EXP_RTCAP_CRSVIS << 16, > + .rw = GENMASK(4, 0), > + .ro = BIT(0) << 16, Bit 0 in both rw and ro -- is it correct? -- With Best Regards, Andy Shevchenko