Jean Delvare wrote:
>
> 1075mV isn't valid under VRM 9.x according to the specs, so I really
> wonder how it can work on some plateforms. I would happily see it
> removed to keep the code more simple. And i2c-sensor-vid will not
> support it anyway.
Ok, removed. The IRU3055 converter controller IC used on the 8RDA boards
allows setting 1.075V.
> As a side note, i2c-sensor-vid.c and i2c-vid.h were written with VID pin
> *read* in mind, not write, so you might have to improve them a bit for
> your own needs.
Something like in the attached patch? I've not added the other VRMs yet,
first need to know if the ATXP1 is used with other VRMs.
>># Update register data if needed
>>for(count = 0; count <= 0x0a; count++) {
>> if((data->valid >> count & 1) == 0) {
>> data->reg[count] = i2c_smbus_read_byte_data(client, count);
>> data->valid |= 1 << count;
>> }
>>}
>
>
> Huh. Looks even more complex. Do you really need to read all 10 first
> registers, while you were previously only reading 2? What is the benefit
> of having a separate "valid" bit for each register since you always read
> them all at once anyway?
For future versions, which are using more (all registers). For general
purpose IOs (used to control other voltages, extra functions) and
watchdog timer. Also a register variable gets only updated at start or
when a value was written.
> That would need to be documented in the code. Actually, the whole way
> the chip works should be, since there is no public datasheet people can
> look at.
Ok, I'll add more comments.
>
> Note that you won't be able to support VRM 10 since it uses 6-bit VID,
> but all previous versions should work fine.
>
I don't know if the ATXP1 is used on boards with < 9.0 at all. Would be
interesting
if somebody knows.
>
> In other words, it uses VRM 9.x. You will need to add validity checkings
> based on VRM version instead, providing you actually want to support
> other VRM versions.
See current patches.
>
> OK, that makes sense. Please add a comment explaining this in the code.
> By slow I mean that SMBus transfer are slows (the bus is typically
> operating at 15kHz), and since atxp1_write does 2 transfers and can be
> called a dozen times to change the vid output once, it looked like it
> would be slower that needed. However, if there is a good reason to do it
> in an incremental way, fine with me. The operation isn't time critical
> and we want it to be safe, agreed. But at least this is a good reason to
> get rid of the readback test in atxp1_write.
>
> Please note that the trick of increasing the vid value by one to
> linearly decrease vcore works with VRM 9.x but wouldn't work with all
> VRM 8.x, which are more complex. This might be a sufficient reason not
> to support VRM 8.x (since we don't know of any system using both VRM 8.x
> and an ATXP1). The fact that you will use i2c-sensor-vid doesn't mean
> that you have to support all VRM versions, just that you will avoid code
> duplication. You can check the VRM version and abort if it is anything
> you don't support (i.e. non-9.x for now).
>
Yes, see patch.
> I don't know if this exist, but it certainly could. If both CPUs are
> handled by an ATXP1 (and thus the same driver), we could have a common
> static counter so that the first client creates cpu0_vid and the second
> would create cpu1_vid.
Not added yet, next patch.
>
> Checking that random registers are 0 won't be very efficient because
> that's the default value for most chips. However, some specific
> registers would be worth checking: 0x3e, 0x3f, 0xfe, 0xff. These are the
> locations of device id and manufacturer id registers for most chips, so
> checking that they have value 0 is certainly great. It will avoid
> attaching to the various temperature sensors that can live at 0x4e.
>
> The fact that registers 0x10-0x1a return the same value as 0x00 will
> also be a very efficient check. You don't have to check them all, a few
> of them will do (remember that each additional read will slow down the
> detection and increase the module load time as a result).
>
> Your original check (CVID > 0) should probably be removed, as it doesn't
> seem to be valid (0 is a valid VID value, 1.850V under VRM 9.x).
>
Done.
Bye,
Sebastian
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: atxp1-0.4.patch
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20050119/4297e2b3/attachment.pl
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: i2c-vid-vrm90.patch
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20050119/4297e2b3/attachment-0001.pl
