Hey Holger,
I've been re-reading this section and now I'm confused by it. I have to
admit, I'm not entirely sure how I came to my original interpretation,
but your guess that I read "unreadable" and stopped is as good a guess
as any.
> Maybe the value was first set in the SDR's when support for the
> optional Get Sensor Thresholds cmd was not yet available in the
> firmware, or by mistake.
After thinking about it, I think this interpretation may be correct. It
seems to be verified by atleast 1 motherboard I have on site that has
the same situation you describe. It has 11b set for the threshold
access support.
[ 3h] = sensor_capabilities.threshold_access_support[ 2b]
yet, it has a bunch of readable thresholds
[ 1h] =
readable_threshold_mask.lower_non_critical_threshold_is_readable[ 1b]
[ 1h] =
readable_threshold_mask.lower_critical_threshold_is_readable[ 1b]
[ 0h] =
readable_threshold_mask.lower_non_recoverable_threshold_is_readable[ 1b]
[ 1h] =
readable_threshold_mask.upper_non_critical_threshold_is_readable[ 1b]
[ 1h] =
readable_threshold_mask.upper_critical_threshold_is_readable[ 1b]
[ 0h] =
readable_threshold_mask.upper_non_recoverable_threshold_is_readable[ 1b]
and it has a bunch of (what appears to be legitimate) values in the threshold
value fields.
[ FFh] = upper_non_recoverable_threshold[ 8b]
[ E4h] = upper_critical_threshold[ 8b]
[ DAh] = upper_non_critical_threshold[ 8b]
[ 0h] = lower_non_recoverable_threshold[ 8b]
[ 80h] = lower_critical_threshold[ 8b]
[ 85h] = lower_non_critical_threshold[ 8b]
However, when calling "Get Sensor Thresholds" against this sensor I get
a completion code of CDh =
"Command illegal for specified sensor or record type."
I think the compliant thing to do would be to get thresholds from the
SDR instead of via Get Sensor Thresholds of the threshold_access ==
fixed/unreadable.
Would this work for you Holger? Or would we still need to the vendor
specific exception? Attached is a patch I made to test this out and it
works on the motherboard I have.
Al
On Tue, 2013-07-02 at 16:33 +0200, Liebig, Holger wrote:
> > Holger,
> >
> > By all means it should adhere to the spec, but I'm confused as to how this
> > 11b value got introduced, when it should have been correct when read from
> > the vendor-supplied SDRs?
> [Liebig, Holger]
> Andy,
> This value is set in our default SDR for almost any system of the past
> few years :(
> It's a shame that it never came up earlier in any review, but the
> situation is now as it is and I'm trying to convince Albert to include
> my patch as a vendor specific workaround.
>
> For non-native speakers it's sometimes difficult to get the full
> meaning and people tend to read what they want to read. If you split
> the wording in the spec into two sentences you get:
>
> Fixed, unreadable, thresholds.
> and
> Which thresholds are supported is reflected by the Reading Mask. The
> threshold value fields report the values that are ‘hard-coded’ in the
> sensor.
>
> The second part makes only limited sense if the thresholds were really
> unreadable, or where the difference to the read only thresholds really
> is. If you read only the first part and skip the second part you end
> up with 'unreadable' and might wonder what "Which thresholds are
> supported is reflected ..." refers to.
>
> Maybe the value was first set in the SDR's when support for the
> optional Get Sensor Thresholds cmd was not yet available in the
> firmware, or by mistake. Nobody I asked can remember why it is set
> this way and many applications check only the Threshold Reading /
> Threshold Writing Mask in the SDR without the capabilities bits.
>
> Again, my hope is that Albert includes the patch as vendor specific
> workaround.
>
> Thanks for your comments,
> Holger
>
> _______________________________________________
> Freeipmi-devel mailing list
> [email protected]
> https://lists.gnu.org/mailman/listinfo/freeipmi-devel
--
Albert Chu
[email protected]
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory
Index: ipmi-sensors/ipmi-sensors-output-common.c
===================================================================
--- ipmi-sensors/ipmi-sensors-output-common.c (revision 9686)
+++ ipmi-sensors/ipmi-sensors-output-common.c (working copy)
@@ -451,8 +451,7 @@
goto cleanup;
}
- if (threshold_access_support == IPMI_SDR_NO_THRESHOLDS_SUPPORT
- || threshold_access_support == IPMI_SDR_FIXED_UNREADABLE_THRESHOLDS_SUPPORT)
+ if (threshold_access_support == IPMI_SDR_NO_THRESHOLDS_SUPPORT)
{
rv = 0;
goto cleanup;
@@ -543,6 +542,15 @@
goto cleanup;
}
+
+ if (threshold_access_support == IPMI_SDR_FIXED_UNREADABLE_THRESHOLDS_SUPPORT)
+ {
+ if (_get_sdr_sensor_thresholds (state_data, obj_cmd_rs) < 0)
+ goto cleanup;
+
+ goto continue_get_sensor_thresholds;
+ }
+
if (ipmi_cmd_get_sensor_thresholds (state_data->ipmi_ctx,
sensor_number,
obj_cmd_rs) < 0)
_______________________________________________
Freeipmi-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/freeipmi-devel
