Messages in this thread | | | Subject | Re: [PATCH 2/7] mfd / platform: cros_ec: move lightbar attributes to its own driver. | From | Enric Balletbo i Serra <> | Date | Fri, 23 Nov 2018 12:52:55 +0100 |
| |
Hi Guenter,
On 22/11/18 18:41, Guenter Roeck wrote: > Hi Enric, > > On Thu, Nov 22, 2018 at 12:33:51PM +0100, Enric Balletbo i Serra wrote: >> The entire way how cros sysfs attibutes are created is broken. >> cros_ec_lightbar should be its own driver and its attributes should be >> associated with a lightbar driver not the mfd driver. In order to retain >> the path, the lightbar attributes are attached to the cros_class. >> >> The patch also adds the sysfs documentation. >> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> >> --- >> > ... >> >> +int cros_ec_attach_attribute_group(struct cros_ec_dev *ec, >> + struct attribute_group *attrs) >> +{ >> + return sysfs_create_group(&ec->class_dev.kobj, attrs); >> +} >> +EXPORT_SYMBOL(cros_ec_attach_attribute_group); >> + >> +void cros_ec_detach_attribute_group(struct cros_ec_dev *ec, >> + struct attribute_group *attrs) >> +{ >> + sysfs_remove_group(&ec->class_dev.kobj, attrs); >> +} >> +EXPORT_SYMBOL(cros_ec_detach_attribute_group); >> + > > Are those two functions necessary ? Why not just call sysfs_create_group > and sysfs_remove_group directly from the calling code ? >
Actually we have cros_ec_dev which registers the cros_ec class, and sysfs/vbc and lightbar using this cros_ec class. I had problems unloading the different modules. For example, when I removed cros_ec_dev modules before cros_ec_sysfs/cros_ec_vbc/cros_ec_lightbar I got a hang.
To solve the hang I did the easy solution that is make these drivers depend on cros_ec_dev so you're not able to unload cros_ec_dev if first you don't unload the sysfs/vbc/lightbar.
Thinking again about it, I don't really understand now why failed in the first place, cros_ec_dev is the parent, so, on remove should call mfd_remove_devices for the subdevices.
So, let me check again this and I'll back to you.
Thanks, Enric
> Thanks, > Guenter >
| |