11273 Want Intel PCH temperature sensor

Review Request #2018 — Created June 22, 2019 and submitted

rm
illumos-gate
master
11273
bb0b690...
general

11273 Want Intel PCH temperature sensor
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Mike Zeller <mike.zeller@joyent.com>



  • 0
  • 0
  • 0
  • 2
  • 2
Description From Last Updated
tsoome
  1. Ship It!
  2. 
      
seeemef@mac.com
  1. LGTM

  2. 
      
domag02
  1. Typo?

  2. usr/src/man/man7d/pchtemp.7d (Diff revision 1)
     
     

    's/the system the ability/the system with the ability/'
    
    1. No, I think this is fine. One could write it with the word 'with', but it's not required.
    2. Ok.
  3. 
      
pwinder
  1. 
      
  2. usr/src/uts/intel/io/pchtemp/pchtemp.c (Diff revision 1)
     
     

    The topo module has this hardcoded path: "/dev/sensors/temperature/pch/ts.0". And the minor node name is based on the instance number. Is it still OK if the minor is not 0?
    Also, if there are multiple sockets, will you get multiple instances. Is that handled ok?

    1. This works fine in the face of multiple sockets because this has nothing to do with the socket (for Intel processors that's the coretemp driver). This in turn is for the chipset, which is independent of the socket. In all of the two socket configurations we tested in (we don't have access to more than 2 socket systems), there's only a single chipset present and it's my rough understanding that's the expectation of most of the system.
      
      Right now, because we only expect a single instance, I believe that hardcoding the instance number to zero should be fine. If it turns out that we encounter platforms with multiple chipsets then we'll have more work to do, but it's hard to know how that's handled until such multiple chipsets exist or how they show up.
    2. Ok - thanks for the explanation

  3. 
      
pwinder
  1. Ship It!
  2. 
      
rm
Review request changed

Status: Closed (submitted)

Loading...