On Mon, Feb 8, 2021 at 22:53:54, Krzysztof Wilczyński <kw@xxxxxxxxx> wrote: > [+cc Bjorn] > > Hi Gustavo, > > [...] > > Thanks for your review. I will wait for a couple of days, before sending > > a new version of this patch series based on your feedback. > > Thank you! > > There might be one more change, and improvement, to be done as per > Bjorn's feedback, see: > > https://urldefense.com/v3/__https://lore.kernel.org/linux-pci/20210208193516.GA406304@bjorn-Precision-5520/__;!!A4F2R9G_pg!Oxp56pU_UN6M2BhfNRSdYqsFUncqVklBj_1IdLQD_w_V6dKRPDO_FjPUystMa5D39SRj8uo$ > > The code in question would be (exceprt from the patch): > > [...] > +static int dw_xdata_pcie_probe(struct pci_dev *pdev, > + const struct pci_device_id *pid) > +{ > + const struct dw_xdata_pcie_data *pdata = (void *)pid->driver_data; > + struct dw_xdata *dw; > [...] > + dw->rg_region.vaddr = pcim_iomap_table(pdev)[pdata->rg_bar]; > + if (!dw->rg_region.vaddr) > + return -ENOMEM; > [...] > > Perhaps something like the following would would? > > void __iomem * const *iomap_table; > > iomap_table = pcim_iomap_table(pdev); > if (!iomap_table) > return -ENOMEM; > > dw->rg_region.vaddr = iomap_table[pdata->rg_bar]; > if (!dw->rg_region.vaddr) > return -ENOMEM; > > With sensible error messages added, of course. What do you think? I think all the improvements are welcome. I will do that. My only doubt is if Bjorn recommends removing the iomap_table[pdata->rg_bar] check, after adding the verification on the pcim_iomap_table, because all other drivers doesn't do that. Thanks! -Gustavo > > Krzysztof
