Re: [1/2] lis3: Add device owner

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ext Guenter Roeck wrote:
On Fri, Aug 27, 2010 at 02:33:20PM -0000, Yuri Ershov wrote:
Add device owner and change /dev/freefall file operations owner
according to the used driver

Signed-off-by: Yuri Kululin <ext-yuri.kululin@xxxxxxxxx>

---
drivers/hwmon/lis3lv02d.c |    8 ++++++--
 drivers/hwmon/lis3lv02d.h |    2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index e278f0e..0f1bd36 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -591,8 +591,7 @@ static int lis3lv02d_misc_fasync(int fd, struct file *file, int on)
 	return fasync_helper(fd, file, on, &lis3_dev.async_queue);
 }
-static const struct file_operations lis3lv02d_misc_fops = {
-	.owner   = THIS_MODULE,
+static struct file_operations lis3lv02d_misc_fops = {
 	.llseek  = no_llseek,
 	.read    = lis3lv02d_misc_read,
 	.open    = lis3lv02d_misc_open,
@@ -1007,6 +1006,11 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
 		goto out;
 	}
+ if (dev->owner)
+		lis3lv02d_misc_fops.owner = dev->owner;
+	else
+		lis3lv02d_misc_fops.owner = THIS_MODULE;
+
If you retained the static assignment to .owner above, you would not need
the else part here.

Yes, you are right. I can change this.

Also, this depends on patch#2, which actually sets dev->owner.
So the problem isn't really addressed w/o patch #2. Given that,
I am not sure if it makes sense to have two separate patches.

Ok, I can merge my patches.

If both patches were applied as one, you might not need the if ()
above in the first place. And if you do, you might still not have solved
the problem completely since .owner would not be set correctly
if dev->owner is NULL.

I've added "if" here to keep the situation "as is" for all other drivers
(HP driver - hp_accel.c and spi part - lis3lv02d_spi.c. I don't have the HW to test) which are using the lis3 core. If we want to avoid the issues with unloading completely all parts should set the dev->owner. Not a problem to add this to HP driver and lis3lv02d_spi.c.

On a higher level, reassigning the owner like this seems to be quite uncommon,
at least in hwmon. I would like to see an Acked-by from Eric to ensure
that the fix is correct.

I tried to use the standard way of module usage control because all functions provided by lis3lv02d_i2c.c, lis3lv02d_spi.c or hp_accel.c can be called through lis3lv02d_misc_fops file ops or joystick poll device.

I can propose another solution. We can use try_module_get(owner) during device registration (in lis3lv02d_add_fs()), release module by using module_put(owner) after device unregister (in lis3lv02d_remove_fs()) and do not touch lis3lv02d_misc_fops. But anyway dev->owner should be set.

Thanks a lot for reviewing,
Yuri K

Thanks,
Guenter


_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux