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

[Suravee Suthikulpanit]

On 09/11/2014 03:17 PM, Olof Johansson wrote:
> On Tue, Sep 9, 2014 at 1:37 PM,  <suravee.suthikulpanit@xxxxxxx> wrote:
> > From: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
> >
> > 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@xxxxxxx>
> > ---
> >   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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/