On 9/16/2014 8:26 PM, Matthew Garrett wrote:
> On Mon, Sep 15, 2014 at 07:47:23PM -0500, [email protected] wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> This patch adds ACPI match table in ahci_platform. The table includes
>> the acpi_device_id to match AMD Seattle SATA controller with following
>> asl structure in DSDT:
>>
>> Device (SATA0)
>> {
>> Name(_HID, "AMDI0600") // Seattle AHSATA
>
> There really ought to be a well-defined PNPID for AHCI, so you can _HID
> to AMD and _CID to something generic. That way we won't have:
>
>> +#ifdef CONFIG_ATA_ACPI
>> +static const struct acpi_device_id ahci_acpi_match[] = {
>> + { "AMDI0600", 0 }, /* AMD Seattle AHCI */
>> + { },
>> +};
>
> utter sadness here. Really, please don't end up in a situation where we
> need to add device-specific IDs to a generic driver.
>
Matthew,
Currently, there is no _CID defined for generic AHCI. We will work on
proposing one, and provide update patches for including the new ID.
Thanks,
Suravee
On Wednesday 01 October 2014 16:19:37 Suravee Suthikulanit wrote:
> On 9/16/2014 8:26 PM, Matthew Garrett wrote:
> > On Mon, Sep 15, 2014 at 07:47:23PM -0500, [email protected] wrote:
> >> From: Suravee Suthikulpanit <[email protected]>
> >>
> >> This patch adds ACPI match table in ahci_platform. The table includes
> >> the acpi_device_id to match AMD Seattle SATA controller with following
> >> asl structure in DSDT:
> >>
> >> Device (SATA0)
> >> {
> >> Name(_HID, "AMDI0600") // Seattle AHSATA
> >
> > There really ought to be a well-defined PNPID for AHCI, so you can _HID
> > to AMD and _CID to something generic. That way we won't have:
> >
> >> +#ifdef CONFIG_ATA_ACPI
> >> +static const struct acpi_device_id ahci_acpi_match[] = {
> >> + { "AMDI0600", 0 }, /* AMD Seattle AHCI */
> >> + { },
> >> +};
> >
> > utter sadness here. Really, please don't end up in a situation where we
> > need to add device-specific IDs to a generic driver.
> >
> Matthew,
>
> Currently, there is no _CID defined for generic AHCI. We will work on
> proposing one, and provide update patches for including the new ID.
>
I think part of the problem is that there is no specification for what
an AHCI device should look like when it's not connected to a PCI
bus, the AHCI document published by Intel just states:
"AHCI is a PCI class device that acts as a data movement engine
between system memory and Serial ATA devices."
It also requires the PCI config space to have the PM capability
registers and (optionally) the MSI capability registers.
There are lots of chips we support in Linux with the ahci-platform
driver, but they are not actually compliant because they cannot
use the pmcap registers but instead typically rely on setting
external clock/phy/regulator/pinctrl registers.
The ARM SBSA document just requires any SATA controller to be AHCI
compliant but does not explain what that means in the case where
it's not a PCI device.
Arnd
On Thu, Oct 02, 2014 at 10:39:18AM +0200, Arnd Bergmann wrote:
> I think part of the problem is that there is no specification for what
> an AHCI device should look like when it's not connected to a PCI
> bus, the AHCI document published by Intel just states:
One thing that has come out of further discussion on this - ACPI defines
a _CLS object, which allows ACPI devices to declare compatibility with a
PCI class device. We don't have support for it at the moment, but it
would be easy enough to add this to the kernel and then just expose the
AHCI PCI class via _CLS. That would avoid us having problems with people
doing similar things with USB hosts.
--
Matthew Garrett | [email protected]
On Monday 06 October 2014 17:31:47 Matthew Garrett wrote:
> On Thu, Oct 02, 2014 at 10:39:18AM +0200, Arnd Bergmann wrote:
>
> > I think part of the problem is that there is no specification for what
> > an AHCI device should look like when it's not connected to a PCI
> > bus, the AHCI document published by Intel just states:
>
> One thing that has come out of further discussion on this - ACPI defines
> a _CLS object, which allows ACPI devices to declare compatibility with a
> PCI class device. We don't have support for it at the moment, but it
> would be easy enough to add this to the kernel and then just expose the
> AHCI PCI class via _CLS. That would avoid us having problems with people
> doing similar things with USB hosts.
Interesting. Does this also define a way to get access to registers
that are normally in PCI config space, provided they are accessible at
all?
I'm guessing that it's not extremely important, given that so far all
platforms are doing ok without the power management registers or MSI,
but I guess it would be nice if we could support those.
Arnd
On Mon, Oct 06, 2014 at 08:19:37PM +0200, Arnd Bergmann wrote:
> Interesting. Does this also define a way to get access to registers
> that are normally in PCI config space, provided they are accessible at
> all?
Unfortunately not. I'd assume that PM registers are expected to be
accessed via the _PS* methods instead. Does MSI make sense outside the
context of PCI interrupts?
--
Matthew Garrett | [email protected]
On Monday 06 October 2014 19:21:53 Matthew Garrett wrote:
> On Mon, Oct 06, 2014 at 08:19:37PM +0200, Arnd Bergmann wrote:
>
> > Interesting. Does this also define a way to get access to registers
> > that are normally in PCI config space, provided they are accessible at
> > all?
>
> Unfortunately not. I'd assume that PM registers are expected to be
> accessed via the _PS* methods instead. Does MSI make sense outside the
> context of PCI interrupts?
Yes, the ARM GIC has a weird sense of what MSI is used for, and
apparently some SoC vendors have started using MSI by default for
all on-chip peripherals.
A patch series to extend MSI to platform devices is currently
under review.
Arnd
On Mon, Oct 06, 2014 at 08:44:35PM +0200, Arnd Bergmann wrote:
> On Monday 06 October 2014 19:21:53 Matthew Garrett wrote:
> > Unfortunately not. I'd assume that PM registers are expected to be
> > accessed via the _PS* methods instead. Does MSI make sense outside the
> > context of PCI interrupts?
>
> Yes, the ARM GIC has a weird sense of what MSI is used for, and
> apparently some SoC vendors have started using MSI by default for
> all on-chip peripherals.
>
> A patch series to extend MSI to platform devices is currently
> under review.
Mm. Yeah, it doesn't seem like there's any ACPI-defined mechanism for
MSI control. Let's chat about this next week?
--
Matthew Garrett | [email protected]