Re: [PATCH v10 2/3] PCI: dwc: intel: PCIe RC controller driver

[Andrew Murray]

On Wed, Dec 11, 2019 at 08:20:22AM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 11, 2019 at 05:59:58PM +0800, Dilip Kota wrote:
> > 
> > On 12/11/2019 7:49 AM, Bjorn Helgaas wrote:
> > > On Fri, Dec 06, 2019 at 03:27:49PM +0800, Dilip Kota wrote:
> > > > Add support to PCIe RC controller on Intel Gateway SoCs.
> > > > PCIe controller is based of Synopsys DesignWare PCIe core.
> > > > 
> > > > Intel PCIe driver requires Upconfigure support, Fast Training
> > > > Sequence and link speed configurations. So adding the respective
> > > > helper functions in the PCIe DesignWare framework.
> > > > It also programs hardware autonomous speed during speed
> > > > configuration so defining it in pci_regs.h.
> > > > 
> > > > Also, mark Intel PCIe driver depends on MSI IRQ Domain
> > > > as Synopsys DesignWare framework depends on the
> > > > PCI_MSI_IRQ_DOMAIN.
> > > > 
> > > > Signed-off-by: Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> > > > Reviewed-by: Andrew Murray <andrew.murray@xxxxxxx>
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > > > Acked-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
> 
> > > > +static void pcie_update_bits(void __iomem *base, u32 ofs, u32 mask, u32 val)
> > > > +{
> > > > +	u32 old;
> > > > +
> > > > +	old = readl(base + ofs);
> > > > +	val = (old & ~mask) | (val & mask);
> > > > +
> > > > +	if (val != old)
> > > > +		writel(val, base + ofs);
> > > I assume this is never used on registers where the "old & ~mask" part
> > > contains RW1C bits?  If there are RW1C bits in that part, this will
> > > corrupt them.
> > There is no impact because RW1C bits of respective registers are 0s at the
> > time of this function call.
> 
> Sounds ... dangerous, but I'll take your word for it.
> 
> > I see, this patch series is merged in the maintainer tree.
> > Should i need to submit as a separate patch on top of maintainer tree or
> > submit the new version of whole patch series?
> > Please let me know the best practice.
> 
> Sorry, I didn't realize this had already been merged to Lorenzo's
> tree.  But it's not upstream (in Linus' tree) yet.  I don't know how
> Andrew and Lorenzo want to handle this.  None of these are important,
> so you could just ignore these comments.
> 
> What I personally would do is rebase the branch, e.g.,
> lpieralisi/pci/dwc, and apply an incremental patch.  But it's up to
> Andrew and Lorenzo whether they want to do anything.

Hi Dilip,

Thanks for taking on this additional feedback.

I'd be happy for you to send an additional patch ontop of your v11 (head of
lpieralisi/pci/dwc) and we'll squash it in.

Thanks,

Andrew Murray

> 
> Bjorn