From: Guenter Roeck <linux@roeck-us.net>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
Jesse Brandeburg <jesse.brandeburg@intel.com>,
Bruce Allan <bruce.w.allan@intel.com>,
Carolyn Wyborny <carolyn.wyborny@intel.com>,
Don Skidmore <donald.c.skidmore@intel.com>,
Greg Rose <gregory.v.rose@intel.com>,
Nithin Nayak Sujir <nsujir@broadcom.com>,
Michael Chan <mchan@broadcom.com>,
e1000-devel@lists.sourceforge.net, lm-sensors@lm-sensors.org
Subject: Re: [lm-sensors] [PATCH 0/5] net: hwmon fixes
Date: Wed, 27 Nov 2013 03:28:19 +0000 [thread overview]
Message-ID: <529566D3.8080801@roeck-us.net> (raw)
In-Reply-To: <1385496510.1586.63.camel@bwh-desktop.uk.level5networks.com>
On 11/26/2013 12:08 PM, Ben Hutchings wrote:
> On Mon, 2013-11-25 at 09:48 -0800, Guenter Roeck wrote:
>> On 11/25/2013 09:15 AM, Ben Hutchings wrote:
>>> On Sat, 2013-11-23 at 09:07 -0800, Guenter Roeck wrote:
>>>> On 11/23/2013 08:48 AM, Ben Hutchings wrote:
>>>>> On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote:
>>>>>> The hwmon subsystem is used by various network drivers to report temperature
>>>>>> sensor and other information. Unfortunately, its use is often not correct.
>>>>>> Typical errors are that the mandatory name sysfs attribute is not created,
>>>>>> that the temperature sensor index starts with 0 instead of 1, and/or that
>>>>>> sysfs attributes are created after the hwmon device was created.
>>>>>
>>>>> As it happens, I was just looking at what we do in sfc
>>>>> (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create
>>>>> the hwmon device before the attributes. I think I avoided the other
>>>>> bugs though.
>>>>>
>>>> Hi Ben,
>>>>
>>>> Yes, I know about that one. It concluded that it would be too invasive
>>>> and risky to try to fix it without access to hardware to test the results.
>>>> That is why I said "fixes _most_ of the problems".
>>>>
>>>> As for why the attributes are created after registration, it was most likely
>>>> because there was no API available to attach the sysfs attributes to
>>>> the hwmon device in a clean way. The new APIs fix that.
>>>
>>> We don't attach them to the hwmon device either, and I would rather not
>>> change that yet because lm-sensors 2 is still widely used.
>>>
>>
>> Hmm .. then there should be no good reason to create the attributes
>> only after hwmon registration.
>>
>> As for lm-sensors 2 ... really ? Seems odd that people would use the
>> latest kernel with 5+ years old versions of applications / libraries.
>> but I guess the world is full of such oddities, so maybe I should not
>> be surprised.
> [...]
>
> As Jean pointed out, the net drivers implementing hwmon aren't supported
> by lm-sensors 2 anyway. So we should go ahead and use the new API in
> sfc. I've opened an internal bug report for this, but it is likely to
> be low priority for the team. But if you write a patch I can test it.
>
You'll get one shortly.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
Jesse Brandeburg <jesse.brandeburg@intel.com>,
Bruce Allan <bruce.w.allan@intel.com>,
Carolyn Wyborny <carolyn.wyborny@intel.com>,
Don Skidmore <donald.c.skidmore@intel.com>,
Greg Rose <gregory.v.rose@intel.com>,
Nithin Nayak Sujir <nsujir@broadcom.com>,
Michael Chan <mchan@broadcom.com>,
e1000-devel@lists.sourceforge.net, lm-sensors@lm-sensors.org
Subject: Re: [PATCH 0/5] net: hwmon fixes
Date: Tue, 26 Nov 2013 19:28:19 -0800 [thread overview]
Message-ID: <529566D3.8080801@roeck-us.net> (raw)
In-Reply-To: <1385496510.1586.63.camel@bwh-desktop.uk.level5networks.com>
On 11/26/2013 12:08 PM, Ben Hutchings wrote:
> On Mon, 2013-11-25 at 09:48 -0800, Guenter Roeck wrote:
>> On 11/25/2013 09:15 AM, Ben Hutchings wrote:
>>> On Sat, 2013-11-23 at 09:07 -0800, Guenter Roeck wrote:
>>>> On 11/23/2013 08:48 AM, Ben Hutchings wrote:
>>>>> On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote:
>>>>>> The hwmon subsystem is used by various network drivers to report temperature
>>>>>> sensor and other information. Unfortunately, its use is often not correct.
>>>>>> Typical errors are that the mandatory name sysfs attribute is not created,
>>>>>> that the temperature sensor index starts with 0 instead of 1, and/or that
>>>>>> sysfs attributes are created after the hwmon device was created.
>>>>>
>>>>> As it happens, I was just looking at what we do in sfc
>>>>> (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create
>>>>> the hwmon device before the attributes. I think I avoided the other
>>>>> bugs though.
>>>>>
>>>> Hi Ben,
>>>>
>>>> Yes, I know about that one. It concluded that it would be too invasive
>>>> and risky to try to fix it without access to hardware to test the results.
>>>> That is why I said "fixes _most_ of the problems".
>>>>
>>>> As for why the attributes are created after registration, it was most likely
>>>> because there was no API available to attach the sysfs attributes to
>>>> the hwmon device in a clean way. The new APIs fix that.
>>>
>>> We don't attach them to the hwmon device either, and I would rather not
>>> change that yet because lm-sensors 2 is still widely used.
>>>
>>
>> Hmm .. then there should be no good reason to create the attributes
>> only after hwmon registration.
>>
>> As for lm-sensors 2 ... really ? Seems odd that people would use the
>> latest kernel with 5+ years old versions of applications / libraries.
>> but I guess the world is full of such oddities, so maybe I should not
>> be surprised.
> [...]
>
> As Jean pointed out, the net drivers implementing hwmon aren't supported
> by lm-sensors 2 anyway. So we should go ahead and use the new API in
> sfc. I've opened an internal bug report for this, but it is likely to
> be low priority for the team. But if you write a patch I can test it.
>
You'll get one shortly.
Thanks,
Guenter
next prev parent reply other threads:[~2013-11-27 3:28 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-23 6:07 [lm-sensors] [PATCH 0/5] net: hwmon fixes Guenter Roeck
2013-11-23 6:07 ` Guenter Roeck
2013-11-23 6:07 ` [lm-sensors] [PATCH 1/5] tg3: Convert to use hwmon_device_register_with_groups Guenter Roeck
2013-11-23 6:07 ` Guenter Roeck
2013-11-23 14:32 ` [lm-sensors] " Jean Delvare
2013-11-23 14:32 ` Jean Delvare
2013-11-26 1:52 ` Nithin Nayak Sujir
2013-11-26 1:52 ` Nithin Nayak Sujir
2013-11-26 2:47 ` [lm-sensors] " Guenter Roeck
2013-11-26 2:47 ` Guenter Roeck
2013-11-26 17:50 ` [lm-sensors] " Nithin Nayak Sujir
2013-11-26 17:50 ` Nithin Nayak Sujir
2013-11-28 23:22 ` [lm-sensors] " David Miller
2013-11-28 23:22 ` David Miller
2013-11-23 6:07 ` [lm-sensors] [PATCH 2/5] igb: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
2013-11-23 6:07 ` Guenter Roeck
2013-11-25 23:16 ` [lm-sensors] " Jeff Kirsher
2013-11-25 23:16 ` Jeff Kirsher
2013-11-23 6:07 ` [lm-sensors] [PATCH 3/5] ixgbe: " Guenter Roeck
2013-11-23 6:07 ` Guenter Roeck
2013-11-25 23:17 ` [lm-sensors] " Jeff Kirsher
2013-11-25 23:17 ` Jeff Kirsher
2013-11-23 6:08 ` [lm-sensors] [PATCH 4/5] igb: Start temperature sensor attribute index with 1 Guenter Roeck
2013-11-23 6:08 ` Guenter Roeck
2013-11-23 12:58 ` [lm-sensors] " Jean Delvare
2013-11-23 12:58 ` Jean Delvare
2013-11-25 23:17 ` Jeff Kirsher
2013-11-25 23:17 ` Jeff Kirsher
2013-11-23 6:08 ` [lm-sensors] [PATCH 5/5] ixgbe: " Guenter Roeck
2013-11-23 6:08 ` Guenter Roeck
2013-11-23 13:01 ` [lm-sensors] " Jean Delvare
2013-11-23 13:01 ` Jean Delvare
2013-11-25 18:08 ` Guenter Roeck
2013-11-25 18:08 ` Guenter Roeck
2013-11-25 23:18 ` Jeff Kirsher
2013-11-25 23:18 ` Jeff Kirsher
2013-11-23 16:48 ` [lm-sensors] [PATCH 0/5] net: hwmon fixes Ben Hutchings
2013-11-23 16:48 ` Ben Hutchings
2013-11-23 17:07 ` [lm-sensors] " Guenter Roeck
2013-11-23 17:07 ` Guenter Roeck
2013-11-25 17:15 ` [lm-sensors] " Ben Hutchings
2013-11-25 17:15 ` Ben Hutchings
2013-11-25 17:48 ` [lm-sensors] " Guenter Roeck
2013-11-25 17:48 ` Guenter Roeck
2013-11-26 20:08 ` [lm-sensors] " Ben Hutchings
2013-11-26 20:08 ` Ben Hutchings
2013-11-27 3:28 ` Guenter Roeck [this message]
2013-11-27 3:28 ` Guenter Roeck
2013-11-25 18:23 ` [lm-sensors] " Jean Delvare
2013-11-25 18:23 ` Jean Delvare
2013-11-25 18:56 ` Ben Hutchings
2013-11-25 18:56 ` Ben Hutchings
2013-11-25 19:34 ` Jean Delvare
2013-11-25 19:34 ` Jean Delvare
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=529566D3.8080801@roeck-us.net \
--to=linux@roeck-us.net \
--cc=bhutchings@solarflare.com \
--cc=bruce.w.allan@intel.com \
--cc=carolyn.wyborny@intel.com \
--cc=davem@davemloft.net \
--cc=donald.c.skidmore@intel.com \
--cc=e1000-devel@lists.sourceforge.net \
--cc=gregory.v.rose@intel.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=lm-sensors@lm-sensors.org \
--cc=mchan@broadcom.com \
--cc=netdev@vger.kernel.org \
--cc=nsujir@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.