Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH v2] ata: add AMD Seattle platform driver | Date | Fri, 05 Feb 2016 15:50:51 +0100 |
| |
On Tuesday 02 February 2016 12:37:58 Brijesh Singh wrote: > Hi, > > On 02/02/2016 08:08 AM, Arnd Bergmann wrote: > > On Monday 01 February 2016 16:15:59 Brijesh Singh wrote: > >>> > >>> This is where we really need the ACPI maintainers to explain the > >>> general policy for dealing with firmware updates. > >>> > >>> I would assume that adding the feature in a later firmware version > >>> is a compatible change, and the feature is non-essential (the > >>> device will work fine with the generic SATA driver, except > >>> the LEDs don't blink), so it's not a big deal, it's just what > >>> you get for having the firmware shipped before the driver is > >>> reviewed (don't do that). > >>> > >> > >> Agreed, the driver should have been reviewed earlier. And now changes in firmware will also require > >> them changing other OSes drivers. > > > > Can you explain that? I would expect the addition of some AML methods > > to be a compatible change. > > > > current DSDT entry looks like this: > > Device (SATA0) > { > ..... > > Name(_CRS, ResourceTemplate() > { > Memory32Fixed(ReadWrite, 0xE03000000, 0x000010000) /* SATA block address */ > Interrupt(ResourceConsumer, Level, ActiveHigh Exclusive,,,) { 387} > Memory32Fixed(ReadWrite, 0xE00000078, 1) /* SGPIO register */ > } > > ...... > } > > Windows driver folks were okay to look at second resource field to map the SGPIO register and program the > registers to blink the LEDs. I think as per ACPI spec, its legal to pass more than one block in resource > template and since AML method is not mandatory for non standard enclosure management hence its entirely > possible that some BIOS vendors may not implement it at all. But if they implement and decide > to expose either AML method or register map but not both then Windows driver may break.
I don't have access to the Windows source code. Is this in the architecture-independent part of their kernel, or only done on ARM64? How do they decide what the second memory range is for?
If this is now a de-facto extension to the PCI_CLASS_STORAGE_SATA_AHCI binding, it should probably be put into the next version of the AHCI spec, and then there is no problem using it.
Arnd
| |