Re: [RFC 1/2] drm/dsi: Create dummy DSI devices

From: Andrzej Hajda <a.hajda@samsung.com>
To: Archit Taneja <architt@codeaurora.org>, dri-devel@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	treding@nvidia.com
Subject: Re: [RFC 1/2] drm/dsi: Create dummy DSI devices
Date: Wed, 19 Aug 2015 10:10:14 +0200	[thread overview]
Message-ID: <55D439E6.3080604@samsung.com> (raw)
In-Reply-To: <1435641851-27295-2-git-send-email-architt@codeaurora.org>

On 06/30/2015 07:24 AM, Archit Taneja wrote:
> We can have devices where the data bus is MIPI DSI, but the control bus
> is something else (i2c, spi etc). A typical example is i2c controlled
> encoder bridge chips.
>
> Such devices too require passing DSI specific parameters (number of data
> lanes, DSI mode flags, color format etc) to their DSI host. For a device
> that isn't 'mipi_dsi_device', there is no way of passing such parameters.
>
> Provide the option of creating a dummy DSI device. The main purpose of
> this would be to attach to a DSI host by calling mipi_dsi_attach, and
> pass DSI params.
>
> Create mipi_dsi_new_dummy for creating a dummy dsi device. The driver
> calling this needs to be aware of the mipi_dsi_host it wants to attach
> to, and also the DSI virtual channel the DSI device intends to use.
>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 78 ++++++++++++++++++++++++++++++++++++++++--
>  include/drm/drm_mipi_dsi.h     |  2 ++
>  2 files changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 2d5ca8ee..9bfe215 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -47,7 +47,14 @@
>  
>  static int mipi_dsi_device_match(struct device *dev, struct device_driver *drv)
>  {
> -	return of_driver_match_device(dev, drv);
> +	if (of_driver_match_device(dev, drv))
> +		return 1;
> +
> +	if (!strcmp(drv->name, "mipi_dsi_dummy") &&
> +			strstr(dev_name(dev), "dummy_dev"))
> +		return 1;

Is this kind of fuzzy matching used in other dummy devs? It looks little bit
scary. You
can at least replace

strstr(dev_name(dev), "dummy_dev"))

with

strstr(dev_name(dev), ".dummy_dev."))

Anyway, currently it should not break anything, am I right?

> +
> +	return 0;
>  }
>  
>  static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
> @@ -171,6 +178,67 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
>  	return dsi;
>  }
>  
> +static int dummy_probe(struct mipi_dsi_device *dsi)
> +{
> +	return 0;
> +}
> +
> +static int dummy_remove(struct mipi_dsi_device *dsi)
> +{
> +	return 0;
> +}
> +
> +static void dummy_shutdown(struct mipi_dsi_device *dsi)
> +{
> +}

I suppose these callbacks are optional, so you can omit them.

> +
> +static struct mipi_dsi_driver dummy_dsi_driver = {
> +	.probe = dummy_probe,
> +	.remove = dummy_remove,
> +	.shutdown = dummy_shutdown,
> +	.driver.name = "mipi_dsi_dummy",
> +};
> +
> +static int mipi_dsi_device_add_dummy(struct mipi_dsi_device *dsi)
> +{
> +	struct mipi_dsi_host *host = dsi->host;
> +
> +	dev_set_name(&dsi->dev, "%s.dummy_dev.%d", dev_name(host->dev),
> +			dsi->channel);
> +
> +	return device_add(&dsi->dev);
> +}
> +
> +struct mipi_dsi_device *mipi_dsi_new_dummy(struct mipi_dsi_host *host, u32 reg)
> +{
> +	struct mipi_dsi_device *dsi;
> +	struct device *dev = host->dev;
> +	int ret;
> +
> +	if (reg > 3) {
> +		dev_err(dev, "invalid reg property %u\n", reg);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	dsi = mipi_dsi_device_alloc(host);
> +	if (IS_ERR(dsi)) {
> +		dev_err(dev, "failed to allocate dummy DSI device %ld\n",
> +			PTR_ERR(dsi));
> +		return dsi;
> +	}
> +
> +	dsi->channel = reg;
> +
> +	ret = mipi_dsi_device_add_dummy(dsi);
> +	if (ret) {
> +		dev_err(dev, "failed to add dummy DSI device %d\n", ret);
> +		kfree(dsi);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return dsi;
> +}
> +
>  int mipi_dsi_host_register(struct mipi_dsi_host *host)
>  {
>  	struct device_node *node;
> @@ -924,7 +992,13 @@ EXPORT_SYMBOL(mipi_dsi_driver_unregister);
>  
>  static int __init mipi_dsi_bus_init(void)
>  {
> -	return bus_register(&mipi_dsi_bus_type);
> +	int ret;
> +
> +	ret = bus_register(&mipi_dsi_bus_type);
> +	if (ret < 0)
> +		return ret;
> +
> +	return mipi_dsi_driver_register(&dummy_dsi_driver);
>  }
>  postcore_initcall(mipi_dsi_bus_init);
>  
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index f1d8d0d..d06ba99 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -174,6 +174,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_new_dummy(struct mipi_dsi_host *host, u32 reg);
> +
>  /**
>   * 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel