From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752842AbcBEOvO (ORCPT ); Fri, 5 Feb 2016 09:51:14 -0500 Received: from mout.kundenserver.de ([217.72.192.74]:50443 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750958AbcBEOvM (ORCPT ); Fri, 5 Feb 2016 09:51:12 -0500 From: Arnd Bergmann To: Brijesh Singh Cc: Tejun Heo , linux-kernel@vger.kernel.org, hdegoede@redhat.com, linux-ide@vger.kernel.org, Graeme Gregory Subject: Re: [PATCH v2] ata: add AMD Seattle platform driver Date: Fri, 05 Feb 2016 15:50:51 +0100 Message-ID: <3134806.iILWebdeN8@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <56B0F786.9010504@amd.com> References: <1452789071-4090-1-git-send-email-brijesh.singh@amd.com> <4743985.XZ7D12RurV@wuerfel> <56B0F786.9010504@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:OSsX1NIlCMPXlOfQnrkILCi26yMU/m/uM/mz5m1QY7IUPxur70D +3o4VfdWgNjSYmnUCSJFJ/3wFGj29q3FeLR5S8Gp1ee6lyeySOiVSREprGvX+KVQsz5V/xD Kjx/tuEvsiZY8+yFun3Dd34JDgKY0on/RA9jT6mmGDmkeFNb1tdej82i3z9Zq5Z95Eub6eX kphAp+UHfNmolbWfqdejw== X-UI-Out-Filterresults: notjunk:1;V01:K0:PQXHQcKHNIg=:aK5EKvk/wBEWfLDvjDXDHh jWCV6c1L1ISagpoKg4O+YsN2CuDCqOUnVOnp0nhEhT/GiLxbwHAe7W+lB4lWQfjgyii7TVfHf Lb6tclp2TwMAZ6INACU2czO/O6AxVU8Dap+wHYqoQit3j1As0P1vuG7s4XdIEf8zykUZBQHGo bUp5rkCU7o0OY6UwlJYX480wqTKHl5RcI/IUqOcOWz+187hGmPqei96AAJ/SfGNVRvPMtajY8 mPp2948D7YZ+7d/vkhAIB6HzcxkpDfI9EQ5Ozod6QIay9bl0zlmIvLpABzFJ642ELPnJGm495 JJMVSpM2AToulpWWfxVX+HZhFlZya6YmgpB+1GSurRAmFD2HedkvhP3MjrnOYMjv0SjroNjbk DlDXNmGm7PF1YYcCtgsoqIFtZ+x+LbmURREhxmjKFB4ncsgs9V0p+9VTLWOWVEl3cESssrJvW OyropPp/ujRn4JgNO7nE8Pw9BXCiwaoS4w2DjFBtsiOONsGeRgjduHGL+zli/H+4DEy8kG6rT yCjepegGBr+4wGWoKBcsyCLoVU/+LpkRWGqElwMPdZIiL03dL3a791hJbofSTqnVy7FCTPGVK WHFKxCC/GkROxHoh45T0pjGj5X04QpY/RxrVk34UE7K527FVlKsxntWijVAnQlSyKlqE/P9Y4 e9RhbF2UepsmgAI0PjXgesQnWplIhKZ8YujlzpAoade1MQzR067IgufUTcmJainchZMgYIWgt FWxwyfMevbMCnKZi Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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