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.
The driver is based on ahci_platform driver.
Signed-off-by: Brijesh Singh <redacted>
CC: robh+dt at kernel.org
CC: pawel.moll at arm.com
CC: mark.rutland at arm.com
CC: ijc+devicetree at hellion.org.uk
CC: galak at codeaurora.org
CC: tj at kernel.org
CC: devicetree at vger.kernel.org
CC: linux-ide at vger.kernel.org
---
.../devicetree/bindings/ata/sata-seattle.txt | 34 ++++
drivers/ata/Kconfig | 8 +
drivers/ata/Makefile | 1 +
drivers/ata/ahci_seattle.c | 226 +++++++++++++++++++++
4 files changed, 269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ata/sata-seattle.txt
create mode 100644 drivers/ata/ahci_seattle.c
@@ -0,0 +1,34 @@+* AHCI SATA Controller++SATA nodes are defined to describe on-chip Serial ATA controllers.+The AMD Seattle SATA controller mostly conforms to the AHCI interface+with some special SGPIO register to contro activity LED interfaces.++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.++Each SATA controller should have its own node.+++Required properties:+- compatible : compatible string should be "amd,seattle-ahci".+- interrupts : <interrupt mapping for SATA IRQ>+- reg : <registers mapping>++Optional properties:+- dma-coherent : Present if dma operations are coherent+- clocks : a list of phandle + clock specifier pairs+- target-supply : regulator for SATA target power+- phys : reference to the SATA PHY node+- phy-names : must be "sata-phy"++Examples:+ sata0 at e0300000 {+ compatible = "amd,seattle-ahci";+ reg = <0x0 0xe0300000 0x0 0xf0000>, <0x0 0xe0000078 0x0 0x1>;+ interrupts = <0x0 0x163 0x4>;+ clocks = <0x2>+ dma-coherent;+ };+
@@ -0,0 +1,226 @@+/*+*AMDSeattleAHCISATAdriver+*+*Copyright(c)2015,AdvancedMicroDevices+*Author:BrijeshSingh<brijesh.singh@amd.com>+*+*basedontheAHCISATAplatformdriverbyJeffGarzikandAntonVorontsov+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/++#include<linux/kernel.h>+#include<linux/module.h>+#include<linux/pm.h>+#include<linux/device.h>+#include<linux/of_device.h>+#include<linux/platform_device.h>+#include<linux/libata.h>+#include<linux/ahci_platform.h>+#include<linux/acpi.h>+#include<linux/pci_ids.h>+#include"ahci.h"++/* SGPIO Control Register definition+*+*BitTypeDescription+*31RWOD7.2(activity)+*30RWOD7.1(locate)+*29RWOD7.0(fault)+*28...8RWOD6.2...OD0.0(3bitsperport,1bitperLED)+*7ROSGPIOfeatureflag+*6:4ROReserved+*3:0RONumberofports(0meansnoportsupported)+*/+#define ACTIVITY_BIT_POS(x) (8 + (3 * x))+#define LOCATE_BIT_POS(x) (ACTIVITY_BIT_POS(x) + 1)+#define FAULT_BIT_POS(x) (LOCATE_BIT_POS(x) + 1)++#define ACTIVITY_MASK 0x00010000+#define LOCATE_MASK 0x00080000+#define FAULT_MASK 0x00400000++#define DRV_NAME "ahci-seattle"++staticssize_tseattle_transmit_led_message(structata_port*ap,u32state,+ssize_tsize);++structseattle_plat_data{+void__iomem*sgpio_ctrl;+};++staticstructata_port_operationsahci_port_ops={+.inherits=&ahci_ops,+};++staticconststructata_port_infoahci_port_info={+.flags=AHCI_FLAG_COMMON,+.pio_mask=ATA_PIO4,+.udma_mask=ATA_UDMA6,+.port_ops=&ahci_port_ops,+};++staticstructata_port_operationsahci_seattle_ops={+.inherits=&ahci_ops,+.transmit_led_message=seattle_transmit_led_message,+};++staticconststructata_port_infoahci_port_seattle_info={+.flags=AHCI_FLAG_COMMON|ATA_FLAG_EM|ATA_FLAG_SW_ACTIVITY,+.link_flags=ATA_LFLAG_SW_ACTIVITY,+.pio_mask=ATA_PIO4,+.udma_mask=ATA_UDMA6,+.port_ops=&ahci_seattle_ops,+};++staticstructscsi_host_templateahci_platform_sht={+AHCI_SHT(DRV_NAME),+};++staticssize_tseattle_transmit_led_message(structata_port*ap,u32state,+ssize_tsize)+{+structahci_host_priv*hpriv=ap->host->private_data;+structahci_port_priv*pp=ap->private_data;+structseattle_plat_data*plat_data=hpriv->plat_data;+unsignedlongflags;+intpmp;+structahci_em_priv*emp;+u32val;++/* get the slot number from the message */+pmp=(state&EM_MSG_LED_PMP_SLOT)>>8;+if(pmp<EM_MAX_SLOTS)+emp=&pp->em_priv[pmp];+else+return-EINVAL;++val=ioread32(plat_data->sgpio_ctrl);+if(state&ACTIVITY_MASK)+val|=1<<ACTIVITY_BIT_POS((ap->port_no));+else+val&=~(1<<ACTIVITY_BIT_POS((ap->port_no)));++if(state&LOCATE_MASK)+val|=1<<LOCATE_BIT_POS((ap->port_no));+else+val&=~(1<<LOCATE_BIT_POS((ap->port_no)));++if(state&FAULT_MASK)+val|=1<<FAULT_BIT_POS((ap->port_no));+else+val&=~(1<<FAULT_BIT_POS((ap->port_no)));++iowrite32(val,plat_data->sgpio_ctrl);++spin_lock_irqsave(ap->lock,flags);++/* save off new led state for port/slot */+emp->led_state=state;++spin_unlock_irqrestore(ap->lock,flags);++returnsize;+}++staticconststructata_port_info*ahci_seattle_get_port_info(+structplatform_device*pdev,structahci_host_priv*hpriv)+{+structdevice*dev=&pdev->dev;+structseattle_plat_data*plat_data;+u32val;++plat_data=devm_kzalloc(dev,sizeof(*plat_data),GFP_KERNEL);+if(IS_ERR(plat_data))+return&ahci_port_info;++plat_data->sgpio_ctrl=devm_ioremap_resource(dev,+platform_get_resource(pdev,IORESOURCE_MEM,1));+if(IS_ERR(plat_data->sgpio_ctrl))+return&ahci_port_info;++val=ioread32(plat_data->sgpio_ctrl);++if(!(val&0xf))+return&ahci_port_info;++hpriv->em_loc=0;+hpriv->em_buf_sz=4;+hpriv->em_msg_type=EM_MSG_TYPE_LED;+hpriv->plat_data=plat_data;++dev_info(dev,"SGPIO LED control is enabled.\n");+return&ahci_port_seattle_info;+}++staticintahci_seattle_probe(structplatform_device*pdev)+{+intrc;+structahci_host_priv*hpriv;++hpriv=ahci_platform_get_resources(pdev);+if(IS_ERR(hpriv))+returnPTR_ERR(hpriv);++rc=ahci_platform_enable_resources(hpriv);+if(rc)+returnrc;++rc=ahci_platform_init_host(pdev,hpriv,+ahci_seattle_get_port_info(pdev,hpriv),+&ahci_platform_sht);+if(rc)+gotodisable_resources;++return0;+disable_resources:+ahci_platform_disable_resources(hpriv);+returnrc;+}++staticSIMPLE_DEV_PM_OPS(ahci_pm_ops,ahci_platform_suspend,+ahci_platform_resume);++#ifdef CONFIG_OF+staticconststructof_device_idahci_of_match[]={+{.compatible="amd,seattle-ahci",},+{}+};+MODULE_DEVICE_TABLE(of,ahci_of_match);+#endif++#ifdef CONFIG_ACPI+staticconststructacpi_device_idahci_acpi_match[]={+{"AMDI0600",0},+{}+};+MODULE_DEVICE_TABLE(acpi,ahci_acpi_match);+#endif++staticstructplatform_driverahci_seattle_driver={+.probe=ahci_seattle_probe,+.remove=ata_platform_remove_one,+.driver={+.name=DRV_NAME,+#ifdef CONFIG_OF+.of_match_table=ahci_of_match,+#endif+#ifdef CONFIG_ACPI+.acpi_match_table=ahci_acpi_match,+#endif+.pm=&ahci_pm_ops,+},+};+module_platform_driver(ahci_seattle_driver);++MODULE_DESCRIPTION("Seattle AHCI SATA platform driver");+MODULE_AUTHOR("Brijesh Singh <brijesh.singh@amd.com>");+MODULE_LICENSE("GPL");+MODULE_ALIAS("platform:"DRV_NAME);
On Thursday 07 January 2016 14:53:22 Brijesh Singh wrote:
quoted hunk
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.
The driver is based on ahci_platform driver.
Signed-off-by: Brijesh Singh <redacted>
CC: robh+dt at kernel.org
CC: pawel.moll at arm.com
CC: mark.rutland at arm.com
CC: ijc+devicetree at hellion.org.uk
CC: galak at codeaurora.org
CC: tj at kernel.org
CC: devicetree at vger.kernel.org
CC: linux-ide at vger.kernel.org
---
.../devicetree/bindings/ata/sata-seattle.txt | 34 ++++
drivers/ata/Kconfig | 8 +
drivers/ata/Makefile | 1 +
drivers/ata/ahci_seattle.c | 226 +++++++++++++++++++++
4 files changed, 269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ata/sata-seattle.txt
create mode 100644 drivers/ata/ahci_seattle.c
@@ -0,0 +1,34 @@+* AHCI SATA Controller++SATA nodes are defined to describe on-chip Serial ATA controllers.+The AMD Seattle SATA controller mostly conforms to the AHCI interface+with some special SGPIO register to contro activity LED interfaces.++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.++Each SATA controller should have its own node.+++Required properties:+- compatible : compatible string should be "amd,seattle-ahci".+- interrupts : <interrupt mapping for SATA IRQ>+- reg : <registers mapping>++Optional properties:+- dma-coherent : Present if dma operations are coherent+- clocks : a list of phandle + clock specifier pairs+- target-supply : regulator for SATA target power+- phys : reference to the SATA PHY node+- phy-names : must be "sata-phy"++Examples:+ sata0 at 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.
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.
+ interrupts = <0x0 0x163 0x4>;
+ clocks = <0x2>
This is not a valid property.
+/* SGPIO Control Register definition
+ *
+ * Bit Type Description
+ * 31 RW OD7.2 (activity)
+ * 30 RW OD7.1 (locate)
+ * 29 RW OD7.0 (fault)
+ * 28...8 RW OD6.2...OD0.0 (3bits per port, 1 bit per LED)
+ * 7 RO SGPIO feature flag
+ * 6:4 RO Reserved
+ * 3:0 RO Number of ports (0 means no port supported)
+ */
The 'reg' property in your example is only 8 bits wide, the above lists
32 bits.
Arnd
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.
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.
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
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.
quoted
+ interrupts = <0x0 0x163 0x4>;
+ clocks = <0x2>
This is not a valid property.
quoted
+/* SGPIO Control Register definition
+ *
+ * Bit Type Description
+ * 31 RW OD7.2 (activity)
+ * 30 RW OD7.1 (locate)
+ * 29 RW OD7.0 (fault)
+ * 28...8 RW OD6.2...OD0.0 (3bits per port, 1 bit per LED)
+ * 7 RO SGPIO feature flag
+ * 6:4 RO Reserved
+ * 3:0 RO Number of ports (0 means no port supported)
+ */
The 'reg' property in your example is only 8 bits wide, the above lists
32 bits.
Arnd
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
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.
Yes, its not part of SATA IP block. We just need a method of pass SGPIO
control register address to driver.
Should I consider adding a property "sgpio-ctrl" to pass the register
address ?
e.g
sata0 at e0300000 {
compatible = "amd,seattle-ahci";
reg = <0 0xe0300000 0 0x800>;
amd,sgpio-ctrl = <0xe0000078>;
interrupts = <0 355 4>;
clocks = <&sataclk_333mhz>;
dma-coherent;
};
quoted
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.
quoted
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.
Yes, its regular AHCI implementation and works well with ahci_platform
driver. In standard ahci_platform driver activity LEDs are blinked
through enclosure management interface. Given the current hardware
limitation it seems like creating a new driver would be cleaner. I am
open to suggestion.
Thanks for all your review feedbacks.
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.
Yes, its not part of SATA IP block. We just need a method of pass SGPIO
control register address to driver.
Should I consider adding a property "sgpio-ctrl" to pass the register
address ?
e.g
sata0 at e0300000 {
compatible = "amd,seattle-ahci";
reg = <0 0xe0300000 0 0x800>;
amd,sgpio-ctrl = <0xe0000078>;
interrupts = <0 355 4>;
clocks = <&sataclk_333mhz>;
dma-coherent;
};
We generally don't refer to register locations with properties other than
'reg', so that approach would be worse. What I'd suggest you do is to
have the sgpio registers in a separate device node, and use the LED
binding to access it, see
Documentation/devicetree/bindings/leds/common.txt
It seems that none of the drivers/ata/ drivers use the leds interface
today, but that can be added to libata-*.c whenever the appropriate
properties are there.
quoted
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.
Yes, its regular AHCI implementation and works well with ahci_platform
driver. In standard ahci_platform driver activity LEDs are blinked
through enclosure management interface. Given the current hardware
limitation it seems like creating a new driver would be cleaner. I am
open to suggestion.
I'd say the code in drivers/ata should be kept completely generic, referring
only to the include/linux/leds.h interfaces and properties added to
Documentation/devicetree/bindings/ata/ahci-platform.txt (if any).
For the driver that actually owns the register, it depends a bit on how
the hardware is structured, and you'd need to look at the datasheet (or
talk to a hardware designer) for that.
I suspect we should have a node for the entire block of registers
around the SGPIO, presumably something like
syscon at 0xe0000000 {
compatible = "amd,$SOC_ID"-lsioc", "syscon";
reg = <0xe0000000 0x1000>; /* find the actual length in the datasheet */
};
That way, any driver that ends up needing a register from this block
can use the syscon interface to get hold of a mapping, and/or you can
have a high-level driver to expose other functionalities. It's probably
best to start doing it either entirely using syscon references from
other drivers, or using no syscon references, and putting everything into
a driver for the register set, but as I said it depends a lot on what
else is in there.
Can you send me a register list of the 0xe0000000 segment for reference?
Arnd
Should I consider adding a property "sgpio-ctrl" to pass the register
address ?
e.g
sata0 at e0300000 {
compatible = "amd,seattle-ahci";
reg = <0 0xe0300000 0 0x800>;
amd,sgpio-ctrl = <0xe0000078>;
interrupts = <0 355 4>;
clocks = <&sataclk_333mhz>;
dma-coherent;
};
We generally don't refer to register locations with properties other than
'reg', so that approach would be worse. What I'd suggest you do is to
have the sgpio registers in a separate device node, and use the LED
binding to access it, see
Documentation/devicetree/bindings/leds/common.txt
It seems that none of the drivers/ata/ drivers use the leds interface
today, but that can be added to libata-*.c whenever the appropriate
properties are there.
libata-*.c implements the "Enclosure management" style led messages but also has hooks
to register a custom led control callback. Since Seattle platform does not support
the "Enclosure management" registers hence ata_port_info we are setting a ATA_FLAG_EM | ATA_FLAG_SW_ACIVITY
to indicate that we can still handle the led messages by our registered callback. I see
that sata_highbank driver is doing something similar.
quoted
Yes, its regular AHCI implementation and works well with ahci_platform
driver. In standard ahci_platform driver activity LEDs are blinked
through enclosure management interface. Given the current hardware
limitation it seems like creating a new driver would be cleaner. I am
open to suggestion.
I'd say the code in drivers/ata should be kept completely generic, referring
only to the include/linux/leds.h interfaces and properties added to
Documentation/devicetree/bindings/ata/ahci-platform.txt (if any).
For the driver that actually owns the register, it depends a bit on how
the hardware is structured, and you'd need to look at the datasheet (or
talk to a hardware designer) for that.
I suspect we should have a node for the entire block of registers
around the SGPIO, presumably something like
syscon at 0xe0000000 {
compatible = "amd,$SOC_ID"-lsioc", "syscon";
reg = <0xe0000000 0x1000>; /* find the actual length in the datasheet */
};
That way, any driver that ends up needing a register from this block
can use the syscon interface to get hold of a mapping, and/or you can
have a high-level driver to expose other functionalities. It's probably
best to start doing it either entirely using syscon references from
other drivers, or using no syscon references, and putting everything into
a driver for the register set, but as I said it depends a lot on what
else is in there.
Can you send me a register list of the 0xe0000000 segment for reference?
Thanks for pointing syncon, are you thinking something like this ?
sgpio0: syscon at e0000078 {
compatible = "amd, seattle-sgpio", syscon";
reg = <0x0 0xe0000078 0x0 0x1>;
};
sata at e0300000 {
compatible = "amd,seattle-ahci";
reg = <0x0 0xe0300000 0x0 0xf0000>;
interrupts = <0x0 0x163 0x4>;
amd,sgpio-ctrl = <&sgpio0>;
dma-coherent;
};
SGPIO0 (0xe0000078) and SGPIO1 (0xe000007C) does not belong to any system control register set.
They are meant to be used by SATA driver only and no other driver should every touch it. It almost feels
its just extension of the IP block but it just happened to be way out of the IP block range.
Other register near to 0xe000_0078 and 0xe000_007C should not be mapped by OS.
But this syscon approach also brings another problem. Currently my code is pretty simple for mapping this regs
+ plat_data->sgpio_ctrl = devm_ioremap_resource(dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 1));
+ if (IS_ERR(plat_data->sgpio_ctrl))
+ return &ahci_port_info;
+
The above code works on ACPI and DT cases. But if we go with syncon approach then we need to handle DT and ACPI differently. Because sycon will provide struct regmap instead of struct resources and also make reading/writing a bit different. I was trying to minimize the DT vs ACPI changes in the driver (other than binding) hence I think defining two ranges in sata controller reg property was much cleaner and it aligns with DSDT.
Just for reference the SATA DSDT looks like this:
Device (SATA0)
{
.....
Name(_CRS, ResourceTemplate()
{
Memory32Fixed(ReadWrite, 0xE03000000, 0x000010000)
Interrupt(ResourceConsumer, Level, ActiveHigh Exclusive,,,) { 387}
Memory32Fixed(ReadWrite, 0xE00000078, 1)
}
......
}
On Friday 08 January 2016 16:21:50 Brijesh Singh wrote:
quoted
quoted
Should I consider adding a property "sgpio-ctrl" to pass the register
address ?
e.g
sata0 at e0300000 {
compatible = "amd,seattle-ahci";
reg = <0 0xe0300000 0 0x800>;
amd,sgpio-ctrl = <0xe0000078>;
interrupts = <0 355 4>;
clocks = <&sataclk_333mhz>;
dma-coherent;
};
We generally don't refer to register locations with properties other than
'reg', so that approach would be worse. What I'd suggest you do is to
have the sgpio registers in a separate device node, and use the LED
binding to access it, see
Documentation/devicetree/bindings/leds/common.txt
It seems that none of the drivers/ata/ drivers use the leds interface
today, but that can be added to libata-*.c whenever the appropriate
properties are there.
libata-*.c implements the "Enclosure management" style led messages but also has hooks
to register a custom led control callback. Since Seattle platform does not support
the "Enclosure management" registers hence ata_port_info we are setting a ATA_FLAG_EM | ATA_FLAG_SW_ACIVITY
to indicate that we can still handle the led messages by our registered callback. I see
that sata_highbank driver is doing something similar.
But if the LEDs are the only thing that is special, I think it makes
more sense to extend the generic driver. This is similar to how the
ahci driver knows how to deal with external PHY, clk, regulator etc
resources. All of those are not part of the AHCI spec, but are common
enough that we want to have them done in a single driver.
We really should not need a special driver just for handling LEDs
when we already deal with more complex stuff, and we have a proper
abstraction for LEDs in the kernel.
quoted
That way, any driver that ends up needing a register from this block
can use the syscon interface to get hold of a mapping, and/or you can
have a high-level driver to expose other functionalities. It's probably
best to start doing it either entirely using syscon references from
other drivers, or using no syscon references, and putting everything into
a driver for the register set, but as I said it depends a lot on what
else is in there.
Can you send me a register list of the 0xe0000000 segment for reference?
Thanks for pointing syncon, are you thinking something like this ?
sgpio0: syscon at e0000078 {
compatible = "amd, seattle-sgpio", syscon";
reg = <0x0 0xe0000078 0x0 0x1>;
};
A syscon is defined as (roughly) a group of otherwise unrelated registers
that happen to be part of the same physical register area because the SoC
designer couldn't find a proper abstraction for them. When you define
a register area with a single byte in it, that is not a syscon.
The sata node should list "generic-ahci" so we can bind the normal
driver. You can leave the "amd,seattle-ahci" in addition in case we
ever need to know the difference to work around a bug, but it's really
not needed for the LED.
SGPIO0 (0xe0000078) and SGPIO1 (0xe000007C) does not belong to any system control register set.
They are meant to be used by SATA driver only and no other driver should every touch it. It almost feels
its just extension of the IP block but it just happened to be way out of the IP block range.
Other register near to 0xe000_0078 and 0xe000_007C should not be mapped by OS.
That is quite normal, a lot of chips have register blocks where one
register is meant for one device only. E.g. the clock controller may have
one register for controlling the clocks of the AHCI device. In the old
days, we would have hacks like what you did to turn on the clocks by poking
the register from the SATA driver, but now we abstract those things using
generic subsystems.
I think you either want a special led controller device node that
refers to the syscon device and exports the LEDs so they can be
accessed by the AHCI device, or do it in the device that contains
the registers.
But this syscon approach also brings another problem. Currently my code is pretty simple for mapping this regs
+ plat_data->sgpio_ctrl = devm_ioremap_resource(dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 1));
+ if (IS_ERR(plat_data->sgpio_ctrl))
+ return &ahci_port_info;
+
The above code works on ACPI and DT cases. But if we go with syncon approach
then we need to handle DT and ACPI differently. Because sycon will provide
struct regmap instead of struct resources and also make reading/writing a
bit different. I was trying to minimize the DT vs ACPI changes in the driver
(other than binding) hence I think defining two ranges in sata controller
reg property was much cleaner and it aligns with DSDT.
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.
Arnd
Hi Arnd,
Thanks for all your valuable feedbacks.
On 01/08/2016 04:47 PM, Arnd Bergmann wrote:
quoted
libata-*.c implements the "Enclosure management" style led messages but also has hooks
to register a custom led control callback. Since Seattle platform does not support
the "Enclosure management" registers hence ata_port_info we are setting a ATA_FLAG_EM | ATA_FLAG_SW_ACIVITY
to indicate that we can still handle the led messages by our registered callback. I see
that sata_highbank driver is doing something similar.
But if the LEDs are the only thing that is special, I think it makes
more sense to extend the generic driver. This is similar to how the
ahci driver knows how to deal with external PHY, clk, regulator etc
resources. All of those are not part of the AHCI spec, but are common
enough that we want to have them done in a single driver.
We really should not need a special driver just for handling LEDs
when we already deal with more complex stuff, and we have a proper
abstraction for LEDs in the kernel.
I took a quick look at LED class and saw ledtrig-ide-disk.c (ledtrig_ide_activity). It seems this approach was used
in deprecate ide-disk driver for blinking the activity led. Are you thinking that we implement something similar
in libahci to control the LED blinking? Looking at current libahci gives me feeling that LED's are considered as
part of AHCI enclosure management implementation and hence LED triggers are not exposed outside the library.
If I am missing something then please correct me.
A syscon is defined as (roughly) a group of otherwise unrelated registers
that happen to be part of the same physical register area because the SoC
designer couldn't find a proper abstraction for them. When you define
a register area with a single byte in it, that is not a syscon.
The sata node should list "generic-ahci" so we can bind the normal
driver. You can leave the "amd,seattle-ahci" in addition in case we
ever need to know the difference to work around a bug, but it's really
not needed for the LED.
quoted
SGPIO0 (0xe0000078) and SGPIO1 (0xe000007C) does not belong to any system control register set.
They are meant to be used by SATA driver only and no other driver should every touch it. It almost feels
its just extension of the IP block but it just happened to be way out of the IP block range.
Other register near to 0xe000_0078 and 0xe000_007C should not be mapped by OS.
That is quite normal, a lot of chips have register blocks where one
register is meant for one device only. E.g. the clock controller may have
one register for controlling the clocks of the AHCI device. In the old
days, we would have hacks like what you did to turn on the clocks by poking
the register from the SATA driver, but now we abstract those things using
generic subsystems.
I think you either want a special led controller device node that
refers to the syscon device and exports the LEDs so they can be
accessed by the AHCI device, or do it in the device that contains
the registers.
quoted
But this syscon approach also brings another problem. Currently my code is pretty simple for mapping this regs
+ plat_data->sgpio_ctrl = devm_ioremap_resource(dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 1));
+ if (IS_ERR(plat_data->sgpio_ctrl))
+ return &ahci_port_info;
+
The above code works on ACPI and DT cases. But if we go with syncon approach
then we need to handle DT and ACPI differently. Because sycon will provide
struct regmap instead of struct resources and also make reading/writing a
bit different. I was trying to minimize the DT vs ACPI changes in the driver
(other than binding) hence I think defining two ranges in sata controller
reg property was much cleaner and it aligns with DSDT.
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 ?
"SATA0" and "SATA1" DSDT Resource template contains two resources (#0 SATA block register and #1 SGPIO control register).
Device (SATA0)
{
Name(_CRS, ResourceTemplate()
{
Memory32Fixed(ReadWrite, 0xE03000000, 0x000010000)
Interrupt(ResourceConsumer, Level, ActiveHigh Exclusive,,,) { 387}
Memory32Fixed(ReadWrite, 0xE00000078, 1)
}
}
Device (SATA1)
{
Name(_CRS, ResourceTemplate()
{
Memory32Fixed(ReadWrite, 0xE0D100000, 0x000010000)
Interrupt(ResourceConsumer, Level, ActiveHigh Exclusive,,,) { 393}
Memory32Fixed(ReadWrite, 0xE0000007C, 1)
}
}
The generic ahci platform driver calls ahci_platform_get_resources() to get the resources.
ahci_platform_get_resources() maps the resource #0. Since the SGPIO control resource is part of SATA device
resource hence I could simply ioremap resource #1 for both DT and ACPI cases. Since windows driver is making
use of same DSDT hence its going to be tricky to modify the DSDT to create a new device entry for SATA LED.
Before creating a new driver, I did attempt to extend the existing ahci_platform driver and was able to get it working but it was not looking good hence created a new driver. One of the issue was knowing when to map
the SGPIO control register in ACPI case. ahci_platform driver does binding based on _CLS mask so I was not sure
whether doing ioremap of resource#1 is safe and would not cause problem on non Seattle platform. If you want then
i can send you patch for quick comment and see if that make sense instead of adding a new driver.
On Monday 11 January 2016 12:56:02 Brijesh Singh wrote:
Hi Arnd,
Thanks for all your valuable feedbacks.
On 01/08/2016 04:47 PM, Arnd Bergmann wrote:
quoted
quoted
libata-*.c implements the "Enclosure management" style led messages but also has hooks
to register a custom led control callback. Since Seattle platform does not support
the "Enclosure management" registers hence ata_port_info we are setting a ATA_FLAG_EM | ATA_FLAG_SW_ACIVITY
to indicate that we can still handle the led messages by our registered callback. I see
that sata_highbank driver is doing something similar.
But if the LEDs are the only thing that is special, I think it makes
more sense to extend the generic driver. This is similar to how the
ahci driver knows how to deal with external PHY, clk, regulator etc
resources. All of those are not part of the AHCI spec, but are common
enough that we want to have them done in a single driver.
We really should not need a special driver just for handling LEDs
when we already deal with more complex stuff, and we have a proper
abstraction for LEDs in the kernel.
I took a quick look at LED class and saw ledtrig-ide-disk.c (ledtrig_ide_activity). It seems this approach was used
in deprecate ide-disk driver for blinking the activity led. Are you thinking that we implement something similar
in libahci to control the LED blinking?
Yes. I'm not overly familiar with the LED framework, but it seems to me that
something along those lines is the right idea. Note that the driver predates
devicetree and it only handles a single LED, so you probably need to do
something a bit more sophisticated to handle each LED separately and connect
it do the right disk through DT.
We actually seem to have quite a number of platforms relying on this:
$ git grep -w "ide-disk"
Documentation/devicetree/bindings/leds/common.txt: "ide-disk" - LED indicates disk activity
Documentation/devicetree/bindings/leds/leds-gpio.txt: linux,default-trigger = "ide-disk";
arch/arm/boot/dts/am57xx-beagle-x15.dts: linux,default-trigger = "ide-disk";
arch/arm/boot/dts/kirkwood-ns2lite.dts: linux,default-trigger = "ide-disk";
arch/arm/boot/dts/kirkwood-topkick.dts: linux,default-trigger = "ide-disk";
arch/arm/mach-davinci/board-dm644x-evm.c: .default_trigger = "ide-disk", },
arch/arm/mach-omap1/board-osk.c: .default_trigger = "ide-disk", },
arch/arm/mach-pxa/spitz.c: .default_trigger = "ide-disk",
arch/mips/txx9/generic/setup.c: "ide-disk",
arch/mips/txx9/rbtx4939/setup.c: "ide-disk",
arch/powerpc/boot/dts/mpc8315erdb.dts: linux,default-trigger = "ide-disk";
arch/powerpc/boot/dts/mpc8377_rdb.dts: linux,default-trigger = "ide-disk";
arch/powerpc/boot/dts/mpc8378_rdb.dts: linux,default-trigger = "ide-disk";
arch/powerpc/boot/dts/mpc8379_rdb.dts: linux,default-trigger = "ide-disk";
arch/unicore32/kernel/gpio.c: .default_trigger = "ide-disk", },
drivers/leds/leds-hp6xx.c: .default_trigger = "ide-disk",
drivers/leds/trigger/Makefile:obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK) += ledtrig-ide-disk.o
drivers/leds/trigger/ledtrig-camera.c: * based on ledtrig-ide-disk.c
drivers/leds/trigger/ledtrig-ide-disk.c: led_trigger_register_simple("ide-disk", &ledtrig_ide);
drivers/macintosh/Kconfig: and the ide-disk LED trigger and configure appropriately through
drivers/macintosh/via-pmu-led.c: .default_trigger = "ide-disk",
I suspect this used to work in the past, but got broken when people moved
away from drivers/ide to drivers/ata, and nobody properly debugged the problem.
If you fix this right, the LEDs on all those platforms should light up again.
Looking at current libahci gives me feeling that LED's are considered as
part of AHCI enclosure management implementation and hence LED triggers
are not exposed outside the library.
If I am missing something then please correct me.
Yes, this sounds correct.
quoted
A syscon is defined as (roughly) a group of otherwise unrelated registers
that happen to be part of the same physical register area because the SoC
designer couldn't find a proper abstraction for them. When you define
a register area with a single byte in it, that is not a syscon.
The sata node should list "generic-ahci" so we can bind the normal
driver. You can leave the "amd,seattle-ahci" in addition in case we
ever need to know the difference to work around a bug, but it's really
not needed for the LED.
quoted
SGPIO0 (0xe0000078) and SGPIO1 (0xe000007C) does not belong to any system control register set.
They are meant to be used by SATA driver only and no other driver should every touch it. It almost feels
its just extension of the IP block but it just happened to be way out of the IP block range.
Other register near to 0xe000_0078 and 0xe000_007C should not be mapped by OS.
That is quite normal, a lot of chips have register blocks where one
register is meant for one device only. E.g. the clock controller may have
one register for controlling the clocks of the AHCI device. In the old
days, we would have hacks like what you did to turn on the clocks by poking
the register from the SATA driver, but now we abstract those things using
generic subsystems.
I think you either want a special led controller device node that
refers to the syscon device and exports the LEDs so they can be
accessed by the AHCI device, or do it in the device that contains
the registers.
quoted
But this syscon approach also brings another problem. Currently my code is pretty simple for mapping this regs
+ plat_data->sgpio_ctrl = devm_ioremap_resource(dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 1));
+ if (IS_ERR(plat_data->sgpio_ctrl))
+ return &ahci_port_info;
+
The above code works on ACPI and DT cases. But if we go with syncon approach
then we need to handle DT and ACPI differently. Because sycon will provide
struct regmap instead of struct resources and also make reading/writing a
bit different. I was trying to minimize the DT vs ACPI changes in the driver
(other than binding) hence I think defining two ranges in sata controller
reg property was much cleaner and it aligns with DSDT.
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
c) calling into the ACPI interpreter to do platform specific hacks
Arnd
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
On Wednesday 13 January 2016 10:55:04 Brijesh Singh wrote:
On 01/12/2016 08:24 AM, Arnd Bergmann wrote:
quoted
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 ?
I don't care what you do with ACPI, just make sure we can use the generic
driver without any platform specific hacks for the DT case.
For ACPI, please talk to the respective maintainers about whether they want
to have a custom driver for this case, or a proper abstraction in the generic
driver.
Arnd
From: Mark Langsdorf <hidden> Date: 2016-01-11 15:33:27
On 01/08/2016 04:21 PM, Brijesh Singh wrote:
Hi,
quoted
We generally don't refer to register locations with properties other than
'reg', so that approach would be worse. What I'd suggest you do is to
have the sgpio registers in a separate device node, and use the LED
binding to access it, see
Documentation/devicetree/bindings/leds/common.txt
It seems that none of the drivers/ata/ drivers use the leds interface
today, but that can be added to libata-*.c whenever the appropriate
properties are there.
libata-*.c implements the "Enclosure management" style led messages but also has hooks
to register a custom led control callback. Since Seattle platform does not support
the "Enclosure management" registers hence ata_port_info we are setting a ATA_FLAG_EM | ATA_FLAG_SW_ACIVITY
to indicate that we can still handle the led messages by our registered callback. I see
that sata_highbank driver is doing something similar.
The sata_highbank driver is doing it wrong and shouldn't have been
accepted in its current condition. Enclosure management really should
be a separate device. Please don't use it as an example.
--Mark Langsdorf
libata-*.c implements the "Enclosure management" style led messages but also has hooks
to register a custom led control callback. Since Seattle platform does not support
the "Enclosure management" registers hence ata_port_info we are setting a ATA_FLAG_EM | ATA_FLAG_SW_ACIVITY
to indicate that we can still handle the led messages by our registered callback. I see
that sata_highbank driver is doing something similar.
The sata_highbank driver is doing it wrong and shouldn't have been
accepted in its current condition. Enclosure management really should
be a separate device. Please don't use it as an example.
Okay. Thanks for letting me know.
Should I consider extending the generic driver to handle this Seattle platform specific feature or go with a new
platform driver approach (i.e the one I am doing right now) ?
-Brijesh
From: Rob Herring <robh@kernel.org> Date: 2016-01-07 22:56:35
On Thu, Jan 07, 2016 at 10:25:38PM +0100, Arnd Bergmann wrote:
On Thursday 07 January 2016 14:53:22 Brijesh Singh wrote:
quoted
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.
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
From: Joe Perches <joe@perches.com> Date: 2016-01-08 20:00:00
On Thu, 2016-01-07 at 14:53 -0600, 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.
From: Hans de Goede <hidden> Date: 2016-01-11 10:24:09
Hi,
On 07-01-16 21:53, 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.
The driver is based on ahci_platform driver.
Signed-off-by: Brijesh Singh <redacted>
This driver looks good from a libahci_platform usage pov. So with the
2 remarks from other reviewers fixed this patch is:
Acked-by: Hans de Goede <redacted>
Regards,
Hans
quoted hunk
CC: robh+dt at kernel.org
CC: pawel.moll at arm.com
CC: mark.rutland at arm.com
CC: ijc+devicetree at hellion.org.uk
CC: galak at codeaurora.org
CC: tj at kernel.org
CC: devicetree at vger.kernel.org
CC: linux-ide at vger.kernel.org
---
.../devicetree/bindings/ata/sata-seattle.txt | 34 ++++
drivers/ata/Kconfig | 8 +
drivers/ata/Makefile | 1 +
drivers/ata/ahci_seattle.c | 226 +++++++++++++++++++++
4 files changed, 269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ata/sata-seattle.txt
create mode 100644 drivers/ata/ahci_seattle.c
@@ -0,0 +1,34 @@+* AHCI SATA Controller++SATA nodes are defined to describe on-chip Serial ATA controllers.+The AMD Seattle SATA controller mostly conforms to the AHCI interface+with some special SGPIO register to contro activity LED interfaces.++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.++Each SATA controller should have its own node.+++Required properties:+- compatible : compatible string should be "amd,seattle-ahci".+- interrupts : <interrupt mapping for SATA IRQ>+- reg : <registers mapping>++Optional properties:+- dma-coherent : Present if dma operations are coherent+- clocks : a list of phandle + clock specifier pairs+- target-supply : regulator for SATA target power+- phys : reference to the SATA PHY node+- phy-names : must be "sata-phy"++Examples:+ sata0 at e0300000 {+ compatible = "amd,seattle-ahci";+ reg = <0x0 0xe0300000 0x0 0xf0000>, <0x0 0xe0000078 0x0 0x1>;+ interrupts = <0x0 0x163 0x4>;+ clocks = <0x2>+ dma-coherent;+ };+
@@ -0,0 +1,226 @@+/*+*AMDSeattleAHCISATAdriver+*+*Copyright(c)2015,AdvancedMicroDevices+*Author:BrijeshSingh<brijesh.singh@amd.com>+*+*basedontheAHCISATAplatformdriverbyJeffGarzikandAntonVorontsov+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/++#include<linux/kernel.h>+#include<linux/module.h>+#include<linux/pm.h>+#include<linux/device.h>+#include<linux/of_device.h>+#include<linux/platform_device.h>+#include<linux/libata.h>+#include<linux/ahci_platform.h>+#include<linux/acpi.h>+#include<linux/pci_ids.h>+#include"ahci.h"++/* SGPIO Control Register definition+*+*BitTypeDescription+*31RWOD7.2(activity)+*30RWOD7.1(locate)+*29RWOD7.0(fault)+*28...8RWOD6.2...OD0.0(3bitsperport,1bitperLED)+*7ROSGPIOfeatureflag+*6:4ROReserved+*3:0RONumberofports(0meansnoportsupported)+*/+#define ACTIVITY_BIT_POS(x) (8 + (3 * x))+#define LOCATE_BIT_POS(x) (ACTIVITY_BIT_POS(x) + 1)+#define FAULT_BIT_POS(x) (LOCATE_BIT_POS(x) + 1)++#define ACTIVITY_MASK 0x00010000+#define LOCATE_MASK 0x00080000+#define FAULT_MASK 0x00400000++#define DRV_NAME "ahci-seattle"++staticssize_tseattle_transmit_led_message(structata_port*ap,u32state,+ssize_tsize);++structseattle_plat_data{+void__iomem*sgpio_ctrl;+};++staticstructata_port_operationsahci_port_ops={+.inherits=&ahci_ops,+};++staticconststructata_port_infoahci_port_info={+.flags=AHCI_FLAG_COMMON,+.pio_mask=ATA_PIO4,+.udma_mask=ATA_UDMA6,+.port_ops=&ahci_port_ops,+};++staticstructata_port_operationsahci_seattle_ops={+.inherits=&ahci_ops,+.transmit_led_message=seattle_transmit_led_message,+};++staticconststructata_port_infoahci_port_seattle_info={+.flags=AHCI_FLAG_COMMON|ATA_FLAG_EM|ATA_FLAG_SW_ACTIVITY,+.link_flags=ATA_LFLAG_SW_ACTIVITY,+.pio_mask=ATA_PIO4,+.udma_mask=ATA_UDMA6,+.port_ops=&ahci_seattle_ops,+};++staticstructscsi_host_templateahci_platform_sht={+AHCI_SHT(DRV_NAME),+};++staticssize_tseattle_transmit_led_message(structata_port*ap,u32state,+ssize_tsize)+{+structahci_host_priv*hpriv=ap->host->private_data;+structahci_port_priv*pp=ap->private_data;+structseattle_plat_data*plat_data=hpriv->plat_data;+unsignedlongflags;+intpmp;+structahci_em_priv*emp;+u32val;++/* get the slot number from the message */+pmp=(state&EM_MSG_LED_PMP_SLOT)>>8;+if(pmp<EM_MAX_SLOTS)+emp=&pp->em_priv[pmp];+else+return-EINVAL;++val=ioread32(plat_data->sgpio_ctrl);+if(state&ACTIVITY_MASK)+val|=1<<ACTIVITY_BIT_POS((ap->port_no));+else+val&=~(1<<ACTIVITY_BIT_POS((ap->port_no)));++if(state&LOCATE_MASK)+val|=1<<LOCATE_BIT_POS((ap->port_no));+else+val&=~(1<<LOCATE_BIT_POS((ap->port_no)));++if(state&FAULT_MASK)+val|=1<<FAULT_BIT_POS((ap->port_no));+else+val&=~(1<<FAULT_BIT_POS((ap->port_no)));++iowrite32(val,plat_data->sgpio_ctrl);++spin_lock_irqsave(ap->lock,flags);++/* save off new led state for port/slot */+emp->led_state=state;++spin_unlock_irqrestore(ap->lock,flags);++returnsize;+}++staticconststructata_port_info*ahci_seattle_get_port_info(+structplatform_device*pdev,structahci_host_priv*hpriv)+{+structdevice*dev=&pdev->dev;+structseattle_plat_data*plat_data;+u32val;++plat_data=devm_kzalloc(dev,sizeof(*plat_data),GFP_KERNEL);+if(IS_ERR(plat_data))+return&ahci_port_info;++plat_data->sgpio_ctrl=devm_ioremap_resource(dev,+platform_get_resource(pdev,IORESOURCE_MEM,1));+if(IS_ERR(plat_data->sgpio_ctrl))+return&ahci_port_info;++val=ioread32(plat_data->sgpio_ctrl);++if(!(val&0xf))+return&ahci_port_info;++hpriv->em_loc=0;+hpriv->em_buf_sz=4;+hpriv->em_msg_type=EM_MSG_TYPE_LED;+hpriv->plat_data=plat_data;++dev_info(dev,"SGPIO LED control is enabled.\n");+return&ahci_port_seattle_info;+}++staticintahci_seattle_probe(structplatform_device*pdev)+{+intrc;+structahci_host_priv*hpriv;++hpriv=ahci_platform_get_resources(pdev);+if(IS_ERR(hpriv))+returnPTR_ERR(hpriv);++rc=ahci_platform_enable_resources(hpriv);+if(rc)+returnrc;++rc=ahci_platform_init_host(pdev,hpriv,+ahci_seattle_get_port_info(pdev,hpriv),+&ahci_platform_sht);+if(rc)+gotodisable_resources;++return0;+disable_resources:+ahci_platform_disable_resources(hpriv);+returnrc;+}++staticSIMPLE_DEV_PM_OPS(ahci_pm_ops,ahci_platform_suspend,+ahci_platform_resume);++#ifdef CONFIG_OF+staticconststructof_device_idahci_of_match[]={+{.compatible="amd,seattle-ahci",},+{}+};+MODULE_DEVICE_TABLE(of,ahci_of_match);+#endif++#ifdef CONFIG_ACPI+staticconststructacpi_device_idahci_acpi_match[]={+{"AMDI0600",0},+{}+};+MODULE_DEVICE_TABLE(acpi,ahci_acpi_match);+#endif++staticstructplatform_driverahci_seattle_driver={+.probe=ahci_seattle_probe,+.remove=ata_platform_remove_one,+.driver={+.name=DRV_NAME,+#ifdef CONFIG_OF+.of_match_table=ahci_of_match,+#endif+#ifdef CONFIG_ACPI+.acpi_match_table=ahci_acpi_match,+#endif+.pm=&ahci_pm_ops,+},+};+module_platform_driver(ahci_seattle_driver);++MODULE_DESCRIPTION("Seattle AHCI SATA platform driver");+MODULE_AUTHOR("Brijesh Singh <brijesh.singh@amd.com>");+MODULE_LICENSE("GPL");+MODULE_ALIAS("platform:"DRV_NAME);