Re: [PATCH v2 1/2] PCI: aardvark: Add support for AER registers on emulated bridge

From: Rob Herring <robh@kernel.org>
To: "Marek Behún" <kabel@kernel.org>
Cc: "Bjorn Helgaas" <helgaas@kernel.org>,
	linux-pci@vger.kernel.org, "Pali Rohár" <pali@kernel.org>
Subject: Re: [PATCH v2 1/2] PCI: aardvark: Add support for AER registers on emulated bridge
Date: Tue, 31 May 2022 14:55:07 -0500	[thread overview]
Message-ID: <20220531195507.GC1808817-robh@kernel.org> (raw)
In-Reply-To: <20220529120813.7dcb5aaf@thinkpad>

On Sun, May 29, 2022 at 12:08:13PM +0200, Marek Behún wrote:
> On Thu, 26 May 2022 15:38:01 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Tue, May 24, 2022 at 03:28:26PM +0200, Marek Behún wrote:
> > > From: Pali Rohár <pali@kernel.org>
> > > 
> > > Aardvark controller supports Advanced Error Reporting configuration
> > > registers.
> > > 
> > > Export these registers on the emulated root bridge via the new .read_ext
> > > and .write_ext methods.
> > > 
> > > Note that in the Advanced Error Reporting Capability header the offset
> > > to the next Extended Capability header is set, but it is not documented
> > > in Armada 3700 Functional Specification. Since this change adds support
> > > only for Advanced Error Reporting, explicitly clear PCI_EXT_CAP_NEXT
> > > bits in AER capability header.
> > > 
> > > Now the pcieport driver correctly detects AER support and allows PCIe
> > > AER driver to start receiving ERR interrupts. Kernel log now says:
> > > 
> > >     [    4.358401] pcieport 0000:00:00.0: AER: enabled with IRQ 52
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>  
> > 
> > Did you mean Reviewed-by? Signed-off-by is only correct if Lorenzo 
> > applied or rewrote these. If he applied them, then Bjorn will pick them 
> > up.
> 
> Hmm. Well, Lorenzo applied the subset I am sending (patches 3 and 5) to
> his tree, with SOB, meaning to send it to Bjorn [1].
> 
> Then we discovered that patch 4 is also required for the _SHIFT
> macros, which was discussed previously that we want to avoid those, and
> use FIELD_PREP() / FIELD_GET() instead [2].
> 
> So I updated the second patch to use FIELD_PREP() / FIELD_GET() instead
> of the _SHIFT macros. I guess this version isn't SOB by Lorenzo, but
> the first version was... I should probably change it to Reviewed-by for
> both patches anyway, right?

I would suggest you send these without either (unless Lorenzo actually 
gave a Reviewed-by) and just state that Lorenzo applied these, but then 
you had to make another change as you described above.

But if Bjorn applies the original and doesn't want to rebase (he 
usually will rebase if needed), then an incremental patch will be 
needed.

Rob