[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <w35is7slftofgexvxqtmz22nabb7g6c2euihmq5yprlninqjhm@2lqsxedcjy6o>
Date: Tue, 5 Nov 2024 14:39:56 +0100
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Philipp Stanner <pstanner@...hat.com>
Cc: linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org
Subject: Re: [PATCH] pwm: Replace deprecated PCI functions
Hello,
[adding Bjorn and linux-pci to Cc:]
On Tue, Nov 05, 2024 at 10:26:42AM +0100, Philipp Stanner wrote:
> diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> index fb3eadf6fbc4..9101a89447d6 100644
> --- a/drivers/pwm/pwm-dwc.c
> +++ b/drivers/pwm/pwm-dwc.c
> @@ -66,20 +66,16 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
>
> pci_set_master(pci);
>
> - ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci));
> - if (ret)
> - return dev_err_probe(dev, ret, "Failed to iomap PCI BAR\n");
> -
> info = (const struct dwc_pwm_info *)id->driver_data;
> ddata = devm_kzalloc(dev, struct_size(ddata, chips, info->nr), GFP_KERNEL);
> if (!ddata)
> return -ENOMEM;
>
> - /*
> - * No need to check for pcim_iomap_table() failure,
> - * pcim_iomap_regions() already does it for us.
> - */
> - ddata->io_base = pcim_iomap_table(pci)[0];
> + ddata->io_base = pcim_iomap_region(pci, 0, "pwm-dwc");
> + ret = PTR_ERR_OR_ZERO(ddata->io_base);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to iomap PCI BAR\n");
I'd say the following is more natural:
ddata->io_base = pcim_iomap_region(pci, 0, "pwm-dwc");
if (IS_ERR(ddata->io_base))
return dev_err_probe(dev, PTR_ERR(ddata->io_base), "Failed to iomap PCI BAR\n");
(maybe with a local variable for ddata->io_base?)
> +
> ddata->info = info;
>
> for (idx = 0; idx < ddata->info->nr; idx++) {
> diff --git a/drivers/pwm/pwm-lpss-pci.c b/drivers/pwm/pwm-lpss-pci.c
> index f7ece2809e6b..823f570afb80 100644
> --- a/drivers/pwm/pwm-lpss-pci.c
> +++ b/drivers/pwm/pwm-lpss-pci.c
> [...]
> @@ -25,12 +26,12 @@ static int pwm_lpss_probe_pci(struct pci_dev *pdev,
> if (err < 0)
> return err;
>
> - err = pcim_iomap_regions(pdev, BIT(0), pci_name(pdev));
> - if (err)
> - return err;
> + io_base = pcim_iomap_region(pdev, 0, "pwm-lpss");
> + if (IS_ERR(io_base))
> + return PTR_ERR(io_base);
>
> info = (struct pwm_lpss_boardinfo *)id->driver_data;
> - chip = devm_pwm_lpss_probe(&pdev->dev, pcim_iomap_table(pdev)[0], info);
> + chip = devm_pwm_lpss_probe(&pdev->dev, io_base, info);
> if (IS_ERR(chip))
> return PTR_ERR(chip);
I remember I didn't like it when pcim_iomap_table(pdev)[0] was
introduced. Glad it can go away.
Best regards
Uwe
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists