Skip to content

Commit

Permalink
Elbert: lm_sensors support for multiple PIM types (#114)
Browse files Browse the repository at this point in the history
Summary:
Updates setup_i2c.sh to add an additional ISL device on pim8ddm PIMs. Additionally, the device names are updated for the SMB ISL/RAA chips to take advantage of the new ISL driver functionality added in [https://github.com/facebookexternal/openbmc.arista/issues/107](https://github.com/facebookexternal/openbmc.arista/pull/107).

lm_sensors is also updated to account for multiple PIM types. Currently, lm_sensors cannot handle sensor differences between pim16q and pim8ddm due to the single elbert.conf configuration file. This change adds two subsequent sensor config file templates, pim16q.conf and pim8ddm.conf, which are used by pim_enable.sh to dynamically apply sensor labels and thresholds based on PIM type. The generated config files can be found in /etc/sensors.d/ (lm_sensors applies all configs found in this directory):

The i2c CIT test is also updated to reflect the device name changes.

```
root@bmc-oob:~# ls /etc/sensors.d/ -al
drwxr-xr-x    2 root     root             0 Nov 18 10:41 .
drwxr-xr-x   33 root     root             0 Nov 18 10:41 ..
-rw-r--r--    1 root     root           776 Nov 18 09:58 .pim16q.conf
-rw-r--r--    1 root     root          1878 Nov 18 09:58 .pim8ddm.conf
-rw-r--r--    1 root     root          7058 Nov 18 09:58 elbert.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim2.conf
-rw-r--r--    1 root     root          1699 Nov 18 10:41 pim3.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim4.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim5.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim6.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim7.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim8.conf
-rw-r--r--    1 root     root           708 Nov 18 10:41 pim9.conf
-rw-r--r--    1 root     root            87 Nov 18 08:46 sensors.conf
```

### Testing

Verified on Elbert with the following PIMs:
```
root@bmc-oob:~# pim_types.sh
PIM 2: PIM16Q
PIM 3: PIM8DDM
PIM 4: PIM16Q
PIM 5: PIM16Q
PIM 6: PIM16Q
PIM 7: PIM16Q
PIM 8: PIM16Q
PIM 9: PIM16Q
```

Focusing on PIMs 2 and 3, note that sensors differ between PIM types:
```
# PIM2
root@bmc-oob:~# sensors | grep i2c-16
lm73-i2c-16-4a
pmbus-i2c-16-16
ucd9090-i2c-16-4e
pmbus-i2c-16-18

# PIM3
root@bmc-oob:~# sensors | grep i2c-17
ucd9090-i2c-17-4e
pmbus-i2c-17-16
isl68224-i2c-17-54 <-- pim8ddm specific ISL
lm73-i2c-17-4a
pmbus-i2c-17-18
```

The ISL68224 on pim8ddm supports voltage monitoring on 3 voltage loops, which the new driver supports:
```
root@bmc-oob:~# sensors isl68224-i2c-17-54
isl68224-i2c-17-54
Adapter: i2c-2-mux (chan_id 1)
PIM3 ISL Vin:        +12.02 V  (crit min =  +0.00 V, min =  +0.00 V)
                               (max = +18.00 V, crit max = +16.00 V)
PIM3 ISL Vmon:        +0.00 V
PIM3 ISL Vout Loop0:  +0.90 V  (crit min =  +0.81 V, crit max =  +0.99 V)
PIM3 ISL Vout Loop1:  +1.55 V  (crit min =  +1.47 V, crit max =  +1.63 V)
PIM3 ISL Vout Loop2:  +1.85 V  (crit min =  +1.76 V, crit max =  +1.94 V)
PIM3 ISL Temp1:       +45.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Temp2:       +47.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Temp3:       +45.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Temp4:       +41.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Temp5:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Temp6:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Temp7:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
PIM3 ISL Pin Loop0:   20.00 W
PIM3 ISL Pin Loop1:   28.00 W
PIM3 ISL Pin Loop2:   16.00 W
PIM3 ISL Pout Loop0:  16.00 W
PIM3 ISL Pout Loop1:  24.00 W
PIM3 ISL Pout Loop2:  14.00 W
PIM3 ISL Iin Loop0:   +1.76 A  (max = +150.00 A, crit max = +50.00 A)
PIM3 ISL Iin Loop1:   +2.38 A  (max = +150.00 A, crit max = +50.00 A)
PIM3 ISL Iin Loop2:   +1.37 A  (max = +150.00 A, crit max = +50.00 A)
PIM3 ISL Iout Loop0: +18.80 A  (max = +3276.70 A, crit max = +126.00 A)
PIM3 ISL Iout Loop1: +15.80 A  (max = +3276.70 A, crit max = +26.00 A)
PIM3 ISL Iout Loop2:  +7.80 A  (max = +3276.70 A, crit max = +12.00 A)
```

The updated ISL driver allows additional sensors on the TH4 ISL/RAA:
```
root@bmc-oob:~# sensors raa228228-i2c-3-60
raa228228-i2c-3-60
Adapter: 1e78a200.i2c-bus
TH4 VRD1 Vin:        +12.04 V  (crit min =  +0.00 V, min =  +0.00 V)
                               (max = +18.00 V, crit max = +16.00 V)
TH4 VRD1 Vmon:        -0.02 V
TH4 VRD1 Vout Loop0:  +0.89 V  (crit min =  +0.80 V, crit max =  +0.98 V)
TH4 VRD1 Temp1:       +33.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD1 Temp2:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD1 Temp3:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD1 Pin:         39.00 W
TH4 VRD1 Pout Loop0:  34.00 W
TH4 VRD1 Iin:         +3.23 A  (max = +150.00 A, crit max = +50.00 A)
TH4 VRD1 Iout Loop0: +38.50 A  (max = +400.00 A, crit max = +715.00 A)

root@bmc-oob:~# sensors isl68226-i2c-3-62
isl68226-i2c-3-62
Adapter: 1e78a200.i2c-bus
TH4 VRD2 Vin:        +12.08 V  (crit min =  +0.00 V, min =  +0.00 V)
                               (max = +18.00 V, crit max = +16.00 V)
TH4 VRD2 Vmon:        +0.00 V
TH4 VRD2 Vout Loop0:  +0.75 V  (crit min =  +0.68 V, crit max =  +0.82 V)
TH4 VRD2 Vout Loop1:  +1.20 V  (crit min =  +1.08 V, crit max =  +1.32 V)
TH4 VRD2 Vout Loop2:  +1.20 V  (crit min =  +1.08 V, crit max =  +1.32 V)
TH4 VRD2 Temp1:       +33.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Temp2:       +33.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Temp3:       +31.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Temp4:       +26.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Temp5:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Temp6:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Temp7:        +0.0 C  (high = +110.0 C, crit low = -40.0 C)
                               (crit = +125.0 C)
TH4 VRD2 Pin Loop0:    2.00 W
TH4 VRD2 Pin Loop1:    0.00 W
TH4 VRD2 Pin Loop2:    0.00 W
TH4 VRD2 Pout Loop0:   2.00 W
TH4 VRD2 Pout Loop1: -1000.00 mW
TH4 VRD2 Pout Loop2: -1000.00 mW
TH4 VRD2 Iin Loop0:   +0.23 A  (max = +150.00 A, crit max = +50.00 A)
TH4 VRD2 Iin Loop1:   +0.08 A  (max = +150.00 A, crit max = +50.00 A)
TH4 VRD2 Iin Loop2:   +0.05 A  (max = +150.00 A, crit max = +50.00 A)
TH4 VRD2 Iout Loop0:  +3.30 A  (max = +3276.70 A, crit max = +191.00 A)
TH4 VRD2 Iout Loop1:  +0.00 A  (max = +3276.70 A, crit max = +20.00 A)
TH4 VRD2 Iout Loop2:  -0.30 A  (max = +3276.70 A, crit max = +20.00 A)
```

test_i2c.py passes for Elbert:
```
root@bmc-oob:/usr/local/bin/tests2# python3 cit_runner.py --platform elbert -r tests.elbert.test_i2c
test_i2c_tree (tests.elbert.test_i2c.ElbertI2cTest)
check if i2c devices are created and drivers are binded properly. ... ok

----------------------------------------------------------------------
Ran 1 test in 4.279s

OK
```

Pull Request resolved: facebookexternal/openbmc.arista#114

Reviewed By: mikechoifb

fbshipit-source-id: e835a38ba3
  • Loading branch information
joancaneus authored and facebook-github-bot committed Nov 19, 2020
1 parent 6b2b156 commit ba8a5bd
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 249 deletions.
278 changes: 42 additions & 236 deletions meta-facebook/meta-elbert/recipes-elbert/lm_sensors/files/elbert.conf
Original file line number Diff line number Diff line change
Expand Up @@ -71,35 +71,50 @@ chip "ucd90160-i2c-3-4e"
ignore in16
ignore temp1

chip "isl68137-i2c-3-60"
# Voltage loop 1 is not used.
chip "raa228228-i2c-3-60"
label in1 "TH4 VRD1 Vin"
label in2 "TH4 VRD1 Vloop0"
label in2 "TH4 VRD1 Vmon"
label in3 "TH4 VRD1 Vout Loop0"
label temp1 "TH4 VRD1 Temp1"
label temp2 "TH4 VRD1 Temp2"
label temp3 "TH4 VRD1 Temp3"
label temp4 "TH4 VRD1 Temp4"
label power1 "TH4 VRD1 Pin"
label power2 "TH4 VRD1 Ploop0"
label power3 "TH4 VRD1 Pout Loop0"
label curr1 "TH4 VRD1 Iin"
label curr2 "TH4 VRD1 Iloop0"
ignore in3
ignore power3
ignore curr3

chip "isl68137-i2c-3-62"
label curr3 "TH4 VRD1 Iout Loop0"
ignore in4
ignore power2
ignore power4
ignore curr2
ignore curr4

chip "isl68226-i2c-3-62"
label in1 "TH4 VRD2 Vin"
label in2 "TH4 VRD2 Vloop0"
label in3 "TH4 VRD2 Vloop1"
label in2 "TH4 VRD2 Vmon"
label in3 "TH4 VRD2 Vout Loop0"
label in4 "TH4 VRD2 Vout Loop1"
label in5 "TH4 VRD2 Vout Loop2"
label temp1 "TH4 VRD2 Temp1"
label temp2 "TH4 VRD2 Temp2"
label temp3 "TH4 VRD2 Temp3"
label temp4 "TH4 VRD2 Temp4"
label power1 "TH4 VRD2 Pin"
label power2 "TH4 VRD2 Ploop0"
label power3 "TH4 VRD2 Ploop1"
label curr1 "TH4 VRD2 Iin"
label curr2 "TH4 VRD2 Iloop0"
label curr3 "TH4 VRD2 Iloop1"
label temp5 "TH4 VRD2 Temp5"
label temp6 "TH4 VRD2 Temp6"
label temp7 "TH4 VRD2 Temp7"
label power1 "TH4 VRD2 Pin Loop0"
label power2 "TH4 VRD2 Pin Loop1"
label power3 "TH4 VRD2 Pin Loop2"
label power4 "TH4 VRD2 Pout Loop0"
label power5 "TH4 VRD2 Pout Loop1"
label power6 "TH4 VRD2 Pout Loop2"
label curr1 "TH4 VRD2 Iin Loop0"
label curr2 "TH4 VRD2 Iin Loop1"
label curr3 "TH4 VRD2 Iin Loop2"
label curr4 "TH4 VRD2 Iout Loop0"
label curr5 "TH4 VRD2 Iout Loop1"
label curr6 "TH4 VRD2 Iout Loop2"

chip "max6581-i2c-4-4d"
label temp1 "SMB T"
Expand Down Expand Up @@ -185,226 +200,14 @@ chip "lm73-i2c-15-4a"
label temp1 "SCM BMC Temp"

# PIMs on i2c-2 mux
chip "pmbus-i2c-16-16"
label in2 "PIM2 POS 3V3_U Vout"
label temp1 "PIM2 POS 3V3_U Temp"
label curr2 "PIM2 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-16-18"
label in2 "PIM2 POS 3V3_L Vout"
label temp1 "PIM2 POS 3V3_L Temp"
label curr2 "PIM2 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-16-4a"
label temp1 "PIM2 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-16-4e"
label in1 "PIM2 POS 12V"
label in2 "PIM2 POS 3V3_E"
label in3 "PIM2 POS 1V2_E"
label in4 "PIM2 POS 3V3_U"
label in5 "PIM2 POS 3V3_L"
label in6 "PIM2 POS 3V8_LEDS"
label temp1 "PIM2 DPM Temp"

chip "pmbus-i2c-17-16"
label in2 "PIM3 POS 3V3_U Vout"
label temp1 "PIM3 POS 3V3_U Temp"
label curr2 "PIM3 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-17-18"
label in2 "PIM3 POS 3V3_L Vout"
label temp1 "PIM3 POS 3V3_L Temp"
label curr2 "PIM3 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-17-4a"
label temp1 "PIM3 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-17-4e"
label in1 "PIM3 POS 12V"
label in2 "PIM3 POS 3V3_E"
label in3 "PIM3 POS 1V2_E"
label in4 "PIM3 POS 3V3_U"
label in5 "PIM3 POS 3V3_L"
label in6 "PIM3 POS 3V8_LEDS"
label temp1 "PIM3 DPM Temp"

chip "pmbus-i2c-18-16"
label in2 "PIM4 POS 3V3_U Vout"
label temp1 "PIM4 POS 3V3_U Temp"
label curr2 "PIM4 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-18-18"
label in2 "PIM4 POS 3V3_L Vout"
label temp1 "PIM4 POS 3V3_L Temp"
label curr2 "PIM4 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-18-4a"
label temp1 "PIM4 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-18-4e"
label in1 "PIM4 POS 12V"
label in2 "PIM4 POS 3V3_E"
label in3 "PIM4 POS 1V2_E"
label in4 "PIM4 POS 3V3_U"
label in5 "PIM4 POS 3V3_L"
label in6 "PIM4 POS 3V8_LEDS"
label temp1 "PIM4 DPM Temp"

chip "pmbus-i2c-23-16"
label in2 "PIM5 POS 3V3_U Vout"
label temp1 "PIM5 POS 3V3_U Temp"
label curr2 "PIM5 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-23-18"
label in2 "PIM5 POS 3V3_L Vout"
label temp1 "PIM5 POS 3V3_L Temp"
label curr2 "PIM5 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-23-4a"
label temp1 "PIM5 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-23-4e"
label in1 "PIM5 POS 12V"
label in2 "PIM5 POS 3V3_E"
label in3 "PIM5 POS 1V2_E"
label in4 "PIM5 POS 3V3_U"
label in5 "PIM5 POS 3V3_L"
label in6 "PIM5 POS 3V8_LEDS"
label temp1 "PIM5 DPM Temp"

chip "pmbus-i2c-20-16"
label in2 "PIM6 POS 3V3_U Vout"
label temp1 "PIM6 POS 3V3_U Temp"
label curr2 "PIM6 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-20-18"
label in2 "PIM6 POS 3V3_L Vout"
label temp1 "PIM6 POS 3V3_L Temp"
label curr2 "PIM6 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-20-4a"
label temp1 "PIM6 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-20-4e"
label in1 "PIM6 POS 12V"
label in2 "PIM6 POS 3V3_E"
label in3 "PIM6 POS 1V2_E"
label in4 "PIM6 POS 3V3_U"
label in5 "PIM6 POS 3V3_L"
label in6 "PIM6 POS 3V8_LEDS"
label temp1 "PIM6 DPM Temp"

chip "pmbus-i2c-21-16"
label in2 "PIM7 POS 3V3_U Vout"
label temp1 "PIM7 POS 3V3_U Temp"
label curr2 "PIM7 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-21-18"
label in2 "PIM7 POS 3V3_L Vout"
label temp1 "PIM7 POS 3V3_L Temp"
label curr2 "PIM7 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-21-4a"
label temp1 "PIM7 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-21-4e"
label in1 "PIM7 POS 12V"
label in2 "PIM7 POS 3V3_E"
label in3 "PIM7 POS 1V2_E"
label in4 "PIM7 POS 3V3_U"
label in5 "PIM7 POS 3V3_L"
label in6 "PIM7 POS 3V8_LEDS"
label temp1 "PIM7 DPM Temp"

chip "pmbus-i2c-22-16"
label in2 "PIM8 POS 3V3_U Vout"
label temp1 "PIM8 POS 3V3_U Temp"
label curr2 "PIM8 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-22-18"
label in2 "PIM8 POS 3V3_L Vout"
label temp1 "PIM8 POS 3V3_L Temp"
label curr2 "PIM8 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-22-4a"
label temp1 "PIM8 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-22-4e"
label in1 "PIM8 POS 12V"
label in2 "PIM8 POS 3V3_E"
label in3 "PIM8 POS 1V2_E"
label in4 "PIM8 POS 3V3_U"
label in5 "PIM8 POS 3V3_L"
label in6 "PIM8 POS 3V8_LEDS"
label temp1 "PIM8 DPM Temp"

chip "pmbus-i2c-19-16"
label in2 "PIM9 POS 3V3_U Vout"
label temp1 "PIM9 POS 3V3_U Temp"
label curr2 "PIM9 POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-19-18"
label in2 "PIM9 POS 3V3_L Vout"
label temp1 "PIM9 POS 3V3_L Temp"
label curr2 "PIM9 POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-19-4a"
label temp1 "PIM9 LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-19-4e"
label in1 "PIM9 POS 12V"
label in2 "PIM9 POS 3V3_E"
label in3 "PIM9 POS 1V2_E"
label in4 "PIM9 POS 3V3_U"
label in5 "PIM9 POS 3V3_L"
label in6 "PIM9 POS 3V8_LEDS"
label temp1 "PIM9 DPM Temp"
# Sensors that differ based on PIM type are defined
# in type-specific files

# PSUs on i2c-5 mux
chip "pmbus-i2c-24-58"
label in1 "PSU1 Vin"
label in2 "PSU1 Vout"
label in2 "PSU1 Vout1"
label in3 "PSU1 Vout2"
label fan1 "PSU1 FAN"
label temp1 "PSU1 Temp1"
label temp2 "PSU1 Temp2"
Expand All @@ -418,7 +221,8 @@ chip "pmbus-i2c-24-58"

chip "pmbus-i2c-25-58"
label in1 "PSU2 Vin"
label in2 "PSU2 Vout"
label in2 "PSU2 Vout1"
label in3 "PSU2 Vout2"
label fan1 "PSU2 FAN"
label temp1 "PSU2 Temp1"
label temp2 "PSU2 Temp2"
Expand All @@ -432,7 +236,8 @@ chip "pmbus-i2c-25-58"

chip "pmbus-i2c-26-58"
label in1 "PSU3 Vin"
label in2 "PSU3 Vout"
label in2 "PSU3 Vout1"
label in3 "PSU3 Vout2"
label fan1 "PSU3 FAN"
label temp1 "PSU3 Temp1"
label temp2 "PSU3 Temp2"
Expand All @@ -446,7 +251,8 @@ chip "pmbus-i2c-26-58"

chip "pmbus-i2c-27-58"
label in1 "PSU4 Vin"
label in2 "PSU4 Vout"
label in2 "PSU4 Vout1"
label in3 "PSU4 Vout2"
label fan1 "PSU4 FAN"
label temp1 "PSU4 Temp1"
label temp2 "PSU4 Temp2"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Sensors specific to pim16q
#
chip "pmbus-i2c-{bus}-16"
label in2 "PIM{pim} POS 3V3_U Vout"
label temp1 "PIM{pim} POS 3V3_U Temp"
label curr2 "PIM{pim} POS 3V3_U Curr"
ignore in1
ignore curr1

chip "pmbus-i2c-{bus}-18"
label in2 "PIM{pim} POS 3V3_L Vout"
label temp1 "PIM{pim} POS 3V3_L Temp"
label curr2 "PIM{pim} POS 3V3_L Curr"
ignore in1
ignore curr1

chip "lm73-i2c-{bus}-4a"
label temp1 "PIM{pim} LM73 Temp"
set temp1_max 100

chip "ucd9090-i2c-{bus}-4e"
label in1 "PIM{pim} POS 12V"
label in2 "PIM{pim} POS 3V3_E"
label in3 "PIM{pim} POS 1V2_E"
label in4 "PIM{pim} POS 3V3_U"
label in5 "PIM{pim} POS 3V3_L"
label in6 "PIM{pim} POS 3V8_LEDS"
label temp1 "PIM{pim} DPM Temp"
Loading

0 comments on commit ba8a5bd

Please sign in to comment.