lkml.org 
[lkml]   [2013]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 5/6] PCI: acpiphp: look _RMV method a bit deeper in the hierarhcy
On Tue, Jul 2, 2013 at 4:44 AM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> Mika Westerberg wrote:
>> The acpiphp driver finds out whether the device is hotpluggable by checking
>> whether it has _RMV method behind it (and if it returns 1). However, at
>> least Acer Aspire S5 with Thunderbolt host router has this method placed
>> behind device called EPUP (endpoint upstream port?) and not directly behind
>> the root port as can be seen from the ASL code below:
>>
>> Device (RP05)
>> {
>> ...
>> Device (HRUP)
>> {
>> Name (_ADR, Zero)
>> Name (_PRW, Package (0x02)
>> {
>> 0x09,
>> 0x04
>> })
>> Device (HRDN)
>> {
>> Name (_ADR, 0x00040000)
>> Name (_PRW, Package (0x02)
>> {
>> 0x09,
>> 0x04
>> })
>> Device (EPUP)
>> {
>> Name (_ADR, Zero)
>> Method (_RMV, 0, NotSerialized)
>> {
>> Return (One)
>> }
>> }
>> }
>> }
>>
>> If we want to support such machines we must look for the _RMV method a bit
>> deeper in the hierarchy. Fix this by changing pcihp_is_ejectable() to check
>> few more devices down from the root port.
>
> We found that this approach is broken. We've got false positive: host
> bridge itself was detected as hotplugable slot %) I think it's not
> acceptable.
>
> Mika has tried few more approaches, but we haven't found anything better
> then hardcoded path like in original workaround patch[1]. It's not generic
> at all, but safe from false positives.
>
> Any thoughts?
>
> [1] http://article.gmane.org/gmane.linux.kernel.pci/19102

The changelog of this patch ([1]) says "Correct ACPI PCI hotplug
implementation should have _RMV method in a PCI slot (device under pci
bridge). In Acer Aspire S5 case we have it deeper in hierarchy ..."

This is exactly what I mean about acpiphp being brittle because of the
assumptions it makes about the ACPI namespace. Is there actually
something in the spec that requires the _RMV method to be where
pcihp_is_ejectable() expects it?

I'm not 100% dead-set against merging the workaround with hard-coded
path, but I still don't think it's a good idea. It "fixes" it for one
particular machine, but there will likely be other machines that
require similar fixes in the future. It makes it harder for somebody
to clean up the design later, because that person won't have an Aspire
S5 to test. It makes it less likely that somebody *will* clean it up
later, because "everything is already working."

That's why my preference (given infinite resources) would be to rework
acpiphp now, while people are interested and can test it. I'm sure
this would be a major redesign of acpiphp and its interaction with
pciehp, but I think it's something we're going to have to do at some
point.

Bjorn


\
 
 \ /
  Last update: 2013-07-02 20:21    [W:0.557 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site