linux-kernel - Re: [PATCH] ata: add AMD Seattle platform driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 7 Jan 2016 16:56:28 -0600
From:	Rob Herring <robh@...nel.org>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Brijesh Singh <brijesh.singh@....com>,
	linux-kernel@...r.kernel.org, mark.rutland@....com,
	devicetree@...r.kernel.org, pawel.moll@....com,
	ijc+devicetree@...lion.org.uk, linux-ide@...r.kernel.org,
	galak@...eaurora.org, tj@...nel.org
Subject: Re: [PATCH] ata: add AMD Seattle platform driver

On Thu, Jan 07, 2016 at 10:25:38PM +0100, Arnd Bergmann wrote:
> On Thursday 07 January 2016 14:53:22 Brijesh Singh wrote:
> > AMD Seattle SATA controller mostly conforms to AHCI interface with some
> > special register to control SGPIO interface. In the case of an AHCI
> > controller, the SGPIO feature is ideally implemented using the
> > "Enclosure Management" register of the AHCI controller, but those
> > registeres are not implemented in the Seattle SoC. Instead SoC
> > (Rev B0 onwards) provides a 32-bit SGPIO control register which should
> > be programmed to control the activity, locate and fault LEDs.

> > +Examples:
> > +        sata0@...00000 {
> > +		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.

SGPIO is really a poor name as it has little to do with GPIO. It's a 
serial protocol to set LED states. Still, I agree this should probably 
be a separate node with perhaps a phandle to syscon.
 
> If so, please implement a GPIO driver for that device, and use the gpio-leds
> driver to drive the LEDs. IIRC there is already a generic way to communicate
> with the LEDs interface from libata, if not you can implement that in order
> to keep the special case out of the platform driver.

There is kernel support for activity LEDs, but the others you want to 
control with ledmon/ledctl utilities rather than LED subsystem. Those 
utilities use an enclosure management sysfs file IIRC. There's no 
kernel support for SGPIO outside of the AHCI enclosure management 
register (at least there wasn't 2 years ago when I last looked). 

Rob