Hi Russell,
Am Mittwoch, den 03.12.2014, 16:30 +0000 schrieb Russell King - ARM Linux:
On Wed, Dec 03, 2014 at 05:20:15PM +0100, Philipp Zabel wrote:
Hi Andy,
It would be better if the bind function would not have to care about platform resources, that should be handled in the probe function. I had a patch to move them:
http://lists.freedesktop.org/archives/dri-devel/2014-May/059630.html
Maybe you could incorporate something like this?
Personally, I hate this idea. Having a two-layered setup means that the when the bind() method is called, the state of struct imx_hdmi is indeterminant.
If it's called immediately from probe, most of the structure will be zeroed, and only those members initialised by the probe function will be set to non-zero values.
However, if the HDMI interface has been previously bound, and is subsequently re-bound, then the structure will most definitely no longer be in a known state on the second bind() call.
This is fragile.
Now, people have tried to tell me that this isn't fragile, but, I now have proof that it is as fragile as I fear. The component helper doesn't yet have that many users, and already we have one user (okay, it's not part of the mainline kernel - it's etnaviv) which contained exactly this kind of bug: it expected its private structures to be zeroed on the bind() call.
So, I /really/ hate this idea. If you really want to do this, then please ensure that the bind() call explicitly zeros the bits of the struct which aren't initialised by the probe() call, so we know that the driver will always start off with a known initial state.
You are right, no I don't want this. When I initially wrote this patch I was under the impression that the memory allocated by devm_kzalloc in bind() wouldn't be freed on unbind(). I remember I stopped pursuing this patch when I got aware of the devres_open/close/remove_group dance in the component framework code, but somehow forgot to drop it altogether locally.
regards Philipp