Re: [PATCH v2] ata: add AMD Seattle platform driver

[Arnd Bergmann <arnd@arndb.de>]

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