linux-kernel - Re: [PATCH v4 2/6] drm/dsi: Refactor device creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 26 Jan 2016 22:35:02 +0530
From:	Archit Taneja <architt@...eaurora.org>
To:	Thierry Reding <treding@...dia.com>
Cc:	dri-devel@...ts.freedesktop.org, a.hajda@...sung.com,
	jani.nikula@...ux.intel.com, linux-kernel@...r.kernel.org,
	airlied@...ux.ie, daniel@...ll.ch, l.stach@...gutronix.de,
	robh@...nel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v4 2/6] drm/dsi: Refactor device creation



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@...sung.com>
>> Signed-off-by: Archit Taneja <architt@...eaurora.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