Re: [PATCH 05/42] PCI: pci-bridge-emul: Add PCIe Root Capabilities Register
From: Pali Rohár
Date: Fri May 07 2021 - 10:40:22 EST
On Thursday 06 May 2021 18:10:09 Bjorn Helgaas wrote:
> On Thu, May 06, 2021 at 05:31:16PM +0200, Pali Rohár wrote:
> > This is 16-bit register at offset 0x1E. Rename current 'rsvd' struct member
> > to 'rootcap'.
>
> "The 16-bit Root Capabilities register is at offset 0x1e in the PCIe
> Capability."
>
> Please make the commit log complete in itself. In some contexts, the
> subject line is not visible at the same time. It's fine to repeat the
> subject in the commit log.
>
> > Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> > Reviewed-by: Marek Behún <kabel@xxxxxxxxxx>
> > Fixes: 23a5fba4d941 ("PCI: Introduce PCI bridge emulated config space common logic")
> > Cc: stable@xxxxxxxxxxxxxxx # e0d9d30b7354 ("PCI: pci-bridge-emul: Fix big-endian support")
>
> I'm not sure how people would deal with *two* SHA1s.
I guess that this is fine per stable document as it mention such example:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
I have already in past sent patches with Fixes:hash1 and CC:stable/hash2
and were taken correctly.
> This patch adds functionality, so it's not really fixing a bug in
> 23a5fba4d941.
I'm not sure what is the correct meaning of Fixes tag. I included it to
easily determinate in which commit was introduced member name "rsvd"
which should have been named "rootcap".
Submitting patches document is not fully clear for me as I understood it
that Fixes and CC:stable are two different things. E.g. it mention
"Attaching a Fixes: tag does not subvert ... the requirement to Cc:
stable@xxxxxxxxxxxxxxx on all stable patch candidates." which I
understood that patch for backporting needs to have Cc:stable:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
But I will change it as needed. Just I did not know what is "the correct
way".
> I see that e0d9d30b7354 came along later and did
> "s/u16 rsvd/__le16 rsvd/".
>
> But it seems like a lot to expect for distros and stable kernel
> maintainers to interpret this.
>
> Personally I think I would omit both Fixes: and the stable tag since
> these two patches (05 and 06) are just adding functionality.
>
> > ---
> > drivers/pci/pci-bridge-emul.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci-bridge-emul.h b/drivers/pci/pci-bridge-emul.h
> > index b31883022a8e..49bbd37ee318 100644
> > --- a/drivers/pci/pci-bridge-emul.h
> > +++ b/drivers/pci/pci-bridge-emul.h
> > @@ -54,7 +54,7 @@ struct pci_bridge_emul_pcie_conf {
> > __le16 slotctl;
> > __le16 slotsta;
> > __le16 rootctl;
> > - __le16 rsvd;
> > + __le16 rootcap;
> > __le32 rootsta;
> > __le32 devcap2;
> > __le16 devctl2;
> > --
> > 2.20.1
> >