| Message ID | 20241014075329.10400-5-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI: Remove most pcim_iounmap_regions() users | expand |
On Mon, Oct 14, 2024 at 9:53 AM Philipp Stanner <pstanner@redhat.com> wrote: > > pcim_iomap_regions() and pcim_iomap_table() have been deprecated by the > PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > Replace those functions with calls to pcim_iomap_region(). > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > Reviewed-by: Andy Shevchenko <andy@kernel.org> > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- This is part of a larger series so I acked it previously but at second glance it doesn't look like it depends on anything that comes before? Should it have been sent separately to the GPIO tree? Should I pick it up independently? Bart
On Mon, 2024-10-14 at 09:59 +0200, Bartosz Golaszewski wrote: > On Mon, Oct 14, 2024 at 9:53 AM Philipp Stanner <pstanner@redhat.com> > wrote: > > > > pcim_iomap_regions() and pcim_iomap_table() have been deprecated by > > the > > PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > > > Replace those functions with calls to pcim_iomap_region(). > > > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > --- > > This is part of a larger series so I acked it previously but at > second > glance it doesn't look like it depends on anything that comes before? > Should it have been sent separately to the GPIO tree? Should I pick > it > up independently? Thx for the offer, but it depends on pcim_iounmap_region(), which only becomes a public symbol through patch No.1 of this series :) P. > > Bart >
On Mon, Oct 14, 2024 at 10:08 AM Philipp Stanner <pstanner@redhat.com> wrote: > > On Mon, 2024-10-14 at 09:59 +0200, Bartosz Golaszewski wrote: > > On Mon, Oct 14, 2024 at 9:53 AM Philipp Stanner <pstanner@redhat.com> > > wrote: > > > > > > pcim_iomap_regions() and pcim_iomap_table() have been deprecated by > > > the > > > PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > > > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > > > > > Replace those functions with calls to pcim_iomap_region(). > > > > > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > > > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > --- > > > > This is part of a larger series so I acked it previously but at > > second > > glance it doesn't look like it depends on anything that comes before? > > Should it have been sent separately to the GPIO tree? Should I pick > > it > > up independently? > > Thx for the offer, but it depends on pcim_iounmap_region(), which only > becomes a public symbol through patch No.1 of this series :) > Then a hint: to make it more obvious to maintainers, I'd change the commit title for patch 1 to say explicitly it makes this function public. In fact: I'd split it and the deprecation into two separate patches. Bart
On Mon, 2024-10-14 at 10:15 +0200, Bartosz Golaszewski wrote: > On Mon, Oct 14, 2024 at 10:08 AM Philipp Stanner > <pstanner@redhat.com> wrote: > > > > On Mon, 2024-10-14 at 09:59 +0200, Bartosz Golaszewski wrote: > > > On Mon, Oct 14, 2024 at 9:53 AM Philipp Stanner > > > <pstanner@redhat.com> > > > wrote: > > > > > > > > pcim_iomap_regions() and pcim_iomap_table() have been > > > > deprecated by > > > > the > > > > PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > > > > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > > > > > > > Replace those functions with calls to pcim_iomap_region(). > > > > > > > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > > > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > > > > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > --- > > > > > > This is part of a larger series so I acked it previously but at > > > second > > > glance it doesn't look like it depends on anything that comes > > > before? > > > Should it have been sent separately to the GPIO tree? Should I > > > pick > > > it > > > up independently? > > > > Thx for the offer, but it depends on pcim_iounmap_region(), which > > only > > becomes a public symbol through patch No.1 of this series :) > > > > Then a hint: to make it more obvious to maintainers, I'd change the > commit title for patch 1 to say explicitly it makes this function > public. In fact: I'd split it and the deprecation into two separate > patches. Yeah, good idea. The maintainer could squash then if two atomic patches are deemed undesirable. Noted. Thank you! P. > > Bart >
On Mon, Oct 14, 2024 at 09:53:25AM +0200, Philipp Stanner wrote: > pcim_iomap_regions() and pcim_iomap_table() have been deprecated by the > PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > Replace those functions with calls to pcim_iomap_region(). > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > Reviewed-by: Andy Shevchenko <andy@kernel.org> > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > drivers/gpio/gpio-merrifield.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c > index 421d7e3a6c66..274afcba31e6 100644 > --- a/drivers/gpio/gpio-merrifield.c > +++ b/drivers/gpio/gpio-merrifield.c > @@ -78,24 +78,24 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id > if (retval) > return retval; > > - retval = pcim_iomap_regions(pdev, BIT(1) | BIT(0), pci_name(pdev)); > - if (retval) > - return dev_err_probe(dev, retval, "I/O memory mapping error\n"); > - > - base = pcim_iomap_table(pdev)[1]; > + base = pcim_iomap_region(pdev, 1, pci_name(pdev)); > + if (IS_ERR(base)) > + return dev_err_probe(dev, PTR_ERR(base), "I/O memory mapping error\n"); > > irq_base = readl(base + 0 * sizeof(u32)); > gpio_base = readl(base + 1 * sizeof(u32)); > > /* Release the IO mapping, since we already get the info from BAR1 */ > - pcim_iounmap_regions(pdev, BIT(1)); > + pcim_iounmap_region(pdev, 1); > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > return -ENOMEM; > > priv->dev = dev; > - priv->reg_base = pcim_iomap_table(pdev)[0]; > + priv->reg_base = pcim_iomap_region(pdev, 0, pci_name(pdev)); > + if (IS_ERR(priv->reg_base)) > + return dev_err_probe(dev, PTR_ERR(base), "I/O memory mapping error\n"); Hi Philipp, There seems to be a mismatch in the use of priv->reg_base and base above. Should the above use PTR_ERR(priv->reg_base) instead of PTR_ERR(base)? > > priv->pin_info.pin_ranges = mrfld_gpio_ranges; > priv->pin_info.nranges = ARRAY_SIZE(mrfld_gpio_ranges); > -- > 2.46.2 > >
On Mon, 2024-10-14 at 13:13 +0100, Simon Horman wrote: > On Mon, Oct 14, 2024 at 09:53:25AM +0200, Philipp Stanner wrote: > > pcim_iomap_regions() and pcim_iomap_table() have been deprecated by > > the > > PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > > > Replace those functions with calls to pcim_iomap_region(). > > > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > --- > > drivers/gpio/gpio-merrifield.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio- > > merrifield.c > > index 421d7e3a6c66..274afcba31e6 100644 > > --- a/drivers/gpio/gpio-merrifield.c > > +++ b/drivers/gpio/gpio-merrifield.c > > @@ -78,24 +78,24 @@ static int mrfld_gpio_probe(struct pci_dev > > *pdev, const struct pci_device_id *id > > if (retval) > > return retval; > > > > - retval = pcim_iomap_regions(pdev, BIT(1) | BIT(0), > > pci_name(pdev)); > > - if (retval) > > - return dev_err_probe(dev, retval, "I/O memory > > mapping error\n"); > > - > > - base = pcim_iomap_table(pdev)[1]; > > + base = pcim_iomap_region(pdev, 1, pci_name(pdev)); > > + if (IS_ERR(base)) > > + return dev_err_probe(dev, PTR_ERR(base), "I/O > > memory mapping error\n"); > > > > irq_base = readl(base + 0 * sizeof(u32)); > > gpio_base = readl(base + 1 * sizeof(u32)); > > > > /* Release the IO mapping, since we already get the info > > from BAR1 */ > > - pcim_iounmap_regions(pdev, BIT(1)); > > + pcim_iounmap_region(pdev, 1); > > > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > if (!priv) > > return -ENOMEM; > > > > priv->dev = dev; > > - priv->reg_base = pcim_iomap_table(pdev)[0]; > > + priv->reg_base = pcim_iomap_region(pdev, 0, > > pci_name(pdev)); > > + if (IS_ERR(priv->reg_base)) > > + return dev_err_probe(dev, PTR_ERR(base), "I/O > > memory mapping error\n"); > > Hi Philipp, > > There seems to be a mismatch in the use of priv->reg_base and base > above. > Should the above use PTR_ERR(priv->reg_base) instead of > PTR_ERR(base)? uff, yes, good catch! Will fix, thx P. > > > > > priv->pin_info.pin_ranges = mrfld_gpio_ranges; > > priv->pin_info.nranges = ARRAY_SIZE(mrfld_gpio_ranges); > > -- > > 2.46.2 > > > > >
diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c index 421d7e3a6c66..274afcba31e6 100644 --- a/drivers/gpio/gpio-merrifield.c +++ b/drivers/gpio/gpio-merrifield.c @@ -78,24 +78,24 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id if (retval) return retval; - retval = pcim_iomap_regions(pdev, BIT(1) | BIT(0), pci_name(pdev)); - if (retval) - return dev_err_probe(dev, retval, "I/O memory mapping error\n"); - - base = pcim_iomap_table(pdev)[1]; + base = pcim_iomap_region(pdev, 1, pci_name(pdev)); + if (IS_ERR(base)) + return dev_err_probe(dev, PTR_ERR(base), "I/O memory mapping error\n"); irq_base = readl(base + 0 * sizeof(u32)); gpio_base = readl(base + 1 * sizeof(u32)); /* Release the IO mapping, since we already get the info from BAR1 */ - pcim_iounmap_regions(pdev, BIT(1)); + pcim_iounmap_region(pdev, 1); priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; priv->dev = dev; - priv->reg_base = pcim_iomap_table(pdev)[0]; + priv->reg_base = pcim_iomap_region(pdev, 0, pci_name(pdev)); + if (IS_ERR(priv->reg_base)) + return dev_err_probe(dev, PTR_ERR(base), "I/O memory mapping error\n"); priv->pin_info.pin_ranges = mrfld_gpio_ranges; priv->pin_info.nranges = ARRAY_SIZE(mrfld_gpio_ranges);
pcim_iomap_regions() and pcim_iomap_table() have been deprecated by the PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate pcim_iomap_table(), pcim_iomap_regions_request_all()"). Replace those functions with calls to pcim_iomap_region(). Signed-off-by: Philipp Stanner <pstanner@redhat.com> Reviewed-by: Andy Shevchenko <andy@kernel.org> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> --- drivers/gpio/gpio-merrifield.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)