On 2013-03-19 08:45, Archit Taneja wrote:
I was trying to come up with a macro which could add default ops to the omap_dss_driver. It isn't straight forward as I thought, because you need to choose either the default op, or the panel driver's op if it exists. For example, I can't create a macro which adds an op for get_timings() to all panel drivers, the panel drivers which already this op defined will have 2 instances of get_timings() in the omap_dss_driver struct.
Yep, I noticed the same a few days ago.
I have been looking around in the kernel to see how undeclared ops in a struct are generally dealt with. Till now, I've noticed that the code which uses this ops takes the responsibility to check whether they are NULL or not.
The easiest way would be to have these default funcs inlined in a header, and every panel driver's omap_dss_driver struct fills in the default op. This way we can make the driver ops const. Do you have any idea of a macro which could do the same as above, and leads to less addition of code?
Why would they need to be inlined?
Another option would be to create global funcs that are used to call the ops. So instead of:
dssdev->dssdrv->foo(dssdev)
the user would call this function:
int dss_foo(struct omap_dss_device *dssdev) { if (dssdev->dssdrv->foo == NULL) return 0; /* or error, depending on case */ return dssdev->dssdrv->foo(dssdev); }
But that'd require adding a bunch of functions, and changing all the callers.
I think the safest way to fix this for now is to add the checks into omapdrm as you do in your original patch. If we go for some other route, I fear that omapfb/omap_vout could be affected, as it could presume that an op being NULL or non-NULL means something. If we change the ops to be always non-NULL, we should go over the uses of those ops to verify they work correctly.
Tomi