Re: [RFC 1/2] drm/dsi: Create dummy DSI devices
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
---
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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
> ---
> 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(>dev, "%s.dummy_dev.%d", dev_name(host->dev),
> + dsi->channel);
> +
> + return device_add(>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(_dsi_bus_type);
> + int ret;
> +
> + ret = bus_register(_dsi_bus_type);
> + if (ret < 0)
> + return ret;
> +
> + return mipi_dsi_driver_register(_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
Re: [RFC 1/2] drm/dsi: Create dummy DSI devices
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 arch...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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 arch...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
