diff mbox

[v3,2/5] drm/dsi: Try to match non-DT dsi devices

Message ID 1448884892-7731-3-git-send-email-architt@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Archit Taneja Nov. 30, 2015, 12:01 p.m. UTC 
Add a device name field in mipi_dsi_device. This name is different from
the actual dev name (which is of the format "hostname.reg"). When the
device is created via DT, this name is set to the modalias string.
In the non-DT case, the driver creating the DSI device provides the
name by populating a filed in mipi_dsi_device_info.

Matching for DT case would be as it was before. For the non-DT case,
we compare the device and driver names. Other buses (like i2c/spi)
perform a non-DT match by comparing the device name and entries in the
driver's id_table. Such a mechanism isn't used for the dsi bus.

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 25 ++++++++++++++++++++++++-
 include/drm/drm_mipi_dsi.h     |  6 ++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

Comments

kernel test robot Nov. 30, 2015, 12:45 p.m. UTC | #1 
Hi Archit,

[auto build test ERROR on: v4.4-rc3]
[also build test ERROR on: next-20151127]

url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
config: x86_64-allyesdebian (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
     if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
         ^
   cc1: some warnings being treated as errors

vim +/of_modalias_node +168 drivers/gpu/drm/drm_mipi_dsi.c

   162	{
   163		struct device *dev = host->dev;
   164		struct mipi_dsi_device_info info = { };
   165		int ret;
   166		u32 reg;
   167	
 > 168		if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
   169			dev_err(dev, "modalias failure on %s\n", node->full_name);
   170			return ERR_PTR(-EINVAL);
   171		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Archit Taneja Dec. 7, 2015, 5:29 a.m. UTC | #2 
Hi,

On 11/30/2015 06:15 PM, kbuild test robot wrote:
> Hi Archit,
>
> [auto build test ERROR on: v4.4-rc3]
> [also build test ERROR on: next-20151127]
>
> url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
> config: x86_64-allyesdebian (attached as .config)
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
>     drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>       if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {

Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
depend on CONFIG_OF? Or is it better to wrap these funcs around
IF_ENABLED(CONFIG_OF)?

Archit

>           ^
>     cc1: some warnings being treated as errors
>
> vim +/of_modalias_node +168 drivers/gpu/drm/drm_mipi_dsi.c
>
>     162	{
>     163		struct device *dev = host->dev;
>     164		struct mipi_dsi_device_info info = { };
>     165		int ret;
>     166		u32 reg;
>     167	
>   > 168		if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>     169			dev_err(dev, "modalias failure on %s\n", node->full_name);
>     170			return ERR_PTR(-EINVAL);
>     171		}
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
Jani Nikula Dec. 7, 2015, 8:45 a.m. UTC | #3 
On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
> Hi,
>
> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>> Hi Archit,
>>
>> [auto build test ERROR on: v4.4-rc3]
>> [also build test ERROR on: next-20151127]
>>
>> url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>> config: x86_64-allyesdebian (attached as .config)
>> reproduce:
>>          # save the attached .config to linux build tree
>>          make ARCH=x86_64
>>
>> All errors (new ones prefixed by >>):
>>
>>     drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>       if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>
> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
> depend on CONFIG_OF?

Please don't.

BR,
Jani.
Archit Taneja Dec. 7, 2015, 8:59 a.m. UTC | #4 
On 12/07/2015 02:15 PM, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>> Hi,
>>
>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>> Hi Archit,
>>>
>>> [auto build test ERROR on: v4.4-rc3]
>>> [also build test ERROR on: next-20151127]
>>>
>>> url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>> config: x86_64-allyesdebian (attached as .config)
>>> reproduce:
>>>           # save the attached .config to linux build tree
>>>           make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>      drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>        if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>
>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>> depend on CONFIG_OF?
>
> Please don't.

Just curious, how did x86 use DSI if the only way to create DSI devices
until now was via DT?

Archit

>
> BR,
> Jani.
>
Jani Nikula Dec. 7, 2015, 9:10 a.m. UTC | #5 
On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>>> Hi,
>>>
>>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>>> Hi Archit,
>>>>
>>>> [auto build test ERROR on: v4.4-rc3]
>>>> [also build test ERROR on: next-20151127]
>>>>
>>>> url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>>> config: x86_64-allyesdebian (attached as .config)
>>>> reproduce:
>>>>           # save the attached .config to linux build tree
>>>>           make ARCH=x86_64
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>      drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>>        if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>>
>>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>>> depend on CONFIG_OF?
>>
>> Please don't.
>
> Just curious, how did x86 use DSI if the only way to create DSI devices
> until now was via DT?

Oh, you want the gory details... we use the DSI code as a library for
abstraction and helpers, without actually creating or registering the
devices.

BR,
Jani.


>
> Archit
>
>>
>> BR,
>> Jani.
>>
Archit Taneja Dec. 7, 2015, 9:18 a.m. UTC | #6 
On 12/07/2015 02:40 PM, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>>> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>>>> Hi,
>>>>
>>>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>>>> Hi Archit,
>>>>>
>>>>> [auto build test ERROR on: v4.4-rc3]
>>>>> [also build test ERROR on: next-20151127]
>>>>>
>>>>> url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>>>> config: x86_64-allyesdebian (attached as .config)
>>>>> reproduce:
>>>>>            # save the attached .config to linux build tree
>>>>>            make ARCH=x86_64
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>>       drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>>>         if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>>>
>>>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>>>> depend on CONFIG_OF?
>>>
>>> Please don't.
>>
>> Just curious, how did x86 use DSI if the only way to create DSI devices
>> until now was via DT?
>
> Oh, you want the gory details... we use the DSI code as a library for
> abstraction and helpers, without actually creating or registering the
> devices.

Okay, got it. I'll go with the IS_ENABLED(CONFIG_OF) approach.

Humble request: Next time if I share something that doesn't make sense, 
please reply with something more than a "Please don't". That just sounds
condescending and doesn't really help me with my cause either.

Thanks,
Archit

>
> BR,
> Jani.
>
>
>>
>> Archit
>>
>>>
>>> BR,
>>> Jani.
>>>
>
Jani Nikula Dec. 7, 2015, 10:07 a.m. UTC | #7 
On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
> On 12/07/2015 02:40 PM, Jani Nikula wrote:
>> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>>> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>>>> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>>>>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>>>>> depend on CONFIG_OF?
>>>>
>>>> Please don't.
>>>
>>> Just curious, how did x86 use DSI if the only way to create DSI devices
>>> until now was via DT?
>>
>> Oh, you want the gory details... we use the DSI code as a library for
>> abstraction and helpers, without actually creating or registering the
>> devices.
>
> Okay, got it. I'll go with the IS_ENABLED(CONFIG_OF) approach.

Thanks, appreciated, so i915 doesn't need to depend on OF.

> Humble request: Next time if I share something that doesn't make sense, 
> please reply with something more than a "Please don't". That just sounds
> condescending and doesn't really help me with my cause either.

That's a fair request, no need to be humble about it. Apologies.

BR,
Jani.
Rob Clark Dec. 7, 2015, 4:55 p.m. UTC | #8 
On Mon, Dec 7, 2015 at 3:45 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 07 Dec 2015, Archit Taneja <architt@codeaurora.org> wrote:
>> Hi,
>>
>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>> Hi Archit,
>>>
>>> [auto build test ERROR on: v4.4-rc3]
>>> [also build test ERROR on: next-20151127]
>>>
>>> url:    https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>> config: x86_64-allyesdebian (attached as .config)
>>> reproduce:
>>>          # save the attached .config to linux build tree
>>>          make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>     drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>       if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>
>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>> depend on CONFIG_OF?
>
> Please don't.

I assume you are not using of_mipi_dsi_device_add()?  We could just
put this one fxn inside #ifdef CONFIG_OF / #endif, I think..

BR,
-R

> BR,
> Jani.
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 82bcdcd..143cce4 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -45,9 +45,26 @@ 
  * subset of the MIPI DCS command set.
  */
 
+static const struct device_type mipi_dsi_device_type;
+
 static int mipi_dsi_device_match(struct device *dev, struct device_driver *drv)
 {
-	return of_driver_match_device(dev, drv);
+	struct mipi_dsi_device *dsi;
+
+	if (dev->type == &mipi_dsi_device_type)
+		dsi = to_mipi_dsi_device(dev);
+	else
+		return 0;
+
+	/* attempt OF style match */
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	/* compare dsi device and driver names */
+	if (!strcmp(dsi->name, drv->name))
+		return 1;
+
+	return 0;
 }
 
 static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
@@ -125,6 +142,7 @@  struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
 	dsi->dev.type = &mipi_dsi_device_type;
 	dsi->dev.of_node = info->node;
 	dsi->channel = info->reg;
+	strlcpy(dsi->name, info->type, sizeof(dsi->name));
 
 	dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);
 
@@ -147,6 +165,11 @@  of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
 	int ret;
 	u32 reg;
 
+	if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
+		dev_err(dev, "modalias failure on %s\n", node->full_name);
+		return ERR_PTR(-EINVAL);
+	}
+
 	ret = of_property_read_u32(node, "reg", &reg);
 	if (ret) {
 		dev_err(dev, "device node %s has no valid reg property: %d\n",
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 90f4f3c..cb084af 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -139,8 +139,11 @@  enum mipi_dsi_pixel_format {
 	MIPI_DSI_FMT_RGB565,
 };
 
+#define DSI_DEV_NAME_SIZE		20
+
 /**
  * struct mipi_dsi_device_info - template for creating a mipi_dsi_device
+ * @type: dsi peripheral chip type
  * @reg: DSI virtual channel assigned to peripheral
  * @node: pointer to OF device node
  *
@@ -148,6 +151,7 @@  enum mipi_dsi_pixel_format {
  * DSI device
  */
 struct mipi_dsi_device_info {
+	char type[DSI_DEV_NAME_SIZE];
 	u32 reg;
 	struct device_node *node;
 };
@@ -156,6 +160,7 @@  struct mipi_dsi_device_info {
  * struct mipi_dsi_device - DSI peripheral device
  * @host: DSI host for this peripheral
  * @dev: driver model device node for this peripheral
+ * @name: dsi peripheral chip type
  * @channel: virtual channel assigned to the peripheral
  * @format: pixel format for video mode
  * @lanes: number of active data lanes
@@ -165,6 +170,7 @@  struct mipi_dsi_device {
 	struct mipi_dsi_host *host;
 	struct device dev;
 
+	char name[DSI_DEV_NAME_SIZE];
 	unsigned int channel;
 	unsigned int lanes;
 	enum mipi_dsi_pixel_format format;