| Message ID | 1374206237-27233-1-git-send-email-himanshu.madhani@qlogic.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 07/18/2013 08:57 PM, Himanshu Madhani wrote: > From: Himanshu Madhani <himanshu.madhani@qlogic.com> > > We need to provide a mechanism for a user space application to query > adapter temperature. This RFC patch is to request input on which of the > following approach would be appropriate way to export this information. > > Here are the two approaches that I am exploring. Please review and > provide your recommendation. > > 1. Enhance ethtool to add capability for driver to advertise its > board temperature as part of "ethtool -i <ethX>" > > For example, following would be output for an interface which > has board temperature field populated via ethtool > > "#ethtool -i p10p1" > driver: qlcnic > version: 5.2.44 > bus-info: 0000:08:00.0 > supports-statistics: yes > supports-test: yes > supports-eeprom-access: yes > supports-register-dump: yes > supports-priv-flags: no > board-temperature: 34 > > There will be update required for ethtool.c in the repository > git://git.kernel.org/pub/scm/network/ethtool/ethtool.git > maintained by Ben Hutchings. > > if we decide to go with ethtool modification approach, I will send > patch to update ethtool repository. Attached patch at the end of this > email is for driver implementation for this approach. > > 2. Add a sysfs hook "brd_temp" which will be used by the application > to query board temperature specific to the particular interface > > for example, driver with "brd_temp" sysfs hook will show temperature > > "#cat /sys/class/net/p10p1/device/brd_temp" > 34 Is it at all likely that other I/O cards have similar temperature sensors? If so, then perhaps a more generic I/O card location would be better? Looking for "temp" under /sys on my 3.5 system I see: /sys/bus/platform/devices/coretemp.0 /sys/bus/platform/drivers/coretemp /sys/bus/platform/drivers/coretemp/coretemp.0 /sys/devices/pci0000:00/0000:00:03.0/0000:0f:00.0/hwmon/hwmon0/temp1_input /sys/devices/platform/hp-wmi/hddtemp /sys/devices/platform/coretemp.0 /sys/devices/platform/coretemp.0/temp3_input /sys/devices/platform/coretemp.0/temp3_label /sys/devices/platform/coretemp.0/temp2_crit_alarm /sys/devices/platform/coretemp.0/temp2_crit /sys/devices/platform/coretemp.0/temp3_crit /sys/devices/platform/coretemp.0/temp4_crit /sys/devices/platform/coretemp.0/temp5_crit /sys/devices/platform/coretemp.0/temp4_input /sys/devices/platform/coretemp.0/temp4_label /sys/devices/platform/coretemp.0/temp3_crit_alarm /sys/devices/platform/coretemp.0/temp4_crit_alarm /sys/devices/platform/coretemp.0/temp5_input /sys/devices/platform/coretemp.0/temp5_label /sys/devices/platform/coretemp.0/temp5_crit_alarm /sys/devices/platform/coretemp.0/temp2_max /sys/devices/platform/coretemp.0/temp3_max /sys/devices/platform/coretemp.0/temp4_max /sys/devices/platform/coretemp.0/temp5_max /sys/devices/platform/coretemp.0/temp2_input /sys/devices/platform/coretemp.0/temp2_label /sys/module/coretemp /sys/module/coretemp/drivers/platform:coretemp Anyhow, it seems that one of your peers presently puts that information in that which is reported by ethtool -S ... eth_red_drops: 0 be_on_die_temperature: 52 rxq0: rx_bytes: 67532255030 ... rick jones -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> static int > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index 38dbafa..2012015 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -101,6 +101,7 @@ struct ethtool_drvinfo { > __u32 testinfo_len; > __u32 eedump_len; /* Size of data from ETHTOOL_GEEPROM (bytes) */ > __u32 regdump_len; /* Size of data from ETHTOOL_GREGS (bytes) */ > + __u32 board_temp; /* board temperature */ > }; The idea is good but a couple of comments. 1. you can't break userspace ABI for ethtool by adding elements. 2. There already exists a hardware monitoring subsystem in Linux and there are applications that use it (like SNMP. If you want this feature to be more than a developer toy, then it should use the existing subsystem API's. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Rick Jones [mailto:rick.jones2@hp.com] > Sent: Friday, July 19, 2013 8:07 AM > To: Himanshu Madhani > Cc: netdev; David Miller; Dept-NX Linux NIC Driver > Subject: Re: [RFC] qlcnic: Enhance ethtool to display board temperature. > > On 07/18/2013 08:57 PM, Himanshu Madhani wrote: > > From: Himanshu Madhani <himanshu.madhani@qlogic.com> > > > > We need to provide a mechanism for a user space application to query > > adapter temperature. This RFC patch is to request input on which of > > the following approach would be appropriate way to export this > information. > > > > Here are the two approaches that I am exploring. Please review and > > provide your recommendation. > > > > 1. Enhance ethtool to add capability for driver to advertise its > > board temperature as part of "ethtool -i <ethX>" > > > > For example, following would be output for an interface which > > has board temperature field populated via ethtool > > > > "#ethtool -i p10p1" > > driver: qlcnic > > version: 5.2.44 > > bus-info: 0000:08:00.0 > > supports-statistics: yes > > supports-test: yes > > supports-eeprom-access: yes > > supports-register-dump: yes > > supports-priv-flags: no > > board-temperature: 34 > > > > There will be update required for ethtool.c in the repository > > git://git.kernel.org/pub/scm/network/ethtool/ethtool.git > > maintained by Ben Hutchings. > > > > if we decide to go with ethtool modification approach, I will send > > patch to update ethtool repository. Attached patch at the end of this > > email is for driver implementation for this approach. > > > > 2. Add a sysfs hook "brd_temp" which will be used by the application > > to query board temperature specific to the particular interface > > > > for example, driver with "brd_temp" sysfs hook will show > > temperature > > > > "#cat /sys/class/net/p10p1/device/brd_temp" > > 34 > > Is it at all likely that other I/O cards have similar temperature sensors? If so, > then perhaps a more generic I/O card location would be better? Looking for > "temp" under /sys on my 3.5 system I see: > > /sys/bus/platform/devices/coretemp.0 > /sys/bus/platform/drivers/coretemp > /sys/bus/platform/drivers/coretemp/coretemp.0 > /sys/devices/pci0000:00/0000:00:03.0/0000:0f:00.0/hwmon/hwmon0/temp1 > _input > /sys/devices/platform/hp-wmi/hddtemp > /sys/devices/platform/coretemp.0 > /sys/devices/platform/coretemp.0/temp3_input > /sys/devices/platform/coretemp.0/temp3_label > /sys/devices/platform/coretemp.0/temp2_crit_alarm > /sys/devices/platform/coretemp.0/temp2_crit > /sys/devices/platform/coretemp.0/temp3_crit > /sys/devices/platform/coretemp.0/temp4_crit > /sys/devices/platform/coretemp.0/temp5_crit > /sys/devices/platform/coretemp.0/temp4_input > /sys/devices/platform/coretemp.0/temp4_label > /sys/devices/platform/coretemp.0/temp3_crit_alarm > /sys/devices/platform/coretemp.0/temp4_crit_alarm > /sys/devices/platform/coretemp.0/temp5_input > /sys/devices/platform/coretemp.0/temp5_label > /sys/devices/platform/coretemp.0/temp5_crit_alarm > /sys/devices/platform/coretemp.0/temp2_max > /sys/devices/platform/coretemp.0/temp3_max > /sys/devices/platform/coretemp.0/temp4_max > /sys/devices/platform/coretemp.0/temp5_max > /sys/devices/platform/coretemp.0/temp2_input > /sys/devices/platform/coretemp.0/temp2_label > /sys/module/coretemp > /sys/module/coretemp/drivers/platform:coretemp > > > Anyhow, it seems that one of your peers presently puts that information in > that which is reported by ethtool -S > > ... > eth_red_drops: 0 > be_on_die_temperature: 52 > rxq0: rx_bytes: 67532255030 > ... Thanks for the comment. I will use ethtool -S <ethX> option to populate board temperature. > > rick jones > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Friday, July 19, 2013 9:11 AM > To: Himanshu Madhani > Cc: netdev; David Miller; Dept-NX Linux NIC Driver > Subject: Re: [RFC] qlcnic: Enhance ethtool to display board temperature. > > > > static int > > diff --git a/include/uapi/linux/ethtool.h > > b/include/uapi/linux/ethtool.h index 38dbafa..2012015 100644 > > --- a/include/uapi/linux/ethtool.h > > +++ b/include/uapi/linux/ethtool.h > > @@ -101,6 +101,7 @@ struct ethtool_drvinfo { > > __u32 testinfo_len; > > __u32 eedump_len; /* Size of data from ETHTOOL_GEEPROM > (bytes) */ > > __u32 regdump_len; /* Size of data from ETHTOOL_GREGS (bytes) > */ > > + __u32 board_temp; /* board temperature */ > > }; > > The idea is good but a couple of comments. > 1. you can't break userspace ABI for ethtool by adding elements. > > 2. There already exists a hardware monitoring subsystem in > Linux and there are applications that use it (like SNMP. If you want this > feature to > be more than a developer toy, then it should use the existing subsystem > API's. Thanks for the comment. I will use ethtool -S ethX option to display board Temperature. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-07-18 at 23:57 -0400, Himanshu Madhani wrote: > From: Himanshu Madhani <himanshu.madhani@qlogic.com> > > We need to provide a mechanism for a user space application to query > adapter temperature. This RFC patch is to request input on which of the > following approach would be appropriate way to export this information. [...] You should create a separate hwmon device. As examples, this is currently done by igb, ixgbe, sfc or tg3. Ben.
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c index 700a463..5335a71 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c @@ -235,6 +235,7 @@ qlcnic_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *drvinfo) { struct qlcnic_adapter *adapter = netdev_priv(dev); u32 fw_major, fw_minor, fw_build; + u32 temp_val = 0; fw_major = QLC_SHARED_REG_RD32(adapter, QLCNIC_FW_VERSION_MAJOR); fw_minor = QLC_SHARED_REG_RD32(adapter, QLCNIC_FW_VERSION_MINOR); fw_build = QLC_SHARED_REG_RD32(adapter, QLCNIC_FW_VERSION_SUB); @@ -246,6 +247,12 @@ qlcnic_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *drvinfo) strlcpy(drvinfo->driver, qlcnic_driver_name, sizeof(drvinfo->driver)); strlcpy(drvinfo->version, QLCNIC_LINUX_VERSIONID, sizeof(drvinfo->version)); + if (qlcnic_82xx_check(adapter)) + temp_val = QLC_SHARED_REG_RD32(adapter, QLCNIC_ASIC_TEMP); + else if (qlcnic_83xx_check(adapter)) + temp_val = QLCRDX(adapter->ahw, QLC_83XX_ASIC_TEMP); + + drvinfo->board_temp = qlcnic_get_temp_val(temp_val); } static int diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index 38dbafa..2012015 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -101,6 +101,7 @@ struct ethtool_drvinfo { __u32 testinfo_len; __u32 eedump_len; /* Size of data from ETHTOOL_GEEPROM (bytes) */ __u32 regdump_len; /* Size of data from ETHTOOL_GREGS (bytes) */ + __u32 board_temp; /* board temperature */ }; #define SOPASS_MAX 6