From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brijesh Singh Subject: Re: [PATCH] ata: add AMD Seattle platform driver Date: Wed, 13 Jan 2016 10:55:04 -0600 Message-ID: <56968168.4010606@amd.com> References: <1452200002-31590-1-git-send-email-brijesh.singh@amd.com> <9923043.nz8DJ6lRiJ@wuerfel> <5693FAC2.4070003@amd.com> <4689728.Nn3Rl2cUeP@wuerfel> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4689728.Nn3Rl2cUeP@wuerfel> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Arnd Bergmann Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, brijesh.singh@amd.com, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org, robh+dt@kernel.org, galak@codeaurora.org, tj@kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-ide@vger.kernel.org Hi, On 01/12/2016 08:24 AM, Arnd Bergmann wrote: >>> >>> Isn't this the thing that ACPI based firmware would handle using AML anyway? >>> I don't think the server people would be too happy to add a new driver >>> each time a SATA controller does the LEDs slightly differently, and this >>> is really the kind of platform specific hack that AML is meant for. >>> >> Sorry I am not able understand your comment, Could you please explain me what you mean by AML is meant for this kind of platform specific hack ? >> > > I meant the sgpio register should not be exposed through a resource on > an ACPI based system but the access be hidden behind a call into an AML > method. > > You basically extend the generic AHCI driver to understand three ways of > blinking the LEDS: > > a) standard AHCI enclosure management > b) the Linux LED subsystem using whatever LED implementation the platform provides Yes I could look into exploring LED implementation for DT case. > c) calling into the ACPI interpreter to do platform specific hacks Thanks for explaining. Now I understood your comment on AML however we need to consider the following: a) activity LED blinking routines are called very frequent (10 to 100ms based on emp->state) and executing AML method that often would introduces its own overhead. b) all BIOS vendors need to implement a new methods in their DSDT and release a new BIOS. c) other OS'es (mainly Windows) driver need to be updated. We don't know how many other SoC's have similar hacky implementation which can take advantage of extending generic AHCI driver to call into AML methods for LED blink. Given some of these causes I think having a platform driver is much cleaner. I can drop DT binding part from this driver and keep just ACPI binding. Thoughts ? -Brijesh > > Arnd > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755397AbcAMRMj (ORCPT ); Wed, 13 Jan 2016 12:12:39 -0500 Received: from mail-bn1on0099.outbound.protection.outlook.com ([157.56.110.99]:1088 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755053AbcAMRMc (ORCPT ); Wed, 13 Jan 2016 12:12:32 -0500 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0O0WHRC-08-Q0E-02 X-M-MSG: Subject: Re: [PATCH] ata: add AMD Seattle platform driver To: Arnd Bergmann References: <1452200002-31590-1-git-send-email-brijesh.singh@amd.com> <9923043.nz8DJ6lRiJ@wuerfel> <5693FAC2.4070003@amd.com> <4689728.Nn3Rl2cUeP@wuerfel> CC: , , , , , , , , , , From: Brijesh Singh Message-ID: <56968168.4010606@amd.com> Date: Wed, 13 Jan 2016 10:55:04 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <4689728.Nn3Rl2cUeP@wuerfel> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.180.168.240] X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(3190300001)(377454003)(24454002)(479174004)(52604005)(189002)(199003)(105586002)(2906002)(47776003)(106466001)(101416001)(5004730100002)(99136001)(586003)(1096002)(1220700001)(93886004)(117636001)(65806001)(36756003)(64126003)(4326007)(65956001)(230700001)(59896002)(3846002)(6116002)(92566002)(65816999)(77096005)(86362001)(87936001)(5008740100001)(2950100001)(83506001)(23746002)(76176999)(87266999)(54356999)(50986999)(97736004)(110136002)(50466002)(4001350100001)(189998001)(62816006);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR12MB0720;H:atltwp02.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0720;2:ONGZQWXzkJivDCDPkxigEei1/ymHjeUxZesy9QdcI/hAoXbijwCKpsefmrflH+A59qMr8czuASN3nV77B/rFD2w8B+SPkJNLmGqXSsQdPrBXfhswS3FH4iEgSsbBRhbX+sib+36EQYzWO9ffTLVUXw==;3:IjU9Wx4pWzyC7V8WXM3LMFfEEd/nQK4kWC+qBf7amKVaGg57YG2qd38w81gUpEonzn1ilmDGMkgSiQ9wFRDe2lybyDiop7vFhLYLpDV07RLcata40gzpIdEgi5uS6t0LbQCJXa4wSpItSKRA90GLQvaDzTu+wheQXgF2jqG0vfifUZAGN1xmZ8zcANGf8IQf7h0i8D/QoUlcNJncmJXw/UbhyaZadqUC1kFWyoZomAA=;25:WySl3F+rCVOU8GKIojX90lwCavGKIt0rioA/FgN4ICwJ12yBVX+v+kqYL60g553zaeSyLy4JwGkzjqK2HicOHFmRL50SbNxrNYuJozTm+RGHiMAHsyjMUkE9lGzL85NYSI+ae1l0cnO7ua2A2Qa/AwE9dhQnyZ0vG6W4nahWBT9psk5/CcG8zcMe/dOQ0oyWdxTbexO8o1j46SNBLtxZd9l1hwaWsemyfC3nDGz5As/FSNjnnW6oeWtjtG4scL62 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0720; X-MS-Office365-Filtering-Correlation-Id: 48805a7f-a5d7-4a2c-d00a-08d31c3a98ea X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0720;20:o/orbkholkaZAG0OubMbxjh8i5HBTpPCu5q1ylytIdsXkCzPQe1eb5v7kfb8TUAN0ajvsCkSRJy9Kw94x/XKFshxgJrHy46liPQFSmjRx2Xw1njzhMjjbuPBoo2p6Kc1AaCIz9y3M0CDBuO0mhblAuOloC70FIUf5ZQbDtz2JJKRmIP+z0y3sQ9H2o7Rl3k4AzX/WcTF3HfMDTOlEHx89AKuO2VlTcND62M8JfuvMYq2H3KfSHEF+FZq6Wj204XfT/KGUbTGBRFTtASdfJxfQeZmjXGiEpBPpyIw2GzTfebCnlbn04YIUEYzM0kT5oBqJ6Y1ZEM0YkzkmYTjB1hPTqZWLcFO1KMj6BIvNKRbKaakwiZZswRXZTnp9nCUxwSnKrhqfVbWcRweE1Q88tOGbPNQXA718BmUhnPMUp44ENCcdn4SvOyGzNcEpvH5g/1uS10XbaOGyCU+8BqflqCxmcKmYTjiksaooykJVzJ0M18Fz6vC//XKbe5OKB/6hmU6;4:Y+Wp534KzZsmrlL/55hK9tvKfcR+r7Gyt1bGKMCwdTzJXj11fuFgrzM4ZZtrbWB91W9cQgpo4MPZEVib5BdtahLoP3CgA/Ac2T2oRb79brhHFO1P3UwoEOSqyiefkV5QHNmAWFZzjhn+bUbq9KpUKEtu5Uy3/uwKnyditFPEEg/2o0iVa9A2ujtKkLOLPISJ/ZXysCGIPq+QpxBgCpKHhiy2CVGVbSWb1weu/GsDCKoCuYXJTCSEy8li49XTBIVGfiGs9C2ts+l42g4FbLLP6GM6ujkm2WWo4nYQPRZnjzduSpdJly3LWJuY80DTgzWhr4YFK/zu9xZwlMc7rOCMkAGunZCSUkgBTjSwZUVNK3np+X6lTJ5xknVTfV7GL888TIruOi8s0DEN2iQbTMQ7qb7AwTRIKKwDS8tuElBZ4DwTpAg/6Ve8iUENT+Ehl09V X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(13018025)(5005006)(13015025)(13017025)(520078)(8121501046)(3002001)(10201501046);SRVR:SN1PR12MB0720;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0720; X-Forefront-PRVS: 08200063E9 X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;SN1PR12MB0720;23:NPpaDaRo0zMd2U0al/sTTyxxcCHiHzWgOOKg6?= =?Windows-1252?Q?i0DoMtNvyIzsacYBNcTXY/ADlyQgL3+B5bBcS4nVXoNJvi2rDudvKAJP?= =?Windows-1252?Q?imrFOeHT5TCVYMOaDHPlE0csGnX+pvF4vQ3Vno6l4dTqFkD8xB40M0Vh?= =?Windows-1252?Q?DZn4zcpH3SrLP8Diz3bn2vc6FsSwAhJUXWbHCTFff0YtOeCAAFpy/fxB?= =?Windows-1252?Q?kRpCFSmTOTwZx70tJMqiz7zbhaZrffEIi1FtMQBDM+uTBxtEwByPht4Z?= =?Windows-1252?Q?BS6yHxbfORRQdG3htrptz9vuHrTQzShUU9pYPPKCFGKbzCsRiznuNjyb?= =?Windows-1252?Q?nXLddrL+h20y/bpxQ+ek09i7VY3laHD6RuSO6fd9zXUjxIius8QIFFC5?= =?Windows-1252?Q?MG2V9koLMGEGcR+YbeL4m0IhX7+PXWkSNkrrKJ7Jz5IBZ0DtcD13CJoA?= =?Windows-1252?Q?49mSxzPDGVKKOoYuHXXstPjbKa+Z6sekARzizt8DZCRn5qKZyV5mSey0?= =?Windows-1252?Q?FFSEe9MY8cYvsvV9KzLlN7l8PdO8xwKtPdyL8drrm3siueo8E2jczsBy?= =?Windows-1252?Q?kJfRclyWFkc/dKKBp3L0pa+uidd1heDRjoOWyo2bhaN5IdLTs4vtXYBC?= =?Windows-1252?Q?VHwgkA/Fti4nN/UV2IJslh8izWmy9QBlKCaufJ+Q43tn5Hnno/vIBt4s?= =?Windows-1252?Q?rEDphTi//WSPOdk88uKG8C9dtr4wHpr3n8DCnaODxketLP1wQIw9bXzI?= =?Windows-1252?Q?P+YcBqUrQEjzmE0GCbuwF+8zahIiWJx6X0xQcfVKMMjgRi0X2Ta7wz8D?= =?Windows-1252?Q?25teir7or+EKaDAtz7D91eomBB4fw60sKIY9ZDbWtuXcmT43T8Ew97iY?= =?Windows-1252?Q?N4vTFMIFhnn7ltX+V6JqtBbK6vTu0JB6BeDi/bGkEgEKiI+pCPP4jkQs?= =?Windows-1252?Q?+7cfhWOeff+FODniSQvuQqdnix6S6IRIJnq6eJB9gFxnq+Cc/EKD2GuM?= =?Windows-1252?Q?1650D5P2m7/6I/JasmIsu7hNIvKPdQ3ksKdNTxcoPaM71H84YHy53rgx?= =?Windows-1252?Q?cdaLqSIAeYi/DvMKAHB5G8R/tC7zY4C+qD0bYHEpbTB3P0qH6iKLDgC8?= =?Windows-1252?Q?hZaWQ6ICwEcnA7+ZIWsz1zZ+6pKjaIRGgku8QgpHG6UWkkW9TQ7uAT1v?= =?Windows-1252?Q?Qxj3vL4ZeXztPrazjL4aeqPqersUEfbf2Nds8ZzylSEXZIns68orWQv3?= =?Windows-1252?Q?TG02jmBetwhxfmYSiQF9XwOx+GZW8jBUxesa2TPeNVPBe+jy2WXDCR43?= =?Windows-1252?Q?ID7W7FW9ucp0SmC8RdWJ88hCJxeqC9DeJP1OA2urMOBwGmYtGrzVXXZj?= =?Windows-1252?Q?t93WGzeJfMt?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0720;5:kKzPjZE66JVeFO9rB1qjco6C+XsqFBNKvfQOS4lHsMsOBu8eyTsCgBMOajedhrNnwx658/tyS52ZMu0vhvZ74gsNO150hy1deFE09/F6EeerSEc+dV5BGPL/PQqHpG2V8w4PjnDFjt7awVaNZi53jg==;24:vHodzk+enWw+prdQAIVDYUmNw4SYZZ3WjoSI1kkur6dSHdf0s5rk4vXOta73bshOc2uITFm80MFzJ6yEOSBqXJxd66VgC1ILWrg1ROs2gEg=;20:EoYa6EPzbpObTkX/QULI7sR8IEXjfpREM2iK+pBIxNhjxiqf5vGo6x6e1CZOkBv2nbGUUexgz0IwklEYlx+EM08agoQYDGRUXmRe11PVt7pRu9fnUZVEnA6XYTuAA9BMfmMym7Eq6sZ5oS/s1vFT4Ra4YkCrxCkySrlW3zy+KCHO6YfOEkDCo1YL2QrmmrEZ4UfN1JpmpsCgFchT0Xq1cxzsxlBZcLxHJQ9IxJ5g6EKWaFJgWxWAh562RGK+Cmsy SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2016 16:57:17.8578 (UTC) X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.222];Helo=[atltwp02.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0720 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 01/12/2016 08:24 AM, Arnd Bergmann wrote: >>> >>> Isn't this the thing that ACPI based firmware would handle using AML anyway? >>> I don't think the server people would be too happy to add a new driver >>> each time a SATA controller does the LEDs slightly differently, and this >>> is really the kind of platform specific hack that AML is meant for. >>> >> Sorry I am not able understand your comment, Could you please explain me what you mean by AML is meant for this kind of platform specific hack ? >> > > I meant the sgpio register should not be exposed through a resource on > an ACPI based system but the access be hidden behind a call into an AML > method. > > You basically extend the generic AHCI driver to understand three ways of > blinking the LEDS: > > a) standard AHCI enclosure management > b) the Linux LED subsystem using whatever LED implementation the platform provides Yes I could look into exploring LED implementation for DT case. > c) calling into the ACPI interpreter to do platform specific hacks Thanks for explaining. Now I understood your comment on AML however we need to consider the following: a) activity LED blinking routines are called very frequent (10 to 100ms based on emp->state) and executing AML method that often would introduces its own overhead. b) all BIOS vendors need to implement a new methods in their DSDT and release a new BIOS. c) other OS'es (mainly Windows) driver need to be updated. We don't know how many other SoC's have similar hacky implementation which can take advantage of extending generic AHCI driver to call into AML methods for LED blink. Given some of these causes I think having a platform driver is much cleaner. I can drop DT binding part from this driver and keep just ACPI binding. Thoughts ? -Brijesh > > Arnd > From mboxrd@z Thu Jan 1 00:00:00 1970 From: brijesh.singh@amd.com (Brijesh Singh) Date: Wed, 13 Jan 2016 10:55:04 -0600 Subject: [PATCH] ata: add AMD Seattle platform driver In-Reply-To: <4689728.Nn3Rl2cUeP@wuerfel> References: <1452200002-31590-1-git-send-email-brijesh.singh@amd.com> <9923043.nz8DJ6lRiJ@wuerfel> <5693FAC2.4070003@amd.com> <4689728.Nn3Rl2cUeP@wuerfel> Message-ID: <56968168.4010606@amd.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 01/12/2016 08:24 AM, Arnd Bergmann wrote: >>> >>> Isn't this the thing that ACPI based firmware would handle using AML anyway? >>> I don't think the server people would be too happy to add a new driver >>> each time a SATA controller does the LEDs slightly differently, and this >>> is really the kind of platform specific hack that AML is meant for. >>> >> Sorry I am not able understand your comment, Could you please explain me what you mean by AML is meant for this kind of platform specific hack ? >> > > I meant the sgpio register should not be exposed through a resource on > an ACPI based system but the access be hidden behind a call into an AML > method. > > You basically extend the generic AHCI driver to understand three ways of > blinking the LEDS: > > a) standard AHCI enclosure management > b) the Linux LED subsystem using whatever LED implementation the platform provides Yes I could look into exploring LED implementation for DT case. > c) calling into the ACPI interpreter to do platform specific hacks Thanks for explaining. Now I understood your comment on AML however we need to consider the following: a) activity LED blinking routines are called very frequent (10 to 100ms based on emp->state) and executing AML method that often would introduces its own overhead. b) all BIOS vendors need to implement a new methods in their DSDT and release a new BIOS. c) other OS'es (mainly Windows) driver need to be updated. We don't know how many other SoC's have similar hacky implementation which can take advantage of extending generic AHCI driver to call into AML methods for LED blink. Given some of these causes I think having a platform driver is much cleaner. I can drop DT binding part from this driver and keep just ACPI binding. Thoughts ? -Brijesh > > Arnd >