Commit 723de0f9171e ("staging: most: remove device from interface
structure") moved registration of driver-provided struct device to
the most subsystem, but did not properly update dim2 driver to
work with that change.
After most subsystem passes driver's dev to register_device(), it
becomes refcounted, and can be only deallocated in the release method.
Provide that by:
- not using devres to allocate the device,
- moving shutdown actions from _remove() to the device release method,
- not calling shutdown actions in _probe() after the device becomes
refcounted.
Should this be 3 patches?
But these three items are deeply interconnected, and fix the issue together. Must not manually free
device structure passed to register_device(), thus must not allocate via devres (because otherwise,
devres will free it). Once not using devres for it, must deallocate it somehow else, thus must rework
the release paths.
Perhaps I just shall not go into these details in the commit message.
Also, driver used to register it's dev itself, to provide a custom
attribute. With the modified most subsystem, this causes duplicate
registration of the same device object. Fix that by adding that custom
attribute to the platform device - that is a better location for
a platform-specific attribute anyway.
Nope, it should be 4 patches.
Unlike the above three, this item could be separated.
Will split into two patches now - the first for this (and with fix to the attributes issue noted below)
and the second for proper device releasing.
Also, why have you not cc:ed the original author of the commit you are
"fixing" here? They are the maintainer of this code, right?
I was under impression that "git send-email" does that automatically...
CCing them now.
One note on your change that would keep me from accepting it even if all
of the above was not an issue:
diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c
index c85b2cdcdca3..22836c8ed554 100644
--- a/drivers/staging/most/dim2/sysfs.c
+++ b/drivers/staging/most/dim2/sysfs.c
@@ -39,11 +39,10 @@ static const struct attribute_group *dev_attr_groups[] = {
int dim2_sysfs_probe(struct device *dev)
{
- dev->groups = dev_attr_groups;
- return device_register(dev);
+ return sysfs_create_groups(&dev->kobj, dev_attr_groups);
No driver code should ever be calling a sysfs_* function, which is a
huge hint that this is incorrect.
You also just raced with userspace and lost, please use the default
attributes for the driver or bus for this, but NEVER manually add and
remove sysfs files, that way lies madness and hard to maintain code.
I'm aware of this race, but still creating attributes on device probe is under wide use in the kernel:
nikita@cobook:~/kernel$ grep -r device_create_file drivers | wc -l
448
Still, in case of dim2 driver, moving to driver's dev_groups is trivial. Preparing that patch now.
Nikita
