[PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller

Discussion:

s***@amd.com

2014-09-09 20:37:15 UTC

From: Suravee Suthikulpanit <***@amd.com>

This patch adds ACPI support for non-PCI SATA contoller in ahci_platform driver.
It adds ACPI matching table in ahci_platform to support AMD Seattle SATA controller
with following ASL structure in DSDT:

Device (SATA0)
{
Name(_HID, "AMDI0600") // Seattle AHSATA
Name (_CCA, 1) // Cache-coherent controller
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xE0300000, 0x00010000)
Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive,,,) { 387 }
})
}

Since ATA driver should not require PCI support for ATA_ACPI,
this patch also removes dependency in the driver/ata/Kconfig.

Signed-off-by: Suravee Suthikulpanit <***@amd.com>
---
drivers/ata/Kconfig | 2 +-
drivers/ata/ahci_platform.c | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index e1b9278..f2e6c9e 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -48,7 +48,7 @@ config ATA_VERBOSE_ERROR

config ATA_ACPI
bool "ATA ACPI Support"
- depends on ACPI && PCI
+ depends on ACPI
default y
help
This option adds support for ATA-related ACPI objects.
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index f61ddb9..3499bab 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -20,6 +20,9 @@
#include <linux/platform_device.h>
#include <linux/libata.h>
#include <linux/ahci_platform.h>
+#ifdef CONFIG_ATA_ACPI
+#include <linux/acpi.h>
+#endif
#include "ahci.h"

static const struct ata_port_info ahci_port_info = {
@@ -87,6 +90,13 @@ static const struct of_device_id ahci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, ahci_of_match);

+#ifdef CONFIG_ATA_ACPI
+static const struct acpi_device_id ahci_acpi_match[] = {
+ { "AMDI0600", 0 }, /* AMD Seattle AHCI */
+ { },
+};
+#endif
+
static struct platform_driver ahci_driver = {
.probe = ahci_probe,
.remove = ata_platform_remove_one,
@@ -94,6 +104,9 @@ static struct platform_driver ahci_driver = {
.name = "ahci",
.owner = THIS_MODULE,
.of_match_table = ahci_of_match,
+#ifdef CONFIG_ATA_ACPI
+ .acpi_match_table = ACPI_PTR(ahci_acpi_match),
+#endif
.pm = &ahci_pm_ops,
},
};

--
1.9.3

Hans de Goede

2014-09-10 10:11:55 UTC

Permalink

Hi,

Looks good to me, one small nitpick inline.

Post by s***@amd.com
---
drivers/ata/Kconfig | 2 +-
drivers/ata/ahci_platform.c | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index e1b9278..f2e6c9e 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -48,7 +48,7 @@ config ATA_VERBOSE_ERROR
config ATA_ACPI
bool "ATA ACPI Support"
- depends on ACPI && PCI
+ depends on ACPI
default y
help
This option adds support for ATA-related ACPI objects.
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index f61ddb9..3499bab 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -20,6 +20,9 @@
#include <linux/platform_device.h>
#include <linux/libata.h>
#include <linux/ahci_platform.h>
+#ifdef CONFIG_ATA_ACPI
+#include <linux/acpi.h>
+#endif
#include "ahci.h"
static const struct ata_port_info ahci_port_info = {
@@ -87,6 +90,13 @@ static const struct of_device_id ahci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, ahci_of_match);
+#ifdef CONFIG_ATA_ACPI
+static const struct acpi_device_id ahci_acpi_match[] = {
+ { "AMDI0600", 0 }, /* AMD Seattle AHCI */
+ { },
+};
+#endif
+
static struct platform_driver ahci_driver = {
.probe = ahci_probe,
.remove = ata_platform_remove_one,
@@ -94,6 +104,9 @@ static struct platform_driver ahci_driver = {
.name = "ahci",
.owner = THIS_MODULE,
.of_match_table = ahci_of_match,
+#ifdef CONFIG_ATA_ACPI
+ .acpi_match_table = ACPI_PTR(ahci_acpi_match),
+#endif

Hmm, is the cast here really necessary ? AFAIK nowadays we usually
don't need this cast macros, and they can hide accidental prototype
differences, so they are better avoided.

Post by s***@amd.com
.pm = &ahci_pm_ops,
},
};

Regards,

Hans

Olof Johansson

2014-09-11 20:17:23 UTC

Permalink

Post by s***@amd.com
This patch adds ACPI support for non-PCI SATA contoller in ahci_platform driver.
It adds ACPI matching table in ahci_platform to support AMD Seattle SATA controller
Device (SATA0)
{
Name(_HID, "AMDI0600") // Seattle AHSATA
Name (_CCA, 1) // Cache-coherent controller
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, 0xE0300000, 0x00010000)
Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive,,,) { 387 }
})
}
Since ATA driver should not require PCI support for ATA_ACPI,
this patch also removes dependency in the driver/ata/Kconfig.
---
drivers/ata/Kconfig | 2 +-
drivers/ata/ahci_platform.c | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index e1b9278..f2e6c9e 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -48,7 +48,7 @@ config ATA_VERBOSE_ERROR
config ATA_ACPI
bool "ATA ACPI Support"
- depends on ACPI && PCI
+ depends on ACPI
default y
help
This option adds support for ATA-related ACPI objects.
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index f61ddb9..3499bab 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -20,6 +20,9 @@
#include <linux/platform_device.h>
#include <linux/libata.h>
#include <linux/ahci_platform.h>
+#ifdef CONFIG_ATA_ACPI
+#include <linux/acpi.h>
+#endif
#include "ahci.h"
static const struct ata_port_info ahci_port_info = {
@@ -87,6 +90,13 @@ static const struct of_device_id ahci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, ahci_of_match);
+#ifdef CONFIG_ATA_ACPI
+static const struct acpi_device_id ahci_acpi_match[] = {
+ { "AMDI0600", 0 }, /* AMD Seattle AHCI */
+ { },
+};
+#endif
+
static struct platform_driver ahci_driver = {
.probe = ahci_probe,
.remove = ata_platform_remove_one,
@@ -94,6 +104,9 @@ static struct platform_driver ahci_driver = {
.name = "ahci",
.owner = THIS_MODULE,
.of_match_table = ahci_of_match,
+#ifdef CONFIG_ATA_ACPI
+ .acpi_match_table = ACPI_PTR(ahci_acpi_match),
+#endif

The whole point of having ACPI_PTR() is to avoid having ifdefs like
these. The structure member is always there, so there's no need to
have it conditionally compiled out.

Come to think of it, the match table should be under CONFIG_ACPI
instead -- why do a separate config option just for this?

-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hans de Goede

2014-09-12 08:14:18 UTC

Permalink

Hi,

Post by Olof Johansson

To clarify this for people like me who were under the impression
that ACPI_PTR() does some kind of cast, this is the definition
of ACPI_PTR:

#ifdef CONFIG_ACPI
#define ACPI_PTR(_ptr) (_ptr)
#else
#define ACPI_PTR(_ptr) (NULL)
#endif

Regards,

Hans

Post by Olof Johansson
-Olof

Suravee Suthikulpanit

2014-09-28 21:51:31 UTC

Permalink

Post by Olof Johansson

Good point. Thanks. I will make change to this, and send out the next
one soon.

Suravee

Arnd Bergmann

2014-09-29 18:16:05 UTC

Permalink

This post might be inappropriate. Click to display it.