Re: [RFC 1/9] PCI: pci-bridge-emul: Update PCIe register behaviors

From: Andy Shevchenko
Date: Sat Feb 08 2020 - 04:56:28 EST


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