* [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
@ 2014-09-09 20:37 suravee.suthikulpanit
2014-09-10 10:11 ` Hans de Goede
2014-09-11 20:17 ` Olof Johansson
0 siblings, 2 replies; 6+ messages in thread
From: suravee.suthikulpanit @ 2014-09-09 20:37 UTC (permalink / raw)
To: tj, hdegoede
Cc: linux-ide, linux-kernel, linux-acpi, linux-arm-kernel,
Suravee Suthikulpanit
From: Suravee Suthikulpanit <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 <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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
2014-09-09 20:37 [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller suravee.suthikulpanit
@ 2014-09-10 10:11 ` Hans de Goede
2014-09-11 20:17 ` Olof Johansson
1 sibling, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2014-09-10 10:11 UTC (permalink / raw)
To: suravee.suthikulpanit, tj
Cc: linux-ide, linux-kernel, linux-acpi, linux-arm-kernel
Hi,
On 09/09/2014 10:37 PM, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <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 <Suravee.Suthikulpanit@amd.com>
Looks good to me, one small nitpick inline.
Acked-by: Hans de Goede <hdegoede@redhat.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.
> .pm = &ahci_pm_ops,
> },
> };
>
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
2014-09-09 20:37 [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller suravee.suthikulpanit
2014-09-10 10:11 ` Hans de Goede
@ 2014-09-11 20:17 ` Olof Johansson
2014-09-12 8:14 ` Hans de Goede
2014-09-28 21:51 ` Suravee Suthikulpanit
1 sibling, 2 replies; 6+ messages in thread
From: Olof Johansson @ 2014-09-11 20:17 UTC (permalink / raw)
To: Suravee Suthikulanit
Cc: Tejun Heo, Hans de Goede, linux-ide, linux-kernel, linux-acpi,
linux-arm-kernel
On Tue, Sep 9, 2014 at 1:37 PM, <suravee.suthikulpanit@amd.com> wrote:
> From: Suravee Suthikulpanit <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 <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
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
2014-09-11 20:17 ` Olof Johansson
@ 2014-09-12 8:14 ` Hans de Goede
2014-09-28 21:51 ` Suravee Suthikulpanit
1 sibling, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2014-09-12 8:14 UTC (permalink / raw)
To: Olof Johansson, Suravee Suthikulanit
Cc: Tejun Heo, linux-ide, linux-kernel, linux-acpi, linux-arm-kernel
Hi,
On 09/11/2014 10:17 PM, Olof Johansson wrote:
> On Tue, Sep 9, 2014 at 1:37 PM, <suravee.suthikulpanit@amd.com> wrote:
>> From: Suravee Suthikulpanit <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 <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
>
> 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?
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
>
>
> -Olof
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
2014-09-11 20:17 ` Olof Johansson
2014-09-12 8:14 ` Hans de Goede
@ 2014-09-28 21:51 ` Suravee Suthikulpanit
2014-09-29 18:16 ` Arnd Bergmann
1 sibling, 1 reply; 6+ messages in thread
From: Suravee Suthikulpanit @ 2014-09-28 21:51 UTC (permalink / raw)
To: Olof Johansson
Cc: Tejun Heo, Hans de Goede, linux-ide, linux-kernel, linux-acpi,
linux-arm-kernel
On 09/11/2014 03:17 PM, Olof Johansson wrote:
> On Tue, Sep 9, 2014 at 1:37 PM, <suravee.suthikulpanit@amd.com> wrote:
>> From: Suravee Suthikulpanit <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 <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
>
> 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
>
Good point. Thanks. I will make change to this, and send out the next
one soon.
Suravee
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
2014-09-28 21:51 ` Suravee Suthikulpanit
@ 2014-09-29 18:16 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2014-09-29 18:16 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Suravee Suthikulpanit, Olof Johansson, linux-ide, linux-acpi,
linux-kernel, Hans de Goede, Tejun Heo
On Sunday 28 September 2014 16:51:31 Suravee Suthikulpanit wrote:
> >> @@ -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
> >> +
I just noticed this patch and stumbled over this hack. Since SBSA requires
any SATA support to be compatible with AHCI and not vendor specific, I think
this needs to be a more generic compatibility string that can be used by
all vendors, while the current one looks very AMD specific for hardware
that is not.
Arnc
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-29 18:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 20:37 [PATCH] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller suravee.suthikulpanit
2014-09-10 10:11 ` Hans de Goede
2014-09-11 20:17 ` Olof Johansson
2014-09-12 8:14 ` Hans de Goede
2014-09-28 21:51 ` Suravee Suthikulpanit
2014-09-29 18:16 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).