[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <56335454.5090109@samsung.com>
Date: Fri, 30 Oct 2015 12:28:20 +0100
From: Andrzej Hajda <a.hajda@...sung.com>
To: Archit Taneja <architt@...eaurora.org>,
dri-devel@...ts.freedesktop.org
Cc: linux-kernel@...r.kernel.org, airlied@...ux.ie, daniel@...ll.ch,
treding@...dia.com, l.stach@...gutronix.de, robh@...nel.org,
linux-arm-msm@...r.kernel.org, jani.nikula@...ux.intel.com
Subject: Re: [RFC v2 1/5] drm/dsi: Refactor device creation
On 10/06/2015 11:24 AM, Archit Taneja wrote:
> Simplify the mipi dsi device creation process. device_initialize and
> 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.
>
> Signed-off-by: Archit Taneja <architt@...eaurora.org>
Reviewed-by: Andrzej Hajda <a.hajda@...sung.com>
Regards
Andrzej
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 61 +++++++++++++++++-------------------------
> include/drm/drm_mipi_dsi.h | 15 +++++++++++
> 2 files changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 2d5ca8ee..245ecfe 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -102,9 +102,18 @@ static const struct device_type mipi_dsi_device_type = {
> .release = mipi_dsi_dev_release,
> };
>
> -static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
> +struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
> + struct mipi_dsi_device_info *info)
> {
> struct mipi_dsi_device *dsi;
> + struct device *dev = host->dev;
> + int r;
> +
> + if (info->reg > 3) {
> + dev_err(dev, "device node has invalid reg property: %u\n",
> + info->reg);
> + return ERR_PTR(-EINVAL);
> + }
>
> dsi = kzalloc(sizeof(*dsi), GFP_KERNEL);
> if (!dsi)
> @@ -114,26 +123,27 @@ static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
> dsi->dev.bus = &mipi_dsi_bus_type;
> dsi->dev.parent = host->dev;
> dsi->dev.type = &mipi_dsi_device_type;
> + dsi->dev.of_node = info->node;
> + dsi->channel = info->reg;
>
> - device_initialize(&dsi->dev);
> -
> - return dsi;
> -}
> -
> -static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
> -{
> - struct mipi_dsi_host *host = dsi->host;
> + dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);
>
> - dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), dsi->channel);
> + r = device_register(&dsi->dev);
> + if (r) {
> + dev_err(dev, "failed to register device: %d\n", r);
> + kfree(dsi);
> + return ERR_PTR(r);
> + }
>
> - return device_add(&dsi->dev);
> + return dsi;
> }
> +EXPORT_SYMBOL(mipi_dsi_device_new);
>
> static struct mipi_dsi_device *
> of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
> {
> - struct mipi_dsi_device *dsi;
> struct device *dev = host->dev;
> + struct mipi_dsi_device_info info = { };
> int ret;
> u32 reg;
>
> @@ -144,31 +154,10 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
> return ERR_PTR(-EINVAL);
> }
>
> - if (reg > 3) {
> - dev_err(dev, "device node %s has invalid reg property: %u\n",
> - node->full_name, reg);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - dsi = mipi_dsi_device_alloc(host);
> - if (IS_ERR(dsi)) {
> - dev_err(dev, "failed to allocate DSI device %s: %ld\n",
> - node->full_name, PTR_ERR(dsi));
> - return dsi;
> - }
> -
> - dsi->dev.of_node = of_node_get(node);
> - dsi->channel = reg;
> + info.reg = reg;
> + info.node = of_node_get(node);
>
> - ret = mipi_dsi_device_add(dsi);
> - if (ret) {
> - dev_err(dev, "failed to add DSI device %s: %d\n",
> - node->full_name, ret);
> - kfree(dsi);
> - return ERR_PTR(ret);
> - }
> -
> - return dsi;
> + return mipi_dsi_device_new(host, &info);
> }
>
> int mipi_dsi_host_register(struct mipi_dsi_host *host)
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index f1d8d0d..90f4f3c 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -140,6 +140,19 @@ enum mipi_dsi_pixel_format {
> };
>
> /**
> + * struct mipi_dsi_device_info - template for creating a mipi_dsi_device
> + * @reg: DSI virtual channel assigned to peripheral
> + * @node: pointer to OF device node
> + *
> + * This is populated and passed to mipi_dsi_device_new to create a new
> + * DSI device
> + */
> +struct mipi_dsi_device_info {
> + u32 reg;
> + struct device_node *node;
> +};
> +
> +/**
> * struct mipi_dsi_device - DSI peripheral device
> * @host: DSI host for this peripheral
> * @dev: driver model device node for this peripheral
> @@ -174,6 +187,8 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload,
> ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,
> size_t num_params, void *data, size_t size);
>
> +struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
> + struct mipi_dsi_device_info *info);
> /**
> * enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
> * @MIPI_DSI_DCS_TEAR_MODE_VBLANK: the TE output line consists of V-Blanking
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists