Bug 3358

Summary: "functional but not present; setting present" is wrong
Product: ACPI Reporter: CASTET Matthieu (castet.matthieu)
Component: Config-OtherAssignee: ykzhao (yakui.zhao)
Status: CLOSED CODE_FIX    
Severity: normal CC: acpi-bugzilla, yakui.zhao
Priority: P2    
Hardware: i386   
OS: Linux   
Kernel Version: 2.6.8 Subsystem:
Regression: --- Bisected commit-id:
Attachments: dsdt
Attempt to fix the problem
patch vs 2.6.23
the same patch as comment #13 but cleaner
patch to workaround the issue
patch for the workaround this issue
patch vs 2.6.24 to fix autoload
Build Linux ACPI device tree and load device driver according to the status of the device.

Description CASTET Matthieu 2004-09-08 00:19:59 UTC
hi,
when developping pnpacpi [1], I discover a bug.
In acpi_bus_get_status, if the device is not present but functional, the device
is set to functional. But it's not good :
all the device disable in bios are set to present
even worse there are 2 printer ports PNP0400 and PNP0401, which cause conflic
because the driver want register the both device...

A workaround sould be to set the device to not functional, so the device will
not be in the available devices.

Matthieu

[1] http://marc.theaimsgroup.com/?l=linux-kernel&m=109430451522335&w=2
Comment 1 Shaohua 2004-09-19 19:28:37 UTC
Matthieu,
Could you please attach your DSDT?
Comment 2 CASTET Matthieu 2004-09-29 10:55:33 UTC
look at
                                    If (DEN)
                                    {
                                        Return (0x0F)
                                    }
                                    Else
                                    {
                                        Return (0x0D)
                                    }
you could see that when DEM = zero (device disable), the device is functional
but not present

So the right way is to suppress (device->status.functional &&
!device->status.present) case...
Comment 3 CASTET Matthieu 2004-09-29 10:58:13 UTC
Created attachment 3742 [details]
dsdt
Comment 4 Shaohua 2004-09-29 20:34:12 UTC
I'm a little confused. 0xD (1101b) means present, disabled, functional.
Comment 5 CASTET Matthieu 2004-09-29 23:24:26 UTC
oupps it is me :
I write the wrong test.
When the device is disable via the bios or LPT is enable, for the ECP device
LAnd (LEqual (Local0, 0x80), LEqual (L1EN, 0x01)) will be false, so it will
return 0x8 (1000b) (functional=1, present=enable=0)

The DEN is used when disable via _DIS.

If (LAnd (LEqual (Local0, 0x80), LEqual (L1EN, 0x01)))                  
                            {   
                                If (DEN)
                                {
                                    Return (0x0F)
                                }
                                Else
                                {
                                    Return (0x0D)
                                }
                            }
                            Else
                            {
                                Return (0x08)
                            }
}
Comment 6 Shaohua 2004-09-29 23:31:06 UTC
Bjorn made the workaround for HP systems. Will send an email to him.
Comment 7 CASTET Matthieu 2004-11-25 09:59:12 UTC
the acpi spec say :
_STA may return bit 0 clear (not present) with bit 3 set (device is functional).
This case is used to indicate
a valid device for which no device driver should be loaded (for example, a
bridge device.) Children of this
device may be present and valid. OSPM should continue enumeration below a device
whose _STA returns
this bit combination.


So it seem clear that if this device should not added in the acpi layer in order
to respect the spec...
The same fix is to be done in pnpacpi...
Comment 8 Luming Yu 2004-12-01 08:16:49 UTC
Here's history for bjorn's workaround:
https://sourceforge.net/mailarchive/message.php?msg_id=6923358
Comment 9 Luming Yu 2004-12-01 08:19:03 UTC
The following is the conclusion till now.
<--quote

On Wednesday 12 May 2004 3:49 pm, Len Brown wrote:
> The ACPI spec gurus say that as of 2.0c,
> _STA.functional is meaningless unless _STA.present.
> 
> However, they also say that the (_STA.functional && !_STA.present)
> implementation was intentional -- apparently to trick some OSPM into
> using a resource for initialization only, but to not load
> a driver.

