On Sat, Mar 30, 2024 at 01:19:11PM +0900, Damien Le Moal wrote: > Introduce the epc core helper function pci_epc_function_is_valid() to > verify that an epc pointer, a physical function number and a virtual > function number are all valid. This avoids repeating the code pattern: > > if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > return err; > > if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > return err; > > in many functions of the endpoint controller core code. > > Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx> One nit below. With that fixed, Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > --- > drivers/pci/endpoint/pci-epc-core.c | 79 +++++++++++------------------ > 1 file changed, 31 insertions(+), 48 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > index da3fc0795b0b..754afd115bbd 100644 > --- a/drivers/pci/endpoint/pci-epc-core.c > +++ b/drivers/pci/endpoint/pci-epc-core.c > @@ -126,6 +126,18 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features > } > EXPORT_SYMBOL_GPL(pci_epc_get_next_free_bar); > > +static inline bool pci_epc_function_is_valid(struct pci_epc *epc, > + u8 func_no, u8 vfunc_no) No need to add 'inline' keyword to function definitions in a .c file. Compiler will handle that. - Mani > +{ > + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > + return false; > + > + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + return false; > + > + return true; > +} > + > /** > * pci_epc_get_features() - get the features supported by EPC > * @epc: the features supported by *this* EPC device will be returned > @@ -143,10 +155,7 @@ const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc, > { > const struct pci_epc_features *epc_features; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return NULL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return NULL; > > if (!epc->ops->get_features) > @@ -216,10 +225,7 @@ int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > if (!epc->ops->raise_irq) > @@ -260,10 +266,7 @@ int pci_epc_map_msi_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc)) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > if (!epc->ops->map_msi_irq) > @@ -291,10 +294,7 @@ int pci_epc_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no) > { > int interrupt; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return 0; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return 0; > > if (!epc->ops->get_msi) > @@ -327,11 +327,10 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u8 interrupts) > int ret; > u8 encode_int; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - interrupts < 1 || interrupts > 32) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (interrupts < 1 || interrupts > 32) > return -EINVAL; > > if (!epc->ops->set_msi) > @@ -359,10 +358,7 @@ int pci_epc_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) > { > int interrupt; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return 0; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return 0; > > if (!epc->ops->get_msix) > @@ -395,11 +391,10 @@ int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - interrupts < 1 || interrupts > 2048) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (interrupts < 1 || interrupts > 2048) > return -EINVAL; > > if (!epc->ops->set_msix) > @@ -426,10 +421,7 @@ EXPORT_SYMBOL_GPL(pci_epc_set_msix); > void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > phys_addr_t phys_addr) > { > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return; > > if (!epc->ops->unmap_addr) > @@ -457,10 +449,7 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > if (!epc->ops->map_addr) > @@ -487,12 +476,11 @@ EXPORT_SYMBOL_GPL(pci_epc_map_addr); > void pci_epc_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > struct pci_epf_bar *epf_bar) > { > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - (epf_bar->barno == BAR_5 && > - epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (epf_bar->barno == BAR_5 && > + epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) > return; > > if (!epc->ops->clear_bar) > @@ -519,18 +507,16 @@ int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > int ret; > int flags = epf_bar->flags; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions || > - (epf_bar->barno == BAR_5 && > - flags & PCI_BASE_ADDRESS_MEM_TYPE_64) || > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > + return -EINVAL; > + > + if ((epf_bar->barno == BAR_5 && flags & PCI_BASE_ADDRESS_MEM_TYPE_64) || > (flags & PCI_BASE_ADDRESS_SPACE_IO && > flags & PCI_BASE_ADDRESS_IO_MASK) || > (upper_32_bits(epf_bar->size) && > !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))) > return -EINVAL; > > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > - return -EINVAL; > - > if (!epc->ops->set_bar) > return 0; > > @@ -559,10 +545,7 @@ int pci_epc_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > { > int ret; > > - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > - return -EINVAL; > - > - if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no)) > return -EINVAL; > > /* Only Virtual Function #1 has deviceID */ > -- > 2.44.0 > -- மணிவண்ணன் சதாசிவம்