linux-kernel - Re: [PATCH v8 6/8] PCI: dwc: Add dw_pcie_host_ops.host

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Ykdj+oJ+0k8UZCwK@robh.at.kernel.org>
Date:   Fri, 1 Apr 2022 15:43:38 -0500
From:   Rob Herring <robh@...nel.org>
To:     Richard Zhu <hongxing.zhu@....com>
Cc:     l.stach@...gutronix.de, bhelgaas@...gle.com, broonie@...nel.org,
        lorenzo.pieralisi@....com, jingoohan1@...il.com,
        festevam@...il.com, francesco.dolcini@...adex.com,
        linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, kernel@...gutronix.de,
        linux-imx@....com
Subject: Re: [PATCH v8 6/8] PCI: dwc: Add dw_pcie_host_ops.host_exit()
 callback

On Fri, Feb 25, 2022 at 11:44:25AM +0800, Richard Zhu wrote:
> When the link never comes up after ->host_init(), some drivers,
> especially those that don't support hotplug, want to turn off clocks
> and power supplies.

Isn't supporting hotplug or not a board level decision? And hotplug 
doesn't have to mean physical plug/unplug. For example, you could have a 
soldered down PCIe device which needs regulators, resets, clocks, etc. 
for that device to be initialized before the link comes up. If that 
device is handled by a module loaded some time later, then the link may 
be down when you probe.

I think the way this all needs to work is with runtime PM. If that's all 
in place, then either you shutdown clocks/power on timeout or via sysfs 
suspend. If there's a child device, then that should prevent suspending.

> Add a new ->host_exit() callback in dw_pcie_host_ops so these
> drivers can clean up if ->host_init() fails.

I'm not really a fan of adding more ops nor the ops which aren't too 
specific about what they do. 'init' and 'exit' can be anything. I'd 
rather see more specific ops with the DWC core driver in charge of 
sequence of operations and the state. 

Rob