Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

Alessandro Suardi
2007-01-02T12:07:55Z

On 1/2/07, Alan <alan@lxorguk.ukuu.org.uk> wrote:
> This is a slight variant on the patch I posted December 16th to fix
> libata combined mode handling. The only real change is that we now
> correctly also reserve BAR1,2,4. That is basically a neatness issue.
>
> Jeff was unhappy about two things
>
> 1. That it didn't work in the case of one channel native one channel
> legacy.
>
> This is a silly complaint because the SFF layer in libata doesn't handle
> this case yet anyway.
>
> 2. The case where combined mode is in use and IDE=n.
>
> In this case the libata quirk code reserves the resources in question
> correctly already.
>
> Once the combined mode stuff is redone properly (2.6.21) then the entire
> mess turns into a single pci_request_regions() for all cases and all the
> ugly resource hackery goes away.
>
> I'm sending this now rather than after running full test suites so that
> it can get the maximal testing in a short time. I'll be running tests on
> this after lunch.
>
> Signed-off-by: Alan Cox <alan@redhat.com>
>
> --- linux.vanilla-2.6.20-rc3/drivers/ata/libata-sff.c 2007-01-01 21:43:27.000000000 +0000
> +++ linux-2.6.20-rc3/drivers/ata/libata-sff.c 2007-01-02 11:15:53.000000000 +0000
> @@ -1027,13 +1027,15 @@
> #endif
> }
>
> - rc = pci_request_regions(pdev, DRV_NAME);
> - if (rc) {
> - disable_dev_on_err = 0;
> - goto err_out;
> - }
> -
> - if (legacy_mode) {
> + if (!legacy_mode) {
> + rc = pci_request_regions(pdev, DRV_NAME);
> + if (rc) {
> + disable_dev_on_err = 0;
> + goto err_out;
> + }
> + } else {
> + /* Deal with combined mode hack. This side of the logic all
> + goes away once the combined mode hack is killed in 2.6.21 */
> if (!request_region(ATA_PRIMARY_CMD, 8, "libata")) {
> struct resource *conflict, res;
> res.start = ATA_PRIMARY_CMD;
> @@ -1071,6 +1073,13 @@
> }
> } else
> legacy_mode |= ATA_PORT_SECONDARY;
> +
> + if (legacy_mode & ATA_PORT_PRIMARY)
> + pci_request_region(pdev, 1, DRV_NAME);
> + if (legacy_mode & ATA_PORT_SECONDARY)
> + pci_request_region(pdev, 3, DRV_NAME);
> + /* If there is a DMA resource, allocate it */
> + pci_request_region(pdev, 4, DRV_NAME);
> }
>
> /* we have legacy mode, but all ports are unavailable */
> @@ -1114,11 +1123,20 @@
> err_out_ent:
> kfree(probe_ent);
> err_out_regions:
> - if (legacy_mode & ATA_PORT_PRIMARY)
> - release_region(ATA_PRIMARY_CMD, 8);
> - if (legacy_mode & ATA_PORT_SECONDARY)
> - release_region(ATA_SECONDARY_CMD, 8);
> - pci_release_regions(pdev);
> + /* All this conditional stuff is needed for the combined mode hack
> + until 2.6.21 when it can go */
> + if (legacy_mode) {
> + pci_release_region(pdev, 4);
> + if (legacy_mode & ATA_PORT_PRIMARY) {
> + release_region(ATA_PRIMARY_CMD, 8);
> + pci_release_region(pdev, 1);
> + }
> + if (legacy_mode & ATA_PORT_SECONDARY) {
> + release_region(ATA_SECONDARY_CMD, 8);
> + pci_release_region(pdev, 3);
> + }
> + } else
> + pci_release_regions(pdev);
> err_out:
> if (disable_dev_on_err)
> pci_disable_device(pdev);
>

Appears to work just fine here (compiles, boots and I'm
typing this email :). The build warnings below seem new
to me - but I guess they're harmless...

CC drivers/ata/libata-sff.o
drivers/ata/libata-sff.c: In function 'ata_pci_init_one':
drivers/ata/libata-sff.c:1078: warning: ignoring return value of
'pci_request_region', declared with attribute warn_unused_result
drivers/ata/libata-sff.c:1080: warning: ignoring return value of
'pci_request_region', declared with attribute warn_unused_result
drivers/ata/libata-sff.c:1082: warning: ignoring return value of
'pci_request_region', declared with attribute warn_unused_result

Thanks, ciao,

--alessandro

"but I thought that I should let you know
the things that I don't always show
might not be worth the time it took"

(Steve Wynn, 'If My Life Was An Open Book')