| Message ID | 20240822134744.44919-6-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Remove pcim_iounmap_regions() | expand |
On Thu, Aug 22, 2024 at 03:47:37PM +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 these functions with the function pcim_iomap_region(). ... > - err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, pci_name(pdev)); > - if (err) > + clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, pci_name(pdev)); > + if (IS_ERR(clock->reg_base)) { > + err = PTR_ERR(clock->reg_base); > goto error_free; > - > - clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO]; > + } Perhaps clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, pci_name(pdev)); err = PTR_ERR_OR_ZERO(clock->reg_base); if (err) goto error_free; This will make your patch smaller and neater. P.S. Do you use --histogram diff algo when preparing patches?
On Thu, 2024-08-22 at 17:44 +0300, Andy Shevchenko wrote: > On Thu, Aug 22, 2024 at 03:47:37PM +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 these functions with the function pcim_iomap_region(). > > ... > > > - err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, > > pci_name(pdev)); > > - if (err) > > + clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, > > pci_name(pdev)); > > + if (IS_ERR(clock->reg_base)) { > > + err = PTR_ERR(clock->reg_base); > > goto error_free; > > - > > - clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO]; > > + } > > Perhaps > > clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, > pci_name(pdev)); > err = PTR_ERR_OR_ZERO(clock->reg_base); > if (err) > goto error_free; > > This will make your patch smaller and neater. > > P.S. Do you use --histogram diff algo when preparing patches? So far not. Should one do that? P. >
On Mon, Aug 26, 2024 at 5:51 PM Philipp Stanner <pstanner@redhat.com> wrote: > On Thu, 2024-08-22 at 17:44 +0300, Andy Shevchenko wrote: > > On Thu, Aug 22, 2024 at 03:47:37PM +0200, Philipp Stanner wrote: ... > > > - err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, > > > pci_name(pdev)); > > > - if (err) > > > + clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, > > > pci_name(pdev)); > > > + if (IS_ERR(clock->reg_base)) { > > > + err = PTR_ERR(clock->reg_base); > > > goto error_free; > > > - > > > - clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO]; > > > + } > > > > Perhaps > > > > clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, > > pci_name(pdev)); > > err = PTR_ERR_OR_ZERO(clock->reg_base); > > if (err) > > goto error_free; > > > > This will make your patch smaller and neater. > > > > P.S. Do you use --histogram diff algo when preparing patches? > > So far not. > Should one do that? Id doesn't alter your code, it's in addition to what I suggested, but as Linus shared that there is no reason to avoid using --histogram not only in Linux kernel, but in general as it produces more human-readable diff:s.
On Mon, 2024-08-26 at 18:41 +0300, Andy Shevchenko wrote: > On Mon, Aug 26, 2024 at 5:51 PM Philipp Stanner <pstanner@redhat.com> > wrote: > > On Thu, 2024-08-22 at 17:44 +0300, Andy Shevchenko wrote: > > > On Thu, Aug 22, 2024 at 03:47:37PM +0200, Philipp Stanner wrote: > > ... > > > > > - err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, > > > > pci_name(pdev)); > > > > - if (err) > > > > + clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, > > > > pci_name(pdev)); > > > > + if (IS_ERR(clock->reg_base)) { > > > > + err = PTR_ERR(clock->reg_base); > > > > goto error_free; > > > > - > > > > - clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO]; > > > > + } > > > > > > Perhaps > > > > > > clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, > > > pci_name(pdev)); > > > err = PTR_ERR_OR_ZERO(clock->reg_base); > > > if (err) > > > goto error_free; > > > > > > This will make your patch smaller and neater. > > > > > > P.S. Do you use --histogram diff algo when preparing patches? > > > > So far not. > > Should one do that? > > Id doesn't alter your code, it's in addition to what I suggested, but > as Linus shared that there is no reason to avoid using --histogram > not > only in Linux kernel, but in general as it produces more > human-readable diff:s. If the Boss says so, one can surely do that \o/ Though if it has 0 disadvantages I'd propose proposing to the git-devs to make it the default. P. >
On Mon, Aug 26, 2024 at 6:52 PM Philipp Stanner <pstanner@redhat.com> wrote: > On Mon, 2024-08-26 at 18:41 +0300, Andy Shevchenko wrote: ... > Though if it has 0 disadvantages I'd propose proposing to the git-devs > to make it the default. It's slower. so the people from https://occ.deadnet.se/about/ won't be happy. And more power consuming, so maybe not so environment friendly after all :-P
diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.c b/drivers/net/ethernet/cavium/common/cavium_ptp.c index 9fd717b9cf69..1849c62cde1d 100644 --- a/drivers/net/ethernet/cavium/common/cavium_ptp.c +++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c @@ -239,11 +239,11 @@ static int cavium_ptp_probe(struct pci_dev *pdev, if (err) goto error_free; - err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, pci_name(pdev)); - if (err) + clock->reg_base = pcim_iomap_region(pdev, PCI_PTP_BAR_NO, pci_name(pdev)); + if (IS_ERR(clock->reg_base)) { + err = PTR_ERR(clock->reg_base); goto error_free; - - clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO]; + } spin_lock_init(&clock->spin_lock); @@ -292,7 +292,7 @@ static int cavium_ptp_probe(struct pci_dev *pdev, clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG); clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN; writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG); - pcim_iounmap_regions(pdev, 1 << PCI_PTP_BAR_NO); + pcim_iounmap_region(pdev, PCI_PTP_BAR_NO); error_free: devm_kfree(dev, clock);
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 these functions with the function pcim_iomap_region(). Signed-off-by: Philipp Stanner <pstanner@redhat.com> --- drivers/net/ethernet/cavium/common/cavium_ptp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)