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

[Andy Shevchenko]

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