Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1209694 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC 1/2] drm/dsi: Create dummy DSI devices Andrzej Hajda <a.hajda@samsung.com> - 2015-08-19 10:20 +0200
    Re: [RFC 1/2] drm/dsi: Create dummy DSI devices Archit Taneja <architt@codeaurora.org> - 2015-08-19 11:40 +0200

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1209741

Hi,

On 08/19/2015 01:40 PM, Andrzej Hajda wrote:
> 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?

I took i2c's dummy dev creation as reference. The i2c_driver struct has
an id_table param, that allows the match function (i2c_device_match) to
not have a special case to check for a dummy device.

We could a 'id_table' entry in mipi_dsi_driver, and a 'name' entry in
mipi_dsi_device. But that would be a bit of an overkill just to support
dummy devices.

I could make the check more thorough by adding a func which does
something similar to 'i2c_verify_client', but I think we would
still need the above string.

I will change "dummy_dev" to ".dummy_dev.". I grepped the kernel for
devices named "dummy_dev", but didn't find anything as such, so it
shouldn't really break anything.

>
>> +
>> +	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.

Right. I will remove these.

Thanks for the review.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web