Re: [PATCH 1/2] Input: touchscreen: ads7846: copy info from pdata to private struct
From: Daniel Mack <zonque@gmail.com>
Date: 2013-05-06 10:34:11
On 06.05.2013 12:25, Mark Brown wrote:
On Sun, May 05, 2013 at 08:24:44PM -0700, Dmitry Torokhov wrote:quoted
On Thu, Apr 25, 2013 at 01:33:52PM +0200, Daniel Mack wrote:quoted
quoted
In preparation for DT bindings, we have to store all runtime information inside struct ads7846. Add more variable to struct ads7846 and refactor some code so the probe-time supplied pdata is not used from any other function than the probe() callback.quoted
I think more common pattern is to allocate platform data structure when parsing device tree, often with devm_kzalloc() so it is cleaned up after driver is unbound.Both are used fairly widely. It's very common to do the separate allocation when converting an existing driver to device tree as the code using the platform data is frequently written with lots of pdata-> in it and may potentially have some different behaviour if there's no platform data at all (though that's a bit questionable) so allocating a new struct is pretty natural and makes for a much less invasive patch. When there's no existing platform data code it's probably more common to embed the structure as this saves an allocation and means that the users can assume that there's a platform data struct there which makes them a little simpler.
The driver as stands right now uses a balanced mix of the two, as some variables are stored in pdata, some are copied over to the private struct. So I had to opt for one of the two approaches, and the one I submitted seemed saner to me, as pdata is eventually only accessed from the probe() function. But I can browse my reflog and switch back to the other approach if that's preferred. The only concern I have is what I already mentioned: the allocation of function pointers which are definitely unused for DT. Thanks, Daniel