diff mbox series

[v7,4/5] gpio: Replace deprecated PCI functions

Message ID 20241014075329.10400-5-pstanner@redhat.com (mailing list archive)
State New
Headers show
Series PCI: Remove most pcim_iounmap_regions() users | expand

Commit Message

Philipp Stanner Oct. 14, 2024, 7:53 a.m. UTC
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(-)

Comments

Bartosz Golaszewski Oct. 14, 2024, 7:59 a.m. UTC | #1
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
Philipp Stanner Oct. 14, 2024, 8:08 a.m. UTC | #2
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
>
Bartosz Golaszewski Oct. 14, 2024, 8:15 a.m. UTC | #3
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
Philipp Stanner Oct. 14, 2024, 8:27 a.m. UTC | #4
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
>
Simon Horman Oct. 14, 2024, 12:13 p.m. UTC | #5
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
> 
>
Philipp Stanner Oct. 14, 2024, 12:59 p.m. UTC | #6
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 mbox series

Patch

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);