Messages in this thread | | | Subject | Re: [PATCH v4 2/6] drm/dsi: Refactor device creation | From | Archit Taneja <> | Date | Tue, 26 Jan 2016 22:35:02 +0530 |
| |
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@samsung.com> >> Signed-off-by: Archit Taneja <architt@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
| |