11273 Want Intel PCH temperature sensor

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

Information
rm
illumos-gate
master
11273
bb0b690...
Reviewers
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




   

   







  

 



 

  


   

   

    
 
tsoome


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




seeemef@mac.com




   

   







  

 



 

  


   

   

    
 
seeemef@mac.com


    
   


  

  

  



    




  1. 
 
    
  
    
   
    LGTM
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




domag02




   

   









  

 



 

  


   

   

    
 
domag02


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Typo?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/man/man7d/pchtemp.7d
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    

    '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. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Ok.
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




pwinder




   

   







  

 



 

  


   

   

    
 
pwinder


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/intel/io/pchtemp/pchtemp.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    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. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Ok - thanks for the explanation
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




pwinder




   

   







  

 



 

  


   

   

    
 
pwinder


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




rm




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...