Ivan Kokshaysky wrote:
> On Sun, Feb 06, 2005 at 03:06:11PM +0000, Matthew Wilcox wrote:
>
>>Looks pretty good to me. For clarity, I'd change:
>>
>>- m7101 = pci_scan_single_device(dev->bus, 0x18);
>>+ m7101 = pci_scan_single_device(dev->bus, PCI_DEVFN(3, 0));
>
>
> No, it's pretty broken regardless of the change. The integrated devices
> on ALi southbridges (IDE, PMU, USB etc.) have programmable IDSELs,
> so using hardcoded devfn is just dangerous. We must figure out what
> the device number PMU will have before we turn it on.
> Something like this:
>
> // NOTE: These values are for m1543. Someone must verify whether
> // they are the same for m1533 or not.
> static char pmu_idsels[4] __devinitdata = { 28, 29, 14, 15 };
>
> ...
>
> /* Get IDSEL mapping for PMU (bits 2-3 of 0x72). */
> pci_read_config_byte(dev, 0x72, &val);
> devnum = pmu_idsel[(val >> 2) & 3] - 11;
> m7101 = pci_get_slot(dev->bus, PCI_DEVFN(devnum, 0));
> if (m7101) {
> /* Failure - there is another device with the same number. */
> pci_dev_put(m7101);
> return;
> }
>
> /* Enable PMU. */
> ...
>
> m7101 = pci_scan_single_device(dev->bus, PCI_DEVFN(devnum, 0));
> ...
In fact, the patch is completely wrong for M1533 south bridge, it will
work only with M1543.
M1533 has different PCI config. register layout - the bit 2 of 0x5F
register is not used for enabling/disabling M7101 but for "on-chip
arbiter control". M7101 can be enabled/disabled using bit 6 of 0x5D
register... M1533 has also different M7101 registers.
The best thing about this is that these two M7101s have the same PCI
device IDs (0x7101).
The south bridges have the same PCI IDs too (0x1533 used by both M1533
and M1543). But according to the datasheet, M1543 should have revision
number at least 0xC0.
--
Ondrej Zary
