acpi_throttle: quirk based on pci info
Andriy Gapon
avg at icyb.net.ua
Sat Mar 1 08:50:16 UTC 2008
on 28/02/2008 15:56 John Baldwin said the following:
> On Wednesday 27 February 2008 05:13:58 pm Andriy Gapon wrote:
>> on 27/02/2008 18:26 John Baldwin said the following:
>>> On Wednesday 27 February 2008 02:54:38 am Andriy Gapon wrote:
>> [snip]
>> John,
>>
>> thank you for this detailed reply!
>> I looked through the code and I think that ichss is the only device that
>> explicitly requires cpu and pci buses to be "configured". acpi_throttle
>> is another one that implicitly requires that.
>> My personal preference is to probe/attach pci first and then go with
>> cpu. This is mostly because pci can provide a lot of useful information
>> and resources to various devices. On the other hand, cpu mostly exists
>> so that others could attach to it (it does provide a little bit, but
>> it's a very little bit). So, in my opinion, it is more likely that a
>> child of cpu would need something from pci than vice versa.
>>
>> If we agree on this order and implement it, then I agree with you that
>> it would be quite easy to modify ichss to be a "normal" child of cpu and
>> use pci_find_device to find a proper pci device. And the rest of the
>> code that uses pci_read_config, bus_set_resource and
>> bus_alloc_resource_any would remain practically the same.
>> I'd even say that this would be a trivial change.
>> And I'd even say that this would be a change in right direction, because
>> ichss would lose most of its 'specialness'.
>
> Actually, we can make ichss rather simple w/o changing it much by having it
> just set a global variable in its PCI probe routine and checking that global
> when attaching to the CPU.
>
> One other thing that concerns me is that cpufreq drivers want to know about
> each other (e.g. smist checks for ichss0, etc.). I'd rather that if we have
> multiple drivers controlling the same back-end hardware (via difference
> interfaces) they all use the same driver name (e.g. speedstep0) and use probe
> priority to decide which driver wins if both ichss0 and smist0 can handle a
> CPU for example.
>
> Here is a patch to make CPUs come after PCI and an attempt to fix ICH. Note
> that if we made ichss_identify() manually look for the PCI device by bsf
> instead of using a probe routine to find it we could remove the pci "driver"
> completely and make it work on kldload. It also fixes a bug that the attempt
> to enable SS via a PCI config register write could never work as it passed a
> cpu0 device_t to pci_read_config/pci_write_config in ichss_probe()
> previously. I moved this to attach() (where it belongs) and used the right
> device_t so this should work now. I have no hardware to test it on though.
>
Here is a patch that implements find by bsf idea in ichss that we also
discussed.
I don't own the actual hardware but I "tested" the patch by temporarily
changing some ids and adding printfs instead of some actions.
Another little difference that I've added (just in case) a protection
against double-adding.
I also tested acpi part of your patch - works great!
Here's one strange thing - in your patch you accidentally have
parameters of device_identify switched, I initially inherited that bug
too. I got no error/warning from compiler whatsoever. It wasn't until I
saw that device_get_unit(parent) returned garbage that I my untrained
eye noticed the mistake.
--
Andriy Gapon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ichss.patch
Type: text/x-patch
Size: 5954 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-acpi/attachments/20080301/7fdeecbc/ichss.bin
More information about the freebsd-acpi
mailing list