On 1/21/2016 9:16 PM, Thierry Reding wrote:
> On Thu, Dec 10, 2015 at 06:11:36PM +0530, Archit Taneja wrote:
>> Simplify the mipi dsi device creation process. device_initialize and
>
> "MIPI" and "DSI", please.
Sure, I'll replace with these and in the other patches.
>
>> device_add don't need to be called separately when creating
>> mipi_dsi_device's. Use device_register instead to simplify things.
>>
>> Create a helper function mipi_dsi_device_new which takes in struct
>> mipi_dsi_device_info and mipi_dsi_host. It clubs the functions
>> mipi_dsi_device_alloc and mipi_dsi_device_add into one.
>>
>> mipi_dsi_device_info acts as a template to populate the dsi device
>> information. This is populated by of_mipi_dsi_device_add and passed to
>> mipi_dsi_device_new.
>>
>> Later on, we'll provide mipi_dsi_device_new as a standalone way to create
>> a dsi device not available via DT.
>>
>> The new device creation process tries to closely follow what's been done
>> in i2c_new_device in i2c-core.
>>
>> Reviewed-by: Andrzej Hajda <a.hajda at samsung.com>
>> Signed-off-by: Archit Taneja <architt at codeaurora.org>
>> ---
>> drivers/gpu/drm/drm_mipi_dsi.c | 61
>> +++++++++++++++++-------------------------
>> include/drm/drm_mipi_dsi.h | 15 +++++++++++
>> 2 files changed, 40 insertions(+), 36 deletions(-)
>
> To be honest, I'm not sure I like this. If you want to have a simpler
> helper, why not implement it using the lower-level helpers. Really the
> only thing you're doing here is add a high-level helper that takes an
> info struct, whereas previously the same would be done by storing the
> info directly in the structure between allocation and addition of the
> device.
>
> Initially the implementation was following that of platform devices, I
> see no reason to deviate from that. What you want here can easily be
I don't see why we need to call device_initialize and device_add
separately for DSI devices. From my (limited) understanding, we should
call these separately if we want to take a reference (using
get_device()), or set up some private data before the bus's
notifier kicks in.
Since the main purpose of the series is not to simplify the device
creation code, I can drop this.
> done by something like:
>
> struct mipi_dsi_device *
> mipi_dsi_device_register_full(struct mipi_dsi_host *host,
> const struct mipi_dsi_device_info *info)
> {
> struct mipi_dsi_device *dsi;
>
> dsi = mipi_dsi_device_alloc(host);
> if (IS_ERR(dsi))
> return dsi;
>
> dsi->dev.of_node = info->node;
> dsi->channel = info->channel;
>
> err = mipi_dsi_device_add(dsi);
> if (err < 0) {
> ...
> }
>
> return dsi;
> }
>
> Thierry
>
This does look less intrusive. I'll consider switching to this.
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
