Re: [RFC PATCH 7/9] cxl/pci: Add atomic notifier callback for CXL PCIe port AER internal errors

[Jonathan Cameron]

> >> +
> >> +static void __iomem *cxl_pci_port_ras(struct pci_dev *pdev)
> >> +{
> >> +	void __iomem *ras_base = NULL;  
> > Don't initialize and...  
> 
> There is possibility the incorrect PCI type is passed and this is intended to
> return NULL for these cases. Should ras_base not be preinitialized in 
> that for the scenario?

From a code point of view at least, nope - just return NULL directly
give it's an error case.

> 
> >> +
> >> +	if ((pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) ||
> >> +	    (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM)) {
> >> +		struct cxl_dport *dport;
> >> +
> >> +		find_cxl_port(&pdev->dev, &dport);
> >> +		ras_base = dport ? dport->regs.ras : NULL;  
> > 		if (dport)
> > 			return dport->regs.ras;  
> >> +	} else if (pci_pcie_type(pdev) == PCI_EXP_TYPE_UPSTREAM) {
> >> +		struct cxl_port *port = pci_to_cxl_uport(pdev);
> >> +
> >> +		ras_base = port ? port->regs.ras : NULL;  
> > 		if (port)
> > 			return port->regs.ras;  
> >> +	}  
> > return NULL;  

This is why you don't need to set ras_base.
If you get here it's always NULL.

> >> +
> >> +	return ras_base;
> >> +}
> >> +
> >> +int port_internal_err_cb(struct notifier_block *unused,
> >> +			 unsigned long event, void *ptr)
> >> +{
> >> +	struct pci_dev *pdev = (struct pci_dev *)ptr;  
> > 
> > I think you can use this notifier for other types of device in future?
> > If it's going to be global definitely want to check here that we
> > actually have a CXL port of some type.
> > 
> > It may be that via the various calls any non CXL device
> > will result in a safe error. However that's not obvious, so an
> > upfront check makes sense (or a per device notifier registration!)
> >   
> 
> cxl_pci_port_ras() performs PCIe type check and sets RAS base to NULL if