On 12/4/24 8:10 PM, Philipp Stanner wrote:
> pcim_iomap_regions() has been deprecated by the PCI subsystem.
>
> Unfortunately, libata-sff uses quite complicated bit mask magic to
> obtain its PCI resources.
>
> Restructure and simplify the PCI resource request code.
>
> Replace pcim_iomap_regions() with pcim_iomap_region().
>
> Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx>
[...]
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 67f277e1c3bf..1d2273c0f447 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -2127,6 +2127,73 @@ static bool ata_resources_present(struct pci_dev *pdev, int port)
> return true;
> }
>
> +static void ata_pci_sff_set_ap_data(struct ata_port *ap, struct ata_host *host,
Maybe ata_pci_sff_set_port_data()?
> + struct pci_dev *pdev, unsigned short base)
Using just 2 tabs here seems to go against the coding style used in libata-sff.c
but no biggie... :-)
> +{
> + void __iomem *ctl_addr;
> +
> + ctl_addr = host->iomap[base + 1];
> + ctl_addr = (void __iomem *)((unsigned long)ctl_addr | ATA_PCI_CTL_OFS);
> +
> + ap->ioaddr.cmd_addr = host->iomap[base];
> + ap->ioaddr.altstatus_addr = ctl_addr;
> + ap->ioaddr.ctl_addr = ctl_addr;
> +
> + ata_sff_std_ports(&ap->ioaddr);
> +
> + ata_port_desc(ap, "cmd 0x%llx ctl 0x%llx",
> + (unsigned long long)pci_resource_start(pdev, base),
> + (unsigned long long)pci_resource_start(pdev, base + 1));
> +}
> +
> +/*
Please use /** here -- the comment seems to otherwise match the kernel-doc
format...
> + * ata_pci_sff_obtain_bars - obtain the PCI BARs associated with an ATA port
> + * @pdev: the PCI device
> + * @host: the ATA host
> + * @ap: the ATA port
> + * @port: @ap's port index in @host
> + *
> + * Returns: Number of successfully ioremaped BARs, a negative code on failure
> + */
> +static int ata_pci_sff_obtain_bars(struct pci_dev *pdev, struct ata_host *host,
> + struct ata_port *ap, unsigned short port)
> +{
> + int ret = 0, bars_mapped = 0;
> + unsigned short i, base;
> + void __iomem *io_tmp;
Maybe call it iomem instead?
> + const char *name = dev_driver_string(&pdev->dev);
> +
> + /*
> + * Port can be 0 or 1.
> + * Port 0 corresponds to PCI BARs 0 and 1, port 1 to BARs 2 and 3.
> + */
> + base = port * 2;
> +
> + /*
Don't indent with spaces please.
> + * Discard disabled ports. Some controllers show their unused channels
> + * this way. Disabled ports are made dummy.
> + */
These 3 lines lack a space before *.
[...]
> + for (i = 0; i < 2; i++) {
> + io_tmp = pcim_iomap_region(pdev, base + i, name);
> + ret = PTR_ERR_OR_ZERO(io_tmp);
> + if (ret != 0)
!= 0 unnecessary.
[...]
> @@ -2148,59 +2215,31 @@ static bool ata_resources_present(struct pci_dev *pdev, int port)
[...]> - if (!mask) {
> - dev_err(gdev, "no available native port\n");
> + if (operational_ports == 0)
Perhaps:
if (!operational_ports)
[...]
MBR, Sergey
