linux-kernel - Re: [PATCH] PCI: imx6: Fix wrong check in imx6_pcie_attach

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YyAy4iBUa5kByPwu@shell.armlinux.org.uk>
Date:   Tue, 13 Sep 2022 08:36:02 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Tang Bin <tangbin@...s.chinamobile.com>
Cc:     hongxing.zhu@....com, l.stach@...gutronix.de,
        lorenzo.pieralisi@....com, robh@...nel.org, kw@...ux.com,
        shawnguo@...nel.org, bhelgaas@...gle.com, s.hauer@...gutronix.de,
        kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
        linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: imx6: Fix wrong check in imx6_pcie_attach_pd()

On Tue, Sep 13, 2022 at 08:31:35AM +0100, Russell King (Oracle) wrote:
> On Tue, Sep 13, 2022 at 02:59:10PM +0800, Tang Bin wrote:
> > In the function imx6_pcie_attach_pd(),
> > dev_pm_domain_attach_by_name() may return NULL in some cases,
> > so IS_ERR() doesn't meet the requirements. Thus fix it.
> 
> NAK. You are clearly doing a mechanical search and replace, and then
> throwing out patches without a care in the world for other people to
> then decide whether the changes are in fact appropriate or not.
> 
> Please don't do that. Please read and understand the code before you
> waste reviewers and developers time - otherwise you will educate
> reviews and developers to ignore your efforts.

It is also highly likely that many of these changes are just plain
broken.

If you read the documentation for this function and the referred
to function:

 * Returns the created virtual device if successfully attached PM domain, NULL
 * when the device don't need a PM domain, else an ERR_PTR() in case of
 * failures. If a power-domain exists for the device, but cannot be found or
 * turned on, then ERR_PTR(-EPROBE_DEFER) is returned to ensure that the device
 * is not probed and to re-try again later.

So, NULL is *not* an error condition. It means that the device does not
need a power domain, which is *not* a failure.

You are probably causing more harm than good by trying to do this
mechanical change all over the kernel.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!