On 05/15/2015 04:50 PM, Rafael J. Wysocki wrote: > On Friday, May 15, 2015 05:55:17 PM Adam Goode wrote: >> On Thu, May 14, 2015 at 6:47 PM, Al Stone <ahs3@xxxxxxxxxx> wrote: >>> On 05/14/2015 06:36 AM, Adam Goode wrote: >>>> On Wed, May 13, 2015 at 11:14 PM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: >>>>> On Wednesday, May 13, 2015 01:07:36 PM Al Stone wrote: >>>>>> On 05/13/2015 10:25 AM, Adam Goode wrote: >>>>>>> The Macmini7,1 addresses SystemCMOS memory in _INI methods. Currently, >>>>>>> this fails since _INI is called before the acpi_cmos_rtc_space_handler >>>>>>> is registered. >>>>>>> >>>>>>> I proposed registering a default handler on the ACPICA list, but was >>>>>>> told that because the device has a _HID it should require a device >>>>>>> driver. >>>>>>> >>>>>>> So, is it possible to register a device driver before _INI is called? >>>>>>> Otherwise, Thunderbolt doesn't get initialized properly on this >>>>>>> hardware. >>>>>> >>>>>> I take it from the question that the _INI methods are using the predefined >>>>>> SystemCMOS OperationRegion, correct? Are the _INI methods invoking _REG >>>>>> before trying to access that region? Looking at the spec, the _INI methods >>>>>> must first call _REG to see if SystemCMOS is available for use (see section >>>>>> 6.5.1), and there is no requirement that SystemCMOS must be available for >>>>>> use by _INI (see 6.5.4). So, if I think about this from the spec point of >>>>>> view, it sounds like the _INI methods are non-compliant. From the kernel >>>>>> perspective, the SystemCMOS region is created at a reasonable time and is >>>>>> available when it is required to be. >>>> >>>> My reading of the ACPI spec is that the OS calls _REG when it updates >>>> region availability. It's not the AML that calls _REG at all. There >>>> are no _REG methods defined for this, so nothing to do. Further >>>> reading of the spec seems to indicate that the OS should be doing a >>>> kind of dependency analysis and registering region handlers before >>>> failing here. I'm not seeing anything really out of spec with the AML >>>> code in this case. >>> >>> Ah, my bad. I misread the _REG part. The OS does call _REG, not the AML. >>> Just the same, that section does say that "control methods must assume all >>> operation regions inaccessible until the _REG(RegionSpace, 1) method is >>> executed." I would take that to mean that _INI cannot assume SystemCMOS >>> is ready to use, unless _REG has been defined in an enclosing scope so the >>> OS knows it is to be executed. >>> >>> Could you point out where the dependency analysis is indicated? I am >>> not seeing that at all; that would seem to require a priori knowledge >>> of all of the regions all of the devices could ever possibly use, and >>> it's not clear to me that can even be conveyed to the OS using the >>> current version of the spec. As someone involved in writing the spec, >>> I want to make sure we're being unambiguous in what is required. >> >> I think you can relax, I believe I read too far into section 6.5.8 >> _DEP (Operation Region Dependencies). It points out that _DEP is >> optional, but goes on to say that you need _REG callbacks to be called >> anyway. Ah. Okey dokey. I will take a look at these sections again, though, just to see if there's a way to make them clearer. >> What is a little confusing to me here is that _REG is per >> address-space, not per address. I guess that makes some sense for some >> kinds of regions. >> >> >>> >>>> I'm guessing that some kind of refactoring of _HID driver attachment >>>> would be a way forward here. But I haven't looked deeply into this >>>> yet. >>> >>> Perhaps; as long as _INI is executed before _HID as required (6.5.1, again). >>> >> >> Hmm, this looks like it's the problem, and does strongly suggest to me >> that the firmware is busted. > > Still, if Windows has no problems working with it, so should we. Yeah, agreed. It's interesting (well, to me, at least :) that this has not shown up before as other _INI functions depending on unregistered regions. Or maybe I just haven't been aware of them before... >> But the spec is confusing to me here, it >> says _INI is run before _HID is "run". What does it mean for _HID to >> run? It's not a method in the traditional sense. I think it is >> implying OS device enumeration? > > _HID may be implemented as a method in which case it will be run. But it is > better to say "evaluated" in any case. :-) Ain't English fun? Yup, it's an object that gets evaluated. I'll try to watch out for that in the future :). > Windows appears to install the CMOS region handler upfront, probably with the > assumption that firmware accessing operation regions in it should know that > the CMOS device is actually present. So would it make sense to reconsider where Linux registers regions, and maybe move them earlier? I can't really tell how prevalent this sort of situation might be in firmware out in the wild; it may be more practical to just handle each region when it becomes an issue like this one. -- ciao, al ----------------------------------- Al Stone Software Engineer Red Hat, Inc. ahs3@xxxxxxxxxx ----------------------------------- -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
