From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johan Hovold Subject: Re: [PATCH v3] usb: core: phy: fix return value checking about devm_of_phy_get_by_index() Date: Fri, 31 Aug 2018 11:41:54 +0200 Message-ID: <20180831094154.GL28861@localhost> References: <626cea85395e3a1f002ef0a241bb508d770a33c0.1535536218.git.chunfeng.yun@mediatek.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <626cea85395e3a1f002ef0a241bb508d770a33c0.1535536218.git.chunfeng.yun@mediatek.com> Sender: linux-kernel-owner@vger.kernel.org To: Chunfeng Yun Cc: Greg Kroah-Hartman , Felipe Balbi , Martin Blumenstingl , Matthias Brugger , Johan Hovold , linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org List-Id: linux-mediatek@lists.infradead.org On Thu, Aug 30, 2018 at 09:16:40AM +0800, Chunfeng Yun wrote: > Use IS_ERR() instead of IS_ERR_OR_NULL() because devm_of_phy_get_by_index() > never return NULL value; > But still need ignore the error of -ENODEV, for more information, please > refer to: > [0] https://lkml.org/lkml/2018/4/19/88 > [1] https://patchwork.kernel.org/patch/10160181/ > > Signed-off-by: Chunfeng Yun > --- > V3: abandon the ternary operator suggested by Johan > > V2: keep the -ENODEV check Perhaps the patch Subject is a little misleading as this is more of a clean up than a fix (as there's no bug here). But apart from that this looks good to me. Reviewed-by: Johan Hovold > --- > drivers/usb/core/phy.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c > index 9879767..38b2c77 100644 > --- a/drivers/usb/core/phy.c > +++ b/drivers/usb/core/phy.c > @@ -23,10 +23,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int index, > struct list_head *list) > { > struct usb_phy_roothub *roothub_entry; > - struct phy *phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > + struct phy *phy; > > - if (IS_ERR_OR_NULL(phy)) { > - if (!phy || PTR_ERR(phy) == -ENODEV) > + phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > + if (IS_ERR(phy)) { > + if (PTR_ERR(phy) == -ENODEV) > return 0; > else > return PTR_ERR(phy); Thanks, Johan 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: [v3] usb: core: phy: fix return value checking about devm_of_phy_get_by_index() From: Johan Hovold Message-Id: <20180831094154.GL28861@localhost> Date: Fri, 31 Aug 2018 11:41:54 +0200 To: Chunfeng Yun Cc: Greg Kroah-Hartman , Felipe Balbi , Martin Blumenstingl , Matthias Brugger , Johan Hovold , linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org List-ID: T24gVGh1LCBBdWcgMzAsIDIwMTggYXQgMDk6MTY6NDBBTSArMDgwMCwgQ2h1bmZlbmcgWXVuIHdy b3RlOgo+IFVzZSBJU19FUlIoKSBpbnN0ZWFkIG9mIElTX0VSUl9PUl9OVUxMKCkgYmVjYXVzZSBk ZXZtX29mX3BoeV9nZXRfYnlfaW5kZXgoKQo+IG5ldmVyIHJldHVybiBOVUxMIHZhbHVlOwo+IEJ1 dCBzdGlsbCBuZWVkIGlnbm9yZSB0aGUgZXJyb3Igb2YgLUVOT0RFViwgZm9yIG1vcmUgaW5mb3Jt YXRpb24sIHBsZWFzZQo+IHJlZmVyIHRvOgo+IFswXSBodHRwczovL2xrbWwub3JnL2xrbWwvMjAx OC80LzE5Lzg4Cj4gWzFdIGh0dHBzOi8vcGF0Y2h3b3JrLmtlcm5lbC5vcmcvcGF0Y2gvMTAxNjAx ODEvCj4gCj4gU2lnbmVkLW9mZi1ieTogQ2h1bmZlbmcgWXVuIDxjaHVuZmVuZy55dW5AbWVkaWF0 ZWsuY29tPgo+IC0tLQo+IFYzOiBhYmFuZG9uIHRoZSB0ZXJuYXJ5IG9wZXJhdG9yIHN1Z2dlc3Rl ZCBieSBKb2hhbgo+IAo+IFYyOiBrZWVwIHRoZSAtRU5PREVWIGNoZWNrCgpQZXJoYXBzIHRoZSBw YXRjaCBTdWJqZWN0IGlzIGEgbGl0dGxlIG1pc2xlYWRpbmcgYXMgdGhpcyBpcyBtb3JlIG9mIGEK Y2xlYW4gdXAgdGhhbiBhIGZpeCAoYXMgdGhlcmUncyBubyBidWcgaGVyZSkuCgpCdXQgYXBhcnQg ZnJvbSB0aGF0IHRoaXMgbG9va3MgZ29vZCB0byBtZS4KClJldmlld2VkLWJ5OiBKb2hhbiBIb3Zv bGQgPGpvaGFuQGtlcm5lbC5vcmc+Cgo+IC0tLQo+ICBkcml2ZXJzL3VzYi9jb3JlL3BoeS5jIHwg NyArKysrLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25z KC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2NvcmUvcGh5LmMgYi9kcml2ZXJzL3Vz Yi9jb3JlL3BoeS5jCj4gaW5kZXggOTg3OTc2Ny4uMzhiMmM3NyAxMDA2NDQKPiAtLS0gYS9kcml2 ZXJzL3VzYi9jb3JlL3BoeS5jCj4gKysrIGIvZHJpdmVycy91c2IvY29yZS9waHkuYwo+IEBAIC0y MywxMCArMjMsMTEgQEAgc3RhdGljIGludCB1c2JfcGh5X3Jvb3RodWJfYWRkX3BoeShzdHJ1Y3Qg ZGV2aWNlICpkZXYsIGludCBpbmRleCwKPiAgCQkJCSAgIHN0cnVjdCBsaXN0X2hlYWQgKmxpc3Qp Cj4gIHsKPiAgCXN0cnVjdCB1c2JfcGh5X3Jvb3RodWIgKnJvb3RodWJfZW50cnk7Cj4gLQlzdHJ1 Y3QgcGh5ICpwaHkgPSBkZXZtX29mX3BoeV9nZXRfYnlfaW5kZXgoZGV2LCBkZXYtPm9mX25vZGUs IGluZGV4KTsKPiArCXN0cnVjdCBwaHkgKnBoeTsKPiAgCj4gLQlpZiAoSVNfRVJSX09SX05VTEwo cGh5KSkgewo+IC0JCWlmICghcGh5IHx8IFBUUl9FUlIocGh5KSA9PSAtRU5PREVWKQo+ICsJcGh5 ID0gZGV2bV9vZl9waHlfZ2V0X2J5X2luZGV4KGRldiwgZGV2LT5vZl9ub2RlLCBpbmRleCk7Cj4g KwlpZiAoSVNfRVJSKHBoeSkpIHsKPiArCQlpZiAoUFRSX0VSUihwaHkpID09IC1FTk9ERVYpCj4g IAkJCXJldHVybiAwOwo+ICAJCWVsc2UKPiAgCQkJcmV0dXJuIFBUUl9FUlIocGh5KTsKClRoYW5r cywKSm9oYW4K From mboxrd@z Thu Jan 1 00:00:00 1970 From: johan@kernel.org (Johan Hovold) Date: Fri, 31 Aug 2018 11:41:54 +0200 Subject: [PATCH v3] usb: core: phy: fix return value checking about devm_of_phy_get_by_index() In-Reply-To: <626cea85395e3a1f002ef0a241bb508d770a33c0.1535536218.git.chunfeng.yun@mediatek.com> References: <626cea85395e3a1f002ef0a241bb508d770a33c0.1535536218.git.chunfeng.yun@mediatek.com> Message-ID: <20180831094154.GL28861@localhost> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Aug 30, 2018 at 09:16:40AM +0800, Chunfeng Yun wrote: > Use IS_ERR() instead of IS_ERR_OR_NULL() because devm_of_phy_get_by_index() > never return NULL value; > But still need ignore the error of -ENODEV, for more information, please > refer to: > [0] https://lkml.org/lkml/2018/4/19/88 > [1] https://patchwork.kernel.org/patch/10160181/ > > Signed-off-by: Chunfeng Yun > --- > V3: abandon the ternary operator suggested by Johan > > V2: keep the -ENODEV check Perhaps the patch Subject is a little misleading as this is more of a clean up than a fix (as there's no bug here). But apart from that this looks good to me. Reviewed-by: Johan Hovold > --- > drivers/usb/core/phy.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c > index 9879767..38b2c77 100644 > --- a/drivers/usb/core/phy.c > +++ b/drivers/usb/core/phy.c > @@ -23,10 +23,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int index, > struct list_head *list) > { > struct usb_phy_roothub *roothub_entry; > - struct phy *phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > + struct phy *phy; > > - if (IS_ERR_OR_NULL(phy)) { > - if (!phy || PTR_ERR(phy) == -ENODEV) > + phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > + if (IS_ERR(phy)) { > + if (PTR_ERR(phy) == -ENODEV) > return 0; > else > return PTR_ERR(phy); Thanks, Johan