Yup, I think somebody said Windows did something special with
this combination.  Figures.

> I'm advocating that the spec be clarified to explicitly
> allow or disallow this idiom.
> 
> In the mean-time, Bjorn's patch is now present in the acpi-test tree.

Sounds good to me.  Thanks for chasing it down!

Bjorn


--quote>
Comment 10 CASTET Matthieu 2004-12-02 08:51:49 UTC
After reading the history for bjorn's workaround, what I understand is the
pseudo PCI root bridges don't need driver and we only need to scan the devices
below them.

So it's follow the spec (that even give bridge as example) and I don't see why
you need to add it in acpi driver layer.
Comment 11 Shaohua 2004-12-02 16:44:02 UTC
Bjorn said they need the hiden MCH devices to do something (I can't recall 
clearly) not adding a MCH driver. But anyway, this workaround is very curious.
Comment 12 CASTET Matthieu 2004-12-17 00:16:46 UTC
Created attachment 4274 [details]
Attempt to fix the problem

I think the parsing device loop should be modified : this hack allow the loop
to parse device child of 'functional but not present' devices. So we should
allow this without modifying present flag and adding the device to the acpi
layer.

I attached an attempt to do that (untested), but I don't know if it is possible
to not register this device in the acpi layer...
Comment 13 Len Brown 2007-09-19 18:55:47 UTC
Created attachment 12872 [details]
patch vs 2.6.23

patch from comment #12 refreshed for linux-2.6.23
Comment 14 Zhang Rui 2007-09-19 19:42:46 UTC
Created attachment 12873 [details]
the same patch as comment #13 but cleaner
Comment 15 Shaohua 2007-09-19 20:38:49 UTC
Created attachment 12874 [details]
patch to workaround the issue

Proposed patch doesn't solve the issue, functional but not present device is stilled be plugged into ACPI layer. To workaround the issue, the new patch does a check in PNP driver.

Len, please check if this is ok.
Comment 16 ykzhao 2007-12-05 17:16:47 UTC
Created attachment 13880 [details]
patch for the workaround this issue

Will someone test this patch on the machine with the unpresent but functional device? 
Thanks.
Comment 17 Len Brown 2008-01-11 19:04:06 UTC
Created attachment 14424 [details]
patch vs 2.6.24 to fix autoload

Yakui,
Please update the patch in comment #16 so that it applies
on top of this "autoload.patch", which is already queued for 2.6.25.
Comment 18 ykzhao 2008-03-03 21:15:44 UTC
Created attachment 15133 [details]
Build Linux ACPI device tree and load device driver according to the status of the device.

The attached is updated patch.
Comment 19 Shaohua 2008-09-09 20:35:46 UTC
yakui, should we mark this resloved and let Len take the patch?
Comment 20 Len Brown 2008-10-17 00:02:05 UTC
patch in comment #18 is queued for 2.6.28:

Author: Zhao Yakui <yakui.zhao@intel.com>  2008-08-11 01:40:22
Committer: Len Brown <len.brown@intel.com>  2008-09-19 14:57:47
Parent: 2f76f1bd9b3ed0992b987d0b3a9b60ee698467d4 (ACPI: Add DMI check to disable power state check in power transition)
Child:  a8c8bdb14ec941cf00103b725683f2d6952bd0b4 (Merge branch 'ec' into test)
Branches: bugfixes, test
Follows: v2.6.27-rc6
Precedes: 

    ACPI : Load device driver according to the status of acpi device
Comment 21 Len Brown 2008-10-24 22:58:15 UTC
shipped in linux-2.6.28-rc1
closed

commit 39a0ad871000d2a016a4fa113a6e53d22aabf25d
Author: Zhao Yakui <yakui.zhao@intel.com>
Date:   Mon Aug 11 13:40:22 2008 +0800

    ACPI : Load device driver according to the status of acpi device