On 2/14/23 23:08, Rick Wertenbroek wrote: > The RK3399 PCIe endpoint core supports only a single PCIe physcial > function (function number 0), therefore return -EINVAL if set_msi() is > called with a function number greater than 0. > The PCIe standard only allows the multi message capability (MMC) value > to be up to 0x5 (32 messages), therefore return -EINVAL if set_msi() is > called with a MMC value of over 0x5. > > Signed-off-by: Rick Wertenbroek <rick.wertenbroek@xxxxxxxxx> > --- > drivers/pci/controller/pcie-rockchip-ep.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c > index b7865a94e..80634b690 100644 > --- a/drivers/pci/controller/pcie-rockchip-ep.c > +++ b/drivers/pci/controller/pcie-rockchip-ep.c > @@ -294,6 +294,16 @@ static int rockchip_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, > struct rockchip_pcie *rockchip = &ep->rockchip; > u32 flags; > > + if (fn) { > + dev_err(&epc->dev, "This endpoint controller only supports a single physical function\n"); > + return -EINVAL; > + } Checking this here is late... Given that at most only one physical function is supported, the check should be in rockchip_pcie_parse_ep_dt(). Something like: err = of_property_read_u8(dev->of_node, "max-functions", &ep->epc->max_functions); if (err < 0 || ep->epc->max_functions > 1) ep->epc->max_functions = 1; And all the macros with the (fn) argument could also be simplified (argument fn removed) since fn will always be 0. > + > + if (mmc > 0x5) { > + dev_err(&epc->dev, "Number of MSI IRQs cannot be more than 32\n"); Long line. Please split it after the comma. > + return -EINVAL; > + } > + > flags = rockchip_pcie_read(rockchip, > ROCKCHIP_PCIE_EP_FUNC_BASE(fn) + > ROCKCHIP_PCIE_EP_MSI_CTRL_REG); Another nice cleanup: define ROCKCHIP_PCIE_EP_MSI_CTRL_REG to include the ROCKCHIP_PCIE_EP_FUNC_BASE(fn) addition so that we do not have to do it here all the time. -- Damien Le Moal Western Digital Research