Re: [PATCH 7/7] pci: aspeed: Add ASPEED PCIe host controller driver

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Jun 23, 2025 at 05:41:13AM +0000, Jacky Chou wrote:

[Apparently you trimmed out some of the lines that show who said what;
there should be more lines here like:

  > On Fri, Jun 13, 2025 at 03:03:55PM +0300, Ilpo Järvinen wrote:
  > > On Fri, 13 Jun 2025, Jacky Chou wrote:
]

> > > +#include <linux/irqchip/chained_irq.h> #include <linux/irqdomain.h>

> > > +/* TLP configuration type 0 and type 1 */
> > > +#define CRG_READ_FMTTYPE(type)		(0x04000000 | (type << 24))
> > > +#define CRG_WRITE_FMTTYPE(type)		(0x44000000 | (type << 24))
> > 
> > These are straight from PCIe spec, right?
> > 
> > I think those should come from defines inside
> > include/uapi/linux/pci_regs.h, there might not be one already, so
> > you might have to add them.
> > 
> > I also think you should actually use the type as boolean, and
> > return one of the two defines based on it. A helper to do that
> > might be generic PCI header material as well.
> > 
> 
> Agreed.  This definition is used on TLP header.  Maybe I will try to
> add some definitions to pci_regs.h or pci.h

This values might come from the PCIe spec, but unless they are needed
outside drivers/pci, any #defines should probably go in
drivers/pci/pci.h.