linux-kernel - Re: [PATCH 2/2] usb: core: phy: fix return value checking about devm_of_phy_get_by

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 25 Jun 2018 19:32:04 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     chunfeng.yun@...iatek.com
Cc:     johan@...nel.org, gregkh@...uxfoundation.org,
        felipe.balbi@...ux.intel.com, matthias.bgg@...il.com,
        linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 2/2] usb: core: phy: fix return value checking about devm_of_phy_get_by_index()

Hi Chunfeng,

On Mon, Jun 25, 2018 at 8:37 AM Chunfeng Yun <chunfeng.yun@...iatek.com> wrote:
>
> On Sun, 2018-06-24 at 20:00 +0200, Martin Blumenstingl wrote:
> > Hello Chunfeng,
> >
> > thank you for the patch!
> >
> > On Fri, Jun 22, 2018 at 8:33 AM Chunfeng Yun <chunfeng.yun@...iatek.com> wrote:
> > >
> > > 1. use IS_ERR() but not IS_ERR_OR_NULL() because devm_of_phy_get_by_index()
> > >     never return NULL value;
> > ACK - good catch!
> >
> > > 2. devm_of_phy_get_by_index() should not fail for a valid index
> > I have learned that the PHY framework can return -ENODEV if the PHY is:
> > 1. supposed to be handled by the legacy USB PHY framework
> > 2. the PHY node is disabled in devicetree
> A little confused, we can avoid this error by not using a disabled or
> "usb-nop-xceiv" phy in host node.
I think that would be the mid or long term goal

> If ignore error of -ENODEV, we will also skip an error case which we
> don't want it happen.
as far as I understand the problem is that there are .dts files out
there which specify "usb-nop-xceiv" in the "phys" property, see [0]
(there might be even some more out-of-tree cases)
so I think the -ENODEV check can only be removed once nothing uses
"usb-nop-xceiv" anymore


Regards
Martin


[0] https://patchwork.kernel.org/patch/10160181/