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.
