platform device as parent of a miscdevice
Hi all, I have a driver that is bound to a platform device. It creates a miscdevice to interact with user space. And it has a "global" structure that stores info which are used throughout the driver. This works ok as far as I can see. The only issue is how to make the "global" struct available to the miscdevice's file operations. Of course, I can make the struct a global variable. Recently, I realized that I could make the platform device the parent of the miscdevice. I came up with something like static int example_open(struct inode *inode, struct file *file) { struct miscdevice *m = file->private_data; struct example_global_info *gbl = dev_get_drvdata(m->parent); ... } static struct file_operations example_fops = { .owner = THIS_MODULE, .open = example_open, ... }; static struct miscdevice example_miscdev = { ... .fops = &example_fops, }; static int example_probe(struct platform_device *pdev) { struct example_global_info *gbl = devm_kzalloc(&pdev->dev, ....); platform_set_drvdata(pdev, gbl); example_miscdev.parent = &pdev->dev; ret = misc_register(&example_miscdev); ... } Yet again, this seems to work ok. I was surprised to see that there's very few (if any) mainline drivers that do something similar. Does this approach make sense? Does it have any implications that I should be aware of (e.g. does the link between pdev and miscdevice cause problems for suspend/resume)? Thanks, Martin
On Tue, Mar 30, 2021 at 06:04:09PM +0200, Martin Kaiser wrote:
Yet again, this seems to work ok. I was surprised to see that there's very few (if any) mainline drivers that do something similar. Does this approach make sense? Does it have any implications that I should be aware of (e.g. does the link between pdev and miscdevice cause problems for suspend/resume)?
Not at all, this is what you should be done, and is what many misc devices do, that's why the parent pointer is there :) What in-kernel misc drivers do not do that that you feel should? thanks, greg k-h
Hi Greg and all, Thus wrote Greg KH (greg@kroah.com):
Not at all, this is what you should be done, and is what many misc devices do, that's why the parent pointer is there :)
thanks for the confirmation.
What in-kernel misc drivers do not do that that you feel should?
My misunderstanding was that quite a few drivers set the parent pointer but hardly anyone uses the parent pointer in the file_operations. It seems that most drivers have their miscdevice as part of their global structure and use container_of in the file_operations, e.g. ipmb_read in drivers/char/ipmi/ipmb_dev_int.c. Best regards, Martin
On Tue, Mar 30, 2021 at 09:27:17PM +0200, Martin Kaiser wrote:
What in-kernel misc drivers do not do that that you feel should?
My misunderstanding was that quite a few drivers set the parent pointer but hardly anyone uses the parent pointer in the file_operations.
What parent pointer in struct file_operations? I don't see one...
It seems that most drivers have their miscdevice as part of their global structure and use container_of in the file_operations, e.g. ipmb_read in drivers/char/ipmi/ipmb_dev_int.c.
That's one way to use it, there are I think at least 3 ways to use it, maybe more... Don't confuse the struct device parent pointer with the private pointer to get back to your local device-specific data. They are different things. thanks, greg k-h
participants (2)
-
Greg KH -
Martin Kaiser