RE: [RESEND v4 1/6] misc: Add Synopsys DesignWare xData IP driver
From: Gustavo Pimentel
Date: Tue Feb 09 2021 - 10:29:31 EST
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