From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754005AbcAGXnt (ORCPT ); Thu, 7 Jan 2016 18:43:49 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:54692 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753855AbcAGXnr (ORCPT ); Thu, 7 Jan 2016 18:43:47 -0500 From: Arnd Bergmann To: Brijesh Singh Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, linux-ide@vger.kernel.org, robh+dt@kernel.org, galak@codeaurora.org, tj@kernel.org Subject: Re: [PATCH] ata: add AMD Seattle platform driver Date: Fri, 08 Jan 2016 00:42:54 +0100 Message-ID: <4983521.tEaWggKCCv@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <568EE596.3060005@amd.com> References: <1452200002-31590-1-git-send-email-brijesh.singh@amd.com> <4652600.rUhSEOUPkl@wuerfel> <568EE596.3060005@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:T74mDdp9rUdHg75Mmj1g/BvXUzpq4bKvTgkPw0lkR/F5PwIW0FI BUazOkmRYkgWKrFbslftOwKiFLIAWli+cU8wVLmCovlClazO0Fs/WZY1SsSKjVKFdIZe6lL UQzFstqGsDpnesr3wG/lIN7dsFc7Hkagaa9kx/ZjVS5iADYcn1kgSApMhIq6q/NQxsXcZYd SQvOXYhJZJLkyGvmyiPxQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:Uv2NG3YZRAI=:iSlnZnfEHBc0Ct7OjcuxNX j03pOH4862mxxankl44oJVHY9gjkG+Vu5zLwLXAzkdVajmUUCKP//7mDuFeBvu5MtZNXyavlh YrG4vaKSxmYHENn1RBG9TX6RM4Y0tXDOVI0cmtwFI4zg6iEY5AFpkPsxyY4V3Pdo7I7TqchI+ RyoMjckG0uSk1xEwpUmc5Unmpti5Yvlfld/d1cWR7rp3UTffHZnkq7WaW3FjLxGrxBtqaVx/b 2rhITwBfrCT/Lh04ZhSIfSxrjEMpdidQW4tDEjnyVoO0Tlal2DHGkIJ5YgFDmnaepKOQSeFLn O12BjCWMWkgS1qfBFW406QqcApse4nmsIAdNS3NmbZvEaM3ngxJrNN981h5lvYpP91/wKinxx +jD08VWGQls8r72z63qs85YIiF1y7Ayg2xuTcMdyL+ay7Mv8hDzXJgHhRoGcdxbdlWVQlrWQB OAeJKZy8/4C+PI6jSVCf8ZX54Ut7G8eANKW75Pu9P62NN0EBvAtbpgUDeIoI5aPkughY6smoa 5G48SnAEY83uFbUlJwNCLlDyNx4SnqSaxsCTCLsP5aZU4eNC6v0EoOnem/RGbbUwg2ZqiRd7B 1WH6U+ocH0dQPg/C7/6uea190wz/1WOFu6Ob9em/nq0FN4ZlCioaX+9OmEPUg2ZxmCU2YSrzk G+lajRpkZbB1gBKoMutO72NtAvf/8R1lzzcZtzYDmXsu+J6k4XpHIjqpmp8FpwVNQUD7/2b2A 4i98pMWYjTNk+nZB Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday 07 January 2016 16:24:22 Brijesh Singh wrote: > >> + > >> +Examples: > >> + sata0@e0300000 { > >> + compatible = "amd,seattle-ahci"; > >> + reg = <0x0 0xe0300000 0x0 0xf0000>, <0x0 0xe0000078 0x0 0x1>; > > > > Looking at the register values, I doubt that the SGPIO is actually part of the > > sata device. More likely, you are pointing in the middle of an actual > > GPIO controller. > > > > That address is SGPIO control register for SATA. The current hardware implementation to control activity LED is not ideal. Of course its a control register "for" SATA, what I meant is that it's not part "of" the SATA IP block, which is hopefully a standard AHCI compliant part as required by SBSA. > A57 does not have access to GPIO's connected to backplane controller > instead SoC has exposed two SGPIO control registers (LSIOC_SGPIO_CONTROL0: > 0xE000_0078 and LSIOC_SGPIO_CONTROL1: 0xE000_007C) to A57. All we > need to do is to program these registers based on the disk activity. > The firmware running on A5 reads the values and generate proper SGPIO > timing and toggles the LEDs etc. It still sounds like SGPIO is not part of the AHCI standard spec, but rather a subset of a device called LSIOC. > These registers are defined in SATA0/1 DSDT resource template and also > documented in SoC BKDG. I just noticed that BKDG has wrong register > definition so will ask documentation folks to fix that. > > This driver is using SGPIO LED control similar to sata_highbank [1] > except bit bang GPIO (which is done by firmware). > > [1] http://lxr.free-electrons.com/source/drivers/ata/sata_highbank.c#L140 This one is rather different: there is a single device that combines registers for AHCI, the PHY attached to it and the LED. This is not SBSA compliant of course, and it requires having a special driver. What you have instead looks like a regular AHCI implementation that should just work with the standard driver as long as you describe how it gets its LEDs. Arnd