* [PATCH] hwmon class driver registration with a device number
@ 2011-10-05 17:13 Himanshu Chauhan
2011-10-05 18:30 ` Guenter Roeck
2011-10-05 19:33 ` Greg KH
0 siblings, 2 replies; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-05 17:13 UTC (permalink / raw)
To: linux-kernel; +Cc: kernelnewbies, Himanshu Chauhan
This patch adds a way to register a hwmon class driver
with a device number rather than the default MK_DEV(0,0).
This would help in creating required "dev" file under
sysfs which in turn can be used in populating /dev tree
during bootup (helpful to driver which want to show up
a char interface along with sysfs).
Signed-off-by: Himanshu Chauhan <hschauhan@nulltrace.org>
---
drivers/hwmon/hwmon.c | 28 ++++++++++++++++++++++------
include/linux/hwmon.h | 1 +
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index a61e781..36961c5 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -31,15 +31,17 @@ static DEFINE_IDR(hwmon_idr);
static DEFINE_SPINLOCK(idr_lock);
/**
- * hwmon_device_register - register w/ hwmon
- * @dev: the device to register
+ * hwmon_device_register_numbered - register hwmon class dev
+ * with specific device number.
+ * @dev: Device to register.
+ * @dev_id: device number to register.
*
- * hwmon_device_unregister() must be called when the device is no
- * longer needed.
+ * hwmon_device_unregister() must be called when the device
+ * is no longer needed.
*
* Returns the pointer to the new device.
*/
-struct device *hwmon_device_register(struct device *dev)
+struct device* hwmon_device_register_numbered(struct device *dev, dev_t dev_id)
{
struct device *hwdev;
int id, err;
@@ -58,7 +60,7 @@ again:
return ERR_PTR(err);
id = id & MAX_ID_MASK;
- hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL,
+ hwdev = device_create(hwmon_class, dev, dev_id, NULL,
HWMON_ID_FORMAT, id);
if (IS_ERR(hwdev)) {
@@ -71,6 +73,20 @@ again:
}
/**
+ * hwmon_device_register - register w/ hwmon
+ * @dev: the device to register
+ *
+ * hwmon_device_unregister() must be called when the device is no
+ * longer needed.
+ *
+ * Returns the pointer to the new device.
+ */
+struct device *hwmon_device_register(struct device *dev)
+{
+ return hwmon_device_register_numbered(dev, MK_DEV(0,0));
+}
+
+/**
* hwmon_device_unregister - removes the previously registered class device
*
* @dev: the class device to destroy
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index 6b6ee70..d816cbe 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -16,6 +16,7 @@
#include <linux/device.h>
+struct device *hwmon_device_register_numbered(struct device *dev, dev_t dev_id);
struct device *hwmon_device_register(struct device *dev);
void hwmon_device_unregister(struct device *dev);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH] hwmon class driver registration with a device number
2011-10-05 17:13 [PATCH] hwmon class driver registration with a device number Himanshu Chauhan
@ 2011-10-05 18:30 ` Guenter Roeck
2011-10-06 4:06 ` Himanshu Chauhan
2011-10-05 19:33 ` Greg KH
1 sibling, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2011-10-05 18:30 UTC (permalink / raw)
To: Himanshu Chauhan
Cc: linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org,
lm-sensors
On Wed, 2011-10-05 at 13:13 -0400, Himanshu Chauhan wrote:
> This patch adds a way to register a hwmon class driver
> with a device number rather than the default MK_DEV(0,0).
> This would help in creating required "dev" file under
> sysfs which in turn can be used in populating /dev tree
> during bootup (helpful to driver which want to show up
> a char interface along with sysfs).
>
> Signed-off-by: Himanshu Chauhan <hschauhan@nulltrace.org>
hwmon patches should be sent to lm-sensors@lm-sensors.org.
I can not comment on the merits of your patch. Unless I am missing
something, which may well be since I only spent a couple of minutes on
it, other device classes don't seem to provide a similar API, so I don't
know if or why it would make sense for hwmon. Maybe a driver which wants
to register a character device interface should do so independently of
hwmon.
On the technical side, EXPORT_SYMBOL is missing.
Guenter
> ---
> drivers/hwmon/hwmon.c | 28 ++++++++++++++++++++++------
> include/linux/hwmon.h | 1 +
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index a61e781..36961c5 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -31,15 +31,17 @@ static DEFINE_IDR(hwmon_idr);
> static DEFINE_SPINLOCK(idr_lock);
>
> /**
> - * hwmon_device_register - register w/ hwmon
> - * @dev: the device to register
> + * hwmon_device_register_numbered - register hwmon class dev
> + * with specific device number.
> + * @dev: Device to register.
> + * @dev_id: device number to register.
> *
> - * hwmon_device_unregister() must be called when the device is no
> - * longer needed.
> + * hwmon_device_unregister() must be called when the device
> + * is no longer needed.
> *
> * Returns the pointer to the new device.
> */
> -struct device *hwmon_device_register(struct device *dev)
> +struct device* hwmon_device_register_numbered(struct device *dev, dev_t dev_id)
> {
> struct device *hwdev;
> int id, err;
> @@ -58,7 +60,7 @@ again:
> return ERR_PTR(err);
>
> id = id & MAX_ID_MASK;
> - hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL,
> + hwdev = device_create(hwmon_class, dev, dev_id, NULL,
> HWMON_ID_FORMAT, id);
>
> if (IS_ERR(hwdev)) {
> @@ -71,6 +73,20 @@ again:
> }
>
> /**
> + * hwmon_device_register - register w/ hwmon
> + * @dev: the device to register
> + *
> + * hwmon_device_unregister() must be called when the device is no
> + * longer needed.
> + *
> + * Returns the pointer to the new device.
> + */
> +struct device *hwmon_device_register(struct device *dev)
> +{
> + return hwmon_device_register_numbered(dev, MK_DEV(0,0));
> +}
> +
> +/**
> * hwmon_device_unregister - removes the previously registered class device
> *
> * @dev: the class device to destroy
> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> index 6b6ee70..d816cbe 100644
> --- a/include/linux/hwmon.h
> +++ b/include/linux/hwmon.h
> @@ -16,6 +16,7 @@
>
> #include <linux/device.h>
>
> +struct device *hwmon_device_register_numbered(struct device *dev, dev_t dev_id);
> struct device *hwmon_device_register(struct device *dev);
>
> void hwmon_device_unregister(struct device *dev);
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] hwmon class driver registration with a device number
2011-10-05 18:30 ` Guenter Roeck
@ 2011-10-06 4:06 ` Himanshu Chauhan
2011-10-06 5:19 ` Guenter Roeck
0 siblings, 1 reply; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-06 4:06 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org,
lm-sensors
Hi,
> I can not comment on the merits of your patch. Unless I am missing
> something, which may well be since I only spent a couple of minutes on
> it, other device classes don't seem to provide a similar API, so I don't
> know if or why it would make sense for hwmon. Maybe a driver which wants
> to register a character device interface should do so independently of
> hwmon.
>
The idea here is to sit in the same class directory as of hwmon. Devices
registered with this interface will have "dev" under, for example,
/sys/class/hwmon/hwmon0/dev. To do the same inside the driver will be
a bit more involved than a call.
In my opinion other classes should also have similar interfaces.
> On the technical side, EXPORT_SYMBOL is missing.
>
I will take care of that.
Regards
Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon class driver registration with a device number
2011-10-06 4:06 ` Himanshu Chauhan
@ 2011-10-06 5:19 ` Guenter Roeck
2011-10-06 7:43 ` [lm-sensors] " Jean Delvare
0 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2011-10-06 5:19 UTC (permalink / raw)
To: Himanshu Chauhan
Cc: linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org,
lm-sensors@lm-sensors.org
On Thu, Oct 06, 2011 at 12:06:59AM -0400, Himanshu Chauhan wrote:
> Hi,
>
> > I can not comment on the merits of your patch. Unless I am missing
> > something, which may well be since I only spent a couple of minutes on
> > it, other device classes don't seem to provide a similar API, so I don't
> > know if or why it would make sense for hwmon. Maybe a driver which wants
> > to register a character device interface should do so independently of
> > hwmon.
> >
>
> The idea here is to sit in the same class directory as of hwmon. Devices
> registered with this interface will have "dev" under, for example,
> /sys/class/hwmon/hwmon0/dev. To do the same inside the driver will be
> a bit more involved than a call.
>
> In my opinion other classes should also have similar interfaces.
>
I think you'll have to spend some more time and effort explaining the "what for".
Apparently no other device class needs this functionality so far, yet you
suggest that such an interface should exist for all device classes.
But you do so without explanation, or in other words without use case.
I for my part have no idea what you would use or need this new interface for,
and if there would be other less intrusive means to accomplish the same goal.
And I would want to see really good reasons to make a change like this.
Specifically looking at the hwmon subsystem, you are expected to use the lm-sensors
library to access all hwmon attributes. So I would expect your explanation to include
exactly what you want to accomplish and why, details why you believe that you can not
use the lm-sensors library, why you believe that the current infrastructure
does not provide the means you need to accomplish your goals, and why you
think that the existing infrastructure can not be modified to let you accomplish
what you want to do without such a - from a conceptual perspective - substantial change.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 5:19 ` Guenter Roeck
@ 2011-10-06 7:43 ` Jean Delvare
2011-10-06 15:12 ` Himanshu Chauhan
2011-10-06 15:15 ` Guenter Roeck
0 siblings, 2 replies; 19+ messages in thread
From: Jean Delvare @ 2011-10-06 7:43 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Himanshu Chauhan, lm-sensors, linux-kernel
I am purposely removing kernelnewbies from the CC list, as it seems
quite irrelevant for this discussion.
On Wed, 5 Oct 2011 22:19:55 -0700, Guenter Roeck wrote:
> On Thu, Oct 06, 2011 at 12:06:59AM -0400, Himanshu Chauhan wrote:
> > Hi,
> >
> > > I can not comment on the merits of your patch. Unless I am missing
> > > something, which may well be since I only spent a couple of minutes on
> > > it, other device classes don't seem to provide a similar API, so I don't
> > > know if or why it would make sense for hwmon. Maybe a driver which wants
> > > to register a character device interface should do so independently of
> > > hwmon.
> > >
> >
> > The idea here is to sit in the same class directory as of hwmon. Devices
> > registered with this interface will have "dev" under, for example,
> > /sys/class/hwmon/hwmon0/dev. To do the same inside the driver will be
> > a bit more involved than a call.
> >
> > In my opinion other classes should also have similar interfaces.
> >
> I think you'll have to spend some more time and effort explaining the "what for".
>
> Apparently no other device class needs this functionality so far, yet you
> suggest that such an interface should exist for all device classes.
Actually a lot of class devices do have a device node:
$ ls -1 /sys/class/*/*/dev | wc -l
252
This includes block, drm, dvb, input, msr, sound and tty class devices,
to name just a few. But this isn't the problem. All these are
documented in Documentation/devices.txt, and have a properly defined
API. There is no such thing for hwmon devices at this point.
> But you do so without explanation, or in other words without use case.
This is indeed the key point. Creating a device node is pointless
without a standard API to make use of it. So Himanshu will have to
document that first.
> I for my part have no idea what you would use or need this new interface for,
> and if there would be other less intrusive means to accomplish the same goal.
> And I would want to see really good reasons to make a change like this.
Another good point.
> Specifically looking at the hwmon subsystem, you are expected to use the lm-sensors
> library to access all hwmon attributes. So I would expect your explanation to include
> exactly what you want to accomplish and why, details why you believe that you can not
> use the lm-sensors library, why you believe that the current infrastructure
> does not provide the means you need to accomplish your goals, and why you
> think that the existing infrastructure can not be modified to let you accomplish
> what you want to do without such a - from a conceptual perspective - substantial change.
I again agree. Which means that Himanshu is still 3 steps away from
getting his patch accepted:
* Explaining why the current sysfs interface is insufficient and can't
be fixed.
* Getting official device node numbers registered for hwmon use.
* Defining an API for these device nodes.
Before then, there's no point in resending this patch, it will be
rejected.
--
Jean Delvare
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 7:43 ` [lm-sensors] " Jean Delvare
@ 2011-10-06 15:12 ` Himanshu Chauhan
2011-10-06 15:46 ` Alan Cox
2011-10-06 15:15 ` Guenter Roeck
1 sibling, 1 reply; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-06 15:12 UTC (permalink / raw)
To: Jean Delvare; +Cc: Guenter Roeck, lm-sensors, linux-kernel
On Thu, Oct 06, 2011 at 09:43:02AM +0200, Jean Delvare wrote:
>
Thanks Jean for summing up the questions :)
> I again agree. Which means that Himanshu is still 3 steps away from
> getting his patch accepted:
> * Explaining why the current sysfs interface is insufficient and can't
> be fixed.
AFAIK, the current sysfs interface is to read standard attributes from
the device. My reasoning is:
1. This will give hwmon devices to have their "dev" in their sysfs folder
as like other classes and in turn be visible in /dev by mdev or udev.
2. Implementing IOCTL if required. I know that kernel hackers believe
they are "backdoors" and "can't be done right" but sometimes there
isn't any get away with them. I don't know when their support will
be completely removed. Rather, I see that instead of deprecation,
new IOCTL vectors like compat_ioctl have been introduced.
> * Getting official device node numbers registered for hwmon use.
Jean, as long as drivers get major device numbers on the fly, I don't
think this is a requirement. Isn't it?
> * Defining an API for these device nodes.
My idea is just this:
1. Get a major number from kernel on the fly.
2. During probing, for each device, call "hwmon_device_register_numbered"
and let mdev create a /dev node for it. I don't say that this interface
be imposed on drivers. If they want they can still call "hwmon_device_register"
if they don't want to implement standard ioctl, read, write calls.
> Before then, there's no point in resending this patch, it will be
> rejected.
>
Thanks for reviewing. These are my thoughts. If you guys think the reasoning
isn't strong enough to get this in mainline, I am happy to accept that :)
At least I know what can get rejected for sure and I am happy to build a
stack of rejected patches for that. :)
-Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 15:12 ` Himanshu Chauhan
@ 2011-10-06 15:46 ` Alan Cox
2011-10-06 16:25 ` Himanshu Chauhan
0 siblings, 1 reply; 19+ messages in thread
From: Alan Cox @ 2011-10-06 15:46 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: Jean Delvare, Guenter Roeck, lm-sensors, linux-kernel
> AFAIK, the current sysfs interface is to read standard attributes from
> the device. My reasoning is:
and any other attributes.
> 1. This will give hwmon devices to have their "dev" in their sysfs folder
> as like other classes and in turn be visible in /dev by mdev or udev.
Well there is in theory no reason a device shouldn't do that, but most
sysfs devices don't need to so making it generic seems odd.
> 2. Implementing IOCTL if required. I know that kernel hackers believe
> they are "backdoors" and "can't be done right" but sometimes there
> isn't any get away with them. I don't know when their support will
> be completely removed. Rather, I see that instead of deprecation,
> new IOCTL vectors like compat_ioctl have been introduced.
Certain things need ioctl, in fact some problems are *very* hard to
solve any other way: transactional updates (changing a set of fields at
once all or none), and query/response data being two.
> 2. During probing, for each device, call "hwmon_device_register_numbered"
> and let mdev create a /dev node for it. I don't say that this interface
> be imposed on drivers. If they want they can still call "hwmon_device_register"
> if they don't want to implement standard ioctl, read, write calls.
I guess I don't see why a device that is more than just a monitoring
interface can't allocate a misc device or similar if it needs one.
For the more complex cases take a look at drivers/staging/iio also.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 15:46 ` Alan Cox
@ 2011-10-06 16:25 ` Himanshu Chauhan
0 siblings, 0 replies; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-06 16:25 UTC (permalink / raw)
To: Alan Cox; +Cc: Jean Delvare, Guenter Roeck, lm-sensors, linux-kernel
On Thu, Oct 06, 2011 at 04:46:00PM +0100, Alan Cox wrote:
>
> I guess I don't see why a device that is more than just a monitoring
> interface can't allocate a misc device or similar if it needs one.
>
Hi Alan,
For a device, for example, /sys/class/hwmon/hwmon0/dev, mdev will create
hwmon0 named device in /dev. User space apps will access the device
with the same name, i.e. /dev/hwmon0. If a driver registers with hwmon
class and with miscellaneous as well, two entries with same name will
be created under /sys; causing conflict. hwmon will create hwmon%d, you
don't have control over that. So the driver has to make sure that it
gives the same device a different name. Why? Doesn't make sense.
If you want to have hwmon device and still want to have char interface,
isn't doing it all at one place more sensible?
May be this interface is not a necessity but it does provides a flexible
way of achieving what I am talking about.
-Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 7:43 ` [lm-sensors] " Jean Delvare
2011-10-06 15:12 ` Himanshu Chauhan
@ 2011-10-06 15:15 ` Guenter Roeck
2011-10-06 16:43 ` Himanshu Chauhan
1 sibling, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2011-10-06 15:15 UTC (permalink / raw)
To: Jean Delvare
Cc: Himanshu Chauhan, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On Thu, 2011-10-06 at 03:43 -0400, Jean Delvare wrote:
> I am purposely removing kernelnewbies from the CC list, as it seems
> quite irrelevant for this discussion.
>
> On Wed, 5 Oct 2011 22:19:55 -0700, Guenter Roeck wrote:
> > On Thu, Oct 06, 2011 at 12:06:59AM -0400, Himanshu Chauhan wrote:
> > > Hi,
> > >
> > > > I can not comment on the merits of your patch. Unless I am missing
> > > > something, which may well be since I only spent a couple of minutes on
> > > > it, other device classes don't seem to provide a similar API, so I don't
> > > > know if or why it would make sense for hwmon. Maybe a driver which wants
> > > > to register a character device interface should do so independently of
> > > > hwmon.
> > > >
> > >
> > > The idea here is to sit in the same class directory as of hwmon. Devices
> > > registered with this interface will have "dev" under, for example,
> > > /sys/class/hwmon/hwmon0/dev. To do the same inside the driver will be
> > > a bit more involved than a call.
> > >
> > > In my opinion other classes should also have similar interfaces.
> > >
> > I think you'll have to spend some more time and effort explaining the "what for".
> >
> > Apparently no other device class needs this functionality so far, yet you
> > suggest that such an interface should exist for all device classes.
>
> Actually a lot of class devices do have a device node:
> $ ls -1 /sys/class/*/*/dev | wc -l
> 252
> This includes block, drm, dvb, input, msr, sound and tty class devices,
> to name just a few. But this isn't the problem. All these are
I meant instances where the major/minor device number is passed to the
class registration function.
Having said that, I realize there are instances where the _minor_ device
number is passed to the class registration function (eg for misc
devices). In that case, though, misc_register() checks if the asked for
minor device already exists, and retains the option to generate a
dynamic minor device. This is different here, where the proposal is to
pass both major and minor device number to the registration function.
Maybe there are instances where both major and minor device number are
passed; as I mentioned before, I did not spend that much time on it. But
you are right - that isn't the point anyway.
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 15:15 ` Guenter Roeck
@ 2011-10-06 16:43 ` Himanshu Chauhan
0 siblings, 0 replies; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-06 16:43 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On Thu, Oct 06, 2011 at 08:15:26AM -0700, Guenter Roeck wrote:
> Having said that, I realize there are instances where the _minor_ device
> number is passed to the class registration function (eg for misc
> devices). In that case, though, misc_register() checks if the asked for
> minor device already exists, and retains the option to generate a
> dynamic minor device. This is different here, where the proposal is to
> pass both major and minor device number to the registration function.
>
> Maybe there are instances where both major and minor device number are
> passed; as I mentioned before, I did not spend that much time on it. But
> you are right - that isn't the point anyway.
>
If misc and other classes can create nodes based on device numbers that
are passed to them, why is it a problem for hwmon? Why does hwmon creates
devices with MKDEV(0,0)? I am still not getting the point.
-Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon class driver registration with a device number
2011-10-05 17:13 [PATCH] hwmon class driver registration with a device number Himanshu Chauhan
2011-10-05 18:30 ` Guenter Roeck
@ 2011-10-05 19:33 ` Greg KH
2011-10-06 4:10 ` Himanshu Chauhan
1 sibling, 1 reply; 19+ messages in thread
From: Greg KH @ 2011-10-05 19:33 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: linux-kernel, kernelnewbies
On Wed, Oct 05, 2011 at 10:43:11PM +0530, Himanshu Chauhan wrote:
> This patch adds a way to register a hwmon class driver
> with a device number rather than the default MK_DEV(0,0).
> This would help in creating required "dev" file under
> sysfs which in turn can be used in populating /dev tree
> during bootup (helpful to driver which want to show up
> a char interface along with sysfs).
How do you later remove a device created with this new interface? As it
is, I think the existing calls will fail, right?
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon class driver registration with a device number
2011-10-05 19:33 ` Greg KH
@ 2011-10-06 4:10 ` Himanshu Chauhan
2011-10-06 18:25 ` Greg KH
0 siblings, 1 reply; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-06 4:10 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, kernelnewbies, lm-sensors
Hi Greg,
>
> How do you later remove a device created with this new interface? As it
> is, I think the existing calls will fail, right?
>
If I have not missed out anything from hwmon_device_unregister(), it shouldn't
fail. Why did you point that out?
-Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon class driver registration with a device number
2011-10-06 4:10 ` Himanshu Chauhan
@ 2011-10-06 18:25 ` Greg KH
2011-10-06 19:07 ` [lm-sensors] " Guenter Roeck
0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2011-10-06 18:25 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: linux-kernel, kernelnewbies, lm-sensors
On Thu, Oct 06, 2011 at 09:40:11AM +0530, Himanshu Chauhan wrote:
> Hi Greg,
>
> >
> > How do you later remove a device created with this new interface? As it
> > is, I think the existing calls will fail, right?
> >
> If I have not missed out anything from hwmon_device_unregister(), it shouldn't
> fail. Why did you point that out?
If you create a device with a call to device_create() with a dev_t set,
it is usually cleaned up with a call to device_destroy(), but you are
right, a simple call to device_unregister() will still work properly.
So nevermind, sorry for the noise.
What you do need to determine is if this is a device node you really
want to be creating in this manner, as it is a new user/kernel API,
right?
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 18:25 ` Greg KH
@ 2011-10-06 19:07 ` Guenter Roeck
2011-10-07 6:42 ` Himanshu Chauhan
0 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2011-10-06 19:07 UTC (permalink / raw)
To: Greg KH
Cc: Himanshu Chauhan, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org
On Thu, Oct 06, 2011 at 02:25:00PM -0400, Greg KH wrote:
> On Thu, Oct 06, 2011 at 09:40:11AM +0530, Himanshu Chauhan wrote:
> > Hi Greg,
> >
> > >
> > > How do you later remove a device created with this new interface? As it
> > > is, I think the existing calls will fail, right?
> > >
> > If I have not missed out anything from hwmon_device_unregister(), it shouldn't
> > fail. Why did you point that out?
>
> If you create a device with a call to device_create() with a dev_t set,
> it is usually cleaned up with a call to device_destroy(), but you are
> right, a simple call to device_unregister() will still work properly.
>
> So nevermind, sorry for the noise.
>
Not entirely noise. Since the new registration call is not exported in the patch,
any code using it won't be compilable as module, which in turn means
that hwmon_device_unregister() was never called.
So while we know that it _should_ work, we also know that it was not tested.
> What you do need to determine is if this is a device node you really
> want to be creating in this manner, as it is a new user/kernel API,
> right?
>
And why, and what for.
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-06 19:07 ` [lm-sensors] " Guenter Roeck
@ 2011-10-07 6:42 ` Himanshu Chauhan
2011-10-07 6:52 ` Greg KH
0 siblings, 1 reply; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-07 6:42 UTC (permalink / raw)
To: Guenter Roeck
Cc: Greg KH, lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org,
kernelnewbies@kernelnewbies.org
On Thu, Oct 06, 2011 at 12:07:52PM -0700, Guenter Roeck wrote:
> And why, and what for.
The initial idea of posting to kernelnewbies was to get a hint on how
the patch would be taken as. I wanted to know if developers will like
the idea behind it or not. I guess, Guenter is not convinced with
any of my reasoning. I am willing to clean it up further only if
I get a positive hint. But it doesn't seem to be going anywhere.
Thanks Guenter, Alan, and Greg for taking your time and reviewing it.
Greg: To answer your last question, if this was taken positively, I
was thinking of having functionality similar to misc device registration.
-Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-07 6:42 ` Himanshu Chauhan
@ 2011-10-07 6:52 ` Greg KH
2011-10-07 9:56 ` Himanshu Chauhan
0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2011-10-07 6:52 UTC (permalink / raw)
To: Himanshu Chauhan
Cc: Guenter Roeck, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org
On Fri, Oct 07, 2011 at 12:12:40PM +0530, Himanshu Chauhan wrote:
> On Thu, Oct 06, 2011 at 12:07:52PM -0700, Guenter Roeck wrote:
> > And why, and what for.
>
> The initial idea of posting to kernelnewbies was to get a hint on how
> the patch would be taken as. I wanted to know if developers will like
> the idea behind it or not. I guess, Guenter is not convinced with
> any of my reasoning. I am willing to clean it up further only if
> I get a positive hint. But it doesn't seem to be going anywhere.
>
> Thanks Guenter, Alan, and Greg for taking your time and reviewing it.
>
> Greg: To answer your last question, if this was taken positively, I
> was thinking of having functionality similar to misc device registration.
But why? What is that device node going to be used for? Who would be
using it in userspace and where would it be tied into in the kernel?
still confused,
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-07 6:52 ` Greg KH
@ 2011-10-07 9:56 ` Himanshu Chauhan
2011-10-07 11:11 ` Jonathan Cameron
2011-10-07 15:46 ` Greg KH
0 siblings, 2 replies; 19+ messages in thread
From: Himanshu Chauhan @ 2011-10-07 9:56 UTC (permalink / raw)
To: Greg KH
Cc: Guenter Roeck, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org
On Thu, Oct 06, 2011 at 11:52:37PM -0700, Greg KH wrote:
> On Fri, Oct 07, 2011 at 12:12:40PM +0530, Himanshu Chauhan wrote:
> > On Thu, Oct 06, 2011 at 12:07:52PM -0700, Guenter Roeck wrote:
> > > And why, and what for.
> >
> > The initial idea of posting to kernelnewbies was to get a hint on how
> > the patch would be taken as. I wanted to know if developers will like
> > the idea behind it or not. I guess, Guenter is not convinced with
> > any of my reasoning. I am willing to clean it up further only if
> > I get a positive hint. But it doesn't seem to be going anywhere.
> >
> > Thanks Guenter, Alan, and Greg for taking your time and reviewing it.
> >
> > Greg: To answer your last question, if this was taken positively, I
> > was thinking of having functionality similar to misc device registration.
>
> But why? What is that device node going to be used for? Who would be
> using it in userspace and where would it be tied into in the kernel?
>
The device node, as I said earlier, can be used for doing IOCTLS. In user space,
applications that manage and monitor system environment will need to use this
interface for querying the sensor's location, for example. In side the kernel,
the driver that is driving the particular hardware sensor can register a char
interface for all this and then register with hwmon with the same major/minor
for usual sysfs export of data.
-Himanshu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-07 9:56 ` Himanshu Chauhan
@ 2011-10-07 11:11 ` Jonathan Cameron
2011-10-07 15:46 ` Greg KH
1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2011-10-07 11:11 UTC (permalink / raw)
To: Himanshu Chauhan
Cc: Greg KH, Guenter Roeck, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org
On 10/07/11 10:56, Himanshu Chauhan wrote:
> On Thu, Oct 06, 2011 at 11:52:37PM -0700, Greg KH wrote:
>> On Fri, Oct 07, 2011 at 12:12:40PM +0530, Himanshu Chauhan wrote:
>>> On Thu, Oct 06, 2011 at 12:07:52PM -0700, Guenter Roeck wrote:
>>>> And why, and what for.
>>>
>>> The initial idea of posting to kernelnewbies was to get a hint on how
>>> the patch would be taken as. I wanted to know if developers will like
>>> the idea behind it or not. I guess, Guenter is not convinced with
>>> any of my reasoning. I am willing to clean it up further only if
>>> I get a positive hint. But it doesn't seem to be going anywhere.
>>>
>>> Thanks Guenter, Alan, and Greg for taking your time and reviewing it.
>>>
>>> Greg: To answer your last question, if this was taken positively, I
>>> was thinking of having functionality similar to misc device registration.
>>
>> But why? What is that device node going to be used for? Who would be
>> using it in userspace and where would it be tied into in the kernel?
>>
> The device node, as I said earlier, can be used for doing IOCTLS. In user space,
> applications that manage and monitor system environment will need to use this
> interface for querying the sensor's location, for example. In side the kernel,
> the driver that is driving the particular hardware sensor can register a char
> interface for all this and then register with hwmon with the same major/minor
> for usual sysfs export of data.
If location is useful info why not propose a sysfs interface for it?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon class driver registration with a device number
2011-10-07 9:56 ` Himanshu Chauhan
2011-10-07 11:11 ` Jonathan Cameron
@ 2011-10-07 15:46 ` Greg KH
1 sibling, 0 replies; 19+ messages in thread
From: Greg KH @ 2011-10-07 15:46 UTC (permalink / raw)
To: Himanshu Chauhan
Cc: Guenter Roeck, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org
On Fri, Oct 07, 2011 at 03:26:21PM +0530, Himanshu Chauhan wrote:
> On Thu, Oct 06, 2011 at 11:52:37PM -0700, Greg KH wrote:
> > On Fri, Oct 07, 2011 at 12:12:40PM +0530, Himanshu Chauhan wrote:
> > > On Thu, Oct 06, 2011 at 12:07:52PM -0700, Guenter Roeck wrote:
> > > > And why, and what for.
> > >
> > > The initial idea of posting to kernelnewbies was to get a hint on how
> > > the patch would be taken as. I wanted to know if developers will like
> > > the idea behind it or not. I guess, Guenter is not convinced with
> > > any of my reasoning. I am willing to clean it up further only if
> > > I get a positive hint. But it doesn't seem to be going anywhere.
> > >
> > > Thanks Guenter, Alan, and Greg for taking your time and reviewing it.
> > >
> > > Greg: To answer your last question, if this was taken positively, I
> > > was thinking of having functionality similar to misc device registration.
> >
> > But why? What is that device node going to be used for? Who would be
> > using it in userspace and where would it be tied into in the kernel?
> >
> The device node, as I said earlier, can be used for doing IOCTLS.
It can? Which ones? New ones you want to propose? If so, why use an
ioctl?
> In user space, applications that manage and monitor system environment
> will need to use this interface for querying the sensor's location,
> for example. In side the kernel, the driver that is driving the
> particular hardware sensor can register a char interface for all this
> and then register with hwmon with the same major/minor for usual sysfs
> export of data.
So you are creating a new user/kernel api here, right? If so, you need
to document it in Documentation/API and we need to be able to approve
that BEFORE we could ever accept the creation of new device nodes like
this, that at the moment, do not do anything.
In other words, you need to prove you need a new ioctl interface before
you can get a patch accepted that implements this.
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-10-07 15:47 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-05 17:13 [PATCH] hwmon class driver registration with a device number Himanshu Chauhan
2011-10-05 18:30 ` Guenter Roeck
2011-10-06 4:06 ` Himanshu Chauhan
2011-10-06 5:19 ` Guenter Roeck
2011-10-06 7:43 ` [lm-sensors] " Jean Delvare
2011-10-06 15:12 ` Himanshu Chauhan
2011-10-06 15:46 ` Alan Cox
2011-10-06 16:25 ` Himanshu Chauhan
2011-10-06 15:15 ` Guenter Roeck
2011-10-06 16:43 ` Himanshu Chauhan
2011-10-05 19:33 ` Greg KH
2011-10-06 4:10 ` Himanshu Chauhan
2011-10-06 18:25 ` Greg KH
2011-10-06 19:07 ` [lm-sensors] " Guenter Roeck
2011-10-07 6:42 ` Himanshu Chauhan
2011-10-07 6:52 ` Greg KH
2011-10-07 9:56 ` Himanshu Chauhan
2011-10-07 11:11 ` Jonathan Cameron
2011-10-07 15:46 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox