From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: [PATCH v4 3/6] drm/dsi: Try to match non-DT dsi devices Date: Tue, 26 Jan 2016 23:34:17 +0530 Message-ID: <56A7B521.6090200@codeaurora.org> References: <1448884892-7731-1-git-send-email-architt@codeaurora.org> <1449751300-2841-1-git-send-email-architt@codeaurora.org> <1449751300-2841-4-git-send-email-architt@codeaurora.org> <20160121160514.GC647@ulmo.nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20160121160514.GC647@ulmo.nvidia.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, a.hajda@samsung.com, dri-devel@lists.freedesktop.org List-Id: linux-arm-msm@vger.kernel.org CgpPbiAxLzIxLzIwMTYgOTozNSBQTSwgVGhpZXJyeSBSZWRpbmcgd3JvdGU6Cj4gT24gVGh1LCBE ZWMgMTAsIDIwMTUgYXQgMDY6MTE6MzdQTSArMDUzMCwgQXJjaGl0IFRhbmVqYSB3cm90ZToKPj4g QWRkIGEgZGV2aWNlIG5hbWUgZmllbGQgaW4gbWlwaV9kc2lfZGV2aWNlLiBUaGlzIG5hbWUgaXMg ZGlmZmVyZW50IGZyb20KPj4gdGhlIGFjdHVhbCBkZXYgbmFtZSAod2hpY2ggaXMgb2YgdGhlIGZv cm1hdCAiaG9zdG5hbWUucmVnIikuIFdoZW4gdGhlCj4+IGRldmljZSBpcyBjcmVhdGVkIHZpYSBE VCwgdGhpcyBuYW1lIGlzIHNldCB0byB0aGUgbW9kYWxpYXMgc3RyaW5nLgo+Cj4gV2h5PyBXaGF0 J3MgdGhlIHVzZSBvZiBzZXR0aW5nIHRoaXMgdG8gdGhlIG1vZGFsaWFzIHN0cmluZz8KClRoZXJl IGlzIG5vIHVzZSB0byBzZXQgaXQgaW4gdGhlIERUIGNhc2UuIEl0J3MganVzdCBzZXQgZm9yIHRo ZSBzYWtlCm9mIGNvbnNpc3RlbmN5IGJldHdlZW4gdGhlIG5vbi1EVCBhbmQgRFQgZGV2aWNlcy4g Rm9yIG5vdywgZHNpLT5uYW1lCmlzIGp1c3QgdXNlZCBmb3IgZGV2aWNlL2RyaXZlciBtYXRjaGlu ZyBmb3Igbm9uLURUIGRldmljZXMuIFRoZXJlJ3MKbm8gaGFybSBpbiBzZXR0aW5nIGl0IHRvIGEg dmFsaWQgbmFtZSBmb3IgRFQgZGV2aWNlcy4KCj4KPj4gSW4gdGhlIG5vbi1EVCBjYXNlLCB0aGUg ZHJpdmVyIGNyZWF0aW5nIHRoZSBEU0kgZGV2aWNlIHByb3ZpZGVzIHRoZQo+PiBuYW1lIGJ5IHBv cHVsYXRpbmcgYSBmaWxlZCBpbiBtaXBpX2RzaV9kZXZpY2VfaW5mby4KPj4KPj4gTWF0Y2hpbmcg Zm9yIERUIGNhc2Ugd291bGQgYmUgYXMgaXQgd2FzIGJlZm9yZS4gRm9yIHRoZSBub24tRFQgY2Fz ZSwKPj4gd2UgY29tcGFyZSB0aGUgZGV2aWNlIGFuZCBkcml2ZXIgbmFtZXMuIE90aGVyIGJ1c2Vz IChsaWtlIGkyYy9zcGkpCj4KPiAiSTJDIiBhbmQgIlNQSSIsIHBsZWFzZS4KPgo+PiBwZXJmb3Jt IGEgbm9uLURUIG1hdGNoIGJ5IGNvbXBhcmluZyB0aGUgZGV2aWNlIG5hbWUgYW5kIGVudHJpZXMg aW4gdGhlCj4+IGRyaXZlcidzIGlkX3RhYmxlLiBTdWNoIGEgbWVjaGFuaXNtIGlzbid0IHVzZWQg Zm9yIHRoZSBkc2kgYnVzLgo+Cj4gIkRTSSIsIHBsZWFzZS4KPgo+Pgo+PiBSZXZpZXdlZC1ieTog QW5kcnplaiBIYWpkYSA8YS5oYWpkYUBzYW1zdW5nLmNvbT4KPj4gU2lnbmVkLW9mZi1ieTogQXJj aGl0IFRhbmVqYSA8YXJjaGl0dEBjb2RlYXVyb3JhLm9yZz4KPj4gLS0tCj4+ICAgZHJpdmVycy9n cHUvZHJtL2RybV9taXBpX2RzaS5jIHwgMjUgKysrKysrKysrKysrKysrKysrKysrKysrLQo+PiAg IGluY2x1ZGUvZHJtL2RybV9taXBpX2RzaS5oICAgICB8ICA2ICsrKysrKwo+PiAgIDIgZmlsZXMg Y2hhbmdlZCwgMzAgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+Pgo+PiBkaWZmIC0tZ2l0 IGEvZHJpdmVycy9ncHUvZHJtL2RybV9taXBpX2RzaS5jIGIvZHJpdmVycy9ncHUvZHJtL2RybV9t aXBpX2RzaS5jCj4+IGluZGV4IDk0MzQ1ODUuLjVhNDY4MDIgMTAwNjQ0Cj4+IC0tLSBhL2RyaXZl cnMvZ3B1L2RybS9kcm1fbWlwaV9kc2kuYwo+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX21p cGlfZHNpLmMKPj4gQEAgLTQ1LDkgKzQ1LDI2IEBACj4+ICAgICogc3Vic2V0IG9mIHRoZSBNSVBJ IERDUyBjb21tYW5kIHNldC4KPj4gICAgKi8KPj4KPj4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgZGV2 aWNlX3R5cGUgbWlwaV9kc2lfZGV2aWNlX3R5cGU7Cj4+ICsKPj4gICBzdGF0aWMgaW50IG1pcGlf ZHNpX2RldmljZV9tYXRjaChzdHJ1Y3QgZGV2aWNlICpkZXYsIHN0cnVjdCBkZXZpY2VfZHJpdmVy ICpkcnYpCj4+ICAgewo+PiAtCXJldHVybiBvZl9kcml2ZXJfbWF0Y2hfZGV2aWNlKGRldiwgZHJ2 KTsKPj4gKwlzdHJ1Y3QgbWlwaV9kc2lfZGV2aWNlICpkc2k7Cj4+ICsKPj4gKwlpZiAoZGV2LT50 eXBlID09ICZtaXBpX2RzaV9kZXZpY2VfdHlwZSkKPj4gKwkJZHNpID0gdG9fbWlwaV9kc2lfZGV2 aWNlKGRldik7Cj4+ICsJZWxzZQo+PiArCQlyZXR1cm4gMDsKPgo+IEkgdGhpbmsgdGhpcyBjaGVj ayBpcyByZWR1bmRhbnQuIEknbSBub3QgYXdhcmUgb2YgYW55IGNhc2Ugd2hlcmUgdGhlIGJ1cwo+ IC0+bWF0Y2goKSBjYWxsYmFjayBpcyBjYWxsZWQgb24gYSBkZXZpY2UgdGhhdCBpc24ndCBvbiBz YWlkIGJ1cy4KCllvdSdyZSByaWdodC4gSSdsbCBkcm9wIHRoaXMuCgo+Cj4+ICsJLyogYXR0ZW1w dCBPRiBzdHlsZSBtYXRjaCAqLwo+PiArCWlmIChvZl9kcml2ZXJfbWF0Y2hfZGV2aWNlKGRldiwg ZHJ2KSkKPj4gKwkJcmV0dXJuIDE7Cj4+ICsKPj4gKwkvKiBjb21wYXJlIGRzaSBkZXZpY2UgYW5k IGRyaXZlciBuYW1lcyAqLwo+Cj4gIkRTSSIsIHBsZWFzZS4KPgo+PiArCWlmICghc3RyY21wKGRz aS0+bmFtZSwgZHJ2LT5uYW1lKSkKPj4gKwkJcmV0dXJuIDE7Cj4+ICsKPj4gKwlyZXR1cm4gMDsK Pj4gICB9Cj4+Cj4+ICAgc3RhdGljIGNvbnN0IHN0cnVjdCBkZXZfcG1fb3BzIG1pcGlfZHNpX2Rl dmljZV9wbV9vcHMgPSB7Cj4+IEBAIC0xMjUsNiArMTQyLDcgQEAgc3RydWN0IG1pcGlfZHNpX2Rl dmljZSAqbWlwaV9kc2lfZGV2aWNlX25ldyhzdHJ1Y3QgbWlwaV9kc2lfaG9zdCAqaG9zdCwKPj4g ICAJZHNpLT5kZXYudHlwZSA9ICZtaXBpX2RzaV9kZXZpY2VfdHlwZTsKPj4gICAJZHNpLT5kZXYu b2Zfbm9kZSA9IGluZm8tPm5vZGU7Cj4+ICAgCWRzaS0+Y2hhbm5lbCA9IGluZm8tPnJlZzsKPj4g KwlzdHJsY3B5KGRzaS0+bmFtZSwgaW5mby0+dHlwZSwgc2l6ZW9mKGRzaS0+bmFtZSkpOwo+Cj4g RG9uJ3QgeW91IG5lZWQgdG8gY2hlY2sgaW5mby0+dHlwZSAhPSBOVUxMIGJlZm9yZSBkb2luZyB0 aGlzPwoKSXQncyBub3QgbmVlZGVkIHdpdGggdGhlIHdheSBzdHJ1Y3QgbWlwaV9kc2lfZGV2aWNl X2luZm8gaXMgY3VycmVudGx5CmRlZmluZWQuCgo+Cj4+Cj4+ICAgCWRldl9zZXRfbmFtZSgmZHNp LT5kZXYsICIlcy4lZCIsIGRldl9uYW1lKGhvc3QtPmRldiksIGluZm8tPnJlZyk7Cj4+Cj4+IEBA IC0xNDgsNiArMTY2LDExIEBAIG9mX21pcGlfZHNpX2RldmljZV9hZGQoc3RydWN0IG1pcGlfZHNp X2hvc3QgKmhvc3QsIHN0cnVjdCBkZXZpY2Vfbm9kZSAqbm9kZSkKPj4gICAJaW50IHJldDsKPj4g ICAJdTMyIHJlZzsKPj4KPj4gKwlpZiAob2ZfbW9kYWxpYXNfbm9kZShub2RlLCBpbmZvLnR5cGUs IHNpemVvZihpbmZvLnR5cGUpKSA8IDApIHsKPj4gKwkJZGV2X2VycihkZXYsICJtb2RhbGlhcyBm YWlsdXJlIG9uICVzXG4iLCBub2RlLT5mdWxsX25hbWUpOwo+PiArCQlyZXR1cm4gRVJSX1BUUigt RUlOVkFMKTsKPj4gKwl9Cj4+ICsKPj4gICAJcmV0ID0gb2ZfcHJvcGVydHlfcmVhZF91MzIobm9k ZSwgInJlZyIsICZyZWcpOwo+PiAgIAlpZiAocmV0KSB7Cj4+ICAgCQlkZXZfZXJyKGRldiwgImRl dmljZSBub2RlICVzIGhhcyBubyB2YWxpZCByZWcgcHJvcGVydHk6ICVkXG4iLAo+PiBkaWZmIC0t Z2l0IGEvaW5jbHVkZS9kcm0vZHJtX21pcGlfZHNpLmggYi9pbmNsdWRlL2RybS9kcm1fbWlwaV9k c2kuaAo+PiBpbmRleCA5MGY0ZjNjLi5jYjA4NGFmIDEwMDY0NAo+PiAtLS0gYS9pbmNsdWRlL2Ry bS9kcm1fbWlwaV9kc2kuaAo+PiArKysgYi9pbmNsdWRlL2RybS9kcm1fbWlwaV9kc2kuaAo+PiBA QCAtMTM5LDggKzEzOSwxMSBAQCBlbnVtIG1pcGlfZHNpX3BpeGVsX2Zvcm1hdCB7Cj4+ICAgCU1J UElfRFNJX0ZNVF9SR0I1NjUsCj4+ICAgfTsKPj4KPj4gKyNkZWZpbmUgRFNJX0RFVl9OQU1FX1NJ WkUJCTIwCj4+ICsKPj4gICAvKioKPj4gICAgKiBzdHJ1Y3QgbWlwaV9kc2lfZGV2aWNlX2luZm8g LSB0ZW1wbGF0ZSBmb3IgY3JlYXRpbmcgYSBtaXBpX2RzaV9kZXZpY2UKPj4gKyAqIEB0eXBlOiBk c2kgcGVyaXBoZXJhbCBjaGlwIHR5cGUKPj4gICAgKiBAcmVnOiBEU0kgdmlydHVhbCBjaGFubmVs IGFzc2lnbmVkIHRvIHBlcmlwaGVyYWwKPj4gICAgKiBAbm9kZTogcG9pbnRlciB0byBPRiBkZXZp Y2Ugbm9kZQo+PiAgICAqCj4+IEBAIC0xNDgsNiArMTUxLDcgQEAgZW51bSBtaXBpX2RzaV9waXhl bF9mb3JtYXQgewo+PiAgICAqIERTSSBkZXZpY2UKPj4gICAgKi8KPj4gICBzdHJ1Y3QgbWlwaV9k c2lfZGV2aWNlX2luZm8gewo+PiArCWNoYXIgdHlwZVtEU0lfREVWX05BTUVfU0laRV07Cj4KPiBX aHkgbGltaXQgb3Vyc2VsdmVzIHRvIDIwIGNoYXJhY3RlcnM/IEFuZCB3aHkgZXZlbiBzbyBjb21w bGljYXRlZD8gSXNuJ3QKPiB0aGUgdHlwZSBhbHdheXMgc3RhdGljIHdoZW4gc29tZW9uZSBzcGVj aWZpZXMgdGhpcz8gQ291bGRuJ3Qgd2Ugc2ltcGx5Cj4gdXNlIGEgY29uc3QgY2hhciAqbmFtZSBo ZXJlIGluc3RlYWQ/CgpJbiB0aGUgY2FzZSB3aGVyZSB0aGUgZGV2aWNlIGlzIHJlZ2lzdGVyZWQg dmlhIERULCB3ZSB3b3VsZCBuZWVkCnNwYWNlIGFsbG9jYXRlZCBmb3IgJ3R5cGUnIHRvIGNvcHkg dGhlIG1vZGFsaWFzIHN0cmluZyBpbnRvIGl0LgpIYXZpbmcgY29uc3QgY2hhciAqdHlwZSB3b3Vs ZCBtYWtlIGl0IGEgYml0IGNvbXBsaWNhdGVkIGZvciB0aGUKRFQgcGF0aC4KClRoZSBtaXBpX2Rz aV9kZXZpY2VfaW5mbyBzdHJ1Y3Qgd2FzIGJhc2VkIG9uCnRoZSBpMmNfYm9hcmRfaW5mby9zcGlf Ym9hcmRfaW5mbyBzdHJ1Y3RzLCBhbmQgdGhleSBoYXZlCnR5cGUvbW9kYWxpYXMgbWVtYmVycyBk ZWNsYXJlZCBhcyBhcnJheSBvZiBjaGFycy4gSSBraW5kIG9mCmZvbGxvd2VkIHN1aXQgd2l0aG91 dCBwdXR0aW5nIHRvIG11Y2ggdGhvdWdodCBvbiBtZW1iZXIgdHlwZS4KCkFyY2hpdAoKPgo+IFRo aWVycnkKPgoKLS0gClF1YWxjb21tIElubm92YXRpb24gQ2VudGVyLCBJbmMuIGlzIGEgbWVtYmVy IG9mIENvZGUgQXVyb3JhIEZvcnVtLAphIExpbnV4IEZvdW5kYXRpb24gQ29sbGFib3JhdGl2ZSBQ cm9qZWN0Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRy aS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRw Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934118AbcAZSE3 (ORCPT ); Tue, 26 Jan 2016 13:04:29 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:32785 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932586AbcAZSE0 (ORCPT ); Tue, 26 Jan 2016 13:04:26 -0500 Subject: Re: [PATCH v4 3/6] drm/dsi: Try to match non-DT dsi devices To: Thierry Reding References: <1448884892-7731-1-git-send-email-architt@codeaurora.org> <1449751300-2841-1-git-send-email-architt@codeaurora.org> <1449751300-2841-4-git-send-email-architt@codeaurora.org> <20160121160514.GC647@ulmo.nvidia.com> Cc: dri-devel@lists.freedesktop.org, a.hajda@samsung.com, jani.nikula@linux.intel.com, linux-kernel@vger.kernel.org, airlied@linux.ie, daniel@ffwll.ch, l.stach@pengutronix.de, robh@kernel.org, linux-arm-msm@vger.kernel.org From: Archit Taneja Message-ID: <56A7B521.6090200@codeaurora.org> Date: Tue, 26 Jan 2016 23:34:17 +0530 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160121160514.GC647@ulmo.nvidia.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1/21/2016 9:35 PM, Thierry Reding wrote: > On Thu, Dec 10, 2015 at 06:11:37PM +0530, Archit Taneja wrote: >> 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. > > Why? What's the use of setting this to the modalias string? There is no use to set it in the DT case. It's just set for the sake of consistency between the non-DT and DT devices. For now, dsi->name is just used for device/driver matching for non-DT devices. There's no harm in setting it to a valid name for DT devices. > >> 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) > > "I2C" and "SPI", please. > >> 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. > > "DSI", please. > >> >> Reviewed-by: Andrzej Hajda >> Signed-off-by: Archit Taneja >> --- >> drivers/gpu/drm/drm_mipi_dsi.c | 25 ++++++++++++++++++++++++- >> include/drm/drm_mipi_dsi.h | 6 ++++++ >> 2 files changed, 30 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c >> index 9434585..5a46802 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; > > I think this check is redundant. I'm not aware of any case where the bus > ->match() callback is called on a device that isn't on said bus. You're right. I'll drop this. > >> + /* attempt OF style match */ >> + if (of_driver_match_device(dev, drv)) >> + return 1; >> + >> + /* compare dsi device and driver names */ > > "DSI", please. > >> + 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)); > > Don't you need to check info->type != NULL before doing this? It's not needed with the way struct mipi_dsi_device_info is currently defined. > >> >> dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg); >> >> @@ -148,6 +166,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", ®); >> 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]; > > Why limit ourselves to 20 characters? And why even so complicated? Isn't > the type always static when someone specifies this? Couldn't we simply > use a const char *name here instead? In the case where the device is registered via DT, we would need space allocated for 'type' to copy the modalias string into it. Having const char *type would make it a bit complicated for the DT path. The mipi_dsi_device_info struct was based on the i2c_board_info/spi_board_info structs, and they have type/modalias members declared as array of chars. I kind of followed suit without putting to much thought on member type. Archit > > Thierry > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project