From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chanwoo Choi Subject: Re: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL Date: Wed, 28 Feb 2018 07:26:51 +0900 Message-ID: <5A95DB2B.6000003@samsung.com> References: <20180227071134.28063-1-a.hajda@samsung.com> <20180227071134.28063-7-a.hajda@samsung.com> <5A953C21.5020007@samsung.com> <87dd3281-0471-ab2a-90c6-3f2d4bdf4750@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Return-path: In-reply-to: <87dd3281-0471-ab2a-90c6-3f2d4bdf4750@samsung.com> Sender: linux-kernel-owner@vger.kernel.org To: Andrzej Hajda , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" Cc: Maciej Purski , Bartlomiej Zolnierkiewicz , Marek Szyprowski , dri-devel@lists.freedesktop.org, Inki Dae , Rob Herring , Mark Rutland , Krzysztof Kozlowski , Archit Taneja , Laurent Pinchart , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-usb@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org Hi, On 2018년 02월 27일 21:05, Andrzej Hajda wrote: > On 27.02.2018 12:08, Chanwoo Choi wrote: >> Hi, >> >> On 2018년 02월 27일 16:11, Andrzej Hajda wrote: >>> From: Maciej Purski >>> >>> Currently MHL chip must be turned on permanently to detect MHL cable. It >>> duplicates micro-USB controller's (MUIC) functionality and consumes >>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip >>> only if it detects MHL cable. >>> >>> Signed-off-by: Maciej Purski >>> Signed-off-by: Andrzej Hajda >>> --- >>> v5: updated extcon API >>> >>> This is rework of the patch by Maciej with following changes: >>> - use micro-USB port bindings to get extcon, instead of extcon property, >>> - fixed remove sequence, >>> - fixed extcon get state logic. >>> >>> Code finding extcon node is hacky IMO, I guess ultimately it should be done >>> via some framework (maybe even extcon), or at least via helper, I hope it >>> can stay as is until the proper solution will be merged. >>> >>> Signed-off-by: Andrzej Hajda >>> --- >>> drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 94 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c >>> index 9e785b8e0ea2..62b0adabcac2 100644 >>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c >>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c >>> @@ -17,6 +17,7 @@ >>> >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -25,6 +26,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> >>> @@ -81,6 +83,10 @@ struct sii8620 { >>> struct edid *edid; >>> unsigned int gen2_write_burst:1; >>> enum sii8620_mt_state mt_state; >>> + struct extcon_dev *extcon; >>> + struct notifier_block extcon_nb; >>> + struct work_struct extcon_wq; >>> + int cable_state; >>> struct list_head mt_queue; >>> struct { >>> int r_size; >>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx) >>> ctx->rc_dev = rc_dev; >>> } >>> >>> +static void sii8620_cable_out(struct sii8620 *ctx) >>> +{ >>> + disable_irq(to_i2c_client(ctx->dev)->irq); >>> + sii8620_hw_off(ctx); >>> +} >>> + >>> +static void sii8620_extcon_work(struct work_struct *work) >>> +{ >>> + struct sii8620 *ctx = >>> + container_of(work, struct sii8620, extcon_wq); >>> + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL); >>> + >>> + if (state == ctx->cable_state) >>> + return; >>> + >>> + ctx->cable_state = state; >>> + >>> + if (state > 0) >>> + sii8620_cable_in(ctx); >>> + else >>> + sii8620_cable_out(ctx); >>> +} >>> + >>> +static int sii8620_extcon_notifier(struct notifier_block *self, >>> + unsigned long event, void *ptr) >>> +{ >>> + struct sii8620 *ctx = >>> + container_of(self, struct sii8620, extcon_nb); >>> + >>> + schedule_work(&ctx->extcon_wq); >>> + >>> + return NOTIFY_DONE; >>> +} >>> + >>> +static int sii8620_extcon_init(struct sii8620 *ctx) >>> +{ >>> + struct extcon_dev *edev; >>> + struct device_node *musb, *muic; >>> + int ret; >>> + >>> + /* get micro-USB connector node */ >>> + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1); >>> + /* next get micro-USB Interface Controller node */ >>> + muic = of_get_next_parent(musb); >>> + >>> + if (!muic) { >>> + dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n"); >>> + return 0; >>> + } >>> + >>> + edev = extcon_find_edev_by_node(muic); >>> + of_node_put(muic); >>> + if (IS_ERR(edev)) { >>> + if (PTR_ERR(edev) == -EPROBE_DEFER) >>> + return -EPROBE_DEFER; >>> + dev_err(ctx->dev, "Invalid or missing extcon\n"); >>> + return PTR_ERR(edev); >>> + } >>> + >>> + ctx->extcon = edev; >>> + ctx->extcon_nb.notifier_call = sii8620_extcon_notifier; >>> + INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work); >>> + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb); >> You better to use devm_extcon_register_notifier(). > > With devm version I risk that in case of device unbind notification will > be called after .remove callback, it seems to me quite dangerous. Or am > I missing something? If you use the cancel_work_sync() in remove() instead of flush_work(), you can use the 'devm_extcon_*'. > > Regards > Andrzej > >> >>> + if (ret) { >>> + dev_err(ctx->dev, "failed to register notifier for MHL\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge) >>> { >>> return container_of(bridge, struct sii8620, bridge); >>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client, >>> if (ret) >>> return ret; >>> >>> + ret = sii8620_extcon_init(ctx); >>> + if (ret < 0) { >>> + dev_err(ctx->dev, "failed to initialize EXTCON\n"); >>> + return ret; >>> + } >>> + >>> i2c_set_clientdata(client, ctx); >>> >>> ctx->bridge.funcs = &sii8620_bridge_funcs; >>> ctx->bridge.of_node = dev->of_node; >>> drm_bridge_add(&ctx->bridge); >>> >>> - sii8620_cable_in(ctx); >>> + if (!ctx->extcon) >>> + sii8620_cable_in(ctx); >>> >>> return 0; >>> } >>> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client) >>> { >>> struct sii8620 *ctx = i2c_get_clientdata(client); >>> >>> - disable_irq(to_i2c_client(ctx->dev)->irq); >>> - sii8620_hw_off(ctx); >>> + if (ctx->extcon) { >>> + extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL, >>> + &ctx->extcon_nb); >> Don't need to unregister the notifier if using devm_extcon_register_notifier(). >> >>> + flush_work(&ctx->extcon_wq); >>> + if (ctx->cable_state > 0) >>> + sii8620_cable_out(ctx); >>> + } else { >>> + sii8620_cable_out(ctx); >>> + } >>> drm_bridge_remove(&ctx->bridge); >>> >>> return 0; >>> >> If you use the resource managed function (devm_extcon_register_notifier), Looks good to me. >> Reviewed-by: Chanwoo Choi >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- Best Regards, Chanwoo Choi Samsung Electronics From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v5,6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL From: Chanwoo Choi Message-Id: <5A95DB2B.6000003@samsung.com> Date: Wed, 28 Feb 2018 07:26:51 +0900 To: Andrzej Hajda , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" Cc: Maciej Purski , Bartlomiej Zolnierkiewicz , Marek Szyprowski , dri-devel@lists.freedesktop.org, Inki Dae , Rob Herring , Mark Rutland , Krzysztof Kozlowski , Archit Taneja , Laurent Pinchart , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-usb@vger.kernel.org List-ID: SGksCgpPbiAyMDE464WEIDAy7JuUIDI37J28IDIxOjA1LCBBbmRyemVqIEhhamRhIHdyb3RlOgo+ IE9uIDI3LjAyLjIwMTggMTI6MDgsIENoYW53b28gQ2hvaSB3cm90ZToKPj4gSGksCj4+Cj4+IE9u IDIwMTjrhYQgMDLsm5QgMjfsnbwgMTY6MTEsIEFuZHJ6ZWogSGFqZGEgd3JvdGU6Cj4+PiBGcm9t OiBNYWNpZWogUHVyc2tpIDxtLnB1cnNraUBzYW1zdW5nLmNvbT4KPj4+Cj4+PiBDdXJyZW50bHkg TUhMIGNoaXAgbXVzdCBiZSB0dXJuZWQgb24gcGVybWFuZW50bHkgdG8gZGV0ZWN0IE1ITCBjYWJs ZS4gSXQKPj4+IGR1cGxpY2F0ZXMgbWljcm8tVVNCIGNvbnRyb2xsZXIncyAoTVVJQykgZnVuY3Rp b25hbGl0eSBhbmQgY29uc3VtZXMKPj4+IHVubmVjZXNzYXJ5IHBvd2VyLiBMZXRzIHVzZSBleHRj b24gYXR0YWNoZWQgdG8gTVVJQyB0byBlbmFibGUgTUhMIGNoaXAKPj4+IG9ubHkgaWYgaXQgZGV0 ZWN0cyBNSEwgY2FibGUuCj4+Pgo+Pj4gU2lnbmVkLW9mZi1ieTogTWFjaWVqIFB1cnNraSA8bS5w dXJza2lAc2Ftc3VuZy5jb20+Cj4+PiBTaWduZWQtb2ZmLWJ5OiBBbmRyemVqIEhhamRhIDxhLmhh amRhQHNhbXN1bmcuY29tPgo+Pj4gLS0tCj4+PiB2NTogdXBkYXRlZCBleHRjb24gQVBJCj4+Pgo+ Pj4gVGhpcyBpcyByZXdvcmsgb2YgdGhlIHBhdGNoIGJ5IE1hY2llaiB3aXRoIGZvbGxvd2luZyBj aGFuZ2VzOgo+Pj4gLSB1c2UgbWljcm8tVVNCIHBvcnQgYmluZGluZ3MgdG8gZ2V0IGV4dGNvbiwg aW5zdGVhZCBvZiBleHRjb24gcHJvcGVydHksCj4+PiAtIGZpeGVkIHJlbW92ZSBzZXF1ZW5jZSwK Pj4+IC0gZml4ZWQgZXh0Y29uIGdldCBzdGF0ZSBsb2dpYy4KPj4+Cj4+PiBDb2RlIGZpbmRpbmcg ZXh0Y29uIG5vZGUgaXMgaGFja3kgSU1PLCBJIGd1ZXNzIHVsdGltYXRlbHkgaXQgc2hvdWxkIGJl IGRvbmUKPj4+IHZpYSBzb21lIGZyYW1ld29yayAobWF5YmUgZXZlbiBleHRjb24pLCBvciBhdCBs ZWFzdCB2aWEgaGVscGVyLCBJIGhvcGUgaXQKPj4+IGNhbiBzdGF5IGFzIGlzIHVudGlsIHRoZSBw cm9wZXIgc29sdXRpb24gd2lsbCBiZSBtZXJnZWQuCj4+Pgo+Pj4gU2lnbmVkLW9mZi1ieTogQW5k cnplaiBIYWpkYSA8YS5oYWpkYUBzYW1zdW5nLmNvbT4KPj4+IC0tLQo+Pj4gIGRyaXZlcnMvZ3B1 L2RybS9icmlkZ2Uvc2lsLXNpaTg2MjAuYyB8IDk3ICsrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKystLQo+Pj4gIDEgZmlsZSBjaGFuZ2VkLCA5NCBpbnNlcnRpb25zKCspLCAzIGRlbGV0 aW9ucygtKQo+Pj4KPj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3NpbC1z aWk4NjIwLmMgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3NpbC1zaWk4NjIwLmMKPj4+IGluZGV4 IDllNzg1YjhlMGVhMi4uNjJiMGFkYWJjYWMyIDEwMDY0NAo+Pj4gLS0tIGEvZHJpdmVycy9ncHUv ZHJtL2JyaWRnZS9zaWwtc2lpODYyMC5jCj4+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdl L3NpbC1zaWk4NjIwLmMKPj4+IEBAIC0xNyw2ICsxNyw3IEBACj4+PiAgCj4+PiAgI2luY2x1ZGUg PGxpbnV4L2Nsay5oPgo+Pj4gICNpbmNsdWRlIDxsaW51eC9kZWxheS5oPgo+Pj4gKyNpbmNsdWRl IDxsaW51eC9leHRjb24uaD4KPj4+ICAjaW5jbHVkZSA8bGludXgvZ3Bpby9jb25zdW1lci5oPgo+ Pj4gICNpbmNsdWRlIDxsaW51eC9pMmMuaD4KPj4+ICAjaW5jbHVkZSA8bGludXgvaW50ZXJydXB0 Lmg+Cj4+PiBAQCAtMjUsNiArMjYsNyBAQAo+Pj4gICNpbmNsdWRlIDxsaW51eC9saXN0Lmg+Cj4+ PiAgI2luY2x1ZGUgPGxpbnV4L21vZHVsZS5oPgo+Pj4gICNpbmNsdWRlIDxsaW51eC9tdXRleC5o Pgo+Pj4gKyNpbmNsdWRlIDxsaW51eC9vZl9ncmFwaC5oPgo+Pj4gICNpbmNsdWRlIDxsaW51eC9y ZWd1bGF0b3IvY29uc3VtZXIuaD4KPj4+ICAjaW5jbHVkZSA8bGludXgvc2xhYi5oPgo+Pj4gIAo+ Pj4gQEAgLTgxLDYgKzgzLDEwIEBAIHN0cnVjdCBzaWk4NjIwIHsKPj4+ICAJc3RydWN0IGVkaWQg KmVkaWQ7Cj4+PiAgCXVuc2lnbmVkIGludCBnZW4yX3dyaXRlX2J1cnN0OjE7Cj4+PiAgCWVudW0g c2lpODYyMF9tdF9zdGF0ZSBtdF9zdGF0ZTsKPj4+ICsJc3RydWN0IGV4dGNvbl9kZXYgKmV4dGNv bjsKPj4+ICsJc3RydWN0IG5vdGlmaWVyX2Jsb2NrIGV4dGNvbl9uYjsKPj4+ICsJc3RydWN0IHdv cmtfc3RydWN0IGV4dGNvbl93cTsKPj4+ICsJaW50IGNhYmxlX3N0YXRlOwo+Pj4gIAlzdHJ1Y3Qg bGlzdF9oZWFkIG10X3F1ZXVlOwo+Pj4gIAlzdHJ1Y3Qgewo+Pj4gIAkJaW50IHJfc2l6ZTsKPj4+ IEBAIC0yMTc1LDYgKzIxODEsNzcgQEAgc3RhdGljIHZvaWQgc2lpODYyMF9pbml0X3JjcF9pbnB1 dF9kZXYoc3RydWN0IHNpaTg2MjAgKmN0eCkKPj4+ICAJY3R4LT5yY19kZXYgPSByY19kZXY7Cj4+ PiAgfQo+Pj4gIAo+Pj4gK3N0YXRpYyB2b2lkIHNpaTg2MjBfY2FibGVfb3V0KHN0cnVjdCBzaWk4 NjIwICpjdHgpCj4+PiArewo+Pj4gKwlkaXNhYmxlX2lycSh0b19pMmNfY2xpZW50KGN0eC0+ZGV2 KS0+aXJxKTsKPj4+ICsJc2lpODYyMF9od19vZmYoY3R4KTsKPj4+ICt9Cj4+PiArCj4+PiArc3Rh dGljIHZvaWQgc2lpODYyMF9leHRjb25fd29yayhzdHJ1Y3Qgd29ya19zdHJ1Y3QgKndvcmspCj4+ PiArewo+Pj4gKwlzdHJ1Y3Qgc2lpODYyMCAqY3R4ID0KPj4+ICsJCWNvbnRhaW5lcl9vZih3b3Jr LCBzdHJ1Y3Qgc2lpODYyMCwgZXh0Y29uX3dxKTsKPj4+ICsJaW50IHN0YXRlID0gZXh0Y29uX2dl dF9zdGF0ZShjdHgtPmV4dGNvbiwgRVhUQ09OX0RJU1BfTUhMKTsKPj4+ICsKPj4+ICsJaWYgKHN0 YXRlID09IGN0eC0+Y2FibGVfc3RhdGUpCj4+PiArCQlyZXR1cm47Cj4+PiArCj4+PiArCWN0eC0+ Y2FibGVfc3RhdGUgPSBzdGF0ZTsKPj4+ICsKPj4+ICsJaWYgKHN0YXRlID4gMCkKPj4+ICsJCXNp aTg2MjBfY2FibGVfaW4oY3R4KTsKPj4+ICsJZWxzZQo+Pj4gKwkJc2lpODYyMF9jYWJsZV9vdXQo Y3R4KTsKPj4+ICt9Cj4+PiArCj4+PiArc3RhdGljIGludCBzaWk4NjIwX2V4dGNvbl9ub3RpZmll cihzdHJ1Y3Qgbm90aWZpZXJfYmxvY2sgKnNlbGYsCj4+PiArCQkJdW5zaWduZWQgbG9uZyBldmVu dCwgdm9pZCAqcHRyKQo+Pj4gK3sKPj4+ICsJc3RydWN0IHNpaTg2MjAgKmN0eCA9Cj4+PiArCQlj b250YWluZXJfb2Yoc2VsZiwgc3RydWN0IHNpaTg2MjAsIGV4dGNvbl9uYik7Cj4+PiArCj4+PiAr CXNjaGVkdWxlX3dvcmsoJmN0eC0+ZXh0Y29uX3dxKTsKPj4+ICsKPj4+ICsJcmV0dXJuIE5PVElG WV9ET05FOwo+Pj4gK30KPj4+ICsKPj4+ICtzdGF0aWMgaW50IHNpaTg2MjBfZXh0Y29uX2luaXQo c3RydWN0IHNpaTg2MjAgKmN0eCkKPj4+ICt7Cj4+PiArCXN0cnVjdCBleHRjb25fZGV2ICplZGV2 Owo+Pj4gKwlzdHJ1Y3QgZGV2aWNlX25vZGUgKm11c2IsICptdWljOwo+Pj4gKwlpbnQgcmV0Owo+ Pj4gKwo+Pj4gKwkvKiBnZXQgbWljcm8tVVNCIGNvbm5lY3RvciBub2RlICovCj4+PiArCW11c2Ig PSBvZl9ncmFwaF9nZXRfcmVtb3RlX25vZGUoY3R4LT5kZXYtPm9mX25vZGUsIDEsIC0xKTsKPj4+ ICsJLyogbmV4dCBnZXQgbWljcm8tVVNCIEludGVyZmFjZSBDb250cm9sbGVyIG5vZGUgKi8KPj4+ ICsJbXVpYyA9IG9mX2dldF9uZXh0X3BhcmVudChtdXNiKTsKPj4+ICsKPj4+ICsJaWYgKCFtdWlj KSB7Cj4+PiArCQlkZXZfaW5mbyhjdHgtPmRldiwgIm5vIGV4dGNvbiBmb3VuZCwgc3dpdGNoaW5n IHRvICdhbHdheXMgb24nIG1vZGVcbiIpOwo+Pj4gKwkJcmV0dXJuIDA7Cj4+PiArCX0KPj4+ICsK Pj4+ICsJZWRldiA9IGV4dGNvbl9maW5kX2VkZXZfYnlfbm9kZShtdWljKTsKPj4+ICsJb2Zfbm9k ZV9wdXQobXVpYyk7Cj4+PiArCWlmIChJU19FUlIoZWRldikpIHsKPj4+ICsJCWlmIChQVFJfRVJS KGVkZXYpID09IC1FUFJPQkVfREVGRVIpCj4+PiArCQkJcmV0dXJuIC1FUFJPQkVfREVGRVI7Cj4+ PiArCQlkZXZfZXJyKGN0eC0+ZGV2LCAiSW52YWxpZCBvciBtaXNzaW5nIGV4dGNvblxuIik7Cj4+ PiArCQlyZXR1cm4gUFRSX0VSUihlZGV2KTsKPj4+ICsJfQo+Pj4gKwo+Pj4gKwljdHgtPmV4dGNv biA9IGVkZXY7Cj4+PiArCWN0eC0+ZXh0Y29uX25iLm5vdGlmaWVyX2NhbGwgPSBzaWk4NjIwX2V4 dGNvbl9ub3RpZmllcjsKPj4+ICsJSU5JVF9XT1JLKCZjdHgtPmV4dGNvbl93cSwgc2lpODYyMF9l eHRjb25fd29yayk7Cj4+PiArCXJldCA9IGV4dGNvbl9yZWdpc3Rlcl9ub3RpZmllcihlZGV2LCBF WFRDT05fRElTUF9NSEwsICZjdHgtPmV4dGNvbl9uYik7Cj4+IFlvdSBiZXR0ZXIgdG8gdXNlIGRl dm1fZXh0Y29uX3JlZ2lzdGVyX25vdGlmaWVyKCkuCj4gCj4gV2l0aCBkZXZtIHZlcnNpb24gSSBy aXNrIHRoYXQgaW4gY2FzZSBvZiBkZXZpY2UgdW5iaW5kIG5vdGlmaWNhdGlvbiB3aWxsCj4gYmUg Y2FsbGVkIGFmdGVyIC5yZW1vdmUgY2FsbGJhY2ssIGl0IHNlZW1zIHRvIG1lIHF1aXRlIGRhbmdl cm91cy4gT3IgYW0KPiBJIG1pc3Npbmcgc29tZXRoaW5nPwoKSWYgeW91IHVzZSB0aGUgY2FuY2Vs X3dvcmtfc3luYygpIGluIHJlbW92ZSgpIGluc3RlYWQgb2YgZmx1c2hfd29yaygpLAp5b3UgY2Fu IHVzZSB0aGUgJ2Rldm1fZXh0Y29uXyonLgoKPiAKPiBSZWdhcmRzCj4gQW5kcnplago+IAo+Pgo+ Pj4gKwlpZiAocmV0KSB7Cj4+PiArCQlkZXZfZXJyKGN0eC0+ZGV2LCAiZmFpbGVkIHRvIHJlZ2lz dGVyIG5vdGlmaWVyIGZvciBNSExcbiIpOwo+Pj4gKwkJcmV0dXJuIHJldDsKPj4+ICsJfQo+Pj4g Kwo+Pj4gKwlyZXR1cm4gMDsKPj4+ICt9Cj4+PiArCj4+PiAgc3RhdGljIGlubGluZSBzdHJ1Y3Qg c2lpODYyMCAqYnJpZGdlX3RvX3NpaTg2MjAoc3RydWN0IGRybV9icmlkZ2UgKmJyaWRnZSkKPj4+ ICB7Cj4+PiAgCXJldHVybiBjb250YWluZXJfb2YoYnJpZGdlLCBzdHJ1Y3Qgc2lpODYyMCwgYnJp ZGdlKTsKPj4+IEBAIC0yMzA3LDEzICsyMzg0LDIwIEBAIHN0YXRpYyBpbnQgc2lpODYyMF9wcm9i ZShzdHJ1Y3QgaTJjX2NsaWVudCAqY2xpZW50LAo+Pj4gIAlpZiAocmV0KQo+Pj4gIAkJcmV0dXJu IHJldDsKPj4+ICAKPj4+ICsJcmV0ID0gc2lpODYyMF9leHRjb25faW5pdChjdHgpOwo+Pj4gKwlp ZiAocmV0IDwgMCkgewo+Pj4gKwkJZGV2X2VycihjdHgtPmRldiwgImZhaWxlZCB0byBpbml0aWFs aXplIEVYVENPTlxuIik7Cj4+PiArCQlyZXR1cm4gcmV0Owo+Pj4gKwl9Cj4+PiArCj4+PiAgCWky Y19zZXRfY2xpZW50ZGF0YShjbGllbnQsIGN0eCk7Cj4+PiAgCj4+PiAgCWN0eC0+YnJpZGdlLmZ1 bmNzID0gJnNpaTg2MjBfYnJpZGdlX2Z1bmNzOwo+Pj4gIAljdHgtPmJyaWRnZS5vZl9ub2RlID0g ZGV2LT5vZl9ub2RlOwo+Pj4gIAlkcm1fYnJpZGdlX2FkZCgmY3R4LT5icmlkZ2UpOwo+Pj4gIAo+ Pj4gLQlzaWk4NjIwX2NhYmxlX2luKGN0eCk7Cj4+PiArCWlmICghY3R4LT5leHRjb24pCj4+PiAr CQlzaWk4NjIwX2NhYmxlX2luKGN0eCk7Cj4+PiAgCj4+PiAgCXJldHVybiAwOwo+Pj4gIH0KPj4+ IEBAIC0yMzIyLDggKzI0MDYsMTUgQEAgc3RhdGljIGludCBzaWk4NjIwX3JlbW92ZShzdHJ1Y3Qg aTJjX2NsaWVudCAqY2xpZW50KQo+Pj4gIHsKPj4+ICAJc3RydWN0IHNpaTg2MjAgKmN0eCA9IGky Y19nZXRfY2xpZW50ZGF0YShjbGllbnQpOwo+Pj4gIAo+Pj4gLQlkaXNhYmxlX2lycSh0b19pMmNf Y2xpZW50KGN0eC0+ZGV2KS0+aXJxKTsKPj4+IC0Jc2lpODYyMF9od19vZmYoY3R4KTsKPj4+ICsJ aWYgKGN0eC0+ZXh0Y29uKSB7Cj4+PiArCQlleHRjb25fdW5yZWdpc3Rlcl9ub3RpZmllcihjdHgt PmV4dGNvbiwgRVhUQ09OX0RJU1BfTUhMLAo+Pj4gKwkJCQkJICAgJmN0eC0+ZXh0Y29uX25iKTsK Pj4gRG9uJ3QgbmVlZCB0byB1bnJlZ2lzdGVyIHRoZSBub3RpZmllciBpZiB1c2luZyBkZXZtX2V4 dGNvbl9yZWdpc3Rlcl9ub3RpZmllcigpLgo+Pgo+Pj4gKwkJZmx1c2hfd29yaygmY3R4LT5leHRj b25fd3EpOwo+Pj4gKwkJaWYgKGN0eC0+Y2FibGVfc3RhdGUgPiAwKQo+Pj4gKwkJCXNpaTg2MjBf Y2FibGVfb3V0KGN0eCk7Cj4+PiArCX0gZWxzZSB7Cj4+PiArCQlzaWk4NjIwX2NhYmxlX291dChj dHgpOwo+Pj4gKwl9Cj4+PiAgCWRybV9icmlkZ2VfcmVtb3ZlKCZjdHgtPmJyaWRnZSk7Cj4+PiAg Cj4+PiAgCXJldHVybiAwOwo+Pj4KPj4gSWYgeW91IHVzZSB0aGUgcmVzb3VyY2UgbWFuYWdlZCBm dW5jdGlvbiAoZGV2bV9leHRjb25fcmVnaXN0ZXJfbm90aWZpZXIpLCBMb29rcyBnb29kIHRvIG1l Lgo+PiBSZXZpZXdlZC1ieTogQ2hhbndvbyBDaG9pIDxjdzAwLmNob2lAc2Ftc3VuZy5jb20+Cj4+ Cj4gCj4gLS0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAi dW5zdWJzY3JpYmUgbGludXgtc2Ftc3VuZy1zb2MiIGluCj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdl IHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKPiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBo dHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwKPiAKPiAKPgo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: cw00.choi@samsung.com (Chanwoo Choi) Date: Wed, 28 Feb 2018 07:26:51 +0900 Subject: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL In-Reply-To: <87dd3281-0471-ab2a-90c6-3f2d4bdf4750@samsung.com> References: <20180227071134.28063-1-a.hajda@samsung.com> <20180227071134.28063-7-a.hajda@samsung.com> <5A953C21.5020007@samsung.com> <87dd3281-0471-ab2a-90c6-3f2d4bdf4750@samsung.com> Message-ID: <5A95DB2B.6000003@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 2018? 02? 27? 21:05, Andrzej Hajda wrote: > On 27.02.2018 12:08, Chanwoo Choi wrote: >> Hi, >> >> On 2018? 02? 27? 16:11, Andrzej Hajda wrote: >>> From: Maciej Purski >>> >>> Currently MHL chip must be turned on permanently to detect MHL cable. It >>> duplicates micro-USB controller's (MUIC) functionality and consumes >>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip >>> only if it detects MHL cable. >>> >>> Signed-off-by: Maciej Purski >>> Signed-off-by: Andrzej Hajda >>> --- >>> v5: updated extcon API >>> >>> This is rework of the patch by Maciej with following changes: >>> - use micro-USB port bindings to get extcon, instead of extcon property, >>> - fixed remove sequence, >>> - fixed extcon get state logic. >>> >>> Code finding extcon node is hacky IMO, I guess ultimately it should be done >>> via some framework (maybe even extcon), or at least via helper, I hope it >>> can stay as is until the proper solution will be merged. >>> >>> Signed-off-by: Andrzej Hajda >>> --- >>> drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 94 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c >>> index 9e785b8e0ea2..62b0adabcac2 100644 >>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c >>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c >>> @@ -17,6 +17,7 @@ >>> >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -25,6 +26,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> >>> @@ -81,6 +83,10 @@ struct sii8620 { >>> struct edid *edid; >>> unsigned int gen2_write_burst:1; >>> enum sii8620_mt_state mt_state; >>> + struct extcon_dev *extcon; >>> + struct notifier_block extcon_nb; >>> + struct work_struct extcon_wq; >>> + int cable_state; >>> struct list_head mt_queue; >>> struct { >>> int r_size; >>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx) >>> ctx->rc_dev = rc_dev; >>> } >>> >>> +static void sii8620_cable_out(struct sii8620 *ctx) >>> +{ >>> + disable_irq(to_i2c_client(ctx->dev)->irq); >>> + sii8620_hw_off(ctx); >>> +} >>> + >>> +static void sii8620_extcon_work(struct work_struct *work) >>> +{ >>> + struct sii8620 *ctx = >>> + container_of(work, struct sii8620, extcon_wq); >>> + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL); >>> + >>> + if (state == ctx->cable_state) >>> + return; >>> + >>> + ctx->cable_state = state; >>> + >>> + if (state > 0) >>> + sii8620_cable_in(ctx); >>> + else >>> + sii8620_cable_out(ctx); >>> +} >>> + >>> +static int sii8620_extcon_notifier(struct notifier_block *self, >>> + unsigned long event, void *ptr) >>> +{ >>> + struct sii8620 *ctx = >>> + container_of(self, struct sii8620, extcon_nb); >>> + >>> + schedule_work(&ctx->extcon_wq); >>> + >>> + return NOTIFY_DONE; >>> +} >>> + >>> +static int sii8620_extcon_init(struct sii8620 *ctx) >>> +{ >>> + struct extcon_dev *edev; >>> + struct device_node *musb, *muic; >>> + int ret; >>> + >>> + /* get micro-USB connector node */ >>> + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1); >>> + /* next get micro-USB Interface Controller node */ >>> + muic = of_get_next_parent(musb); >>> + >>> + if (!muic) { >>> + dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n"); >>> + return 0; >>> + } >>> + >>> + edev = extcon_find_edev_by_node(muic); >>> + of_node_put(muic); >>> + if (IS_ERR(edev)) { >>> + if (PTR_ERR(edev) == -EPROBE_DEFER) >>> + return -EPROBE_DEFER; >>> + dev_err(ctx->dev, "Invalid or missing extcon\n"); >>> + return PTR_ERR(edev); >>> + } >>> + >>> + ctx->extcon = edev; >>> + ctx->extcon_nb.notifier_call = sii8620_extcon_notifier; >>> + INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work); >>> + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb); >> You better to use devm_extcon_register_notifier(). > > With devm version I risk that in case of device unbind notification will > be called after .remove callback, it seems to me quite dangerous. Or am > I missing something? If you use the cancel_work_sync() in remove() instead of flush_work(), you can use the 'devm_extcon_*'. > > Regards > Andrzej > >> >>> + if (ret) { >>> + dev_err(ctx->dev, "failed to register notifier for MHL\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge) >>> { >>> return container_of(bridge, struct sii8620, bridge); >>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client, >>> if (ret) >>> return ret; >>> >>> + ret = sii8620_extcon_init(ctx); >>> + if (ret < 0) { >>> + dev_err(ctx->dev, "failed to initialize EXTCON\n"); >>> + return ret; >>> + } >>> + >>> i2c_set_clientdata(client, ctx); >>> >>> ctx->bridge.funcs = &sii8620_bridge_funcs; >>> ctx->bridge.of_node = dev->of_node; >>> drm_bridge_add(&ctx->bridge); >>> >>> - sii8620_cable_in(ctx); >>> + if (!ctx->extcon) >>> + sii8620_cable_in(ctx); >>> >>> return 0; >>> } >>> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client) >>> { >>> struct sii8620 *ctx = i2c_get_clientdata(client); >>> >>> - disable_irq(to_i2c_client(ctx->dev)->irq); >>> - sii8620_hw_off(ctx); >>> + if (ctx->extcon) { >>> + extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL, >>> + &ctx->extcon_nb); >> Don't need to unregister the notifier if using devm_extcon_register_notifier(). >> >>> + flush_work(&ctx->extcon_wq); >>> + if (ctx->cable_state > 0) >>> + sii8620_cable_out(ctx); >>> + } else { >>> + sii8620_cable_out(ctx); >>> + } >>> drm_bridge_remove(&ctx->bridge); >>> >>> return 0; >>> >> If you use the resource managed function (devm_extcon_register_notifier), Looks good to me. >> Reviewed-by: Chanwoo Choi >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- Best Regards, Chanwoo Choi Samsung Electronics