From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Keeping Subject: Re: [PATCH v3 23/24] drm/rockchip: dw-mipi-dsi: add reset control Date: Thu, 16 Feb 2017 14:11:04 +0000 Message-ID: <20170216141104.5ecca1df.john@metanate.com> References: <20170129132444.25251-1-john@metanate.com> <20170129132444.25251-24-john@metanate.com> <58A3CD45.9090309@rock-chips.com> <20170215123944.798ae363.john@metanate.com> <58A50A91.6010202@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <58A50A91.6010202@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Zhong Cc: linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org List-Id: linux-rockchip.vger.kernel.org T24gVGh1LCAxNiBGZWIgMjAxNyAxMDoxMjozMyArMDgwMCwgQ2hyaXMgWmhvbmcgd3JvdGU6Cgo+ IE9uIDAyLzE1LzIwMTcgMDg6MzkgUE0sIEpvaG4gS2VlcGluZyB3cm90ZToKPiA+IE9uIFdlZCwg MTUgRmViIDIwMTcgMTE6Mzg6NDUgKzA4MDAsIENocmlzIFpob25nIHdyb3RlOgo+ID4gIAo+ID4+ IE9uIDAxLzI5LzIwMTcgMDk6MjQgUE0sIEpvaG4gS2VlcGluZyB3cm90ZTogIAo+ID4+PiBJbiBv cmRlciB0byBmdWxseSByZXNldCB0aGUgc3RhdGUgb2YgdGhlIE1JUEkgY29udHJvbGxlciB3ZSBt dXN0IGFzc2VydAo+ID4+PiB0aGlzIHJlc2V0Lgo+ID4+Pgo+ID4+PiBUaGlzIGlzIHNsaWdodGx5 IG1vcmUgY29tcGxpY2F0ZWQgdGhhbiBpdCBjb3VsZCBiZSBpbiBvcmRlciB0byBtYWludGFpbgo+ ID4+PiBjb21wYXRpYmlsaXR5IHdpdGggZGV2aWNlIHRyZWVzIHRoYXQgZG8gbm90IHNwZWNpZnkg dGhlIHJlc2V0IHByb3BlcnR5Lgo+ID4+Pgo+ID4+PiBTaWduZWQtb2ZmLWJ5OiBKb2huIEtlZXBp bmcgPGpvaG5AbWV0YW5hdGUuY29tPgo+ID4+PiBSZXZpZXdlZC1ieTogQ2hyaXMgWmhvbmcgPHp5 d0Byb2NrLWNoaXBzLmNvbT4KPiA+Pj4gLS0tCj4gPj4+IHYzOgo+ID4+PiAtIEFkZCBDaHJpcycg UmV2aWV3ZWQtYnkKPiA+Pj4gVW5jaGFuZ2VkIGluIHYyCj4gPj4+Cj4gPj4+ICAgIGRyaXZlcnMv Z3B1L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaS5jIHwgMzAgKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrCj4gPj4+ICAgIDEgZmlsZSBjaGFuZ2VkLCAzMCBpbnNlcnRpb25zKCspCj4gPj4+ Cj4gPj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvZHctbWlwaS1kc2ku YyBiL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaS5jCj4gPj4+IGluZGV4IDU4 Y2I4YWNlMmZlOC4uY2YzY2E2YjBjYmRiIDEwMDY0NAo+ID4+PiAtLS0gYS9kcml2ZXJzL2dwdS9k cm0vcm9ja2NoaXAvZHctbWlwaS1kc2kuYwo+ID4+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vcm9j a2NoaXAvZHctbWlwaS1kc2kuYwo+ID4+PiBAQCAtMTMsNiArMTMsNyBAQAo+ID4+PiAgICAjaW5j bHVkZSA8bGludXgvbW9kdWxlLmg+Cj4gPj4+ICAgICNpbmNsdWRlIDxsaW51eC9vZl9kZXZpY2Uu aD4KPiA+Pj4gICAgI2luY2x1ZGUgPGxpbnV4L3JlZ21hcC5oPgo+ID4+PiArI2luY2x1ZGUgPGxp bnV4L3Jlc2V0Lmg+Cj4gPj4+ICAgICNpbmNsdWRlIDxsaW51eC9tZmQvc3lzY29uLmg+Cj4gPj4+ ICAgICNpbmNsdWRlIDxkcm0vZHJtX2F0b21pY19oZWxwZXIuaD4KPiA+Pj4gICAgI2luY2x1ZGUg PGRybS9kcm1fY3J0Yy5oPgo+ID4+PiBAQCAtMTEyNCw2ICsxMTI1LDcgQEAgc3RhdGljIGludCBk d19taXBpX2RzaV9iaW5kKHN0cnVjdCBkZXZpY2UgKmRldiwgc3RydWN0IGRldmljZSAqbWFzdGVy LAo+ID4+PiAgICAJCQlvZl9tYXRjaF9kZXZpY2UoZHdfbWlwaV9kc2lfZHRfaWRzLCBkZXYpOwo+ ID4+PiAgICAJY29uc3Qgc3RydWN0IGR3X21pcGlfZHNpX3BsYXRfZGF0YSAqcGRhdGEgPSBvZl9p ZC0+ZGF0YTsKPiA+Pj4gICAgCXN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYgPSB0b19wbGF0 Zm9ybV9kZXZpY2UoZGV2KTsKPiA+Pj4gKwlzdHJ1Y3QgcmVzZXRfY29udHJvbCAqYXBiX3JzdDsK PiA+Pj4gICAgCXN0cnVjdCBkcm1fZGV2aWNlICpkcm0gPSBkYXRhOwo+ID4+PiAgICAJc3RydWN0 IGR3X21pcGlfZHNpICpkc2k7Cj4gPj4+ICAgIAlzdHJ1Y3QgcmVzb3VyY2UgKnJlczsKPiA+Pj4g QEAgLTExNjIsNiArMTE2NCwzNCBAQCBzdGF0aWMgaW50IGR3X21pcGlfZHNpX2JpbmQoc3RydWN0 IGRldmljZSAqZGV2LCBzdHJ1Y3QgZGV2aWNlICptYXN0ZXIsCj4gPj4+ICAgIAkJcmV0dXJuIHJl dDsKPiA+Pj4gICAgCX0KPiA+Pj4gICAgCj4gPj4+ICsJLyoKPiA+Pj4gKwkgKiBOb3RlIHRoYXQg dGhlIHJlc2V0IHdhcyBub3QgZGVmaW5lZCBpbiB0aGUgaW5pdGlhbCBkZXZpY2UgdHJlZSwgc28K PiA+Pj4gKwkgKiB3ZSBoYXZlIHRvIGJlIHByZXBhcmVkIGZvciBpdCBub3QgYmVpbmcgZm91bmQu Cj4gPj4+ICsJICovCj4gPj4+ICsJYXBiX3JzdCA9IGRldm1fcmVzZXRfY29udHJvbF9nZXQoZGV2 LCAiYXBiIik7Cj4gPj4+ICsJaWYgKElTX0VSUihhcGJfcnN0KSkgewo+ID4+PiArCQlpZiAoUFRS X0VSUihhcGJfcnN0KSA9PSAtRU5PREVWKSB7ICAKPiA+PiBBY2NvcmRpbmcgdG8gWzBdLCBJIHRo aW5rIGl0IHNob3VsZCBiZSAtRU5PRU5UIGhlcmUuICAKPiA+IE5pY2UgY2F0Y2gsIEknbGwgZml4 IHRoaXMuCj4gPiAgCj4gPj4gWzBdCj4gPj4gaHR0cHM6Ly9naXQua2VybmVsLm9yZy9jZ2l0L2xp bnV4L2tlcm5lbC9naXQvbmV4dC9saW51eC1uZXh0LmdpdC9jb21taXQvP2lkPTNkODEyMTZmZGU0 NjVlNzZjNWVhZTk4ZjYxZDM2NjYxNjM2MzQzOTUKPiA+Pgo+ID4+IGNvbW1pdCAzZDgxMjE2ZmRl NDY1ZTc2YzVlYWU5OGY2MWQzNjY2MTYzNjM0Mzk1Cj4gPj4gQXV0aG9yOiBBbGJhbiBCZWRlbCA8 YWxiZXVAZnJlZS5mcj4KPiA+PiBEYXRlOiAgIFR1ZSBTZXAgMSAxNzoyODozMSAyMDE1ICswMjAw Cj4gPj4KPiA+PiAgICAgICByZXNldDogRml4IG9mX3Jlc2V0X2NvbnRyb2xfZ2V0KCkgZm9yIGNv bnNpc3RlbnQgcmV0dXJuIHZhbHVlcwo+ID4+Cj4gPj4gICAgICAgV2hlbiBvZl9yZXNldF9jb250 cm9sX2dldCgpIGlzIGNhbGxlZCB3aXRob3V0IGNvbm5lY3Rpb24gSUQgaXQgcmV0dXJucwo+ID4+ ICAgICAgIC1FTk9FTlQgd2hlbiB0aGUgJ3Jlc2V0cycgcHJvcGVydHkgZG9lc24ndCBleGlzdHMg b3IgaXMgYW4gZW1wdHkgZW50cnkuCj4gPj4gICAgICAgSG93ZXZlciB3aGVuIGEgY29ubmVjdGlv biBJRCBpcyBnaXZlbiBpdCByZXR1cm5zIC1FSU5WQUwgd2hlbiB0aGUKPiA+PiAncmVzZXRzJwo+ ID4+ICAgICAgIHByb3BlcnR5IGRvZXNuJ3QgZXhpc3RzIG9yIHRoZSByZXF1ZXN0ZWQgbmFtZSBj YW4ndCBiZSBmb3VuZC4gVGhpcyBpcwo+ID4+ICAgICAgIGJlY2F1c2UgdGhlIGVycm9yIGNvZGUg cmV0dXJuZWQgYnkgb2ZfcHJvcGVydHlfbWF0Y2hfc3RyaW5nKCkgaXMganVzdAo+ID4+ICAgICAg IHBhc3NlZCBkb3duIGFzIGFuIGluZGV4IHRvIG9mX3BhcnNlX3BoYW5kbGVfd2l0aF9hcmdzKCks IHdoaWNoIHRoZW4KPiA+PiAgICAgICByZXR1cm5zIC1FSU5WQUwuCj4gPj4KPiA+PiAgICAgICBU byBnZXQgYSBjb25zaXN0ZW50IHJldHVybiB2YWx1ZSB3aXRoIGJvdGggY29kZSBwYXRocyB3ZSBt dXN0IHJldHVybgo+ID4+ICAgICAgIC1FTk9FTlQgd2hlbiBvZl9wcm9wZXJ0eV9tYXRjaF9zdHJp bmcoKSBmYWlscy4KPiA+Pgo+ID4+ICAgICAgIFNpZ25lZC1vZmYtYnk6IEFsYmFuIEJlZGVsIDxh bGJldUBmcmVlLmZyPgo+ID4+ICAgICAgIFNpZ25lZC1vZmYtYnk6IFBoaWxpcHAgWmFiZWwgPHAu emFiZWxAcGVuZ3V0cm9uaXguZGU+Cj4gPj4KPiA+PiAgCj4gPj4+ICsJCQlhcGJfcnN0ID0gTlVM TDsKPiA+Pj4gKwkJfSBlbHNlIHsKPiA+Pj4gKwkJCWRldl9lcnIoZGV2LCAiVW5hYmxlIHRvIGdl dCByZXNldCBjb250cm9sOiAlZFxuIiwgcmV0KTsgIAo+IEFsc28sIHdlIGNhbiBub3QgZ2V0IGVy cm9yIG51bWJlciBmcm9tICJyZXQiIGhlcmUuCgpHb29kIHBvaW50LCBJJ2xsIGZpeCB0aGlzLgoK CkpvaG4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: john@metanate.com (John Keeping) Date: Thu, 16 Feb 2017 14:11:04 +0000 Subject: [PATCH v3 23/24] drm/rockchip: dw-mipi-dsi: add reset control In-Reply-To: <58A50A91.6010202@rock-chips.com> References: <20170129132444.25251-1-john@metanate.com> <20170129132444.25251-24-john@metanate.com> <58A3CD45.9090309@rock-chips.com> <20170215123944.798ae363.john@metanate.com> <58A50A91.6010202@rock-chips.com> Message-ID: <20170216141104.5ecca1df.john@metanate.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, 16 Feb 2017 10:12:33 +0800, Chris Zhong wrote: > On 02/15/2017 08:39 PM, John Keeping wrote: > > On Wed, 15 Feb 2017 11:38:45 +0800, Chris Zhong wrote: > > > >> On 01/29/2017 09:24 PM, John Keeping wrote: > >>> In order to fully reset the state of the MIPI controller we must assert > >>> this reset. > >>> > >>> This is slightly more complicated than it could be in order to maintain > >>> compatibility with device trees that do not specify the reset property. > >>> > >>> Signed-off-by: John Keeping > >>> Reviewed-by: Chris Zhong > >>> --- > >>> v3: > >>> - Add Chris' Reviewed-by > >>> Unchanged in v2 > >>> > >>> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 30 ++++++++++++++++++++++++++++++ > >>> 1 file changed, 30 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > >>> index 58cb8ace2fe8..cf3ca6b0cbdb 100644 > >>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > >>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > >>> @@ -13,6 +13,7 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> #include > >>> #include > >>> #include > >>> @@ -1124,6 +1125,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, > >>> of_match_device(dw_mipi_dsi_dt_ids, dev); > >>> const struct dw_mipi_dsi_plat_data *pdata = of_id->data; > >>> struct platform_device *pdev = to_platform_device(dev); > >>> + struct reset_control *apb_rst; > >>> struct drm_device *drm = data; > >>> struct dw_mipi_dsi *dsi; > >>> struct resource *res; > >>> @@ -1162,6 +1164,34 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, > >>> return ret; > >>> } > >>> > >>> + /* > >>> + * Note that the reset was not defined in the initial device tree, so > >>> + * we have to be prepared for it not being found. > >>> + */ > >>> + apb_rst = devm_reset_control_get(dev, "apb"); > >>> + if (IS_ERR(apb_rst)) { > >>> + if (PTR_ERR(apb_rst) == -ENODEV) { > >> According to [0], I think it should be -ENOENT here. > > Nice catch, I'll fix this. > > > >> [0] > >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=3d81216fde465e76c5eae98f61d3666163634395 > >> > >> commit 3d81216fde465e76c5eae98f61d3666163634395 > >> Author: Alban Bedel > >> Date: Tue Sep 1 17:28:31 2015 +0200 > >> > >> reset: Fix of_reset_control_get() for consistent return values > >> > >> When of_reset_control_get() is called without connection ID it returns > >> -ENOENT when the 'resets' property doesn't exists or is an empty entry. > >> However when a connection ID is given it returns -EINVAL when the > >> 'resets' > >> property doesn't exists or the requested name can't be found. This is > >> because the error code returned by of_property_match_string() is just > >> passed down as an index to of_parse_phandle_with_args(), which then > >> returns -EINVAL. > >> > >> To get a consistent return value with both code paths we must return > >> -ENOENT when of_property_match_string() fails. > >> > >> Signed-off-by: Alban Bedel > >> Signed-off-by: Philipp Zabel > >> > >> > >>> + apb_rst = NULL; > >>> + } else { > >>> + dev_err(dev, "Unable to get reset control: %d\n", ret); > Also, we can not get error number from "ret" here. Good point, I'll fix this. John From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932116AbdBPOLe (ORCPT ); Thu, 16 Feb 2017 09:11:34 -0500 Received: from dougal.metanate.com ([90.155.101.14]:23763 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755087AbdBPOLc (ORCPT ); Thu, 16 Feb 2017 09:11:32 -0500 Date: Thu, 16 Feb 2017 14:11:04 +0000 From: John Keeping To: Chris Zhong Cc: Mark Yao , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 23/24] drm/rockchip: dw-mipi-dsi: add reset control Message-ID: <20170216141104.5ecca1df.john@metanate.com> In-Reply-To: <58A50A91.6010202@rock-chips.com> References: <20170129132444.25251-1-john@metanate.com> <20170129132444.25251-24-john@metanate.com> <58A3CD45.9090309@rock-chips.com> <20170215123944.798ae363.john@metanate.com> <58A50A91.6010202@rock-chips.com> Organization: Metanate Ltd X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 16 Feb 2017 10:12:33 +0800, Chris Zhong wrote: > On 02/15/2017 08:39 PM, John Keeping wrote: > > On Wed, 15 Feb 2017 11:38:45 +0800, Chris Zhong wrote: > > > >> On 01/29/2017 09:24 PM, John Keeping wrote: > >>> In order to fully reset the state of the MIPI controller we must assert > >>> this reset. > >>> > >>> This is slightly more complicated than it could be in order to maintain > >>> compatibility with device trees that do not specify the reset property. > >>> > >>> Signed-off-by: John Keeping > >>> Reviewed-by: Chris Zhong > >>> --- > >>> v3: > >>> - Add Chris' Reviewed-by > >>> Unchanged in v2 > >>> > >>> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 30 ++++++++++++++++++++++++++++++ > >>> 1 file changed, 30 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > >>> index 58cb8ace2fe8..cf3ca6b0cbdb 100644 > >>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > >>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > >>> @@ -13,6 +13,7 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> #include > >>> #include > >>> #include > >>> @@ -1124,6 +1125,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, > >>> of_match_device(dw_mipi_dsi_dt_ids, dev); > >>> const struct dw_mipi_dsi_plat_data *pdata = of_id->data; > >>> struct platform_device *pdev = to_platform_device(dev); > >>> + struct reset_control *apb_rst; > >>> struct drm_device *drm = data; > >>> struct dw_mipi_dsi *dsi; > >>> struct resource *res; > >>> @@ -1162,6 +1164,34 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, > >>> return ret; > >>> } > >>> > >>> + /* > >>> + * Note that the reset was not defined in the initial device tree, so > >>> + * we have to be prepared for it not being found. > >>> + */ > >>> + apb_rst = devm_reset_control_get(dev, "apb"); > >>> + if (IS_ERR(apb_rst)) { > >>> + if (PTR_ERR(apb_rst) == -ENODEV) { > >> According to [0], I think it should be -ENOENT here. > > Nice catch, I'll fix this. > > > >> [0] > >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=3d81216fde465e76c5eae98f61d3666163634395 > >> > >> commit 3d81216fde465e76c5eae98f61d3666163634395 > >> Author: Alban Bedel > >> Date: Tue Sep 1 17:28:31 2015 +0200 > >> > >> reset: Fix of_reset_control_get() for consistent return values > >> > >> When of_reset_control_get() is called without connection ID it returns > >> -ENOENT when the 'resets' property doesn't exists or is an empty entry. > >> However when a connection ID is given it returns -EINVAL when the > >> 'resets' > >> property doesn't exists or the requested name can't be found. This is > >> because the error code returned by of_property_match_string() is just > >> passed down as an index to of_parse_phandle_with_args(), which then > >> returns -EINVAL. > >> > >> To get a consistent return value with both code paths we must return > >> -ENOENT when of_property_match_string() fails. > >> > >> Signed-off-by: Alban Bedel > >> Signed-off-by: Philipp Zabel > >> > >> > >>> + apb_rst = NULL; > >>> + } else { > >>> + dev_err(dev, "Unable to get reset control: %d\n", ret); > Also, we can not get error number from "ret" here. Good point, I'll fix this. John